Re: Commit: PR 22875: Stop strip corrupting unknown relocs

["Maciej W. Rozycki" <macro at mips dot com>]

On Mon, 5 Mar 2018, Alan Modra wrote:

> > > Oh, and the bad symbol index gets you a segfault on trying to run
> > > strip -g, at this line
> > >       if ((*ptr->sym_ptr_ptr)->the_bfd->xvec != abfd->xvec
> > > in elf64-mip.c:mips_elf64_write_rela.  the_bfd is NULL.
> > 
> > Adding this fixes the problem:
> > 
> > @@ -4112,7 +4114,8 @@ mips_elf64_write_rela (bfd *abfd, asection *sec,
> >        int_rela.r_addend = ptr->addend;
> >        int_rela.r_ssym = RSS_UNDEF;
> >  
> > -      if ((*ptr->sym_ptr_ptr)->the_bfd->xvec != abfd->xvec
> > +      if (! bfd_is_const_section ((*ptr->sym_ptr_ptr)->section)
> > +         && (*ptr->sym_ptr_ptr)->the_bfd->xvec != abfd->xvec
> >           && ! _bfd_elf_validate_reloc (abfd, ptr))
> >         {
> >           *failedp = TRUE;
> 
> Or the simpler patch of just testing the_bfd for NULL, and doing the
> same in mips_elf64_write_rel too.  However, that workaround results in
> objcopy silently modifying the copied object file, which is why I
> decided to not post a patch for this and leave it to Maciej to fix
> properly.

 I think this is the right fix here however, complementing the same change 
made for generic ELF with:

commit e35765a9a2eaff0df62757f3e6480c8ba5ab8ee8
Author: Ian Lance Taylor <ian@airs.com>
Date:   Sun Dec 15 19:59:18 1996 +0000

which I believe has been incorrectly recorded with repository conversion 
and actually is:

	* elfcode.h (write_relocs): Handle absolute symbol.

from:

commit c86158e591edd8450f49f8cd75f82e4313d4b6d8
Author: Ian Lance Taylor <ian@airs.com>
Date:   Fri Aug 30 22:09:51 1996 +0000

("Add SH ELF support."), which also for some reason updated RELA only and 
not REL (which has been since fixed with:

commit 947216bf8f343c1440e85633b5bf2f2394f87bc4
Author: Alan Modra <amodra@gmail.com>
Date:   Thu Nov 28 11:55:43 2002 +0000

).  Absolute symbol handling has been since partially fixed for n64 MIPS 
with:

commit 99022dfb1d4b1de6f394026ffd5b738b522aa9f6
Author: Richard Sandiford <rdsandiford@googlemail.com>
Date:   Thu Oct 7 19:15:29 2004 +0000

If the fix makes `objcopy' modify a copied object file, then so be it; 
garbage in, garbage out applies.  At least it will not crash, and I 
expect the same will happen with other ELF targets hitting the same 
condition in `elf_write_relocs'.  So if you want a better fix, then it 
would have to go into the latter function in the first place.

 And I have now actually verified that the x86-64 target does exactly the 
same, except for an extra warning message:

$ readelf -r strip-13mod.o

Relocation section '.rela.text' at offset 0x44 contains 2 entries:
  Offset          Info           Type           Sym. Value    Sym. Name + Addend
000000000000  00f100000000 R_X86_64_NONE    readelf: Error:  bad symbol index: 000000f1 in reloc
000000000000  000000000000 R_X86_64_NONE                        0
$ objcopy strip-13mod.o strip-13mod-out.o
objcopy: strip-13mod.o(.text): relocation 0 has invalid symbol index 241
$ readelf -r strip-13mod-out.o

Relocation section '.rela.text' at offset 0xc8 contains 2 entries:
  Offset          Info           Type           Sym. Value    Sym. Name + Addend
000000000000  000000000000 R_X86_64_NONE                        f1
000000000000  000000000000 R_X86_64_NONE                        0
$

so I am going to apply your proposed fix and will look into implementing 
the missing warning message too.  Maybe we actually want a test case for 
this as well (but we need to get conditionals for RELA vs REL vs n64-MIPS 
relocations sorted out first in the test framework).

  Maciej