Allow yaml2obj to order implicit sections for ELF

[kastiglione]

Summary

This change allows yaml input to control the order of implicitly added sections
(.symtab, .strtab, .shstrtab). The order is controlled by adding a
placeholder section of the given name to the Sections field.

This change is to support changes in D39582, where it is desirable to control
the location of the .dynsym section.

[jakehehrlich]

Overall looks good to me. I just have one nit about the lookup function.

tools/yaml2obj/yaml2elf.cpp

78 ↗

(On Diff #121936)

I think you should use an Optional here instead of returning a maximal value. Also can you implement one of the lookup functions in terms of the other to dedup code?

[kastiglione]

Rename lookup to get and make it assert when key not found

[kastiglione]

tools/yaml2obj/yaml2elf.cpp

78 ↗

(On Diff #121936)

I deduped, and instead of returning an optional, I added an assert. The intended semantics of this function is it's called only for inputs known to exist. The result should never be a none value. Thoughts?

[jakehehrlich]

LGTM

[kastiglione]

llvm/trunk/tools/yaml2obj/yaml2elf.cpp

79–81

The uninitialized Idx broke some builds. What's the expedient way to fix this, another diff?

[kastiglione]

llvm/trunk/tools/yaml2obj/yaml2elf.cpp

79–81

Addressed in r317626.

llvm/trunk/lib/ObjectYAML/ELFYAML.cpp

 
 void ScalarEnumerationTraits<ELFYAML::ELF_SHT>::enumeration(
     IO &IO, ELFYAML::ELF_SHT &Value) {
   const auto *Object = static_cast<ELFYAML::Object *>(IO.getContext());
   assert(Object && "The IO context is not initialized");
 #define ECase(X) IO.enumCase(Value, #X, ELF::X)
   ECase(SHT_NULL);
   ECase(SHT_PROGBITS);
-  // No SHT_SYMTAB. Use the top-level `Symbols` key instead.
+  ECase(SHT_SYMTAB);
   // FIXME: Issue a diagnostic with this information.
   ECase(SHT_STRTAB);
   ECase(SHT_RELA);
   ECase(SHT_HASH);
   ECase(SHT_DYNAMIC);
   ECase(SHT_NOTE);
   ECase(SHT_NOBITS);
   ECase(SHT_REL);

llvm/trunk/test/tools/yaml2obj/section-ordering.yaml

+# Ensures that implicitly added sections can be ordered within Sections.
+# RUN: yaml2obj %s -o %t
+# RUN: llvm-readobj -sections %t | FileCheck %s
+
+!ELF
+FileHeader:
+  Class:           ELFCLASS64
+  Data:            ELFDATA2LSB
+  Type:            ET_EXEC
+  Machine:         EM_X86_64
+Sections:
+  - Name:            .text
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC, SHF_EXECINSTR ]
+  - Name:            .symtab
+    Type:            SHT_SYMTAB
+  - Name:            .data
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC, SHF_WRITE ]
+  - Name:            .shstrtab
+    Type:            SHT_STRTAB
+  - Name:            .strtab
+    Type:            SHT_STRTAB
+
+# CHECK: Name: .text
+# CHECK: Name: .symtab
+# CHECK: Name: .data
+# CHECK: Name: .shstrtab
+# CHECK: Name: .strtab

llvm/trunk/tools/yaml2obj/yaml2elf.cpp

public:
   /// \returns true if name is not present in the map
   bool lookup(StringRef Name, unsigned &Idx) const {
     StringMap<int>::const_iterator I = Map.find(Name);
     if (I == Map.end())
       return true;
     Idx = I->getValue();
     return false;
   }
+  /// asserts if name is not present in the map
+  unsigned get(StringRef Name) const {
+    unsigned Idx;
+    assert(!lookup(Name, Idx) && "Expected section not found in index");
+    return Idx;
+  }
+  unsigned size() const { return Map.size(); }
 };
 } // end anonymous namespace
 
 template <class T>
 static size_t arrayDataSize(ArrayRef<T> A) {
   return A.size() * sizeof(T);
 }
 
bool writeSectionContent(Elf_Shdr &SHeader,
                            ContiguousBlobAccumulator &CBA);
   bool writeSectionContent(Elf_Shdr &SHeader, const ELFYAML::Group &Group,
                            ContiguousBlobAccumulator &CBA);
   bool writeSectionContent(Elf_Shdr &SHeader,
                            const ELFYAML::MipsABIFlags &Section,
                            ContiguousBlobAccumulator &CBA);
 
   // - SHT_NULL entry (placed first, i.e. 0'th entry)
-  // - symbol table (.symtab) (placed third to last)
-  // - string table (.strtab) (placed second to last)
-  // - section header string table (.shstrtab) (placed last)
-  unsigned getDotSymTabSecNo() const { return Doc.Sections.size() + 1; }
-  unsigned getDotStrTabSecNo() const { return Doc.Sections.size() + 2; }
-  unsigned getDotShStrTabSecNo() const { return Doc.Sections.size() + 3; }
-  unsigned getSectionCount() const { return Doc.Sections.size() + 4; }
+  // - symbol table (.symtab) (defaults to third to last)
+  // - string table (.strtab) (defaults to second to last)
+  // - section header string table (.shstrtab) (defaults to last)
+  unsigned getDotSymTabSecNo() const { return SN2I.get(".symtab"); }
+  unsigned getDotStrTabSecNo() const { return SN2I.get(".strtab"); }
+  unsigned getDotShStrTabSecNo() const { return SN2I.get(".shstrtab"); }
+  unsigned getSectionCount() const { return SN2I.size() + 1; }
 
   ELFState(const ELFYAML::Object &D) : Doc(D) {}
 
 public:
   static int writeELF(raw_ostream &OS, const ELFYAML::Object &Doc);
 };
+
+static const char * const ImplicitSecNames[] = {".symtab", ".strtab", ".shstrtab"};
 } // end anonymous namespace
 
 template <class ELFT>
 void ELFState<ELFT>::initELFHeader(Elf_Ehdr &Header) {
   using namespace llvm::ELF;
   zero(Header);
   Header.e_ident[EI_MAG0] = 0x7f;
   Header.e_ident[EI_MAG1] = 'E';

 bool ELFState<ELFT>::initSectionHeaders(std::vector<Elf_Shdr> &SHeaders,
                                         ContiguousBlobAccumulator &CBA) {
   // Ensure SHN_UNDEF entry is present. An all-zero section header is a
   // valid SHN_UNDEF entry since SHT_NULL == 0.
   Elf_Shdr SHeader;
   zero(SHeader);
   SHeaders.push_back(SHeader);
 
-  for (const auto &Sec : Doc.Sections)
-    DotShStrtab.add(Sec->Name);
-  DotShStrtab.finalize();
-
   for (const auto &Sec : Doc.Sections) {
     zero(SHeader);
     SHeader.sh_name = DotShStrtab.getOffset(Sec->Name);
     SHeader.sh_type = Sec->Type;
     SHeader.sh_flags = Sec->Flags;
     SHeader.sh_addr = Sec->Address;
     SHeader.sh_addralign = Sec->AddressAlign;
 
bool ELFState<ELFT>::writeSectionContent(Elf_Shdr &SHeader,
   Flags.flags1 = Section.Flags1;
   Flags.flags2 = Section.Flags2;
   OS.write((const char *)&Flags, sizeof(Flags));
 
   return true;
 }
 
 template <class ELFT> bool ELFState<ELFT>::buildSectionIndex() {
-  SN2I.addName(".symtab", getDotSymTabSecNo());
-  SN2I.addName(".strtab", getDotStrTabSecNo());
-  SN2I.addName(".shstrtab", getDotShStrTabSecNo());
-
   for (unsigned i = 0, e = Doc.Sections.size(); i != e; ++i) {
     StringRef Name = Doc.Sections[i]->Name;
     if (Name.empty())
       continue;
     // "+ 1" to take into account the SHT_NULL entry.
     if (SN2I.addName(Name, i + 1)) {
       errs() << "error: Repeated section name: '" << Name
              << "' at YAML section number " << i << ".\n";
       return false;
     }
+    DotShStrtab.add(Name);
   }
+
+  auto SecNo = 1 + Doc.Sections.size();
+  // Add special sections after input sections, if necessary.
+  for (const auto &Name : ImplicitSecNames)
+    if (!SN2I.addName(Name, SecNo)) {
+      // Account for this section, since it wasn't in the Doc
+      ++SecNo;
+      DotShStrtab.add(Name);
+    }
+
+  DotShStrtab.finalize();
   return true;
 }
 
 template <class ELFT>
 bool
 ELFState<ELFT>::buildSymbolIndex(std::size_t &StartIndex,
                                  const std::vector<ELFYAML::Symbol> &Symbols) {
   for (const auto &Sym : Symbols) {
int ELFState<ELFT>::writeELF(raw_ostream &OS, const ELFYAML::Object &Doc) {
 
   // XXX: This offset is tightly coupled with the order that we write
   // things to `OS`.
   const size_t SectionContentBeginOffset = Header.e_ehsize +
                                            Header.e_phentsize * Header.e_phnum +
                                            Header.e_shentsize * Header.e_shnum;
   ContiguousBlobAccumulator CBA(SectionContentBeginOffset);
 
-  // Doc might not contain .symtab, .strtab and .shstrtab sections,
-  // but we will emit them, so make sure to add them to ShStrTabSHeader.
-  State.DotShStrtab.add(".symtab");
-  State.DotShStrtab.add(".strtab");
-  State.DotShStrtab.add(".shstrtab");
-
   std::vector<Elf_Shdr> SHeaders;
+  SHeaders.reserve(State.SN2I.size());
   if(!State.initSectionHeaders(SHeaders, CBA))
     return 1;
 
-  // .symtab section.
-  Elf_Shdr SymtabSHeader;
-  State.initSymtabSectionHeader(SymtabSHeader, CBA);
-  SHeaders.push_back(SymtabSHeader);
-
-  // .strtab string table header.
-  Elf_Shdr DotStrTabSHeader;
-  State.initStrtabSectionHeader(DotStrTabSHeader, ".strtab", State.DotStrtab,
-                                CBA);
-  SHeaders.push_back(DotStrTabSHeader);
-
-  // .shstrtab string table header.
-  Elf_Shdr ShStrTabSHeader;
-  State.initStrtabSectionHeader(ShStrTabSHeader, ".shstrtab", State.DotShStrtab,
-                                CBA);
-  SHeaders.push_back(ShStrTabSHeader);
+  // Populate SHeaders with implicit sections not present in the Doc
+  for (const auto &Name : ImplicitSecNames)
+    if (State.SN2I.get(Name) >= SHeaders.size())
+      SHeaders.push_back({});
+
+  // Initialize the implicit sections
+  auto Index = State.SN2I.get(".symtab");
+  State.initSymtabSectionHeader(SHeaders[Index], CBA);
+  Index = State.SN2I.get(".strtab");
+  State.initStrtabSectionHeader(SHeaders[Index], ".strtab", State.DotStrtab, CBA);
+  Index = State.SN2I.get(".shstrtab");
+  State.initStrtabSectionHeader(SHeaders[Index], ".shstrtab", State.DotShStrtab, CBA);
 
   // Now we can decide segment offsets
   State.setProgramHeaderLayout(PHeaders, SHeaders);
 
   OS.write((const char *)&Header, sizeof(Header));
   writeArrayData(OS, makeArrayRef(PHeaders));
   writeArrayData(OS, makeArrayRef(SHeaders));
   CBA.writeBlobToStream(OS);