[PATCH] D53993: [ELF] Set sh_link to 0 in static link to make GNU strip happy

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: ELF/SyntheticSections.cpp:1494
 void RelocationBaseSection::finalizeContents() {
-  // If all relocations are R_*_{,I}RELATIVE they don't refer to any dynamic
-  // symbol and we don't need a dynamic symbol table. If that is the case, use
-  // the index of the regular symbol table section (if exists) or 0.
-  if (In.DynSymTab)
-    getParent()->Link = In.DynSymTab->getParent()->SectionIndex;
-  else if (In.SymTab)
-    getParent()->Link = In.SymTab->getParent()->SectionIndex;
-  else
-    getParent()->Link = 0;
+  // When linking glibc statically, .rela?.plt contains R_*_IRELATIVE
+  // relocations due to IFUNC (e.g. strcpy). Just set sh_link to 0 as there is
----------------
MaskRay wrote:
> MaskRay wrote:
> > ruiu wrote:
> > > Looks like this new comment is not easy to understand for those who do not already know the problem you are trying to fix in this. What is this `link` supposed to have by the spec? You shouldn't focus too much on a workaround when describing something, but focus on what it is supposed to be (and briefly mention the deviation from the expected situation.)
> > D52830 says:
> > 
> > ELF spec (https://docs.oracle.com/cd/E19683-01/816-1386/chapter6-94076/index.html)
> > says that for SHT_REL and SHT_RELA, sh_link references the associated symbol table
> > and sh_info the "section to which the relocation applies."
> > 
> > 
> > I think either .symtab and .dynsym can be used. `binutils/readelf.c` emits the warning when the pointed symbol table section is of neither SHT_SYMTAB nor SHT_DYNSYM.
> > 
> > 	case SHT_REL:
> > 	case SHT_RELA:
> > 	  if (section->sh_link < 1
> > 	      || section->sh_link >= filedata->file_header.e_shnum
> > 	      || (filedata->section_headers[section->sh_link].sh_type != SHT_SYMTAB
> > 		  && filedata->section_headers[section->sh_link].sh_type != SHT_DYNSYM))
> > 	    warn (_("[%2u]: Link field (%u) should index a symtab section.\n"),
> > 		  i, section->sh_link);
> > 	  break;
> > 
> > Let me report an issue on binutils and simplify the comment with a link to that bug
> Simplified it with a link to https://docs.oracle.com/cd/E19683-01/816-1386/6m7qcoblh/index.html
> 
> Basically it reverts the sh_link part of D52830
> 
> I am happy to change the comments at your request.
Or how about this:

  if (In.DynSymTab)
    getParent()->Link = In.DynSymTab->getParent()->SectionIndex;
  else if (In.SymTab && In.RelaIplt != this) // && In.RelaIplt != this  is the new condition
    getParent()->Link = In.SymTab->getParent()->SectionIndex;
  else
    getParent()->Link = 0;

.rela.plt (In.RelaIplt) is a special relocation section invented for IFUNC. We can keep the work around to a minimum case.


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D53993