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

[Georgii Rymar via llvm-branch-commits]

Author: Georgii Rymar
Date: 2021-01-12T14:08:59+03:00
New Revision: c15a57cc1a86bfb72f4fa0e7d265494babc3b412

URL: https://github.com/llvm/llvm-project/commit/c15a57cc1a86bfb72f4fa0e7d265494babc3b412
DIFF: https://github.com/llvm/llvm-project/commit/c15a57cc1a86bfb72f4fa0e7d265494babc3b412.diff

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

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.

Differential revision: https://reviews.llvm.org/D93697

Added: 
    

Modified: 
    llvm/test/tools/obj2yaml/ELF/no-symtab.yaml
    llvm/tools/obj2yaml/elf2yaml.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/test/tools/obj2yaml/ELF/no-symtab.yaml b/llvm/test/tools/obj2yaml/ELF/no-symtab.yaml
index 8f9fb8285645..132ddfbbc321 100644
--- a/llvm/test/tools/obj2yaml/ELF/no-symtab.yaml
+++ b/llvm/test/tools/obj2yaml/ELF/no-symtab.yaml
@@ -37,3 +37,37 @@ FileHeader:
   Data:  ELFDATA2LSB
   Type:  ET_DYN
 Symbols: []
+
+## A symbol table without the null entry is non-conforming.
+## 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

diff  --git a/llvm/tools/obj2yaml/elf2yaml.cpp b/llvm/tools/obj2yaml/elf2yaml.cpp
index f29b1ebca7de..89bbee49657a 100644
--- a/llvm/tools/obj2yaml/elf2yaml.cpp
+++ b/llvm/tools/obj2yaml/elf2yaml.cpp
@@ -55,7 +55,7 @@ class ELFDumper {
   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();
@@ -219,9 +219,12 @@ bool ELFDumper<ELFT>::shouldPrintSection(const ELFYAML::Section &S,
   // 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, which should not normally 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;
 }
@@ -325,17 +328,13 @@ template <class ELFT> Expected<ELFYAML::Object *> ELFDumper<ELFT>::dump() {
     }
   }
 
-  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
@@ -516,6 +515,13 @@ 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();
 }
 
@@ -621,30 +627,33 @@ ELFDumper<ELFT>::dumpSections() {
 }
 
 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();