[llvm] c74751d - [obj2yaml] - Fix the crash in getUniquedSectionName().

[Georgii Rymar via llvm-commits]

Author: Georgii Rymar
Date: 2021-01-11T15:04:00+03:00
New Revision: c74751d4b5bd8e6753ba8dbc5baa88d7c8929c5c

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

LOG: [obj2yaml] - Fix the crash in getUniquedSectionName().

`getUniquedSectionName(const Elf_Shdr *Sec)` assumes that
`Sec` is not `nullptr`.

I've found one place in `getUniquedSymbolName` where it is
not true (because of that we crash when trying to dump
unnamed null section symbols).

Patch fixes the crash and changes the signature of the
`getUniquedSectionName` section to accept a reference.

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

Added: 
    

Modified: 
    llvm/test/tools/obj2yaml/ELF/symbol.yaml
    llvm/tools/obj2yaml/elf2yaml.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/test/tools/obj2yaml/ELF/symbol.yaml b/llvm/test/tools/obj2yaml/ELF/symbol.yaml
index 3684d4f70c6c..3afe2d13d162 100644
--- a/llvm/test/tools/obj2yaml/ELF/symbol.yaml
+++ b/llvm/test/tools/obj2yaml/ELF/symbol.yaml
@@ -25,3 +25,44 @@ Symbols:
   - Name:  bar
     Size:  0x1
     Value: 0x1
+
+## Check how we dump unnamed section symbols.
+## Check we are able to handle the section symbol for the null section.
+## Document we name them with a section name they describe.
+
+# RUN: yaml2obj --docnum=2 %s -o %t2
+# RUN: obj2yaml %t2 | FileCheck %s --check-prefix=SECTION-SYM
+
+# SECTION-SYM:      --- !ELF
+# SECTION-SYM-NEXT: FileHeader:
+# SECTION-SYM-NEXT:   Class: ELFCLASS64
+# SECTION-SYM-NEXT:   Data:  ELFDATA2LSB
+# SECTION-SYM-NEXT:   Type:  ET_REL
+# SECTION-SYM-NEXT: Sections:
+# SECTION-SYM-NEXT:   - Name: .section
+# SECTION-SYM-NEXT:     Type: SHT_PROGBITS
+# SECTION-SYM-NEXT: Symbols:
+# SECTION-SYM-NEXT:   - Type: STT_SECTION
+# SECTION-SYM-NEXT:   - Name:    .section
+# SECTION-SYM-NEXT:     Type:    STT_SECTION
+# SECTION-SYM-NEXT:     Section: .section
+# SECTION-SYM-NEXT:   - Name:    .section
+# SECTION-SYM-NEXT:     Type:    STT_SECTION
+# SECTION-SYM-NEXT:     Section: .section
+# SECTION-SYM-NEXT: ...
+
+--- !ELF
+FileHeader:
+  Class: ELFCLASS64
+  Data:  ELFDATA2LSB
+  Type:  ET_REL
+Sections:
+  - Name: .section
+    Type: SHT_PROGBITS
+Symbols:
+  - Type:  STT_SECTION
+    Index: 0
+  - Type:  STT_SECTION
+    Index: 1
+  - Type:  STT_SECTION
+    Index: 1

diff  --git a/llvm/tools/obj2yaml/elf2yaml.cpp b/llvm/tools/obj2yaml/elf2yaml.cpp
index da32eaba5a69..dacbaaf482c0 100644
--- a/llvm/tools/obj2yaml/elf2yaml.cpp
+++ b/llvm/tools/obj2yaml/elf2yaml.cpp
@@ -37,7 +37,7 @@ class ELFDumper {
 
   BumpPtrAllocator StringAllocator;
 
-  Expected<StringRef> getUniquedSectionName(const Elf_Shdr *Sec);
+  Expected<StringRef> getUniquedSectionName(const Elf_Shdr &Sec);
   Expected<StringRef> getUniquedSymbolName(const Elf_Sym *Sym,
                                            StringRef StrTable,
                                            const Elf_Shdr *SymTab);
@@ -115,13 +115,12 @@ ELFDumper<ELFT>::ELFDumper(const object::ELFFile<ELFT> &O,
 
 template <class ELFT>
 Expected<StringRef>
-ELFDumper<ELFT>::getUniquedSectionName(const Elf_Shdr *Sec) {
-  unsigned SecIndex = Sec - &Sections[0];
-  assert(&Sections[SecIndex] == Sec);
+ELFDumper<ELFT>::getUniquedSectionName(const Elf_Shdr &Sec) {
+  unsigned SecIndex = &Sec - &Sections[0];
   if (!SectionNames[SecIndex].empty())
     return SectionNames[SecIndex];
 
-  auto NameOrErr = Obj.getSectionName(*Sec);
+  auto NameOrErr = Obj.getSectionName(Sec);
   if (!NameOrErr)
     return NameOrErr;
   StringRef Name = *NameOrErr;
@@ -150,10 +149,12 @@ ELFDumper<ELFT>::getUniquedSymbolName(const Elf_Sym *Sym, StringRef StrTable,
     return SymbolNameOrErr;
   StringRef Name = *SymbolNameOrErr;
   if (Name.empty() && Sym->getType() == ELF::STT_SECTION) {
-    auto ShdrOrErr = Obj.getSection(*Sym, SymTab, ShndxTables.lookup(SymTab));
+    Expected<const Elf_Shdr *> ShdrOrErr =
+        Obj.getSection(*Sym, SymTab, ShndxTables.lookup(SymTab));
     if (!ShdrOrErr)
       return ShdrOrErr.takeError();
-    return getUniquedSectionName(*ShdrOrErr);
+    // The null section has no name.
+    return (*ShdrOrErr == nullptr) ? "" : getUniquedSectionName(**ShdrOrErr);
   }
 
   // Symbols in .symtab can have duplicate names. For example, it is a common
@@ -678,7 +679,7 @@ Error ELFDumper<ELFT>::dumpSymbol(const Elf_Sym *Sym, const Elf_Shdr *SymTab,
   if (!Shdr)
     return Error::success();
 
-  auto NameOrErr = getUniquedSectionName(Shdr);
+  auto NameOrErr = getUniquedSectionName(*Shdr);
   if (!NameOrErr)
     return NameOrErr.takeError();
   S.Section = NameOrErr.get();
@@ -755,7 +756,7 @@ Error ELFDumper<ELFT>::dumpCommonSection(const Elf_Shdr *Shdr,
 
   S.OriginalSecNdx = Shdr - &Sections[0];
 
-  auto NameOrErr = getUniquedSectionName(Shdr);
+  Expected<StringRef> NameOrErr = getUniquedSectionName(*Shdr);
   if (!NameOrErr)
     return NameOrErr.takeError();
   S.Name = NameOrErr.get();
@@ -764,14 +765,14 @@ Error ELFDumper<ELFT>::dumpCommonSection(const Elf_Shdr *Shdr,
     S.EntSize = static_cast<llvm::yaml::Hex64>(Shdr->sh_entsize);
 
   if (Shdr->sh_link != ELF::SHN_UNDEF) {
-    auto LinkSection = Obj.getSection(Shdr->sh_link);
+    Expected<const Elf_Shdr *> LinkSection = Obj.getSection(Shdr->sh_link);
     if (!LinkSection)
       return make_error<StringError>(
           "unable to resolve sh_link reference in section '" + S.Name +
               "': " + toString(LinkSection.takeError()),
           inconvertibleErrorCode());
 
-    NameOrErr = getUniquedSectionName(*LinkSection);
+    NameOrErr = getUniquedSectionName(**LinkSection);
     if (!NameOrErr)
       return NameOrErr.takeError();
     S.Link = NameOrErr.get();
@@ -795,7 +796,7 @@ Error ELFDumper<ELFT>::dumpCommonRelocationSection(
   if (!InfoSection)
     return InfoSection.takeError();
 
-  auto NameOrErr = getUniquedSectionName(*InfoSection);
+  Expected<StringRef> NameOrErr = getUniquedSectionName(**InfoSection);
   if (!NameOrErr)
     return NameOrErr.takeError();
   S.RelocatableSec = NameOrErr.get();
@@ -1462,10 +1463,10 @@ ELFDumper<ELFT>::dumpGroupSection(const Elf_Shdr *Shdr) {
       continue;
     }
 
-    auto SHdrOrErr = Obj.getSection(Member);
+    Expected<const Elf_Shdr *> SHdrOrErr = Obj.getSection(Member);
     if (!SHdrOrErr)
       return SHdrOrErr.takeError();
-    auto NameOrErr = getUniquedSectionName(*SHdrOrErr);
+    Expected<StringRef> NameOrErr = getUniquedSectionName(**SHdrOrErr);
     if (!NameOrErr)
       return NameOrErr.takeError();
     S->Members->push_back({*NameOrErr});