[yaml2obj] - Allow setting st_value explicitly for Symbol.

[grimar]

Summary

In some cases it is useful to explicitly set symbol's st_name value.
For example, I am using it in a patch for LLD to remove the broken
binary from a test case and replace it with a YAML test.

[jhenderson]

I have a slight concern here, in that the approach prevents us using a name that is actually a number, and that might be confusing. I think we might want to have a different field (e.g. "NameIndex" or similar) explicitly for this situation. One and only one of the two fields can then be specified.

What do you think?

[grimar]

In D61180#1482531, @jhenderson wrote:

I have a slight concern here, in that the approach prevents us using a name that is actually a number, and that might be confusing. I think we might want to have a different field (e.g. "NameIndex" or similar) explicitly for this situation. One and only one of the two fields can then be specified.

What do you think?

I thought that it is impossible to have a symbol name that is a number. Because valid C name must begin with a letter of the alphabet or an underscore.
I.e. I am not sure we should care about this case. At least now. Should we?

[jhenderson]

In D61180#1482568, @grimar wrote:

I thought that it is impossible to have a symbol name that is a number. Because valid C name must begin with a letter of the alphabet or an underscore.
I.e. I am not sure we should care about this case. At least now. Should we?

Maybe not in C, but there's nothing in ELF that requires this, and I believe it might be possible to name them like that in assembly using double quotes.

[grimar]

Ok. Planning changes. Will take some time since I will be off starting from tomorrow by EOW because of holidays here.

[grimar]

• Reimplemented as was suggested during the review.

[jhenderson]

Some comment nits, otherwise LGTM.

test/tools/yaml2obj/symbol-name.yaml

2

FWIW, I liked what you did in other tests with the double '#' symbols to indicate comments. I wouldn't object to you doing it here either.

tools/yaml2obj/yaml2elf.cpp

483

name index containing offset -> NameIndex, which contains the name offset,
remove "in YAML", since theoretically, we could use this bit of code via other methods in the future.

484

It is -> This is
a Name -> the specified Name

test/tools/yaml2obj/symbol-name.yaml

+# RUN: yaml2obj %s > %t
+# RUN: llvm-readobj --symbols %t | FileCheck %s
+
+# Check we are able to use integers for symbol names and
+# yaml2obj use them as explicit st_name value for symbols.
+
+# CHECK: Name: test (1)
+# CHECK: Name: test (1)
+# CHECK: Name: est (2)
+
+!ELF
+FileHeader:
+  Class:     ELFCLASS64
+  Data:      ELFDATA2LSB
+  Type:      ET_REL
+  Machine:   EM_X86_64
+Symbols:
+  - Name:    test
+  - Name:    1
+  - Name:    2

tools/yaml2obj/yaml2elf.cpp

 
 template <class ELFT>
 void ELFState<ELFT>::addSymbols(ArrayRef<ELFYAML::Symbol> Symbols,
                                 std::vector<Elf_Sym> &Syms,
                                 const StringTableBuilder &Strtab) {
   for (const auto &Sym : Symbols) {
     Elf_Sym Symbol;
     zero(Symbol);
-    if (!Sym.Name.empty())
+
+    // If name described in YAML as an integer, we assume it is an offset and
+    // use it. It is useful for preparing broken objects. Otherwise, we treat it
+    // as a string name and add to the string table builder.
+    if (!Sym.Name.empty()) {
+      unsigned SymName = 0;
+      if (!to_integer(Sym.Name, SymName))
         Symbol.st_name = Strtab.getOffset(Sym.Name);
+      else
+        Symbol.st_name = SymName;
+    }
+
     Symbol.setBindingAndType(Sym.Binding, Sym.Type);
     if (!Sym.Section.empty()) {
       unsigned Index;
       if (SN2I.lookup(Sym.Section, Index)) {
         WithColor::error() << "Unknown section referenced: '" << Sym.Section
                            << "' by YAML symbol " << Sym.Name << ".\n";
         exit(1);
       }