[llvm] add -o flag to llvm-bitcode-strip

[rmaz]

Summary

Add the -o flag to specify an output path for llvm-bitcode-strip.
This matches the interface to the Xcode bitcode_strip tool.

[jhenderson]

I was going to suggest that the option should be documented in the llvm-bitcode-strip command guide, but it looks like that this documentation hasn't been written yet :(

llvm/test/tools/llvm-objcopy/MachO/bitcode-strip.test

2

5

You shouldn't just run a naked not ... without checking the error message produced by llvm-bitcode-strip

6

You should check that %t2 contains the output you expect, whatever that may be.

30

I suspect you only need 0 or 1 sections for the test case to be sufficient?

llvm/tools/llvm-objcopy/ObjcopyOptions.cpp

1211

Are you sure you want this behaviour? Prior to this patch, llvm-bitcode-strip will modify things in-place, like llvm-strip does.

[rmaz]

llvm/tools/llvm-objcopy/ObjcopyOptions.cpp

1211

Yes, this matches the behaviour of bitcode_strip:

Usage: bitcode_strip input [-r | -m | -l] [-keep_cs] -o output

[rmaz]

update test to check error message, remove unused sections and verify output is unchanged.

[jhenderson]

llvm/tools/llvm-objcopy/ObjcopyOptions.cpp

1214

I'd change this comment: the error message already has llvm-bitcode-strip in the prefix, so no need to repeat it in the body of the message. Perhaps simply "-o is a required argument" (alternative ideas also welcome)?

[rmaz]

reword error message

[rmaz]

update test

[jhenderson]

LGTM.

llvm/test/tools/llvm-objcopy/MachO/bitcode-strip.test

+## Test output flag is required.
+
+# RUN: yaml2obj %s -o %t
+# RUN: not llvm-bitcode-strip %t 2>&1 | FileCheck --check-prefix=MISSING-ARG %s
+# RUN: llvm-bitcode-strip %t -o %t2
+# RUN: cmp %t %t2
+
+# MISSING-ARG: llvm-bitcode-strip: error: llvm-bitcode-strip expects a single output path specified with -o
+
+--- !mach-o
+FileHeader:
+  magic:           0xFEEDFACF
+  cputype:         0x01000007
+  cpusubtype:      0x00000003
+  filetype:        0x00000001
+  ncmds:           1
+  sizeofcmds:      152
+  flags:           0x00002000
+  reserved:        0x00000000
+LoadCommands:
+  - cmd:             LC_SEGMENT_64
+    cmdsize:         152
+    segname:         __TEXT
+    vmaddr:          0
+    vmsize:          4
+    fileoff:         184
+    filesize:        4
+    maxprot:         7
+    initprot:        7
+    nsects:          1
+    flags:           0
+    Sections:
+      - sectname:        __text
+        segname:         __TEXT
+        addr:            0x0000000000000000
+        content:         'AABBCCDD'
+        size:            4
+        offset:          184
+        align:           0
+        reloff:          0x00000000
+        nreloc:          0
+        flags:           0x80000400
+        reserved1:       0x00000000
+        reserved2:       0x00000000
+        reserved3:       0x00000000

llvm/tools/llvm-objcopy/BitcodeStripOpts.td

 
 include "llvm/Option/OptParser.td"
 
 def help : Flag<["--"], "help">;
 
 def h : Flag<["-"], "h">, Alias<help>;
 
 def version : Flag<["--"], "version">,
-              HelpText<"Print the version and exit.">;
+              HelpText<"Print the version and exit">;
 
 def V : Flag<["-"], "V">,
         Alias<version>,
         HelpText<"Alias for --version">;
+
+def output : JoinedOrSeparate<["-"], "o">, HelpText<"Write output to <file>">,
+             MetaVarName<"<file>">;

llvm/tools/llvm-objcopy/ObjcopyOptions.cpp

objcopy::parseBitcodeStripOptions(ArrayRef<const char *> ArgsArr) {
   SmallVector<StringRef, 2> Positional;
   for (auto *Arg : InputArgs.filtered(BITCODE_STRIP_INPUT))
     Positional.push_back(Arg->getValue());
   if (Positional.size() > 1)
     return createStringError(errc::invalid_argument,
                              "llvm-bitcode-strip expects a single input file");
   assert(!Positional.empty());
   Config.InputFilename = Positional[0];
-  Config.OutputFilename = Positional[0];
+
+  if (!InputArgs.hasArg(BITCODE_STRIP_output)) {
+    return createStringError(errc::invalid_argument,
+                             "-o is a required argument");
+  }
+  Config.OutputFilename = InputArgs.getLastArgValue(BITCODE_STRIP_output);
 
   DC.CopyConfigs.push_back(std::move(ConfigMgr));
   return std::move(DC);
 }
 
 // ParseStripOptions returns the config and sets the input arguments. If a
 // help flag is set then ParseStripOptions will print the help messege and
 // exit.