[PATCH] D47414: [llvm-objcopy] Fix null symbol handling

[Paul Semel via Phabricator via llvm-commits]

paulsemel updated this revision to Diff 149273.
paulsemel edited the summary of this revision.

Repository:
  rL LLVM

https://reviews.llvm.org/D47414

Files:
  test/tools/llvm-objcopy/keep-file-symbols.test
  test/tools/llvm-objcopy/strip-all-and-keep-symbol.test
  tools/llvm-objcopy/Object.cpp
  tools/llvm-objcopy/Object.h
  tools/llvm-objcopy/llvm-objcopy.cpp


Index: tools/llvm-objcopy/llvm-objcopy.cpp
===================================================================
--- tools/llvm-objcopy/llvm-objcopy.cpp
+++ tools/llvm-objcopy/llvm-objcopy.cpp
@@ -292,9 +292,7 @@
         return true;
       }
 
-      // TODO: We might handle the 'null symbol' in a different way
-      // by probably handling it the same way as we handle 'null section' ?
-      if (Config.StripUnneeded && !Sym.Referenced && Sym.Index != 0 &&
+      if (Config.StripUnneeded && !Sym.Referenced &&
           (Sym.Binding == STB_LOCAL || Sym.getShndx() == SHN_UNDEF) &&
           Sym.Type != STT_FILE && Sym.Type != STT_SECTION)
         return true;
Index: tools/llvm-objcopy/Object.h
===================================================================
--- tools/llvm-objcopy/Object.h
+++ tools/llvm-objcopy/Object.h
@@ -367,7 +367,9 @@
                  SectionBase *DefinedIn, uint64_t Value, uint8_t Visibility,
                  uint16_t Shndx, uint64_t Sz);
   void addSymbolNames();
-  bool empty() const { return Symbols.empty(); }
+  // We don't want to count the null symbol for the SymbolTable emptyness
+  // as the user might not alter it in anyway.
+  bool empty() const { return Symbols.size() == 1; }
   const SectionBase *getStrTab() const { return SymbolNames; }
   const Symbol *getSymbolByIndex(uint32_t Index) const;
   Symbol *getSymbolByIndex(uint32_t Index);
Index: tools/llvm-objcopy/Object.cpp
===================================================================
--- tools/llvm-objcopy/Object.cpp
+++ tools/llvm-objcopy/Object.cpp
@@ -200,8 +200,8 @@
 }
 
 void SymbolTableSection::updateSymbols(function_ref<void(Symbol &)> Callable) {
-  for (auto &Sym : Symbols)
-    Callable(*Sym);
+  for (auto Sym = std::begin(Symbols) + 1; Sym != std::end(Symbols); Sym++)
+    Callable(**Sym);
   std::stable_partition(
       std::begin(Symbols), std::end(Symbols),
       [](const SymPtr &Sym) { return Sym->Binding == STB_LOCAL; });
@@ -211,7 +211,7 @@
 void SymbolTableSection::removeSymbols(
     function_ref<bool(const Symbol &)> ToRemove) {
   Symbols.erase(
-      std::remove_if(std::begin(Symbols), std::end(Symbols),
+      std::remove_if(std::begin(Symbols) + 1, std::end(Symbols),
                      [ToRemove](const SymPtr &Sym) { return ToRemove(*Sym); }),
       std::end(Symbols));
   Size = Symbols.size() * EntrySize;
Index: test/tools/llvm-objcopy/strip-all-and-keep-symbol.test
===================================================================
--- test/tools/llvm-objcopy/strip-all-and-keep-symbol.test
+++ test/tools/llvm-objcopy/strip-all-and-keep-symbol.test
@@ -58,6 +58,15 @@
 
 #CHECK: Symbols [
 #CHECK-NEXT:  Symbol {
+#CHECK-NEXT:    Name:
+#CHECK-NEXT:    Value: 0x0
+#CHECK-NEXT:    Size: 0
+#CHECK-NEXT:    Binding: Local
+#CHECK-NEXT:    Type: None
+#CHECK-NEXT:    Other: 0
+#CHECK-NEXT:    Section: Undefined
+#CHECK-NEXT:  }
+#CHECK-NEXT:  Symbol {
 #CHECK-NEXT:    Name: foo
 #CHECK-NEXT:    Value: 0x1000
 #CHECK-NEXT:    Size: 8
Index: test/tools/llvm-objcopy/keep-file-symbols.test
===================================================================
--- test/tools/llvm-objcopy/keep-file-symbols.test
+++ test/tools/llvm-objcopy/keep-file-symbols.test
@@ -29,6 +29,15 @@
 
 #STRIPALL: Symbols [
 #STRIPALL-NEXT:  Symbol {
+#STRIPALL-NEXT:    Name:
+#STRIPALL-NEXT:    Value: 0x0
+#STRIPALL-NEXT:    Size: 0
+#STRIPALL-NEXT:    Binding: Local
+#STRIPALL-NEXT:    Type: None
+#STRIPALL-NEXT:    Other: 0
+#STRIPALL-NEXT:    Section: Undefined
+#STRIPALL-NEXT:  }
+#STRIPALL-NEXT:  Symbol {
 #STRIPALL-NEXT:    Name: foo
 #STRIPALL-NEXT:    Value: 0x0
 #STRIPALL-NEXT:    Size: 0


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D47414.149273.patch
Type: text/x-patch
Size: 3660 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180531/26ad36c5/attachment.bin>