[yaml2obj] - Change NameIndex to StName for Symbol.

[grimar]

Summary

It is consistent with the approach we use for Section struct.

[jhenderson]

llvm/include/llvm/ObjectYAML/ELFYAML.h

112

Any particular reason for this blank line?

llvm/test/tools/yaml2obj/ELF/symbol-name.yaml

26–27

It might be interesting to have something that references the symbol by name too in this case (e.g. a relocation or group section).

[grimar]

llvm/include/llvm/ObjectYAML/ELFYAML.h

112

Yes, it is intentional. I'd like to separate 'St*' things. They have a different nature.

[grimar]

• Addressed review comments.

llvm/test/tools/yaml2obj/ELF/symbol-name.yaml

26–27

Done.

[jhenderson]

LGTM, with the comment changes, but you might want to wait to give others a chance to comment before committing.

llvm/test/tools/yaml2obj/ELF/symbol-name.yaml

26

has a priority -> has priority
but symbol name -> but the symbol name

27

string symbol -> symbol string

[grimar]

• Addressed comments.

llvm/include/llvm/ObjectYAML/ELFYAML.h

struct ProgramHeader {
   Optional<llvm::yaml::Hex64> FileSize;
   Optional<llvm::yaml::Hex64> MemSize;
   Optional<llvm::yaml::Hex64> Offset;
   std::vector<SectionName> Sections;
 };
 
 struct Symbol {
   StringRef Name;
-  Optional<uint32_t> NameIndex;
   ELF_STT Type;
   StringRef Section;
   Optional<ELF_SHN> Index;
   ELF_STB Binding;
   llvm::yaml::Hex64 Value;
   llvm::yaml::Hex64 Size;
   Optional<uint8_t> Other;
+
+  Optional<uint32_t> StName;
 };
 
 struct SectionOrType {
   StringRef sectionNameOrType;
 };
 
 struct DynamicEntry {
   ELF_DYNTAG Tag;

llvm/lib/ObjectYAML/ELFEmitter.cpp

ELFState<ELFT>::toELFSymbols(ArrayRef<ELFYAML::Symbol> Symbols,
 
   size_t I = 0;
   for (const ELFYAML::Symbol &Sym : Symbols) {
     Elf_Sym &Symbol = Ret[++I];
 
     // If NameIndex, which contains the name offset, is explicitly specified, we
     // use it. This is useful for preparing broken objects. Otherwise, we add
     // the specified Name to the string table builder to get its offset.
-    if (Sym.NameIndex)
-      Symbol.st_name = *Sym.NameIndex;
+    if (Sym.StName)
+      Symbol.st_name = *Sym.StName;
     else if (!Sym.Name.empty())
       Symbol.st_name = Strtab.getOffset(ELFYAML::dropUniqueSuffix(Sym.Name));
 
     Symbol.setBindingAndType(Sym.Binding, Sym.Type);
     if (!Sym.Section.empty())
       Symbol.st_shndx = toSectionIndex(Sym.Section, "", Sym.Name);
     else if (Sym.Index)
       Symbol.st_shndx = *Sym.Index;

llvm/lib/ObjectYAML/ELFYAML.cpp

struct NormalizedOther {
   Optional<std::vector<StOtherPiece>> Other;
   std::string UnknownFlagsHolder;
 };
 
 } // end anonymous namespace
 
 void MappingTraits<ELFYAML::Symbol>::mapping(IO &IO, ELFYAML::Symbol &Symbol) {
   IO.mapOptional("Name", Symbol.Name, StringRef());
-  IO.mapOptional("NameIndex", Symbol.NameIndex);
+  IO.mapOptional("StName", Symbol.StName);
   IO.mapOptional("Type", Symbol.Type, ELFYAML::ELF_STT(0));
   IO.mapOptional("Section", Symbol.Section, StringRef());
   IO.mapOptional("Index", Symbol.Index);
   IO.mapOptional("Binding", Symbol.Binding, ELFYAML::ELF_STB(0));
   IO.mapOptional("Value", Symbol.Value, Hex64(0));
   IO.mapOptional("Size", Symbol.Size, Hex64(0));
 
   // Symbol's Other field is a bit special. It is usually a field that
   // represents st_other and holds the symbol visibility. However, on some
   // platforms, it can contain bit fields and regular values, or even sometimes a
   // crazy mix of them (see comments for NormalizedOther). Because of this, we
   // need special handling.
   MappingNormalization<NormalizedOther, Optional<uint8_t>> Keys(IO,
                                                                 Symbol.Other);
   IO.mapOptional("Other", Keys->Other);
 }
 
 StringRef MappingTraits<ELFYAML::Symbol>::validate(IO &IO,
                                                    ELFYAML::Symbol &Symbol) {
   if (Symbol.Index && Symbol.Section.data())
     return "Index and Section cannot both be specified for Symbol";
-  if (Symbol.NameIndex && !Symbol.Name.empty())
-    return "Name and NameIndex cannot both be specified for Symbol";
   return StringRef();
 }
 
 static void commonSectionMapping(IO &IO, ELFYAML::Section &Section) {
   IO.mapOptional("Name", Section.Name, StringRef());
   IO.mapRequired("Type", Section.Type);
   IO.mapOptional("Flags", Section.Flags);
   IO.mapOptional("Address", Section.Address, Hex64(0));

llvm/test/tools/yaml2obj/ELF/symbol-name.yaml

 
 --- !ELF
 FileHeader:
   Class:     ELFCLASS64
   Data:      ELFDATA2LSB
   Type:      ET_REL
   Machine:   EM_X86_64
 Symbols:
-  - Name:      test
-  - NameIndex: 1
-  - NameIndex: 2
-  - Name:      1
-  - Name:      2
+  - Name:   test
+  - StName: 1
+  - StName: 2
+  - Name:   1
+  - Name:   2
 
-## Check we do not allow specifying both Name and NameIndex at once.
-# RUN: not yaml2obj --docnum=2 %s 2>&1 | FileCheck %s --check-prefix=ERR
-# ERR: error: Name and NameIndex cannot both be specified for Symbol
+## Check we allow specifying both Name and StName at once.
+## In this case StName has priority, but the symbol Name is still added
+## to the symbol string table.
+
+# RUN: yaml2obj --docnum=2 %s -o %t2
+# RUN: llvm-readobj --symbols --sections --section-data -r --expand-relocs %t2 | FileCheck %s --check-prefix=BOTH
+
+# BOTH:      Name: .strtab
+# BOTH:      SectionData (
+# BOTH-NEXT:   0000: 00666F6F 00 |.foo.|
+# BOTH-NEXT: )
+
+# BOTH:       Relocations [
+# BOTH-NEXT:   Section (1) .rela.data {
+# BOTH-NEXT:     Relocation {
+# BOTH-NEXT:       Offset: 0x0
+# BOTH-NEXT:       Type: R_X86_64_NONE (0)
+# BOTH-NEXT:       Symbol: - (1)
+# BOTH-NEXT:       Addend: 0x0
+# BOTH-NEXT:     }
+# BOTH-NEXT:   }
+# BOTH-NEXT: ]
+
+# BOTH:      Symbols [
+# BOTH-NEXT:   Symbol {
+# BOTH-NEXT:     Name:  (0)
+# BOTH-NEXT:     Value: 0x0
+# BOTH-NEXT:     Size: 0
+# BOTH-NEXT:     Binding: Local (0x0)
+# BOTH-NEXT:     Type: None (0x0)
+# BOTH-NEXT:     Other: 0
+# BOTH-NEXT:     Section: Undefined (0x0)
+# BOTH-NEXT:   }
+# BOTH-NEXT:   Symbol {
+# BOTH-NEXT:     Name:  (0)
+# BOTH-NEXT:     Value: 0x0
+# BOTH-NEXT:     Size: 0
+# BOTH-NEXT:     Binding: Local (0x0)
+# BOTH-NEXT:     Type: None (0x0)
+# BOTH-NEXT:     Other: 0
+# BOTH-NEXT:     Section: Undefined (0x0)
+# BOTH-NEXT:   }
+# BOTH-NEXT: ]
 
 --- !ELF
 FileHeader:
-  Class:     ELFCLASS64
-  Data:      ELFDATA2LSB
-  Type:      ET_REL
-  Machine:   EM_X86_64
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_REL
+  Machine: EM_X86_64
+Sections:
+  - Name: .rela.data
+    Type: SHT_RELA
+    Relocations:
+      - Offset: 0x0
+        Type:   R_X86_64_NONE
+        Symbol: foo
 Symbols:
-  - Name:      foo
-    NameIndex: 0
+  - Name:   foo
+    StName: 0