[PATCH] D46830: [llvm-objcopy] Add --keep-file-symbols option

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: test/tools/llvm-objcopy/keep-file-symbols.test:37-48
+#STRIPALL: Symbols [
+#STRIPALL-NEXT:  Symbol {
+#STRIPALL-NEXT:    Name: foo
+#STRIPALL-NEXT:    Value: 0x0
+#STRIPALL-NEXT:    Size: 0
+#STRIPALL-NEXT:    Binding: Local
+#STRIPALL-NEXT:    Type: File
----------------
paulsemel wrote:
> jhenderson wrote:
> > Wait, shouldn't there be a null symbol here? Null symbols are required... That's probably a bug unrelated to your change, but should be fixed ASAP.
> Well, yes, I added a TODO for this in my previous patch, about this dummy symbol. We shouldn't pass it to the removeSymbols function as @jakehehrlich stated :)
> Do you want me to work on it ? :)
If you don't mind. What we currently generate is actually illegal ELF. I was under the impression that the TODO was for coming up with a better way for handling the null symbol, and I assumed that it wasn't being stripped (unless the whole table was), hence why I didn't comment on it before.


Repository:
  rL LLVM

https://reviews.llvm.org/D46830