[PATCH] D46369: [llvm-objcopy] Add --strip-symbol (-N) option

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

Not sure this is specifically an issue with your change, but I think we need a specific test for the first non-local in a symbol table being removed. This is because that affects the sh_info field of that section, but I suspect we don't (deliberately) test that anywhere.



================
Comment at: tools/llvm-objcopy/Object.cpp:352
+                  if (ToRemove(*Reloc.RelocSymbol))
+                    error("not stripping symbol `" + Reloc.RelocSymbol->Name +
+                          "' because it is named in a relocation");
----------------
Test case?


================
Comment at: tools/llvm-objcopy/Object.cpp:385
+          "referenced by the section " +
+          this->Name);
+  }
----------------
Test case?

I think this is okay, but not ideal - the section name for a .group section is officially unspecified, but essentially always ".group" so the message won't provide a useful way of identifying the section. Perhaps the input section index should be printed instead/as well?


Repository:
  rL LLVM

https://reviews.llvm.org/D46369