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

[grimar]

Summary

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.

[jhenderson]

LGTM.

llvm/tools/obj2yaml/elf2yaml.cpp

128

Seems to me like this assert is rather unnecessary? That's just saying that the previous line's calculation is correct.

[grimar]

llvm/tools/obj2yaml/elf2yaml.cpp

128

Yeah. I'll remove it before committing.

llvm/test/tools/obj2yaml/ELF/symbol.yaml

FileHeader:
   Type:  ET_REL
 Symbols:
   - Name:  foo
     Size:  0x0
     Value: 0x0
   - 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

llvm/tools/obj2yaml/elf2yaml.cpp

class ELFDumper {
   DenseMap<StringRef, uint32_t> UsedSectionNames;
   std::vector<std::string> SectionNames;
 
   DenseMap<StringRef, uint32_t> UsedSymbolNames;
   std::vector<std::string> SymbolNames;
 
   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);
   Expected<StringRef> getSymbolName(uint32_t SymtabNdx, uint32_t SymbolNdx);
 
   const object::ELFFile<ELFT> &Obj;
   std::unique_ptr<DWARFContext> DWARFCtx;
 

 
 template <class ELFT>
 ELFDumper<ELFT>::ELFDumper(const object::ELFFile<ELFT> &O,
                            std::unique_ptr<DWARFContext> DCtx)
     : Obj(O), DWARFCtx(std::move(DCtx)) {}
 
 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];
+  assert(&Sections[SecIndex] == &Sec);
   if (!SectionNames[SecIndex].empty())
     return SectionNames[SecIndex];
 
-  auto NameOrErr = Obj.getSectionName(*Sec);
+  auto NameOrErr = Obj.getSectionName(Sec);
   if (!NameOrErr)
     return NameOrErr;
   StringRef Name = *NameOrErr;
   // In some specific cases we might have more than one section without a
   // name (sh_name == 0). It normally doesn't happen, but when we have this case
   // it doesn't make sense to uniquify their names and add noise to the output.
   if (Name.empty())
     return "";

 Expected<StringRef>
 ELFDumper<ELFT>::getUniquedSymbolName(const Elf_Sym *Sym, StringRef StrTable,
                                       const Elf_Shdr *SymTab) {
   Expected<StringRef> SymbolNameOrErr = Sym->getName(StrTable);
   if (!SymbolNameOrErr)
     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
   // situation for local symbols in a relocatable object. Here we assign unique
   // suffixes for such symbols so that we can differentiate them.
   if (SymTab->sh_type == ELF::SHT_SYMTAB) {
     unsigned Index = Sym - SymTable.data();
     if (!SymbolNames[Index].empty())
Error ELFDumper<ELFT>::dumpSymbol(const Elf_Sym *Sym, const Elf_Shdr *SymTab,
 
   auto ShdrOrErr = Obj.getSection(*Sym, SymTab, ShndxTables.lookup(SymTab));
   if (!ShdrOrErr)
     return ShdrOrErr.takeError();
   const Elf_Shdr *Shdr = *ShdrOrErr;
   if (!Shdr)
     return Error::success();
 
-  auto NameOrErr = getUniquedSectionName(Shdr);
+  auto NameOrErr = getUniquedSectionName(*Shdr);
   if (!NameOrErr)
     return NameOrErr.takeError();
   S.Section = NameOrErr.get();
 
   return Error::success();
 }
 
 template <class ELFT>
Error ELFDumper<ELFT>::dumpCommonSection(const Elf_Shdr *Shdr,
   if (Shdr->sh_flags)
     S.Flags = static_cast<ELFYAML::ELF_SHF>(Shdr->sh_flags);
   if (Shdr->sh_addr)
     S.Address = static_cast<uint64_t>(Shdr->sh_addr);
   S.AddressAlign = Shdr->sh_addralign;
 
   S.OriginalSecNdx = Shdr - &Sections[0];
 
-  auto NameOrErr = getUniquedSectionName(Shdr);
+  Expected<StringRef> NameOrErr = getUniquedSectionName(*Shdr);
   if (!NameOrErr)
     return NameOrErr.takeError();
   S.Name = NameOrErr.get();
 
   if (Shdr->sh_entsize != getDefaultShEntSize<ELFT>(S.Type, S.Name))
     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();
   }
 
   return Error::success();
 }
 
 template <class ELFT>
 Error ELFDumper<ELFT>::dumpCommonRelocationSection(
     const Elf_Shdr *Shdr, ELFYAML::RelocationSection &S) {
   if (Error E = dumpCommonSection(Shdr, S))
     return E;
 
   // Having a zero sh_info field is normal: .rela.dyn is a dynamic
   // relocation section that normally has no value in this field.
   if (!Shdr->sh_info)
     return Error::success();
 
   auto InfoSection = Obj.getSection(Shdr->sh_info);
   if (!InfoSection)
     return InfoSection.takeError();
 
-  auto NameOrErr = getUniquedSectionName(*InfoSection);
+  Expected<StringRef> NameOrErr = getUniquedSectionName(**InfoSection);
   if (!NameOrErr)
     return NameOrErr.takeError();
   S.RelocatableSec = NameOrErr.get();
 
   return Error::success();
 }
 
 template <class ELFT>
ELFDumper<ELFT>::dumpGroupSection(const Elf_Shdr *Shdr) {
 
   S->Members.emplace();
   for (Elf_Word Member : *MembersOrErr) {
     if (Member == llvm::ELF::GRP_COMDAT) {
       S->Members->push_back({"GRP_COMDAT"});
       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});
   }
   return S.release();
 }
 
 template <class ELFT>