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

[jhenderson]

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.

[grimar]

LGTM.

tools/yaml2obj/yaml2elf.cpp

868

This place looks a bit overcomplicated (seems requires refactoring), but it is not a problem of this patch.

[jhenderson]

I forgot to run all the test before putting this patch up for review, and llvm-readobj/demangle.test is failing in a non-trivial way, which I'll need to fix. It may result in a different change in yaml2obj going in first, to fix segment offsets that contain .dynstr and .dynsym based on my first look.

[grimar]

tools/yaml2obj/yaml2elf.cpp

868

I committed NFC in r355832. Open for comments.

[jhenderson]

tools/yaml2obj/yaml2elf.cpp

868

Seems reasonable. I'll rebase later, once I've fixed up the other issue. I've got some higher priority work to do first, so this may have to go onto the backburner for a week or two.

[grimar]

I posted https://reviews.llvm.org/D66532 on review to fix this.

[jhenderson]

Abandoned in favour of D66532.

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  0000000100000000 R_X86_64_NONE          0000000011223344 static
+# CHECK-NEXT: 0000000000000018  0000000200000000 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
+    Size: 32
+  - 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
+DynamicSymbols:
+  Global:
+    - Name: dynamic
+      Section: .data
+      Value: 0x12345678
+    - Name: both
+      Section: .data
+      Value: 0x87654321
+Symbols:
+  Global:
+    - Name: static
+      Section: .data
+      Value: 0x11223344
+    - Name: both
+      Section: .data
+      Value: 0x88776655

tools/yaml2obj/yaml2elf.cpp

class ELFState {
   /// The future ".shstrtab" section.
   StringTableBuilder DotShStrtab{StringTableBuilder::ELF};
 
   /// The future ".dynstr" section.
   StringTableBuilder DotDynstr{StringTableBuilder::ELF};
 
   NameToIdxMap SN2I;
   NameToIdxMap SymN2I;
+  NameToIdxMap DynSymN2I;
   const ELFYAML::Object &Doc;
 
   bool buildSectionIndex();
   bool buildSymbolIndex(std::size_t &StartIndex,
+                        NameToIdxMap &IndexMap,
                         const std::vector<ELFYAML::Symbol> &Symbols);
   void initELFHeader(Elf_Ehdr &Header);
   void initProgramHeaders(std::vector<Elf_Phdr> &PHeaders);
   bool initSectionHeaders(std::vector<Elf_Shdr> &SHeaders,
                           ContiguousBlobAccumulator &CBA);
   void initSymtabSectionHeader(Elf_Shdr &SHeader, SymtabType STType,
                                ContiguousBlobAccumulator &CBA);
   void initStrtabSectionHeader(Elf_Shdr &SHeader, StringRef Name,
assert((Section.Type == llvm::ELF::SHT_REL ||
           Section.Type == llvm::ELF::SHT_RELA) &&
          "Section type is not SHT_REL nor SHT_RELA");
 
   bool IsRela = Section.Type == llvm::ELF::SHT_RELA;
   SHeader.sh_entsize = IsRela ? sizeof(Elf_Rela) : sizeof(Elf_Rel);
   SHeader.sh_size = SHeader.sh_entsize * Section.Relocations.size();
 
   auto &OS = CBA.getOSAndAlignedOffset(SHeader.sh_offset, SHeader.sh_addralign);
+  const NameToIdxMap &SymIndexMap =
+      Section.Link == ".dynsym" ? DynSymN2I : SymN2I;
 
   for (const auto &Rel : Section.Relocations) {
     unsigned SymIdx = 0;
     // Some special relocation, R_ARM_v4BX for instance, does not have
     // an external reference.  So it ignores the return value of lookup()
     // here.
     if (Rel.Symbol)
-      SymN2I.lookup(*Rel.Symbol, SymIdx);
+      SymIndexMap.lookup(*Rel.Symbol, SymIdx);
 
     if (IsRela) {
       Elf_Rela REntry;
       zero(REntry);
       REntry.r_offset = Rel.Offset;
       REntry.r_addend = Rel.Addend;
       REntry.setSymbolAndType(SymIdx, Rel.Type, isMips64EL(Doc));
       OS.write((const char *)&REntry, sizeof(REntry));
template <class ELFT> bool ELFState<ELFT>::buildSectionIndex() {
 
   DotShStrtab.finalize();
   return true;
 }
 
 template <class ELFT>
 bool
 ELFState<ELFT>::buildSymbolIndex(std::size_t &StartIndex,
+                                 NameToIdxMap &IndexMap,
                                  const std::vector<ELFYAML::Symbol> &Symbols) {
   for (const auto &Sym : Symbols) {
     ++StartIndex;
     if (Sym.Name.empty())
       continue;
-    if (SymN2I.addName(Sym.Name, StartIndex)) {
+    if (IndexMap.addName(Sym.Name, StartIndex)) {
       WithColor::error() << "Repeated symbol name: '" << Sym.Name << "'.\n";
       return false;
     }
   }
   return true;
 }
 
 template <class ELFT> void ELFState<ELFT>::finalizeStrings() {
int ELFState<ELFT>::writeELF(raw_ostream &OS, const ELFYAML::Object &Doc) {
   // 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;
 
   std::size_t StartSymIndex = 0;
-  if (!State.buildSymbolIndex(StartSymIndex, Doc.Symbols.Local) ||
-      !State.buildSymbolIndex(StartSymIndex, Doc.Symbols.Global) ||
-      !State.buildSymbolIndex(StartSymIndex, Doc.Symbols.Weak))
+  if (!State.buildSymbolIndex(StartSymIndex, State.SymN2I, Doc.Symbols.Local) ||
+      !State.buildSymbolIndex(StartSymIndex, State.SymN2I,
+                              Doc.Symbols.Global) ||
+      !State.buildSymbolIndex(StartSymIndex, State.SymN2I, Doc.Symbols.Weak))
+    return 1;
+  StartSymIndex = 0;
+  if (!State.buildSymbolIndex(StartSymIndex, State.DynSymN2I,
+                              Doc.DynamicSymbols.Local) ||
+      !State.buildSymbolIndex(StartSymIndex, State.DynSymN2I,
+                              Doc.DynamicSymbols.Global) ||
+      !State.buildSymbolIndex(StartSymIndex, State.DynSymN2I,
+                              Doc.DynamicSymbols.Weak))
     return 1;
 
   Elf_Ehdr Header;
   State.initELFHeader(Header);
 
   // TODO: Flesh out section header support.
 
   std::vector<Elf_Phdr> PHeaders;
   State.initProgramHeaders(PHeaders);