[PATCH] D59496: [llvm-objcopy] - Fix a st_name of the first symbol table entry.

[George Rimar via Phabricator via llvm-commits]

grimar created this revision.
grimar added reviewers: jhenderson, jakehehrlich, alexshap, rupprecht.
Herald added subscribers: MaskRay, arichardson, emaste.
Herald added a reviewer: espindola.

Spec says about the first symbol table entry that index 0 both designates the first entry in the table
and serves as the undefined symbol index. It should have zero value.
Hence the first symbol table entry has no name. And so has to have a st_name == 0.
(http://refspecs.linuxbase.org/elf/gabi4+/ch4.symtab.html)

Currently, we do not emit zero value for the first symbol table entry.
That happens because we add empty strings to the string builder, which
for each such case adds a zero byte:
(https://github.com/llvm-mirror/llvm/blob/master/lib/MC/StringTableBuilder.cpp#L185)
After the string optimization performed it might return non zero indexes for the
empty string requested.

It was revealed during the development and review of D59488 <https://reviews.llvm.org/D59488>.
The patch fixes this issue for the case above and other sections with no names.


https://reviews.llvm.org/D59496

Files:
  test/tools/llvm-objcopy/ELF/symtab-null-entry.test
  tools/llvm-objcopy/ELF/Object.cpp


Index: tools/llvm-objcopy/ELF/Object.cpp
===================================================================
--- tools/llvm-objcopy/ELF/Object.cpp
+++ tools/llvm-objcopy/ELF/Object.cpp
@@ -297,7 +297,10 @@
   Visitor.visit(*this);
 }
 
-void StringTableSection::addString(StringRef Name) { StrTabBuilder.add(Name); }
+void StringTableSection::addString(StringRef Name) {
+  assert(!Name.empty());
+  StrTabBuilder.add(Name);
+}
 
 uint32_t StringTableSection::findIndex(StringRef Name) const {
   return StrTabBuilder.getOffset(Name);
@@ -470,7 +473,7 @@
 void SymbolTableSection::finalize() {
   uint32_t MaxLocalIndex = 0;
   for (auto &Sym : Symbols) {
-    Sym->NameIndex = SymbolNames->findIndex(Sym->Name);
+    Sym->NameIndex = Sym->Name.empty() ? 0 : SymbolNames->findIndex(Sym->Name);
     if (Sym->Binding == STB_LOCAL)
       MaxLocalIndex = std::max(MaxLocalIndex, Sym->Index);
   }
@@ -492,8 +495,11 @@
   }
   // Add all of our strings to SymbolNames so that SymbolNames has the right
   // size before layout is decided.
+  // Ignore the symbols with empty names, e.g special first symbol entry,
+  // some sections symbols etc.
   for (auto &Sym : Symbols)
-    SymbolNames->addString(Sym->Name);
+    if (!Sym->Name.empty())
+      SymbolNames->addString(Sym->Name);
 }
 
 const Symbol *SymbolTableSection::getSymbolByIndex(uint32_t Index) const {
Index: test/tools/llvm-objcopy/ELF/symtab-null-entry.test
===================================================================
--- /dev/null
+++ test/tools/llvm-objcopy/ELF/symtab-null-entry.test
@@ -0,0 +1,28 @@
+# RUN: yaml2obj %s > %t
+# RUN: llvm-objcopy %t %t2
+# RUN: llvm-readobj -t %t2 | FileCheck %s
+
+## Check that all values of the first symbol are zeroes.
+
+# CHECK:      Symbols [
+# CHECK-NEXT:  Symbol {
+# CHECK-NEXT:    Name:  (0)
+# CHECK-NEXT:    Value: 0x0
+# CHECK-NEXT:    Size: 0
+# CHECK-NEXT:    Binding: Local (0x0)
+# CHECK-NEXT:    Type: None (0x0)
+# CHECK-NEXT:    Other: 0
+# CHECK-NEXT:    Section: Undefined (0x0)
+# CHECK-NEXT:  }
+
+!ELF
+FileHeader:
+  Class:           ELFCLASS64
+  Data:            ELFDATA2LSB
+  Type:            ET_EXEC
+  Machine:         EM_X86_64
+Symbols:
+  Global:
+  # We need to have a symbol, otherwise the original
+  # issue that was fixed did not reproduce.
+    - Name:     foo


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D59496.191110.patch
Type: text/x-patch
Size: 2314 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190318/e07f32e5/attachment.bin>