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

[Paul Semel via Phabricator via llvm-commits]

paulsemel added inline comments.


================
Comment at: test/tools/llvm-objcopy/keep-file-symbols.test:28-35
+  Weak:
+    - Name:     baz
+      Type:     STT_FUNC
+      Section:  .text
+  Global:
+    - Name:     foobar
+      Type:     STT_FUNC
----------------
jhenderson wrote:
> You only need two symbols - one file symbol and one non-file symbol.
You're totally right. I'll remove those :)


================
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
----------------
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 ? :)


Repository:
  rL LLVM

https://reviews.llvm.org/D46830