[llvm-objcopy] Add support for .dynamic, .dynsym, and .dynstr

[jakehehrlich]

Summary

This change adds support for sections involved in dynamic loading such as SHT_DYNAMIC, SHT_DYNSYM, and allocated string tables.

The two added binaries used for tests can be downloaded here and here

[jhenderson]

I take it yaml2obj cannot create dynamic section/symbol table information? If it could, it would mean that no pre-built binaries would be needed for the tests.

A thought on the duplication of DynamicSymbolTableSection and DynamicSection: what would you think of having a shared class called something like "SectionWithStringTable", and then implementing these two section types as inheriting from that one. The only logic needed in DynamicSection/DynamicSymbolTableSection would then be the classof method.

test/tools/llvm-objcopy/dynsym.test

4

Please also check the section header data like in the other tests.

tools/llvm-objcopy/Object.cpp

405

Full stop.

414

Ditto. I'd also replace the last word "anyway" with "either".

422

DynamicSection not DynamicSymbolTableSection!

485

As with other cases, we need to check that the Link value is actually a valid section index, or we will get a crash. Since this is a recurring theme, perhaps Object should gain a getSection() method that does the error checking?

489

Ditto.

tools/llvm-objcopy/Object.h

197–200

As the DynamicSection class currently appears after this class, I think that either a) the comment should be moved to the second class to appear in order (i.e. DynamicSection), or b) the DynamicSection should be moved to before the DynamicSymbolTableSection. This means that the comment refers to something that a user reading top-down would have already read about.

[jakehehrlich]

In D36560#856314, @jhenderson wrote:

I take it yaml2obj cannot create dynamic section/symbol table information? If it could, it would mean that no pre-built binaries would be needed for the tests.

This is correct. It might not be so hard to add though. It supports just

A thought on the duplication of DynamicSymbolTableSection and DynamicSection: what would you think of having a shared class called something like "SectionWithStringTable", and then implementing these two section types as inheriting from that one. The only logic needed in DynamicSection/DynamicSymbolTableSection would then be the classof method.

I like that. I'll upload that in the next diff when I fix these other changes.

[jakehehrlich]

1. Added and used getSection methods to help with problem of constantly needed to access and error check sections
2. Used SectionWithStrTab change recomended
3. Fixed typos

[jhenderson]

tools/llvm-objcopy/Object.cpp

397

speical -> special

tools/llvm-objcopy/Object.h

210

Has this been clang-formatted? The line looks a bit long.

243

Not really important, but I'm not a massive fan of this name - maybe getSectionOfType?

[jakehehrlich]

1. Changed name of method to getSectionOfType
2. Fixed typos

[jhenderson]

LGTM with the suggested argument name changes.

tools/llvm-objcopy/Object.cpp

380

The last argument name now needs changing both here and in the header. It may be wise to change ErrMsg as well to clearly disambiguate the two. Suggestions: ErrMsg -> IndexErrMsg; CastErrMsg -> TypeErrMsg.

test/tools/llvm-objcopy/dynamic.test

+# RUN: llvm-objcopy %p/Inputs/dynamic.so %t
+# RUN: llvm-readobj -dynamic-table %t | FileCheck %s
+# RUN: llvm-readobj -sections %t | FileCheck -check-prefix=LINK %s
+
+#CHECK: DynamicSection [
+#CHECK-NEXT:  Tag                Type                 Name/Value
+#CHECK-NEXT:  0x0000000000000006 SYMTAB               0x1C8
+#CHECK-NEXT:  0x000000000000000B SYMENT               24
+#CHECK-NEXT:  0x0000000000000005 STRTAB               0x210
+#CHECK-NEXT:  0x000000000000000A STRSZ                5
+#CHECK-NEXT:  0x0000000000000004 HASH                 0x1F8
+#CHECK-NEXT:  0x0000000000000000 NULL                 0x0
+#CHECK-NEXT:]
+
+#LINK:         Index: 3
+#LINK-NEXT:    Name: .dynstr
+
+#LINK:         Name: .dynamic
+#LINK-NEXT:    Type: SHT_DYNAMIC
+#LINK-NEXT:    Flags [
+#LINK-NEXT:      SHF_ALLOC
+#LINK-NEXT:      SHF_WRITE
+#LINK-NEXT:    ]
+#LINK-NEXT:    Address:
+#LINK-NEXT:    Offset:
+#LINK-NEXT:    Size:
+#LINK-NEXT:    Link: 3

test/tools/llvm-objcopy/dynstr.test

+# RUN: yaml2obj %s > %t
+# RUN: llvm-objcopy %t %t2
+# RUN: llvm-readobj -sections -section-data %t2 | FileCheck %s
+
+!ELF
+FileHeader:
+  Class:           ELFCLASS64
+  Data:            ELFDATA2LSB
+  Type:            ET_EXEC
+  Machine:         EM_X86_64
+Sections:
+  - Name:            .dynstr
+    Type:            SHT_STRTAB
+    Flags:           [ SHF_ALLOC ]
+    Address:         0x1000
+    Content:         "002EDEADBEEF002EBAADF00D00"
+
+#CHECK:         Name: .dynstr
+#CHECK-NEXT:    Type: SHT_STRTAB
+#CHECK-NEXT:    Flags [
+#CHECK-NEXT:      SHF_ALLOC
+#CHECK-NEXT:    ]
+#CHECK-NEXT:    Address: 0x1000
+#CHECK-NEXT:    Offset:
+#CHECK-NEXT:    Size: 13
+#CHECK-NEXT:    Link:
+#CHECK-NEXT:    Info:
+#CHECK-NEXT:    AddressAlignment:
+#CHECK-NEXT:    EntrySize:
+#CHECK-NEXT:    SectionData (
+#CHECK-NEXT:      0000: 002EDEAD BEEF002E BAADF00D 00
+#CHECK-NEXT:    )

test/tools/llvm-objcopy/dynsym.test

+# RUN: llvm-objcopy %p/Inputs/dynsym.so %t
+# RUN: llvm-readobj -dyn-symbols %t | FileCheck %s
+# RUN: llvm-readobj -sections %t | FileCheck -check-prefix=LINK %s
+
+#LINK:         Name: .dynsym
+#LINK-NEXT:    Type: SHT_DYNSYM
+#LINK-NEXT:    Flags [
+#LINK-NEXT:      SHF_ALLOC
+#LINK-NEXT:    ]
+#LINK-NEXT:    Address:
+#LINK-NEXT:    Offset:
+#LINK-NEXT:    Size:
+#LINK-NEXT:    Link: 3
+
+#LINK:         Index: 3
+#LINK-NEXT:    Name: .dynstr
+
+#CHECK: DynamicSymbols [
+#CHECK-NEXT:  Symbol {
+#CHECK-NEXT:    Name: @ (0)
+#CHECK-NEXT:    Value: 0x0
+#CHECK-NEXT:    Size: 0
+#CHECK-NEXT:    Binding: Local
+#CHECK-NEXT:    Type: None (0x0)
+#CHECK-NEXT:    Other: 0
+#CHECK-NEXT:    Section: Undefined (0x0)
+#CHECK-NEXT:  }
+#CHECK-NEXT:  Symbol {
+#CHECK-NEXT:    Name: bang@ (1)
+#CHECK-NEXT:    Value: 0x4000
+#CHECK-NEXT:    Size: 8
+#CHECK-NEXT:    Binding: Global (0x1)
+#CHECK-NEXT:    Type: Object (0x1)
+#CHECK-NEXT:    Other: 0
+#CHECK-NEXT:    Section: .bss (0x7)
+#CHECK-NEXT:  }
+#CHECK-NEXT:  Symbol {
+#CHECK-NEXT:    Name: bar@ (6)
+#CHECK-NEXT:    Value: 0x1001
+#CHECK-NEXT:    Size: 0
+#CHECK-NEXT:    Binding: Global (0x1)
+#CHECK-NEXT:    Type: Function (0x2)
+#CHECK-NEXT:    Other: 0
+#CHECK-NEXT:    Section: .text (0x4)
+#CHECK-NEXT:  }
+#CHECK-NEXT:  Symbol {
+#CHECK-NEXT:    Name: baz@ (10)
+#CHECK-NEXT:    Value: 0x2000
+#CHECK-NEXT:    Size: 0
+#CHECK-NEXT:    Binding: Global (0x1)
+#CHECK-NEXT:    Type: Object (0x1)
+#CHECK-NEXT:    Other: 0
+#CHECK-NEXT:    Section: .data (0x5)
+#CHECK-NEXT:  }
+#CHECK-NEXT:  Symbol {
+#CHECK-NEXT:    Name: foo@ (14)
+#CHECK-NEXT:    Value: 0x1000
+#CHECK-NEXT:    Size: 0
+#CHECK-NEXT:    Binding: Global (0x1)
+#CHECK-NEXT:    Type: Function (0x2)
+#CHECK-NEXT:    Other: 0
+#CHECK-NEXT:    Section: .text (0x4)
+#CHECK-NEXT:  }
+#CHECK-NEXT:]

tools/llvm-objcopy/Object.h

public:
   void addRelocation(Relocation Rel) { Relocations.push_back(Rel); }
   void finalize() override;
   void writeSection(llvm::FileOutputBuffer &Out) const override;
   static bool classof(const SectionBase *S) {
     return S->Type == llvm::ELF::SHT_REL || S->Type == llvm::ELF::SHT_RELA;
   }
 };
 
+class SectionWithStrTab : public Section {
+private:
+  StringTableSection *StrTab;
+
+public:
+  SectionWithStrTab(llvm::ArrayRef<uint8_t> Data) : Section(Data) {}
+  void setStrTab(StringTableSection *StringTable) { StrTab = StringTable; }
+  void finalize() override;
+  static bool classof(const SectionBase *S);
+};
+
+class DynamicSymbolTableSection : public SectionWithStrTab {
+public:
+  DynamicSymbolTableSection(llvm::ArrayRef<uint8_t> Data) : SectionWithStrTab(Data) {}
+  static bool classof(const SectionBase *S) {
+    return S->Type == llvm::ELF::SHT_DYNSYM;
+  }
+};
+
+class DynamicSection : public SectionWithStrTab {
+public:
+  DynamicSection(llvm::ArrayRef<uint8_t> Data) : SectionWithStrTab(Data) {}
+  static bool classof(const SectionBase *S) {
+    return S->Type == llvm::ELF::SHT_DYNAMIC;
+  }
+};
+
 template <class ELFT> class Object {
 private:
   typedef std::unique_ptr<SectionBase> SecPtr;
   typedef std::unique_ptr<Segment> SegPtr;
 
   typedef typename ELFT::Shdr Elf_Shdr;
   typedef typename ELFT::Ehdr Elf_Ehdr;
   typedef typename ELFT::Phdr Elf_Phdr;
 
   void initSymbolTable(const llvm::object::ELFFile<ELFT> &ElfFile,
                        SymbolTableSection *SymTab);
   SecPtr makeSection(const llvm::object::ELFFile<ELFT> &ElfFile,
                      const Elf_Shdr &Shdr);
   void readProgramHeaders(const llvm::object::ELFFile<ELFT> &ElfFile);
   void readSectionHeaders(const llvm::object::ELFFile<ELFT> &ElfFile);
 
+  SectionBase *getSection(uint16_t Index, llvm::Twine ErrMsg);
+
+  template <class T>
+  T *getSectionCast(uint16_t Index, llvm::Twine ErrMsg, llvm::Twine CastErrMsg);
+
 protected:
   StringTableSection *SectionNames;
   SymbolTableSection *SymbolTable;
   std::vector<SecPtr> Sections;
   std::vector<SegPtr> Segments;
 
   void writeHeader(llvm::FileOutputBuffer &Out) const;
   void writeProgramHeaders(llvm::FileOutputBuffer &Out) const;

tools/llvm-objcopy/Object.cpp

 void RelocationSection<ELFT>::writeSection(llvm::FileOutputBuffer &Out) const {
   uint8_t *Buf = Out.getBufferStart() + Offset;
   if (Type == SHT_REL)
     writeRel(reinterpret_cast<Elf_Rel *>(Buf));
   else
     writeRel(reinterpret_cast<Elf_Rela *>(Buf));
 }
 
+bool SectionWithStrTab::classof(const SectionBase *S) {
+  return isa<DynamicSymbolTableSection>(S) || isa<DynamicSection>(S);
+}
+
+void SectionWithStrTab::finalize() { this->Link = StrTab->Index; }
+
 // Returns true IFF a section is wholly inside the range of a segment
 static bool sectionWithinSegment(const SectionBase &Section,
                                  const Segment &Segment) {
   // If a section is empty it should be treated like it has a size of 1. This is
   // to clarify the case when an empty section lies on a boundary between two
   // segments and ensures that the section "belongs" to the second segment and
   // not the first.
   uint64_t SecSize = Section.Size ? Section.Size : 1;
void Object<ELFT>::readProgramHeaders(const ELFFile<ELFT> &ElfFile) {
   }
 }
 
 template <class ELFT>
 void Object<ELFT>::initSymbolTable(const llvm::object::ELFFile<ELFT> &ElfFile,
                                    SymbolTableSection *SymTab) {
 
   SymTab->Size = 0;
-  if (SymbolTable->Link - 1 >= Sections.size())
-    error("Symbol table has link index of " + Twine(SymbolTable->Link) +
-          " which is not a valid index");
-
-  if (auto StrTab =
-          dyn_cast<StringTableSection>(Sections[SymbolTable->Link - 1].get()))
-    SymTab->setStrTab(StrTab);
-  else
-    error("Symbol table has link index of " + Twine(SymbolTable->Link) +
-          "which is not a string table");
+
+  SymTab->setStrTab(getSectionCast<StringTableSection>(
+      SymbolTable->Link,
+      "Symbol table has link index of " + Twine(SymbolTable->Link) +
+          " which is not a valid index",
+      "Symbol table has link index of " + Twine(SymbolTable->Link) +
+          " which is not a string table"));
 
   const Elf_Shdr &Shdr = *unwrapOrError(ElfFile.getSection(SymTab->Index));
   StringRef StrTabData = unwrapOrError(ElfFile.getStringTableForSymtab(Shdr));
 
   for (const auto &Sym : unwrapOrError(ElfFile.symbols(&Shdr))) {
     SectionBase *DefSection = nullptr;
     StringRef Name = unwrapOrError(Sym.getName(StrTabData));
+
     if (Sym.st_shndx >= SHN_LORESERVE) {
-      if (!isValidReservedSectionIndex(Sym.st_shndx)) {
+      if (!isValidReservedSectionIndex(Sym.st_shndx))
         error(
             "Symbol '" + Name +
             "' has unsupported value greater than or equal to SHN_LORESERVE: " +
             Twine(Sym.st_shndx));
-      }
-    } else if (Sym.st_shndx != SHN_UNDEF) {
-      if (Sym.st_shndx >= Sections.size())
-        error("Symbol '" + Name +
-              "' is defined in invalid section with index " +
+    } else if (Sym.st_shndx != SHN_UNDEF)
+      DefSection = getSection(
+          Sym.st_shndx,
+          "Symbol '" + Name + "' is defined in invalid section with index " +
               Twine(Sym.st_shndx));
-      DefSection = Sections[Sym.st_shndx - 1].get();
-    }
+
     SymTab->addSymbol(Name, Sym.getBinding(), Sym.getType(), DefSection,
                       Sym.getValue(), Sym.st_shndx, Sym.st_size);
   }
 }
 
 template <class ELFT>
 static void getAddend(uint64_t &ToSet, const Elf_Rel_Impl<ELFT, false> &Rel) {}
 
for (const auto &Rel : RelRange) {
     getAddend(ToAdd.Addend, Rel);
     ToAdd.Type = Rel.getType(false);
     ToAdd.RelocSymbol = SymbolTable->getSymbolByIndex(Rel.getSymbol(false));
     Relocs->addRelocation(ToAdd);
   }
 }
 
 template <class ELFT>
+SectionBase *Object<ELFT>::getSection(uint16_t Index, Twine ErrMsg) {
+  if (Index == SHN_UNDEF || Index > Sections.size())
+    error(ErrMsg);
+  return Sections[Index - 1].get();
+}
+
+template <class ELFT>
+template <class T>
+T *Object<ELFT>::getSectionCast(uint16_t Index, Twine ErrMsg,
+                                Twine CastErrMsg) {
+  if (T *TSec = llvm::dyn_cast<T>(getSection(Index, ErrMsg)))
+    return TSec;
+  error(CastErrMsg);
+}
+
+template <class ELFT>
 std::unique_ptr<SectionBase>
 Object<ELFT>::makeSection(const llvm::object::ELFFile<ELFT> &ElfFile,
                           const Elf_Shdr &Shdr) {
   ArrayRef<uint8_t> Data;
   switch (Shdr.sh_type) {
   case SHT_REL:
   case SHT_RELA:
     return llvm::make_unique<RelocationSection<ELFT>>();
   case SHT_STRTAB:
+    // If a string table is allocated we don't want to mess with it. That would
+    // mean altering the memory image. There are no speical link types or
+    // anything so we can just use a Section.
+    if (Shdr.sh_flags & SHF_ALLOC) {
+      Data = unwrapOrError(ElfFile.getSectionContents(&Shdr));
+      return llvm::make_unique<Section>(Data);
+    }
     return llvm::make_unique<StringTableSection>();
+  case SHT_HASH:
+  case SHT_GNU_HASH:
+    // Hash tables should refer to SHT_DYNSYM which we're not going to change.
+    // Because of this we don't need to mess with the hash tables either.
+    Data = unwrapOrError(ElfFile.getSectionContents(&Shdr));
+    return llvm::make_unique<Section>(Data);
+  case SHT_DYNSYM:
+    Data = unwrapOrError(ElfFile.getSectionContents(&Shdr));
+    return llvm::make_unique<DynamicSymbolTableSection>(Data);
+  case SHT_DYNAMIC:
+    Data = unwrapOrError(ElfFile.getSectionContents(&Shdr));
+    return llvm::make_unique<DynamicSection>(Data);
   case SHT_SYMTAB: {
     auto SymTab = llvm::make_unique<SymbolTableSectionImpl<ELFT>>();
     SymbolTable = SymTab.get();
     return std::move(SymTab);
   }
   case SHT_NOBITS:
     return llvm::make_unique<Section>(Data);
   default:
     Data = unwrapOrError(ElfFile.getSectionContents(&Shdr));
     return llvm::make_unique<Section>(Data);
   }
 }
 
 template <class ELFT>
 void Object<ELFT>::readSectionHeaders(const ELFFile<ELFT> &ElfFile) {
void Object<ELFT>::readSectionHeaders(const ELFFile<ELFT> &ElfFile) {
   if (SymbolTable)
     initSymbolTable(ElfFile, SymbolTable);
 
   // Now that all sections and symbols have been added we can add
   // relocations that reference symbols and set the link and info fields for
   // relocation sections.
   for (auto &Section : Sections) {
     if (auto RelSec = dyn_cast<RelocationSection<ELFT>>(Section.get())) {
-      if (RelSec->Link - 1 >= Sections.size() || RelSec->Link == 0) {
-        error("Link field value " + Twine(RelSec->Link) + " in section " +
-              RelSec->Name + " is invalid");
-      }
-      if (RelSec->Info - 1 >= Sections.size() || RelSec->Info == 0) {
-        error("Info field value " + Twine(RelSec->Link) + " in section " +
-              RelSec->Name + " is invalid");
-      }
-      auto SymTab =
-          dyn_cast<SymbolTableSection>(Sections[RelSec->Link - 1].get());
-      if (SymTab == nullptr) {
-        error("Link field of relocation section " + RelSec->Name +
-              " is not a symbol table");
-      }
+
+      auto SymTab = getSectionCast<SymbolTableSection>(
+          RelSec->Link,
+          "Link field value " + Twine(RelSec->Link) + " in section " +
+              RelSec->Name + " is invalid",
+          "Link field value " + Twine(RelSec->Link) + " in section " +
+              RelSec->Name + " is not a symbol table");
       RelSec->setSymTab(SymTab);
-      RelSec->setSection(Sections[RelSec->Info - 1].get());
+
+      RelSec->setSection(getSection(RelSec->Info,
+                                    "Info field value " + Twine(RelSec->Link) +
+                                        " in section " + RelSec->Name +
+                                        " is invalid"));
+
       auto Shdr = unwrapOrError(ElfFile.sections()).begin() + RelSec->Index;
       if (RelSec->Type == SHT_REL)
         initRelocations(RelSec, SymTab, unwrapOrError(ElfFile.rels(Shdr)));
       else
         initRelocations(RelSec, SymTab, unwrapOrError(ElfFile.relas(Shdr)));
     }
+
+    if (auto Sec = dyn_cast<SectionWithStrTab>(Section.get())) {
+      Sec->setStrTab(getSectionCast<StringTableSection>(
+          Sec->Link,
+          "Link field value " + Twine(Sec->Link) + " in section " + Sec->Name +
+              " is invalid",
+          "Link field value " + Twine(Sec->Link) + " in section " + Sec->Name +
+              " is not a string table"));
+    }
   }
 }
 
 template <class ELFT> Object<ELFT>::Object(const ELFObjectFile<ELFT> &Obj) {
   const auto &ElfFile = *Obj.getELFFile();
   const auto &Ehdr = *ElfFile.getHeader();
 
   std::copy(Ehdr.e_ident, Ehdr.e_ident + 16, Ident);
   Type = Ehdr.e_type;
   Machine = Ehdr.e_machine;
   Version = Ehdr.e_version;
   Entry = Ehdr.e_entry;
   Flags = Ehdr.e_flags;
 
   readSectionHeaders(ElfFile);
   readProgramHeaders(ElfFile);
 
-  SectionNames =
-      dyn_cast<StringTableSection>(Sections[Ehdr.e_shstrndx - 1].get());
+  SectionNames = getSectionCast<StringTableSection>(
+      Ehdr.e_shstrndx,
+      "e_shstrndx field value " + Twine(Ehdr.e_shstrndx) + " in elf header " +
+          " is invalid",
+      "e_shstrndx field value " + Twine(Ehdr.e_shstrndx) + " in elf header " +
+          " is not a string table");
 }
 
 template <class ELFT>
 void Object<ELFT>::writeHeader(FileOutputBuffer &Out) const {
   uint8_t *Buf = Out.getBufferStart();
   Elf_Ehdr &Ehdr = *reinterpret_cast<Elf_Ehdr *>(Buf);
   std::copy(Ident, Ident + 16, Ehdr.e_ident);
   Ehdr.e_type = Type;