[ELF] - Assign sh_link field of SHT_GNU_versym section to DynSymTab section index

[grimar]

Summary

.gnu.version should have sh_link field initialized with index of DynSymTab section.

GNU documentation looks misses that, but Sun docs mention it, according to
https://docs.oracle.com/cd/E19120-01/open.solaris/819-0690/chapter6-54676/index.html
versym sh_link is indeed supposed to point to the .dynsym section.

Binutils readelf tool also relies on that:
https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=blob;f=binutils/readelf.c;h=b6454d353279dc57745cd5a2d68b5f3f69f8e17c;hb=5522f910cb539905d6adfdceab208ddfa5e84557#l9988

Both gold/bfd do the same + after this patch I am able to see this section in readelf output, was unable before in my case.

[emaste]

I did not find anywhere in documentation that sh_link of this one should be set to something, but readelf using it:

ELF Tool Chain readelf collects the version strings from the verdef and verneed sections, without using the versym section sh_link. I think the GNU readelf source you linked to is actually a bug.

That said, from https://docs.oracle.com/cd/E19120-01/open.solaris/819-0690/chapter6-54676/index.html versym sh_link is indeed supposed to point to the .dynsym section:

The number of elements of the array must equal the number of symbol table entries that are contained in the associated symbol table. This number is determined by the section's sh_link value.

I think it would be valid to have another string section (say, .verstr) and have the verdef/verneed sections reference it in sh_link. In that case versym sh_link still needs to be the dynsym section.

[• rafael]

LGTM

[ruiu]

LGTM with a nit.

ELF/OutputSections.cpp

1421 ↗

(On Diff #59528)

Please add a comment saying that setting sh_link is not required by any spec but we do since readelf assumes it.

[ruiu]

"readelf as of June 2016" (so that we can remove this code in, say, 2026.)

[emaste]

ELF/OutputSections.cpp

1421 ↗

(On Diff #59528)

It is actually required (in the link I posted above). I just think it's also a bug in readelf.

versym sh_link has to point to .dynsym per above, while I believe verdef and verneed do not have to (but in practice always do), while readelf relies on all three pointing to .dynsym.

[emaste]

The link I pasted is for the sun doc not a GNU one, but I believe much/all of the detail is the same. The section header types SHT_SUNW_version and SHT_GNU_version have the same value.

[ruiu]

Ah, OK, then this should be fine. Thanks!

[grimar]

ELF/OutputSections.cpp

1421 ↗

(On Diff #59528)

I think verdef and verneed should point to DynStrTab in sh_link field, when
versym points to DynSymTab. At least that how that stuff works in binutils now I believe.

[emaste]

I think verdef and verneed should point to DynStrTab in sh_link field, when
versym points to DynSymTab. At least that how that stuff works in binutils now I believe.

Sorry for the confusion, you are correct. I dropped one level of indirection.

I believe these are true:

• versym must point to dynsym (as fixed by this change)
• dynsym must point to dynstr (already a given)
• typically verdef, versym and dynsym all point to dynstr
• no spec requires verdef and versym to point to dynstr
• GNU readelf will not work correctly if verdef or versym do not point to dynstr because it accesses the string table for version strings via versym->sh_info->sh_info not verdef->sh_info and verneed->sh_info (the presumed bug)

lld/trunk/ELF/OutputSections.cpp

VersionTableSection<ELFT>::VersionTableSection()
     : OutputSectionBase<ELFT>(".gnu.version", SHT_GNU_versym, SHF_ALLOC) {
   this->Header.sh_addralign = sizeof(uint16_t);
 }
 
 template <class ELFT> void VersionTableSection<ELFT>::finalize() {
   this->Header.sh_size =
       sizeof(Elf_Versym) * (Out<ELFT>::DynSymTab->getSymbols().size() + 1);
   this->Header.sh_entsize = sizeof(Elf_Versym);
+  // At the moment of june 2016 GNU docs does not mention that sh_link field
+  // should be set, but Sun docs do. Also readelf relies on this field.
+  this->Header.sh_link = Out<ELFT>::DynSymTab->SectionIndex;
 }
 
 template <class ELFT> void VersionTableSection<ELFT>::writeTo(uint8_t *Buf) {
   auto *OutVersym = reinterpret_cast<Elf_Versym *>(Buf) + 1;
   for (const std::pair<SymbolBody *, size_t> &P :
        Out<ELFT>::DynSymTab->getSymbols()) {
     if (auto *SS = dyn_cast<SharedSymbol<ELFT>>(P.first))
       OutVersym->vs_index = SS->VersionId;

lld/trunk/test/ELF/verneed.s

 # REQUIRES: x86
 # RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t.o
 # RUN: ld.lld %t.o %S/Inputs/verneed1.so %S/Inputs/verneed2.so -o %t
 # RUN: llvm-readobj -sections -dyn-symbols -dynamic-table %t | FileCheck %s
 # RUN: llvm-objdump -s %t | FileCheck --check-prefix=CONTENTS %s
 
-# CHECK:        Index: 2
+# CHECK:        Section {
+# CHECK:         Index: 1
+# CHECK-NEXT:    Name: .dynsym (1)
+# CHECK-NEXT:    Type: SHT_DYNSYM (0xB)
+# CHECK-NEXT:    Flags [ (0x2)
+# CHECK-NEXT:      SHF_ALLOC (0x2)
+# CHECK-NEXT:    ]
+# CHECK-NEXT:    Address: 0x101C8
+# CHECK-NEXT:    Offset: 0x1C8
+# CHECK-NEXT:    Size: 96
+# CHECK-NEXT:    Link: 5
+# CHECK-NEXT:    Info: 1
+# CHECK-NEXT:    AddressAlignment: 8
+# CHECK-NEXT:    EntrySize: 24
+# CHECK-NEXT:   }
+# CHECK-NEXT:   Section {
+# CHECK-NEXT:   Index: 2
 # CHECK-NEXT:   Name: .gnu.version (9)
 # CHECK-NEXT:   Type: SHT_GNU_versym (0x6FFFFFFF)
 # CHECK-NEXT:   Flags [ (0x2)
 # CHECK-NEXT:     SHF_ALLOC (0x2)
 # CHECK-NEXT:   ]
 # CHECK-NEXT:   Address: 0x10228
 # CHECK-NEXT:   Offset: 0x228
 # CHECK-NEXT:   Size: 8
-# CHECK-NEXT:   Link: 0
+# CHECK-NEXT:   Link: 1
 # CHECK-NEXT:   Info: 0
 # CHECK-NEXT:   AddressAlignment: 2
 # CHECK-NEXT:   EntrySize: 2
 # CHECK-NEXT: }
 # CHECK-NEXT: Section {
 # CHECK-NEXT:   Index: 3
 # CHECK-NEXT:   Name: .gnu.version_r (22)
 # CHECK-NEXT:   Type: SHT_GNU_verneed (0x6FFFFFFE)