.reloc improvement

[Maciej W. Rozycki]

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?

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;