[yaml2obj] - Lookup relocation symbols in dynamic symbol when .dynsym referenced.

[grimar]

Summary

This fixes https://bugs.llvm.org/show_bug.cgi?id=40337.

Previously, it was always assumed that relocations referenced symbols in the static symbol table.
Now, if the Link field references a section called ".dynsym" it will look up these symbols
in the dynamic symbol table.

This patch is heavily based on D59097 by James Henderson

[grimar]

lib/ObjectYAML/ELFEmitter.cpp

1001 ↗

(On Diff #216389)

btw, since this code is now used for dynamic symbols and yaml2obj is
generally a tool that makes possible to produce the broken binaries,
perhaps I'd try to remove this restriction logic for both regular and dynamic symbols
in a follow up.

[jhenderson]

LGTM, with my suggested changes. Thanks for picking this up. I've been too busy doing other things to get back around to it.

lib/ObjectYAML/ELFEmitter.cpp

1001 ↗

(On Diff #216389)

Seems like a reasonable plan to me.

1015 ↗

(On Diff #216389)

Maybe rename this to buildSymbolIndexes, since there are more than one.

test/tools/yaml2obj/dynamic-relocations.yaml

62 ↗

(On Diff #216389)

Minor point, but it might make sense to have both first in one of the two tables so that it doesn't have the same index. If I'm not mistaken, in the currents situation, the unpatched yaml2obj would actually generate the right thing as things stand for both.

[grimar]

test/tools/yaml2obj/dynamic-relocations.yaml

62 ↗

(On Diff #216389)

Good point, I did that, thanks!

llvm/trunk/lib/ObjectYAML/ELFEmitter.cpp

template <class ELFT> class ELFState {
   /// The future ".shstrtab" section.
   StringTableBuilder DotShStrtab{StringTableBuilder::ELF};
 
   /// The future ".dynstr" section.
   StringTableBuilder DotDynstr{StringTableBuilder::ELF};
 
   NameToIdxMap SN2I;
   NameToIdxMap SymN2I;
+  NameToIdxMap DynSymN2I;
   ELFYAML::Object &Doc;
 
   bool buildSectionIndex();
-  bool buildSymbolIndex(ArrayRef<ELFYAML::Symbol> Symbols);
+  bool buildSymbolIndexes();
   void initELFHeader(Elf_Ehdr &Header);
   void initProgramHeaders(std::vector<Elf_Phdr> &PHeaders);
   bool initImplicitHeader(ELFState<ELFT> &State, ContiguousBlobAccumulator &CBA,
                           Elf_Shdr &Header, StringRef SecName,
                           ELFYAML::Section *YAMLSec);
   bool initSectionHeaders(ELFState<ELFT> &State,
                           std::vector<Elf_Shdr> &SHeaders,
                           ContiguousBlobAccumulator &CBA);
bool ELFState<ELFT>::writeSectionContent(
   unsigned Index = 0;
   if (!Section.RelocatableSec.empty() &&
       !convertSectionIndex(SN2I, Section.Name, Section.RelocatableSec, Index))
     return false;
   SHeader.sh_info = Index;
 
   auto &OS = CBA.getOSAndAlignedOffset(SHeader.sh_offset, SHeader.sh_addralign);
 
+  const NameToIdxMap &SymMap = Section.Link == ".dynsym" ? DynSymN2I : SymN2I;
   for (const auto &Rel : Section.Relocations) {
     unsigned SymIdx = 0;
     // If a relocation references a symbol, try to look one up in the symbol
     // table. If it is not there, treat the value as a symbol index.
-    if (Rel.Symbol && !SymN2I.lookup(*Rel.Symbol, SymIdx) &&
+    if (Rel.Symbol && !SymMap.lookup(*Rel.Symbol, SymIdx) &&
         !to_integer(*Rel.Symbol, SymIdx)) {
       WithColor::error() << "Unknown symbol referenced: '" << *Rel.Symbol
                          << "' at YAML section '" << Section.Name << "'.\n";
       return false;
     }
 
     if (IsRela) {
       Elf_Rela REntry;
if (!SN2I.addName(Name, I)) {
       return false;
     }
   }
 
   DotShStrtab.finalize();
   return true;
 }
 
-template <class ELFT>
-bool ELFState<ELFT>::buildSymbolIndex(ArrayRef<ELFYAML::Symbol> Symbols) {
+static bool buildSymbolsMap(ArrayRef<ELFYAML::Symbol> V, NameToIdxMap &Map) {
   bool GlobalSymbolSeen = false;
   std::size_t I = 0;
-  for (const auto &Sym : Symbols) {
+  for (const ELFYAML::Symbol &Sym : V) {
     ++I;
 
     StringRef Name = Sym.Name;
     if (Sym.Binding.value == ELF::STB_LOCAL && GlobalSymbolSeen) {
       WithColor::error() << "Local symbol '" + Name +
                                 "' after global in Symbols list.\n";
       return false;
     }
     if (Sym.Binding.value != ELF::STB_LOCAL)
       GlobalSymbolSeen = true;
 
-    if (!Name.empty() && !SymN2I.addName(Name, I)) {
+    if (!Name.empty() && !Map.addName(Name, I)) {
       WithColor::error() << "Repeated symbol name: '" << Name << "'.\n";
       return false;
     }
   }
   return true;
 }
 
+template <class ELFT> bool ELFState<ELFT>::buildSymbolIndexes() {
+  return buildSymbolsMap(Doc.Symbols, SymN2I) &&
+         buildSymbolsMap(Doc.DynamicSymbols, DynSymN2I);
+}
+
 template <class ELFT> void ELFState<ELFT>::finalizeStrings() {
   // Add the regular symbol names to .strtab section.
   for (const ELFYAML::Symbol &Sym : Doc.Symbols)
     DotStrtab.add(dropUniqueSuffix(Sym.Name));
   DotStrtab.finalize();
 
   // Add the dynamic symbol names to .dynstr section.
   for (const ELFYAML::Symbol &Sym : Doc.DynamicSymbols)

 int ELFState<ELFT>::writeELF(raw_ostream &OS, ELFYAML::Object &Doc) {
   ELFState<ELFT> State(Doc);
 
   // Finalize .strtab and .dynstr sections. We do that early because want to
   // finalize the string table builders before writing the content of the
   // sections that might want to use them.
   State.finalizeStrings();
 
-  if (!State.buildSectionIndex())
-    return 1;
-
-  if (!State.buildSymbolIndex(Doc.Symbols))
+  if (!State.buildSectionIndex() || !State.buildSymbolIndexes())
     return 1;
 
   Elf_Ehdr Header;
   State.initELFHeader(Header);
 
   // TODO: Flesh out section header support.
 
   std::vector<Elf_Phdr> PHeaders;

llvm/trunk/test/tools/llvm-readobj/demangle.test

- Name:         .rela.dyn
     Flags:        [ SHF_ALLOC ]
     Link:         .dynsym
     Info:         .text.foo
     Address:      0x200
     AddressAlign: 0x100
     EntSize:      0x18
     Relocations:
       - Offset: 0x10
-        ## FIXME: This should be a lookup in the corresponding symbol table, not necessarily the static symbol table.
-        ## See https://bugs.llvm.org/show_bug.cgi?id=40337.
-        Symbol: _Z3fooc
+        Symbol: _Z3fooi
         Type:   R_X86_64_PC32
         Addend: 0x4
   - Name:            .dynamic
     Type:            SHT_DYNAMIC
     Flags:           [ SHF_ALLOC ]
     Address:         0x0000000000001000
     Link:            .dynstr
     AddressAlign:    0x0000000000001000

llvm/trunk/test/tools/yaml2obj/dynamic-relocations.yaml

+## Show that yaml2obj uses the correct set of symbols for relocation sections
+## referencing the dynamic symbol table.
+
+# RUN: yaml2obj %s -o %t
+# RUN: llvm-readelf -r %t | FileCheck %s
+
+# CHECK:      Relocation section '.rela.dyn' at offset {{.*}} contains 2 entries:
+# CHECK-NEXT:     Offset             Info             Type      Symbol's Value  Symbol's Name
+# CHECK-NEXT: 0000000000000000  0000000100000000 R_X86_64_NONE 0000000012345678 dynamic
+# CHECK-NEXT: 0000000000000008  0000000200000000 R_X86_64_NONE 0000000087654321 both
+# CHECK-EMPTY:
+# CHECK-NEXT: Relocation section '.rela.data' at offset {{.*}} contains 2 entries:
+# CHECK-NEXT:     Offset             Info             Type      Symbol's Value  Symbol's Name
+# CHECK-NEXT: 0000000000000010  0000000200000000 R_X86_64_NONE 0000000011223344 static
+# CHECK-NEXT: 0000000000000018  0000000100000000 R_X86_64_NONE 0000000088776655 both
+
+!ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_DYN
+  Machine: EM_X86_64
+Sections:
+  - Name: .data
+    Type: SHT_PROGBITS
+  - Name: .rela.dyn
+    Type: SHT_REL
+    Link: .dynsym
+    Info: .data
+    Relocations:
+      - Offset: 0
+        Type: R_X86_64_NONE
+        Symbol: dynamic
+      - Offset: 8
+        Type: R_X86_64_NONE
+        Symbol: both
+  - Name: .rela.data
+    Type: SHT_REL
+    Link: .symtab
+    Info: .data
+    Relocations:
+      - Offset: 16
+        Type: R_X86_64_NONE
+        Symbol: static
+      - Offset: 24
+        Type: R_X86_64_NONE
+        Symbol: both
+Symbols:
+    - Name: both
+      Section: .data
+      Value: 0x88776655
+      Binding: STB_GLOBAL
+    - Name: static
+      Section: .data
+      Value: 0x11223344
+      Binding: STB_GLOBAL
+DynamicSymbols:
+    - Name: dynamic
+      Section: .data
+      Value: 0x12345678
+      Binding: STB_GLOBAL
+    - Name: both
+      Section: .data
+      Value: 0x87654321
+      Binding: STB_GLOBAL

llvm/trunk/test/tools/yaml2obj/dynamic-symbols.yaml

 # Ensures that implicitly added sections can be ordered within Sections.
 # RUN: yaml2obj %s -o %t
 # RUN: llvm-readobj --sections %t | FileCheck %s --check-prefix=SECTION
 # RUN: llvm-nm --dynamic %t | FileCheck %s --check-prefix=SYMBOL
 
 !ELF
 FileHeader:
   Class:           ELFCLASS64
   Data:            ELFDATA2LSB
   Type:            ET_DYN
   Machine:         EM_X86_64
 Sections:
   - Name: .data
     Type: SHT_PROGBITS
     Flags: [ SHF_ALLOC, SHF_WRITE ]
 DynamicSymbols:
+  - Name: dynlocal
+    Type: STT_OBJECT
+    Section: .data
+    Binding: STB_LOCAL
   - Name: dynglobal
     Type: STT_OBJECT
     Section: .data
     Binding: STB_GLOBAL
   - Name: dynweak
     Type: STT_OBJECT
     Section: .data
     Binding: STB_WEAK
-  - Name: dynlocal
-    Type: STT_OBJECT
-    Section: .data
-    Binding: STB_LOCAL
 
 # SECTION: Name: .dynsym
 # SECTION-NEXT: Type: SHT_DYNSYM
 # SECTION-NEXT: Flags
 # SECTION-NEXT: SHF_ALLOC
 # SECTION: Name: .dynstr
 # SECTION-NEXT: Type: SHT_STRTAB
 # SECTION-NEXT: Flags
 # SECTION-NEXT: SHF_ALLOC
 
+# SYMBOL-DAG: d dynlocal
 # SYMBOL-DAG: D dynglobal
 # SYMBOL-DAG: V dynweak
-# SYMBOL-DAG: d dynlocal