[yaml2obj][obj2yaml] - Teach yaml2obj/obj2yaml tools about STB_GNU_UNIQUE symbols.

[grimar]

Summary

yaml2obj/obj2yaml does not support the symbols with STB_GNU_UNIQUE yet.
Currently, obj2yaml fails with llvm_unreachable when met such a symbol.

I faced it when investigated the https://bugs.llvm.org/show_bug.cgi?id=41196.

[jhenderson]

LGTM.

The way symbols are handled in yaml2obj looks like it's desperately in need of an overhaul. It looks very fragile and easy to miss one thing when modifying it like you did here. That's a separate issue though.

tools/obj2yaml/elf2yaml.cpp

268 ↗

(On Diff #192430)

Sounds to me like llvm_unreachable here is wrong, because it is reachable by any value not recognised. It should just be a normal error saying what the unknown binding value is. That can be a separate patch though if you want.

[grimar]

In D59875#1445681, @jhenderson wrote:

LGTM.

The way symbols are handled in yaml2obj looks like it's desperately in need of an overhaul. It looks very fragile and easy to miss one thing when modifying it like you did here. That's a separate issue though.

I also feel I do not like the way how yaml2obj describes/handles symbols now. At least currently for each binding, we might need to extend the syntax and it does not seem to me as a reasonable/nice approach in general.
I think we should change/improve something here, let me take a look closer, I'll try to suggest something here after the additional investigation.

tools/obj2yaml/elf2yaml.cpp

268 ↗

(On Diff #192430)

Yeah, I noticed that but was not sure because I would not expect to see any another kind of bindings here (I just never heard about other ones I think). But now, after your comment, I think you're right, that perhaps better be an error just in case. I'll prepare a patch.

[jhenderson]

tools/obj2yaml/elf2yaml.cpp

268 ↗

(On Diff #192430)

Thanks. Even if there are no other legitimate bindings, a bad tool could write an invalid value.

llvm/trunk/include/llvm/ObjectYAML/ELFYAML.h

struct Symbol {
   ELF_STT Type;
   StringRef Section;
   Optional<ELF_SHN> Index;
   llvm::yaml::Hex64 Value;
   llvm::yaml::Hex64 Size;
   uint8_t Other;
 };
 
-struct LocalGlobalWeakSymbols {
+struct SymbolsDef {
   std::vector<Symbol> Local;
   std::vector<Symbol> Global;
   std::vector<Symbol> Weak;
+  std::vector<Symbol> GNUUnique;
 };
 
 struct SectionOrType {
   StringRef sectionNameOrType;
 };
 
 struct DynamicEntry {
   ELF_DYNTAG Tag;

 struct Object {
   FileHeader Header;
   std::vector<ProgramHeader> ProgramHeaders;
   std::vector<std::unique_ptr<Section>> Sections;
   // Although in reality the symbols reside in a section, it is a lot
   // cleaner and nicer if we read them from the YAML as a separate
   // top-level key, which automatically ensures that invariants like there
   // being a single SHT_SYMTAB section are upheld.
-  LocalGlobalWeakSymbols Symbols;
-  LocalGlobalWeakSymbols DynamicSymbols;
+  SymbolsDef Symbols;
+  SymbolsDef DynamicSymbols;
 };
 
 } // end namespace ELFYAML
 } // end namespace llvm
 
 LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::ELFYAML::DynamicEntry)
 LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::ELFYAML::ProgramHeader)
 LLVM_YAML_IS_SEQUENCE_VECTOR(std::unique_ptr<llvm::ELFYAML::Section>)

 };
 
 template <>
 struct MappingTraits<ELFYAML::Symbol> {
   static void mapping(IO &IO, ELFYAML::Symbol &Symbol);
   static StringRef validate(IO &IO, ELFYAML::Symbol &Symbol);
 };
 
-template <>
-struct MappingTraits<ELFYAML::LocalGlobalWeakSymbols> {
-  static void mapping(IO &IO, ELFYAML::LocalGlobalWeakSymbols &Symbols);
+template <> struct MappingTraits<ELFYAML::SymbolsDef> {
+  static void mapping(IO &IO, ELFYAML::SymbolsDef &Symbols);
 };
 
 template <> struct MappingTraits<ELFYAML::DynamicEntry> {
   static void mapping(IO &IO, ELFYAML::DynamicEntry &Rel);
 };
 
 template <> struct MappingTraits<ELFYAML::VerdefEntry> {
   static void mapping(IO &IO, ELFYAML::VerdefEntry &E);

llvm/trunk/lib/ObjectYAML/ELFYAML.cpp

if (Symbol.Index && Symbol.Section.data()) {
     return "Index and Section cannot both be specified for Symbol";
   }
   if (Symbol.Index && *Symbol.Index == ELFYAML::ELF_SHN(ELF::SHN_XINDEX)) {
     return "Large indexes are not supported";
   }
   return StringRef();
 }
 
-void MappingTraits<ELFYAML::LocalGlobalWeakSymbols>::mapping(
-    IO &IO, ELFYAML::LocalGlobalWeakSymbols &Symbols) {
+void MappingTraits<ELFYAML::SymbolsDef>::mapping(IO &IO,
+                                                 ELFYAML::SymbolsDef &Symbols) {
   IO.mapOptional("Local", Symbols.Local);
   IO.mapOptional("Global", Symbols.Global);
   IO.mapOptional("Weak", Symbols.Weak);
+  IO.mapOptional("GNUUnique", Symbols.GNUUnique);
 }
 
 static void commonSectionMapping(IO &IO, ELFYAML::Section &Section) {
   IO.mapOptional("Name", Section.Name, StringRef());
   IO.mapRequired("Type", Section.Type);
   IO.mapOptional("Flags", Section.Flags, ELFYAML::ELF_SHF(0));
   IO.mapOptional("Address", Section.Address, Hex64(0));
   IO.mapOptional("Link", Section.Link, StringRef());

llvm/trunk/test/tools/obj2yaml/elf-gnu-unique-symbols.yaml

+# RUN: yaml2obj %s -o %t
+# RUN: obj2yaml %t | FileCheck %s
+
+## Check obj2yaml is able to dump the STB_GNU_UNIQUE symbol.
+
+# CHECK:      --- !ELF
+# CHECK-NEXT: FileHeader:
+# CHECK-NEXT:   Class:     ELFCLASS64
+# CHECK-NEXT:   Data:      ELFDATA2LSB
+# CHECK-NEXT:   OSABI:     ELFOSABI_GNU
+# CHECK-NEXT:   Type:      ET_REL
+# CHECK-NEXT:   Machine:   EM_X86_64
+# CHECK-NEXT: Symbols:
+# CHECK-NEXT:   GNUUnique:
+# CHECK-NEXT:     - Name:  foo
+# CHECK-NEXT:       Type:  STT_OBJECT
+# CHECK-NEXT: DynamicSymbols: {}
+# CHECK-NEXT: ...
+
+--- !ELF
+FileHeader:
+  Class:     ELFCLASS64
+  Data:      ELFDATA2LSB
+  OSABI:     ELFOSABI_GNU
+  Type:      ET_REL
+  Machine:   EM_X86_64
+Symbols:
+  GNUUnique:
+    - Name:  foo
+      Type:  STT_OBJECT

llvm/trunk/test/tools/yaml2obj/elf-gnu-unique-symbols.yaml

+# RUN: yaml2obj %s -o %t
+# RUN: llvm-readobj -symbols %t | FileCheck %s
+
+## Check yaml2obj is able to create the STB_GNU_UNIQUE symbol.
+
+# CHECK:        Name: foo
+# CHECK-NEXT:   Value:
+# CHECK-NEXT:   Size:
+# CHECK-NEXT:   Binding: Unique
+
+--- !ELF
+FileHeader:
+  Class:     ELFCLASS64
+  Data:      ELFDATA2LSB
+  OSABI:     ELFOSABI_GNU
+  Type:      ET_REL
+  Machine:   EM_X86_64
+Symbols:
+  GNUUnique:
+    - Name:  foo
+      Type:  STT_OBJECT

llvm/trunk/tools/obj2yaml/elf2yaml.cpp

class ELFDumper {
   Expected<StringRef> getUniquedSectionName(const Elf_Shdr *Sec);
   Expected<StringRef> getSymbolName(const Elf_Sym *Sym, StringRef StrTable,
                                     const Elf_Shdr *SymTab);
 
   const object::ELFFile<ELFT> &Obj;
   ArrayRef<Elf_Word> ShndxTable;
 
   std::error_code dumpSymbols(const Elf_Shdr *Symtab,
-                              ELFYAML::LocalGlobalWeakSymbols &Symbols);
+                              ELFYAML::SymbolsDef &Symbols);
   std::error_code dumpSymbol(const Elf_Sym *Sym, const Elf_Shdr *SymTab,
                              StringRef StrTable, ELFYAML::Symbol &S);
   std::error_code dumpCommonSection(const Elf_Shdr *Shdr, ELFYAML::Section &S);
   std::error_code dumpCommonRelocationSection(const Elf_Shdr *Shdr,
                                               ELFYAML::RelocationSection &S);
   template <class RelT>
   std::error_code dumpRelocation(const RelT *Rel, const Elf_Shdr *SymTab,
                                  ELFYAML::Relocation &R);
if (auto EC = dumpSymbols(Symtab, Y->Symbols))
     return EC;
   if (auto EC = dumpSymbols(DynSymtab, Y->DynamicSymbols))
     return EC;
 
   return Y.release();
 }
 
 template <class ELFT>
-std::error_code
-ELFDumper<ELFT>::dumpSymbols(const Elf_Shdr *Symtab,
-                             ELFYAML::LocalGlobalWeakSymbols &Symbols) {
+std::error_code ELFDumper<ELFT>::dumpSymbols(const Elf_Shdr *Symtab,
+                                             ELFYAML::SymbolsDef &Symbols) {
   if (!Symtab)
     return std::error_code();
 
   auto StrTableOrErr = Obj.getStringTableForSymtab(*Symtab);
   if (!StrTableOrErr)
     return errorToErrorCode(StrTableOrErr.takeError());
   StringRef StrTable = *StrTableOrErr;
 
case ELF::STB_LOCAL:
       Symbols.Local.push_back(S);
       break;
     case ELF::STB_GLOBAL:
       Symbols.Global.push_back(S);
       break;
     case ELF::STB_WEAK:
       Symbols.Weak.push_back(S);
       break;
+    case ELF::STB_GNU_UNIQUE:
+      Symbols.GNUUnique.push_back(S);
+      break;
     default:
       llvm_unreachable("Unknown ELF symbol binding");
     }
   }
 
   return std::error_code();
 }
 

llvm/trunk/tools/yaml2obj/yaml2elf.cpp

class ELFState {
   /// The future ".dynstr" section.
   StringTableBuilder DotDynstr{StringTableBuilder::ELF};
 
   NameToIdxMap SN2I;
   NameToIdxMap SymN2I;
   const ELFYAML::Object &Doc;
 
   bool buildSectionIndex();
-  bool buildSymbolIndex(const ELFYAML::LocalGlobalWeakSymbols &);
+  bool buildSymbolIndex(const ELFYAML::SymbolsDef &);
   void initELFHeader(Elf_Ehdr &Header);
   void initProgramHeaders(std::vector<Elf_Phdr> &PHeaders);
   bool initSectionHeaders(std::vector<Elf_Shdr> &SHeaders,
                           ContiguousBlobAccumulator &CBA);
   void initSymtabSectionHeader(Elf_Shdr &SHeader, SymtabType STType,
                                ContiguousBlobAccumulator &CBA);
   void initStrtabSectionHeader(Elf_Shdr &SHeader, StringRef Name,
                                StringTableBuilder &STB,
std::vector<Elf_Sym> Syms;
     Elf_Sym Sym;
     zero(Sym);
     Syms.push_back(Sym);
   }
 
   addSymbols(Symbols.Local, Syms, ELF::STB_LOCAL, Strtab);
   addSymbols(Symbols.Global, Syms, ELF::STB_GLOBAL, Strtab);
   addSymbols(Symbols.Weak, Syms, ELF::STB_WEAK, Strtab);
+  addSymbols(Symbols.GNUUnique, Syms, ELF::STB_GNU_UNIQUE, Strtab);
 
   writeArrayData(
       CBA.getOSAndAlignedOffset(SHeader.sh_offset, SHeader.sh_addralign),
       makeArrayRef(Syms));
   SHeader.sh_size = arrayDataSize(makeArrayRef(Syms));
 }
 
 template <class ELFT>
if (!SN2I.addName(Name, SecNo)) {
       DotShStrtab.add(Name);
     }
 
   DotShStrtab.finalize();
   return true;
 }
 
 template <class ELFT>
-bool ELFState<ELFT>::buildSymbolIndex(
-    const ELFYAML::LocalGlobalWeakSymbols &Symbols) {
+bool ELFState<ELFT>::buildSymbolIndex(const ELFYAML::SymbolsDef &Symbols) {
   std::size_t I = 0;
   for (const std::vector<ELFYAML::Symbol> &V :
-       {Symbols.Local, Symbols.Global, Symbols.Weak}) {
+       {Symbols.Local, Symbols.Global, Symbols.Weak, Symbols.GNUUnique}) {
     for (const auto &Sym : V) {
       ++I;
       if (Sym.Name.empty())
         continue;
       if (SymN2I.addName(Sym.Name, I)) {
         WithColor::error() << "Repeated symbol name: '" << Sym.Name << "'.\n";
         return false;
       }
     }
   }
   return true;
 }
 
 template <class ELFT> void ELFState<ELFT>::finalizeStrings() {
   auto AddSymbols = [](StringTableBuilder &StrTab,
-                       const ELFYAML::LocalGlobalWeakSymbols &Symbols) {
+                       const ELFYAML::SymbolsDef &Symbols) {
     for (const auto &Sym : Symbols.Local)
       StrTab.add(Sym.Name);
     for (const auto &Sym : Symbols.Global)
       StrTab.add(Sym.Name);
     for (const auto &Sym : Symbols.Weak)
       StrTab.add(Sym.Name);
+    for (const auto &Sym : Symbols.GNUUnique)
+      StrTab.add(Sym.Name);
   };
 
   // Add the regular symbol names to .strtab section.
   AddSymbols(DotStrtab, Doc.Symbols);
   DotStrtab.finalize();
 
   if (!hasDynamicSymbols())
     return;
int ELFState<ELFT>::writeELF(raw_ostream &OS, const ELFYAML::Object &Doc) {
   OS.write((const char *)&Header, sizeof(Header));
   writeArrayData(OS, makeArrayRef(PHeaders));
   writeArrayData(OS, makeArrayRef(SHeaders));
   CBA.writeBlobToStream(OS);
   return 0;
 }
 
 template <class ELFT> bool ELFState<ELFT>::hasDynamicSymbols() const {
-  return Doc.DynamicSymbols.Global.size() > 0 ||
-         Doc.DynamicSymbols.Weak.size() > 0 ||
-         Doc.DynamicSymbols.Local.size() > 0;
+  return !Doc.DynamicSymbols.Global.empty() ||
+         !Doc.DynamicSymbols.Weak.empty() ||
+         !Doc.DynamicSymbols.Local.empty() ||
+         !Doc.DynamicSymbols.GNUUnique.empty();
 }
 
 template <class ELFT>
 SmallVector<const char *, 5> ELFState<ELFT>::implicitSectionNames() const {
   if (!hasDynamicSymbols())
     return {".symtab", ".strtab", ".shstrtab"};
   return {".symtab", ".strtab", ".shstrtab", ".dynsym", ".dynstr"};
 }