[PATCH 1/2] MIPS: Compressed PLT/stubs support

Richard Sandiford rdsandiford@googlemail.com
Sat Mar 9 09:58:00 GMT 2013


Thanks for the updates.

"Maciej W. Rozycki" <macro@codesourcery.com> writes:
>> "Maciej W. Rozycki" <macro@codesourcery.com> writes:
>> > @@ -3215,25 +3325,19 @@ static bfd_vma
>> >  mips_elf_gotplt_index (struct bfd_link_info *info,
>> >  		       struct elf_link_hash_entry *h)
>> >  {
>> > -  bfd_vma plt_index, got_address, got_value;
>> > +  bfd_vma got_address, got_value;
>> >    struct mips_elf_link_hash_table *htab;
>> >  
>> >    htab = mips_elf_hash_table (info);
>> >    BFD_ASSERT (htab != NULL);
>> >  
>> > -  BFD_ASSERT (h->plt.offset != (bfd_vma) -1);
>> > -
>> > -  /* This function only works for VxWorks, because a non-VxWorks .got.plt
>> > -     section starts with reserved entries.  */
>> > -  BFD_ASSERT (htab->is_vxworks);
>> > -
>> > -  /* Calculate the index of the symbol's PLT entry.  */
>> > -  plt_index = (h->plt.offset - htab->plt_header_size) / htab->plt_entry_size;
>> > +  BFD_ASSERT (h->plt.plist != NULL);
>> > +  BFD_ASSERT (h->plt.plist->gotplt_index != MINUS_ONE);
>> >  
>> >    /* Calculate the address of the associated .got.plt entry.  */
>> >    got_address = (htab->sgotplt->output_section->vma
>> >  		 + htab->sgotplt->output_offset
>> > -		 + plt_index * 4);
>> > +		 + h->plt.plist->gotplt_index * 4);
>> >  
>> >    /* Calculate the value of _GLOBAL_OFFSET_TABLE_.  */
>> >    got_value = (htab->root.hgot->root.u.def.section->output_section->vma
>> 
>> If we remove the is_vxworks assert, I think we should use MIPS_ELF_GOT_SIZE
>> instead of 4.
>
>  Not sure if this is related to this change, but I see no problem with 
> that either.  I've updated all the references throughout.

It was related because the patch kept a VxWorks assumption while removing
the assert for VxWorks.

> Index: binutils-fsf-trunk-quilt/bfd/elfxx-mips.c
> ===================================================================
> --- binutils-fsf-trunk-quilt.orig/bfd/elfxx-mips.c	2013-03-08 11:09:04.000000000 +0000
> +++ binutils-fsf-trunk-quilt/bfd/elfxx-mips.c	2013-03-09 02:43:31.765430204 +0000
> @@ -319,6 +319,32 @@ struct mips_elf_hash_sort_data
>    long max_non_got_dynindx;
>  };
>  
> +/* We make up to two PLT entries if needed, one for standard MIPS code
> +   and one for compressed code, either of MIPS16 or microMIPS one.  We
> +   keep the record of a stub if one is used instead separately, for
> +   easier processing.  */

s/either of MIPS16 or microMIPS one/either a MIPS16 or microMIPS one/.
Suggest "We keep a separate record of traditional lazy-binding stubs,
for easier processing."

> @@ -5124,13 +5232,55 @@ mips_elf_calculate_relocation (bfd *abfd
>  		|| h->root.root.type == bfd_link_hash_defweak)
>  	       && h->root.root.u.def.section)
>  	{
> -	  sec = h->root.root.u.def.section;
> -	  if (sec->output_section)
> -	    symbol = (h->root.root.u.def.value
> -		      + sec->output_section->vma
> -		      + sec->output_offset);
> +	  if (h->use_plt_entry)
> +	    {
> +	      bfd_boolean micromips_p = MICROMIPS_P (abfd);
> +	      bfd_vma plt_offset;
> +	      bfd_vma isa_bit;
> +	      bfd_vma val;
> +
> +	      BFD_ASSERT (h->root.plt.plist != NULL);
> +	      BFD_ASSERT (h->root.plt.plist->mips_offset != MINUS_ONE
> +			  || h->root.plt.plist->comp_offset != MINUS_ONE);
> +
> +	      plt_offset = htab->plt_header_size;
> +	      if (h->root.plt.plist->comp_offset == MINUS_ONE
> +		  || (h->root.plt.plist->mips_offset != MINUS_ONE
> +		      && r_type != R_MIPS16_26 && r_type != R_MICROMIPS_26_S1))
> +		{
> +		  isa_bit = 0;
> +		  target_is_16_bit_code_p = FALSE;
> +		  target_is_micromips_code_p = FALSE;
> +		  plt_offset += h->root.plt.plist->mips_offset;
> +		}
> +	      else
> +		{
> +		  isa_bit = 1;
> +		  target_is_16_bit_code_p = !micromips_p;
> +		  target_is_micromips_code_p = micromips_p;
> +		  plt_offset += (htab->plt_mips_offset
> +				 + h->root.plt.plist->comp_offset);
> +		}
> +	      BFD_ASSERT (plt_offset <= htab->splt->size);
> +
> +	      sec = htab->splt;
> +	      val = plt_offset + isa_bit;
> +	      /* For VxWorks, point at the PLT load stub rather than the
> +	         lazy resolution stub.  */
> +	      if (htab->is_vxworks)
> +		val += 8;
> +	      symbol = sec->output_section->vma + sec->output_offset + val;
> +	    }
>  	  else
> -	    symbol = h->root.root.u.def.value;
> +	    {
> +	      sec = h->root.root.u.def.section;
> +	      if (sec->output_section)
> +		symbol = (h->root.root.u.def.value
> +			  + sec->output_section->vma
> +			  + sec->output_offset);
> +	      else
> +		symbol = h->root.root.u.def.value;
> +	    }
>  	}
>        else if (h->root.root.type == bfd_link_hash_undefweak)
>  	/* We allow relocations against undefined weak symbols, giving
> @@ -5177,12 +5327,6 @@ mips_elf_calculate_relocation (bfd *abfd
>  	{
>  	  return bfd_reloc_notsupported;
>  	}
> -
> -      target_is_16_bit_code_p = ELF_ST_IS_MIPS16 (h->root.other);
> -      /* If the output section is the PLT section,
> -         then the target is not microMIPS.  */
> -      target_is_micromips_code_p = (htab->splt != sec
> -				    && ELF_ST_IS_MICROMIPS (h->root.other));
>      }
>  
>    /* If this is a reference to a 16-bit function with a stub, we need

The block of code that you're changing here is simply calculating the
symbol value.  Now that size_dynamic_sections has set that up for us,
these two hunks should be dropped, and the test for whether to use a
compressed PLT entry instead of the symbol value should be made
afterwards, as it is for things like hard-float and LA25 stubs.
Maybe something like:

  /* If compressed PLT entries are available, make sure that we use them
     for MIPS16 and microMIPS calls.  */
  else if ((r_type == R_MIPS16_26 || r_type == R_MICROMIPS_26_S1)
	   && h != NULL
	   && h->use_plt
	   && h->root.plt.plist->comp_offset != MINUS_ONE)
    {
      sec = htab->splt;
      symbol = (sec->output_section->vma
		+ sec->output_offset
		+ htab->plt_header_size
		+ htab->plt_mips_offset
		+ h->root.plt.plist->comp_offset
		+ 1);
      target_is_16_bit_code_p = !MICROMIPS_P (abfd);
      target_is_micromips_code_p = MICROMIPS_P (abfd);
    }

at the end of the:

  /* If this is a reference to a 16-bit function with a stub, we need
     to redirect the relocation to the stub unless:

chain of ifs.

> -      /* Make room for the .got.plt entry and the R_MIPS_JUMP_SLOT
> -	 relocation.  */
> -      htab->sgotplt->size += MIPS_ELF_GOT_SIZE (dynobj);
> +      htab->splt->size = htab->plt_mips_offset + htab->plt_comp_offset;
> +      htab->sgotplt->size = htab->plt_got_index * MIPS_ELF_GOT_SIZE (dynobj);

These last two lines should be done in size_dynamic_sections rather
than once for each symbol.  I.e.:

> @@ -8985,18 +9318,60 @@ _bfd_mips_elf_size_dynamic_sections (bfd
>  	    = (bfd_byte *) ELF_DYNAMIC_INTERPRETER (output_bfd);
>  	}
>  
> -      /* Create a symbol for the PLT, if we know that we are using it.  */
> -      if (htab->splt && htab->splt->size > 0 && htab->root.hplt == NULL)
> +      /* Figure out the size of the PLT header if we know that we
> +         are using it.  For the sake of cache alignment always use
> +         a standard header whenever any standard entries are present
> +         even if microMIPS entries are present as well.  This also
> +         lets the microMIPS header rely on the value of $v0 only set
> +         by microMIPS entries, for a small size reduction.
> +
> +         Set symbol table entry values for symbols that use the
> +         address of their PLT entry now that we can calculate it.
> +
> +         Also create the _PROCEDURE_LINKAGE_TABLE_ symbol if we
> +         haven't already in _bfd_elf_create_dynamic_sections.  */
> +      if (htab->splt && htab->splt->size > 0)
>  	{
> +	  bfd_boolean micromips_p = (MICROMIPS_P (output_bfd)
> +				     && !htab->plt_mips_offset);
> +	  unsigned int other = micromips_p ? STO_MICROMIPS : 0;
> +	  bfd_vma isa_bit = micromips_p;
>  	  struct elf_link_hash_entry *h;
> +	  bfd_vma size;
>  
>  	  BFD_ASSERT (htab->use_plts_and_copy_relocs);
>  
> -	  h = _bfd_elf_define_linkage_sym (dynobj, info, htab->splt,
> -					   "_PROCEDURE_LINKAGE_TABLE_");
> -	  htab->root.hplt = h;
> -	  if (h == NULL)
> -	    return FALSE;
> +	  if (htab->is_vxworks && info->shared)
> +	    size = 4 * ARRAY_SIZE (mips_vxworks_shared_plt0_entry);
> +	  else if (htab->is_vxworks)
> +	    size = 4 * ARRAY_SIZE (mips_vxworks_exec_plt0_entry);
> +	  else if (ABI_64_P (output_bfd))
> +	    size = 4 * ARRAY_SIZE (mips_n64_exec_plt0_entry);
> +	  else if (ABI_N32_P (output_bfd))
> +	    size = 4 * ARRAY_SIZE (mips_n32_exec_plt0_entry);
> +	  else if (!micromips_p)
> +	    size = 4 * ARRAY_SIZE (mips_o32_exec_plt0_entry);
> +	  else
> +	    size = 2 * ARRAY_SIZE (micromips_o32_exec_plt0_entry);
> +
> +	  htab->plt_header_is_comp = micromips_p;
> +	  htab->plt_header_size = size;
> +	  htab->splt->size += size;

	  htab->splt->size = (size
			      + htab->plt_mips_offset
			      + htab->plt_comp_offset;
	  htab->sgotplt->size = (htab->plt_got_index
				 * MIPS_ELF_GOT_SIZE (dynobj));

> +	      /* ADDIUPC has a span of +/-16MB, check we're in range.  */
> +	      if (gotpc_offset + 0x1000000 >= 0x2000000)
> +		{
> +		  (*_bfd_error_handler)
> +		    (_("%B: `%A' offset of %ld from `%A' "
> +		       "beyond the range of ADDIUPC"),
> +		     output_bfd,
> +		     htab->sgotplt->output_section,
> +		     htab->splt->output_section,
> +		     (long) gotpc_offset);

The last two arguments should be swapped.

> +      /* ADDIUPC has a span of +/-16MB, check we're in range.  */
> +      if (gotpc_offset + 0x1000000 >= 0x2000000)
> +	{
> +	  (*_bfd_error_handler)
> +	    (_("%B: `%A' offset of %ld from `%A' beyond the range of ADDIUPC"),
> +	     output_bfd,
> +	     htab->sgotplt->output_section,
> +	     htab->splt->output_section,
> +	     (long) gotpc_offset);

Same here.

> +  /* Calculating the exact amount of space required for symbols would
> +     require two passes over the PLT, so just pessimise assuming two
> +     PLT slots per relocation.  */
> +  count = relplt->size / hdr->sh_entsize;
> +  counti = count * bed->s->int_rels_per_ext_rel;
> +  size = 2 * count * sizeof (asymbol);
> +  size += count * (sizeof (mipssuffix) +
> +		   (micromips_p ? sizeof (microsuffix) : sizeof (m16suffix)));
> +  addlen = 2 * (sizeof ("+0x") - 1 + 8);
> +#ifdef BFD64
> +  addlen += 2 * 8 * (bed->s->elfclass == ELFCLASS64);
> +#endif

Now that there are no addends (thanks), the last four lines should be dropped.

OK with those changes if they work, otherwise let me know.

Richard



More information about the Binutils mailing list