[obj2yaml] - Don't crash when an object has an empty symbol table.

[grimar]

Summary

Currently we crash when we have an object with SHT_SYMTAB/SHT_DYNSYM sections
of size 0.

With this patch instead of the crash we start to dump them properly.

[MaskRay]

According to http://www.sco.com/developers/gabi/latest/ch4.symtab.html , an empty SHT_SYMTAB/SHT_DYNSYM is non-conforming.

1. I think Optional<std::vector<ELFYAML::Symbol>> and various emplace is to get rid of Symbols: [] when the size is 0. Can we just print Symbols: [] along with Size: 0? From Size: 0 it should be clear there is no (normally implicit) index 0.
2. If yaml2obj is going to support multiple symbol tables, what will the display style be like? The use cases are narrow but raising this may helps think about how point 1 should be handled.

llvm/test/tools/obj2yaml/ELF/no-symtab.yaml

41

A symbol table without index 0 is non-conforming, according to http://www.sco.com/developers/gabi/latest/ch4.symtab.html

llvm/tools/obj2yaml/elf2yaml.cpp

230

It happens with the non-conforming empty case.

[grimar]

1. I think Optional<std::vector<ELFYAML::Symbol>> and various emplace is to get rid of Symbols: [] when the size is 0.

Right.

Can we just print Symbols: [] along with Size: 0? From Size: 0 it should be clear there is no (normally implicit) index 0.

No, because yaml2obj doesn't accept "Symbols" and "Size" together. "Symbols: []" means we have a zero entry actually.

If yaml2obj is going to support multiple symbol tables, what will the display style be like? The use cases are narrow but raising this may helps think about how point 1 should be handled.

I guess we will need to get rid of "Symbols" and "DynamicSymbols" tags and move these keys to corresponding sections.

[jhenderson]

llvm/tools/obj2yaml/elf2yaml.cpp

230

[grimar]

• Updated code and test case comments to address suggestions from reviewers.

[jhenderson]

LGTM.

llvm/test/tools/obj2yaml/ELF/no-symtab.yaml

 # SYMTAB-NEXT: ...
 
 --- !ELF
 FileHeader:
   Class: ELFCLASS64
   Data:  ELFDATA2LSB
   Type:  ET_DYN
 Symbols: []
+
+## Normally symbol tables can't be empty because of the first null
+## entry they contain. Check we dump them when they are.
+## Check we don't print "Symbols" and "DynamicSymbols" keys in this case.
+
+# RUN: yaml2obj --docnum=3 %s -o %t3
+# RUN: obj2yaml %t3 | FileCheck %s --check-prefix=EMPTY
+
+# EMPTY:      Sections:
+# EMPTY-NEXT:   - Name:    .symtab
+# EMPTY-NEXT:     Type:    SHT_SYMTAB
+# EMPTY-NEXT:     Link:    .strtab
+## TODO: we shouldn't dump the default "EntSize" value.
+# EMPTY-NEXT:     EntSize: 0x18
+# EMPTY-NEXT:     Size:    0x0
+# EMPTY-NEXT:   - Name:    .dynsym
+# EMPTY-NEXT:     Type:    SHT_DYNSYM
+# EMPTY-NEXT:     Flags:   [ SHF_ALLOC ]
+## TODO: we shouldn't dump the default "EntSize" value.
+# EMPTY-NEXT:     EntSize: 0x18
+# EMPTY-NEXT:     Size:    0x0
+# EMPTY-NEXT: ...
+
+--- !ELF
+FileHeader:
+  Class: ELFCLASS64
+  Data:  ELFDATA2LSB
+  Type:  ET_DYN
+Sections:
+  - Name: .symtab
+    Type: SHT_SYMTAB
+    Size: 0
+  - Name: .dynsym
+    Type: SHT_DYNSYM
+    Size: 0

llvm/tools/obj2yaml/elf2yaml.cpp

class ELFDumper {
 
   Expected<std::vector<ELFYAML::ProgramHeader>>
   dumpProgramHeaders(ArrayRef<std::unique_ptr<ELFYAML::Chunk>> Sections);
 
   Optional<DWARFYAML::Data>
   dumpDWARFSections(std::vector<std::unique_ptr<ELFYAML::Chunk>> &Sections);
 
   Error dumpSymbols(const Elf_Shdr *Symtab,
-                    std::vector<ELFYAML::Symbol> &Symbols);
+                    Optional<std::vector<ELFYAML::Symbol>> &Symbols);
   Error dumpSymbol(const Elf_Sym *Sym, const Elf_Shdr *SymTab,
                    StringRef StrTable, ELFYAML::Symbol &S);
   Expected<std::vector<std::unique_ptr<ELFYAML::Chunk>>> dumpSections();
   Error dumpCommonSection(const Elf_Shdr *Shdr, ELFYAML::Section &S);
   Error dumpCommonRelocationSection(const Elf_Shdr *Shdr,
                                     ELFYAML::RelocationSection &S);
   template <class RelT>
   Error dumpRelocation(const RelT *Rel, const Elf_Shdr *SymTab,
bool ELFDumper<ELFT>::shouldPrintSection(const ELFYAML::Section &S,
 
   // Normally we use "Symbols:" and "DynamicSymbols:" to describe contents of
   // symbol tables. We also build and emit corresponding string tables
   // implicitly. But sometimes it is important to preserve positions and virtual
   // addresses of allocatable sections, e.g. for creating program headers.
   // Generally we are trying to reduce noise in the YAML output. Because
   // of that we do not print non-allocatable versions of such sections and
   // assume they are placed at the end.
+  // We also dump symbol tables when the Size field is set. It happens when they
+  // are empty, what normally should not happen.
   if (S.Type == ELF::SHT_STRTAB || S.Type == ELF::SHT_SYMTAB ||
-      S.Type == ELF::SHT_DYNSYM)
-    return S.Flags.getValueOr(ELFYAML::ELF_SHF(0)) & ELF::SHF_ALLOC;
+      S.Type == ELF::SHT_DYNSYM) {
+    return S.Size || S.Flags.getValueOr(ELFYAML::ELF_SHF(0)) & ELF::SHF_ALLOC;
+  }
 
   return true;
 }
 
 template <class ELFT>
 static void dumpSectionOffsets(const typename ELFT::Ehdr &Header,
                                ArrayRef<ELFYAML::ProgramHeader> Phdrs,
                                std::vector<std::unique_ptr<ELFYAML::Chunk>> &V,
if (Sec.sh_type == ELF::SHT_SYMTAB) {
       } else {
         return createStringError(errc::invalid_argument,
                                  "unable to read extended section indexes: " +
                                      toString(TableOrErr.takeError()));
       }
     }
   }
 
-  if (SymTab) {
-    Y->Symbols.emplace();
-    if (Error E = dumpSymbols(SymTab, *Y->Symbols))
+  if (SymTab)
+    if (Error E = dumpSymbols(SymTab, Y->Symbols))
       return std::move(E);
-  }
 
-  if (DynSymTab) {
-    Y->DynamicSymbols.emplace();
-    if (Error E = dumpSymbols(DynSymTab, *Y->DynamicSymbols))
+  if (DynSymTab)
+    if (Error E = dumpSymbols(DynSymTab, Y->DynamicSymbols))
       return std::move(E);
-  }
 
   // We dump all sections first. It is simple and allows us to verify that all
   // sections are valid and also to generalize the code. But we are not going to
   // keep all of them in the final output (see comments for
   // 'shouldPrintSection()'). Undesired chunks will be removed later.
   Expected<std::vector<std::unique_ptr<ELFYAML::Chunk>>> ChunksOrErr =
       dumpSections();
   if (!ChunksOrErr)

 }
 
 template <class ELFT>
 Expected<ELFYAML::RawContentSection *>
 ELFDumper<ELFT>::dumpPlaceholderSection(const Elf_Shdr *Shdr) {
   auto S = std::make_unique<ELFYAML::RawContentSection>();
   if (Error E = dumpCommonSection(Shdr, *S.get()))
     return std::move(E);
+
+  // Normally symbol tables should not be empty. We dump the "Size"
+  // key when they are.
+  if ((Shdr->sh_type == ELF::SHT_SYMTAB || Shdr->sh_type == ELF::SHT_DYNSYM) &&
+      !Shdr->sh_size)
+    S->Size.emplace();
+
   return S.release();
 }
 
 template <class ELFT>
 Expected<std::vector<std::unique_ptr<ELFYAML::Chunk>>>
 ELFDumper<ELFT>::dumpSections() {
   std::vector<std::unique_ptr<ELFYAML::Chunk>> Ret;
   auto Add = [&](Expected<ELFYAML::Chunk *> SecOrErr) -> Error {
for (const Elf_Shdr &Sec : Sections) {
     if (Error E = Add(dumpContentSection(&Sec)))
       return std::move(E);
   }
 
   return std::move(Ret);
 }
 
 template <class ELFT>
-Error ELFDumper<ELFT>::dumpSymbols(const Elf_Shdr *Symtab,
-                             std::vector<ELFYAML::Symbol> &Symbols) {
+Error ELFDumper<ELFT>::dumpSymbols(
+    const Elf_Shdr *Symtab, Optional<std::vector<ELFYAML::Symbol>> &Symbols) {
   if (!Symtab)
     return Error::success();
 
-  auto StrTableOrErr = Obj.getStringTableForSymtab(*Symtab);
-  if (!StrTableOrErr)
-    return StrTableOrErr.takeError();
-  StringRef StrTable = *StrTableOrErr;
-
   auto SymtabOrErr = Obj.symbols(Symtab);
   if (!SymtabOrErr)
     return SymtabOrErr.takeError();
 
+  if (SymtabOrErr->empty())
+    return Error::success();
+
+  auto StrTableOrErr = Obj.getStringTableForSymtab(*Symtab);
+  if (!StrTableOrErr)
+    return StrTableOrErr.takeError();
+
   if (Symtab->sh_type == ELF::SHT_SYMTAB) {
     SymTable = *SymtabOrErr;
     SymbolNames.resize(SymTable.size());
   }
 
+  Symbols.emplace();
   for (const auto &Sym : (*SymtabOrErr).drop_front()) {
     ELFYAML::Symbol S;
-    if (auto EC = dumpSymbol(&Sym, Symtab, StrTable, S))
+    if (auto EC = dumpSymbol(&Sym, Symtab, *StrTableOrErr, S))
       return EC;
-    Symbols.push_back(S);
+    Symbols->push_back(S);
   }
 
   return Error::success();
 }
 
 template <class ELFT>
 Error ELFDumper<ELFT>::dumpSymbol(const Elf_Sym *Sym, const Elf_Shdr *SymTab,
                                   StringRef StrTable, ELFYAML::Symbol &S) {