[llvm-strip] Expose --strip-unneeded option

[jhenderson]

LGTM, with the suggested changes.

test/tools/llvm-objcopy/strip-unneeded.test

7 ↗

(On Diff #150102)

Please put a new line in before this line. It'll help make the llvm-strip part clearer.

8–9 ↗

(On Diff #150102)

You don't need to copy the input file here, since we don't use it again at this point.

tools/llvm-objcopy/llvm-objcopy.cpp

586 ↗

(On Diff #150102)

This comment looks a little out of place now with the changes you and @alexshap are making in this area. It probably needs updating or (re)moving.

[paulsemel]

Added Jame's suggestions

tools/llvm-objcopy/llvm-objcopy.cpp

586 ↗

(On Diff #150102)

I think you're right. Maybe we should strip it as it is not really useful ? What do you think @alexshap ?

[jakehehrlich]

Hey I'm away but I got a bug report requesting -x as an alias in
llvm-strip. I haven't looked at the code for this patch so it might already
be solved here but I'd appreciate it being added. No worries either way.
I'll just add it when aight get back of it isn't there.

Best,
Jake

[paulsemel]

In D47818#1124135, @jakehehrlich wrote:

Hey I'm away but I got a bug report requesting -x as an alias in
llvm-strip. I haven't looked at the code for this patch so it might already
be solved here but I'd appreciate it being added. No worries either way.
I'll just add it when aight get back of it isn't there.

Best,
Jake

Hi Jake,

Are you talking about this revision : https://reviews.llvm.org/D47750 ?
@alexshap will land it soon I think. Btw @alexshap, I'll wait for you to land it before landing this one :)

[jakehehrlich]

Ah yes. Thanks!

llvm/trunk/test/tools/llvm-objcopy/strip-unneeded.test

 # RUN: yaml2obj %s > %t
+# RUN: cp %t %t1
 # RUN: llvm-objcopy --strip-unneeded %t %t2
+# Verify that llvm-objcopy has not modified the input.
+# RUN: cmp %t %t1
 # RUN: llvm-readobj -symbols %t2 | FileCheck %s
 
+# Verify that llvm-strip modifies the symbol table the same way.
+# RUN: llvm-strip --strip-unneeded %t
+# RUN: cmp %t %t2
+
 !ELF
 FileHeader:
   Class:           ELFCLASS64
   Data:            ELFDATA2LSB
   Type:            ET_REL
   Machine:         EM_X86_64
 Sections:
   - Name:            .text

llvm/trunk/tools/llvm-objcopy/StripOpts.td

 
 def K : JoinedOrSeparate<["-"], "K">,
         Alias<keep_symbol>;
 
 def discard_all : Flag<["-", "--"], "discard-all">,
                   HelpText<"Remove all local symbols except file and section symbols">;
 def x : Flag<["-"], "x">,
         Alias<discard_all>;
+
+def strip_unneeded : Flag<["-", "--"], "strip-unneeded">,
+                      HelpText<"Remove all symbols not needed by relocations">;

llvm/trunk/tools/llvm-objcopy/llvm-objcopy.cpp

CopyConfig ParseStripOptions(ArrayRef<const char *> ArgsArr) {
   if (Positional.size() > 2)
     error("Support for multiple input files is not implemented yet");
 
   CopyConfig Config;
   Config.InputFilename = Positional[0];
   Config.OutputFilename =
       InputArgs.getLastArgValue(STRIP_output, Positional[0]);
 
-  // Strip debug info only.
   Config.StripDebug = InputArgs.hasArg(STRIP_strip_debug);
   
   Config.DiscardAll = InputArgs.hasArg(STRIP_discard_all);
-  
-  if (!Config.StripDebug && !Config.DiscardAll)
+  Config.StripUnneeded = InputArgs.hasArg(STRIP_strip_unneeded);
+  if (!Config.StripDebug && !Config.StripUnneeded && !Config.DiscardAll)
     Config.StripAll = true;
 
   for (auto Arg : InputArgs.filtered(STRIP_remove_section))
     Config.ToRemove.push_back(Arg->getValue());
 
   for (auto Arg : InputArgs.filtered(STRIP_keep_symbol))
     Config.SymbolsToKeep.push_back(Arg->getValue());