[ELF] - Set sh_info and sh_link for .rela.plt sections.

[grimar]

Summary

This is https://bugs.llvm.org/show_bug.cgi?id=37538,

Currently, LLD may set both sh_link and sh_info for
.rela.plt section to zero when we have only .rela.iplt section part used.

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."

When we set the sh_link field, for the regular case we use the .dynsym index.
For .rela.iplt sections, it is unclear what is the associated symbol table,
because R_*_RELATIVE relocations do not use symbol names and we might have no
.dynsym section at all so this patch uses .symtab section index.

That helps to suppress the warnings generated by GNU Binutils.
The behavior is consistent with GNU linkers and at least does not violate
the spec it seems.

[ruiu]

In the commit message, please emphasize that this is what ELF spec requires, rather than emphasizing that GNU tools complain.

ELF/SyntheticSections.cpp

1498 ↗

(On Diff #168109)

Why don't you directly assign it to getParent()->Link?

[emaste]

https://docs.oracle.com/cd/E19683-01/816-1386/chapter6-94076/index.html

For SHT_REL and SHT_RELA, sh_link references the associated symbol table and sh_info the "section to which the relocation applies."

[grimar]

In D52830#1253865, @ruiu wrote:

In the commit message, please emphasize that this is what ELF spec requires, rather than emphasizing that GNU tools complain.

It's a bit uncovered part of the spec. As Ed mentioned, spec says "sh_link references the associated symbol table".
Technically, when we have only R_*_RELATIVE relocations in .rela.plt there is no .dynsym and hence no associated symbol table.
This patch assigns the index of .symtab then, but that should be useless for any tools as R_*_RELATIVE
just do not need symbols names and this assignment is a stub to suppress complaints.
That is why I did not say that it is required by ELF.

I'll update this patch with a better commit message.

[grimar]

• Addressed comments.

ELF/SyntheticSections.cpp

1498 ↗

(On Diff #168109)

Just tried to be consistent with the existent code. Fixed.

[ruiu]

ELF/SyntheticSections.cpp

1496–1497 ↗

(On Diff #168253)

You do that not because GNU tools complain but because that's the right thing to do according to ELF spec, no?

[grimar]

Updated the comment.

ELF/SyntheticSections.cpp

1496–1497 ↗

(On Diff #168253)

I would not say it is clear to me that it is required.
Spec says about sh_link that it contains "The section header index of the associated symbol table. ".
In the case of .rela.iplt we can live without the symbol table. In that case, this association does not make sense.

Spec does not say what should we do when there is no symbol table associated. With the same success, it probably
could be an empty dynamic section we could create for consistency.

Or, another example, with -strip-all we do not create the .symtab. And tools would complain again I guess,
though I see nothing obviously wrong with that scenario.

Perhaps I am overthinking. I'll shorten the comment.

[emaste]

This change LGTM.

ELF/SyntheticSections.cpp

1496–1497 ↗

(On Diff #168253)

The issue I encountered was with sh_info where according to the ELF spec I believe this change is a correctness fix (and did not trigger warnings from GNU tools anyhow).

For sh_link in an ifunc-using statically-linked binary I think it is arguable that this change is only to appease GNU readelf.

[emaste]

This change looks good to me; testing it out in the FreeBSD tree now.

[emaste]

I backported the change to lld 6.0 currently in FreeBSD-HEAD and it has the expected behaviour, but for my test case I think sh_info should reference .got.plt not .plt - note the address marked with ######## below.

That said, in practice it will address the issue we have with ELF Tool Chain's strip either way (the important part is that sh_info references a section that isn't being removed.

There are 36 section headers, starting at offset 0x434420:

Section Headers:
  [Nr] Name              Type            Addr             Off    Size   ES Flg Lk Inf Al
  [ 0]                   NULL            0000000000000000 000000 000000 00      0   0  0
  [ 1] .note.tag         NOTE            0000000000200238 000238 000030 00   A  0   0  4
  [ 2] .rodata           PROGBITS        0000000000200280 000280 017f20 00 AMS  0   0 64
  [ 3] .rela.plt         RELA            00000000002181a0 0181a0 000018 18   A 33   9  8
                                                                                   **
  [ 4] .eh_frame_hdr     PROGBITS        00000000002181b8 0181b8 003b54 00   A  0   0  1
  [ 5] .eh_frame         PROGBITS        000000000021bd10 01bd10 0100a0 00   A  0   0  8
  [ 6] .text             PROGBITS        000000000022c000 02c000 0bdbf8 00  AX  0   0 16
  [ 7] .init             PROGBITS        00000000002e9bf8 0e9bf8 000013 00  AX  0   0  4
  [ 8] .fini             PROGBITS        00000000002e9c0c 0e9c0c 00000e 00  AX  0   0  4
  [ 9] .plt              PROGBITS        00000000002e9c20 0e9c20 000010 00  AX  0   0 16
   **
  [10] .data             PROGBITS        00000000002ea000 0ea000 003aa0 00  WA  0   0 16
  [11] .got.plt          PROGBITS        00000000002edaa0 0edaa0 000008 00  WA  0   0  8
...                                      ################
  [33] .symtab           SYMTAB          0000000000000000 4148f8 012d38 18     35 1582  8
  [34] .shstrtab         STRTAB          0000000000000000 427630 00016f 00      0   0  1
  [35] .strtab           STRTAB          0000000000000000 42779f 00cc7b 00      0   0  1
Key to Flags:
  W (write), A (alloc), X (execute), M (merge), S (strings)
  I (info), L (link order), G (group), x (unknown)
  O (extra OS processing required) o (OS specific), p (processor specific)

nuc% readelf -r make.full

Relocation section with addend (.rela.plt):
r_offset     r_info       r_type              st_value         st_name + r_addend
0000002edaa0 000000000025 R_X86_64_IRELATIVE  0000000000000000  + 2e91f0
############
nuc%

[grimar]

In D52830#1258953, @emaste wrote:

I backported the change to lld 6.0 currently in FreeBSD-HEAD and it has the expected behaviour, but for my test case I think sh_info should reference .got.plt not .plt - note the address marked with ######## below.

I referenced '.plt' mostly for naming consistency ('.rela.plt' -> '.plt') (And gold seems do the same).
But I think you are right since it relocates entries in .got.plt first of all, so technically it would be correct to reference the '.got.plt'.
It also makes the implementation a bit simpler. I'll update the patch.

[grimar]

• Updated in according to recent comments.

[ruiu]

LGTM

[MaskRay]

This has issue with GNU strip. I'll try giving an example (and probably patch) tomorrow.

lld/trunk/ELF/SyntheticSections.cpp

void RelocationBaseSection::addReloc(const DynamicReloc &Reloc) {
   if (Reloc.Type == Target->RelativeRel)
     ++NumRelativeRelocs;
   Relocs.push_back(Reloc);
 }
 
 void RelocationBaseSection::finalizeContents() {
   // If all relocations are R_*_RELATIVE they don't refer to any
   // dynamic symbol and we don't need a dynamic symbol table. If that
-  // is the case, just use 0 as the link.
-  getParent()->Link =
-      In.DynSymTab ? In.DynSymTab->getParent()->SectionIndex : 0;
+  // is the case, just use the index of the regular symbol table section.
+  getParent()->Link = In.DynSymTab ? In.DynSymTab->getParent()->SectionIndex
+                                   : In.SymTab->getParent()->SectionIndex;
+
+  if (In.RelaIplt == this || In.RelaPlt == this)
+    getParent()->Info = In.GotPlt->getParent()->SectionIndex;
 }
 
 RelrBaseSection::RelrBaseSection()
     : SyntheticSection(SHF_ALLOC,
                        Config->UseAndroidRelrTags ? SHT_ANDROID_RELR : SHT_RELR,
                        Config->Wordsize, ".relr.dyn") {}
 
 template <class ELFT>

lld/trunk/test/ELF/aarch64-gnu-ifunc.s

 // CHECK-NEXT:  Name: .rela.plt
 // CHECK-NEXT:  Type: SHT_RELA
 // CHECK-NEXT:  Flags [
 // CHECK-NEXT:    SHF_ALLOC
 // CHECK-NEXT:  ]
 // CHECK-NEXT:  Address: [[RELA:.*]]
 // CHECK-NEXT:  Offset: 0x158
 // CHECK-NEXT:  Size: 48
-// CHECK-NEXT:  Link: 0
-// CHECK-NEXT:  Info: 0
+// CHECK-NEXT:  Link: 6
+// CHECK-NEXT:  Info: 4
 // CHECK-NEXT:  AddressAlignment: 8
 // CHECK-NEXT:  EntrySize: 24
 // CHECK-NEXT: }
 // CHECK:      Relocations [
 // CHECK-NEXT:   Section ({{.*}}) .rela.plt {
 // CHECK-NEXT:     0x220000 R_AARCH64_IRELATIVE
 // CHECK-NEXT:     0x220008 R_AARCH64_IRELATIVE
 // CHECK-NEXT:   }

lld/trunk/test/ELF/dynamic-reloc.s

 // CHECK-NEXT: Type: SHT_RELA
 // CHECK-NEXT: Flags [
 // CHECK-NEXT:   SHF_ALLOC
 // CHECK-NEXT: ]
 // CHECK-NEXT: Address: [[RELAADDR:.*]]
 // CHECK-NEXT: Offset:
 // CHECK-NEXT: Size: [[RELASIZE:.*]]
 // CHECK-NEXT: Link: 1
-// CHECK-NEXT: Info: 0
+// CHECK-NEXT: Info: 7
 // CHECK-NEXT: AddressAlignment: 8
 // CHECK-NEXT: EntrySize: 24
 
 // CHECK:      Name: .text
 // CHECK-NEXT: Type: SHT_PROGBITS
 // CHECK-NEXT: Flags [
 // CHECK-NEXT:   SHF_ALLOC
 // CHECK-NEXT:   SHF_EXECINSTR

lld/trunk/test/ELF/gnu-ifunc-i386.s

 // CHECK-NEXT:  Name: .rel.plt
 // CHECK-NEXT:  Type: SHT_REL
 // CHECK-NEXT:  Flags [
 // CHECK-NEXT:    SHF_ALLOC
 // CHECK-NEXT:  ]
 // CHECK-NEXT:  Address: [[RELA:.*]]
 // CHECK-NEXT:  Offset: 0xD4
 // CHECK-NEXT:  Size: 16
-// CHECK-NEXT:  Link: 0
-// CHECK-NEXT:  Info: 0
+// CHECK-NEXT:  Link: 6
+// CHECK-NEXT:  Info: 4
 // CHECK-NEXT:  AddressAlignment: 4
 // CHECK-NEXT:  EntrySize: 8
 // CHECK-NEXT: }
 // CHECK:     Relocations [
 // CHECK-NEXT:   Section ({{.*}}) .rel.plt {
 // CHECK-NEXT:     0x402000 R_386_IRELATIVE
 // CHECK-NEXT:     0x402004 R_386_IRELATIVE
 // CHECK-NEXT:   }

lld/trunk/test/ELF/gnu-ifunc.s

 // CHECK-NEXT:  Name: .rela.plt
 // CHECK-NEXT:  Type: SHT_RELA
 // CHECK-NEXT:  Flags [
 // CHECK-NEXT:    SHF_ALLOC
 // CHECK-NEXT:  ]
 // CHECK-NEXT:  Address: [[RELA:.*]]
 // CHECK-NEXT:  Offset: 0x158
 // CHECK-NEXT:  Size: 48
-// CHECK-NEXT:  Link: 0
-// CHECK-NEXT:  Info: 0
+// CHECK-NEXT:  Link: [[SYMTAB:.*]]
+// CHECK-NEXT:  Info: [[GOTPLT:.*]]
 // CHECK-NEXT:  AddressAlignment: 8
 // CHECK-NEXT:  EntrySize: 24
 // CHECK-NEXT: }
+// CHECK:      Index: [[GOTPLT]]
+// CHECK-NEXT: Name: .got.plt
+// CHECK:      Index: [[SYMTAB]]
+// CHECK-NEXT: Name: .symtab
+// CHECK-NEXT: Type: SHT_SYMTAB
 // CHECK:      Relocations [
 // CHECK-NEXT:   Section ({{.*}}) .rela.plt {
 // CHECK-NEXT:     0x202000 R_X86_64_IRELATIVE
 // CHECK-NEXT:     0x202008 R_X86_64_IRELATIVE
 // CHECK-NEXT:   }
 // CHECK-NEXT: ]
 // CHECK:      Symbols [
 // CHECK-NEXT:  Symbol {