This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: .reloc improvement


On Nov 14, 2011, at 3:22 PM, Maciej W. Rozycki wrote:

> On Thu, 27 Oct 2011, Maciej W. Rozycki wrote:
> 
>> On Tue, 6 Sep 2011, Maciej W. Rozycki wrote:
>> 
>>> On Mon, 22 Aug 2011, Alan Modra wrote:
>>> 
>>>>> Please note that on REL MIPS targets local symbols have to be retained in 
>>>>> several cases where crucial information is lost if a relocation against 
>>>>> such a symbol is converted to one against the containing section's symbol.  
>>>>> This applies to all the PC-relative relocations, where the relocatable 
>>>>> field is too narrow to hold arbitrary addends, and also, more recently, to 
>>>>> microMIPS targets, where linker relaxation has to know local symbol 
>>>>> (label) addresses to perform branch displacement recalculations and to 
>>>>> make the LUI/ADDIU->ADDIUPC relaxation.  The latter is a linker's internal 
>>>>> limitation and may possibly be lifted (possibly by using the RELA 
>>>>> representation internally even on REL targets), but the former is an 
>>>>> inherent problem of the object file format used.
>>>>> 
>>>>> Just making sure these issues are not missed -- I can't have a look at 
>>>>> your change at the moment and see what exact implications it has, sorry.
>>>> 
>>>> The implication of my change is that the programmer will need to be
>>>> careful in choosing symbols used with .reloc.  While that isn't ideal,
>>>> .reloc is such a low-level interface that you need to know what you're
>>>> doing if you use it.
>>> 
>>> Problem is on REL targets some MIPS relocation types can never ever be 
>>> safely converted to refer to a section symbol + addend instead of the 
>>> intended symbol.  You'd therefore make .reloc useless for these types.
>>> 
>>> Yes, REL shouldn't have been chosen for the MIPS ABI in the first place, 
>>> with the complication observed here being the tip of an iceberg only, but 
>>> there you go.  The choice was later fixed with the new ABIs made for 
>>> 64-bit MIPS processors, but the old ABI still remains for 32-bit ones.
>>> 
>>>> Alternatively, I'm quite willing to disable the symbol to section symbol 
>>>> conversion for REL targets.
>>> 
>>> Good idea, that sounds like a plan to me.  That could be made platform 
>>> specific if that made anybody's life easier.
>> 
>> Where are we with this problem?  Your change regressed this program (an 
>> excerpt from a test case I prepared a while ago and was about to integrate 
>> with our test suite):
>> 
>> $ cat reloc.s
>> 	.text
>> 
>> 	.fill	0x1000000
>> 
>> 	.set	micromips
>> foo:
>> 	nop32
>> 	.reloc	., R_MICROMIPS_PC23_S2, 0f
>> 	.fill	0
>> 	addiu	$2, $pc, 8
>> 	nop32
>> 0:
>> 
>> from this:
>> 
>> $ mips-sde-elf-as -32 -o reloc.o reloc.s
>> $ mips-sde-elf-objdump -dr reloc.o
>> 
>> reloc.o:     file format elf32-tradbigmips
>> 
>> 
>> Disassembly of section .text:
>> 
>> 00000000 <foo-0x1000000>:
>> 	...
>> 
>> 01000000 <foo>:
>> 1000000:	0000 0000 	nop
>> 1000004:	7900 0002 	addiu	v0,$pc,8
>> 			1000004: R_MICROMIPS_PC23_S2	.L1
>> 1000008:	0000 0000 	nop
>> 
>> to this:
>> 
>> $ mips-sde-elf-as -o reloc.o reloc.s
>> reloc.s: Assembler messages:
>> reloc.s:8: Error: relocation overflow
> 
> Long time, no response, so I went ahead and implemented this fix.  It 
> unbreaks the test case for me and causes no regressions for mips-sde-elf 
> (although I have no slightest idea how much coverage in the test suite 
> .reloc has).
> 
> OK to apply?  Since this is a regression OK for 2.22 too?

It potentially affects all targets.

Ok for the branch if you have the OK for mainline.

Tristan.

> 
> 2011-11-14  Maciej W. Rozycki  <macro@codesourcery.com>
> 
> 	gas/
> 	write.c (dump_section_relocs): Don't convert PC-relative relocs 
> 	that have an in-place addend narrower than the addresses used.
> 
>  Maciej
> 
> binutils-gas-reloc-pcrel-inplace.diff
> Index: binutils-fsf-trunk-quilt/gas/write.c
> ===================================================================
> --- binutils-fsf-trunk-quilt.orig/gas/write.c	2011-11-14 13:50:51.000000000 +0000
> +++ binutils-fsf-trunk-quilt/gas/write.c	2011-11-14 14:19:07.795860362 +0000
> @@ -654,15 +654,21 @@ dump_section_relocs (bfd *abfd ATTRIBUTE
> static void
> resolve_reloc_expr_symbols (void)
> {
> +  bfd_vma addr_mask = 1;
>   struct reloc_list *r;
> 
> +  /* Avoid a shift by the width of type.  */
> +  addr_mask <<= bfd_arch_bits_per_address (stdoutput) - 1;
> +  addr_mask <<= 1;
> +  addr_mask -= 1;
> +
>   for (r = reloc_list; r; r = r->next)
>     {
> +      reloc_howto_type *howto = r->u.a.howto;
>       expressionS *symval;
>       symbolS *sym;
>       bfd_vma offset, addend;
>       asection *sec;
> -      reloc_howto_type *howto;
> 
>       resolve_symbol_value (r->u.a.offset_sym);
>       symval = symbol_get_value_expression (r->u.a.offset_sym);
> @@ -709,7 +715,16 @@ resolve_reloc_expr_symbols (void)
> 	    }
> 	  else if (sym != NULL)
> 	    {
> -	      if (S_IS_LOCAL (sym) && !symbol_section_p (sym))
> +	      /* Convert relocs against local symbols to refer to the
> +	         corresponding section symbol plus offset instead.  Keep
> +	         PC-relative relocs of the REL variety intact though to
> +		 prevent the offset from overflowing the relocated field,
> +	         unless it has enough bits to cover the whole address
> +	         space.  */
> +	      if (S_IS_LOCAL (sym) && !symbol_section_p (sym)
> +		  && !(howto->partial_inplace
> +		       && howto->pc_relative
> +		       && howto->src_mask != addr_mask))
> 		{
> 		  asection *symsec = S_GET_SEGMENT (sym);
> 		  if (!(((symsec->flags & SEC_MERGE) != 0
> @@ -730,8 +745,6 @@ resolve_reloc_expr_symbols (void)
> 	  sym = abs_section_sym;
> 	}
> 
> -      howto = r->u.a.howto;
> -
>       r->u.b.sec = sec;
>       r->u.b.s = symbol_get_bfdsym (sym);
>       r->u.b.r.sym_ptr_ptr = &r->u.b.s;


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]