[PATCH] D47750: [llvm-strip] Expose --discard-all option

[Paul Semel via Phabricator via llvm-commits]

paulsemel added a comment.

First, thanks for the patch :)
I think @jhenderson pointed out something important. If you do `strip -x foo -o bar`, you will notice that it will only discard local symbols from the binary.
This is not really what's happening with this behavior.. (as you mentioned, this removes symbol table, and that's why you can't test the `-x` option alone)
Maybe we should fix this ? What do you think ? :)



================
Comment at: tools/llvm-objcopy/llvm-objcopy.cpp:588-589
   Config.StripDebug = InputArgs.hasArg(STRIP_strip_debug);
   if (!Config.StripDebug)
     Config.StripAll = true;
 
----------------
Maybe you should do something like (see my comment above for the reason):
```
Config.DiscardAll = InputArgs.hasArg(STRIP_discard_all);
if (!Config.StripDebug && !Config.DiscardAll)
  Config.StripAll = true;
```


================
Comment at: tools/llvm-objcopy/llvm-objcopy.cpp:591-592
 
+  Config.DiscardAll = InputArgs.hasArg(STRIP_discard_all);
+
   for (auto Arg : InputArgs.filtered(STRIP_remove_section))
----------------
And thus remove those lines :)


Repository:
  rL LLVM

https://reviews.llvm.org/D47750