[llvm-strip] Add support for '--' for delimiting options from input files

[sdmitriev]

Summary

This will allow to use llvm-strip with file names that begin with dashes.

[jhenderson]

llvm/test/tools/llvm-objcopy/dash-dash.test

36

You don't need to make this CHECK unique to objcopy, since llvm-strip and llvm-objcopy produce the same string. Just use --check-prefix=CHECK-NO-INPUT.

38

I'd probably reorder the test cases so that equivalent cases for llvm-strip and llvm-objcopy are side-by-side, i.e. something like:

## llvm-objcopy case 1
## llvm-strip case 1

## llvm-objcopy case 2
## llvm-strip case 2

## llvm-objcopy case 3
## llvm-strip case 3

(where case 1/2/3 are the three distinct cases you test here).

40

Use cp to copy test-obj rather than running yaml2obj again unnecessarily.

41–42

I'd avoid using -o anywhere in these tests, since that's an option itself, and just muddies the waters. Instead, use 2+ input files, which will be modified in-place (you may need to make copies of these files first so you have pristine versions for use later). Same applies in the below cases too.

[sdmitriev]

Addressed review comments.

[jhenderson]

llvm/test/tools/llvm-objcopy/dash-dash.test

12

I think you should have two inputs here, much like llvm-objcopy has two positional arguments. Same goes below. In other words, test the same set of positional arguments as llvm-objcopy does (it's just that in llvm-strip's case, they are both inputs, not one input and one output).

[sdmitriev]

Addressed review comments.

[jhenderson]

llvm/test/tools/llvm-objcopy/dash-dash.test

13–14

I think this needs to be as suggested in the edit. This will ensure that the files have actually been stripped too.

26–27

Similar to above, do a run without the dash dash, and compare the modified file against one of these two files too.

[jhenderson]

LGTM, thanks!

llvm/test/tools/llvm-objcopy/dash-dash.test

 ## Create a clean temporary directory and cd to it to use relative paths.
 # RUN: rm -rf %t && mkdir %t && cd %t
 
-## Check that llvm-objcopy correctly treats paths after '--' as file paths,
+## Check that tools correctly treat paths after '--' as file paths,
 ## even if they start with dashes.
 # RUN: yaml2obj %s -o test-obj
 # RUN: llvm-objcopy --only-section=.test test-obj -- --only-section
 # RUN: llvm-objcopy --only-section=.test test-obj out-obj1
 # RUN: cmp -- out-obj1 --only-section
 
+# RUN: cp -- test-obj --strip-symbol
+# RUN: cp -- test-obj strip-obj1
+# RUN: llvm-strip --remove-section=.test strip-obj1 -- --strip-symbol
+# RUN: cp -- test-obj no-dash-dash1
+# RUN: llvm-strip --remove-section=.test no-dash-dash1
+# RUN: cmp -- no-dash-dash1 strip-obj1
+# RUN: cmp -- no-dash-dash1 --strip-symbol
+
 ## Check the case when '--' is specified with input files but no options.
 # RUN: llvm-objcopy -- test-obj --add-section
 # RUN: llvm-objcopy test-obj out-obj2
 # RUN: cmp -- out-obj2 --add-section
 
+# RUN: cp -- test-obj --keep-symbol
+# RUN: cp -- test-obj strip-obj2
+# RUN: llvm-strip -- strip-obj2 --keep-symbol
+# RUN: cp -- test-obj no-dash-dash2
+# RUN: llvm-strip no-dash-dash2
+# RUN: cmp -- no-dash-dash2 strip-obj2
+# RUN: cmp -- no-dash-dash2 --keep-symbol
+
 ## Check the case when '--' is specified with options but no input files.
 # RUN: not llvm-objcopy --only-section=.test -- 2>&1 | FileCheck %s --check-prefixes=CHECK-NO-INPUT
+# RUN: not llvm-strip --remove-section=.test -- 2>&1 | FileCheck %s --check-prefixes=CHECK-NO-INPUT
 
 # CHECK-NO-INPUT: error: no input file specified
 
 --- !ELF
 FileHeader:
   Class: ELFCLASS64
   Data:  ELFDATA2LSB
   Type:  ET_REL

llvm/test/tools/llvm-objcopy/tool-help-message.test

 # RUN: llvm-objcopy -h | FileCheck --check-prefix=OBJCOPY-USAGE %s --match-full-lines
 # RUN: llvm-objcopy --help | FileCheck --check-prefix=OBJCOPY-USAGE %s --match-full-lines
 # RUN: not llvm-objcopy 2>&1 | FileCheck --check-prefix=OBJCOPY-USAGE %s --match-full-lines
 # RUN: not llvm-objcopy -- 2>&1 | FileCheck --check-prefix=OBJCOPY-USAGE %s --match-full-lines
 # RUN: not llvm-objcopy -abcabc 2>&1 | FileCheck --check-prefix=UNKNOWN-ARG %s
 # RUN: not llvm-objcopy --abcabc 2>&1 | FileCheck --check-prefix=UNKNOWN-ARG %s
 # RUN: not llvm-objcopy --strip-debug 2>&1 | FileCheck %s --check-prefix=NO-INPUT-FILES
 
 # RUN: llvm-strip -h | FileCheck --check-prefix=STRIP-USAGE %s --match-full-lines
 # RUN: llvm-strip --help | FileCheck --check-prefix=STRIP-USAGE %s --match-full-lines
 # RUN: not llvm-strip 2>&1 | FileCheck --check-prefix=STRIP-USAGE %s --match-full-lines
+# RUN: not llvm-strip -- 2>&1 | FileCheck --check-prefix=STRIP-USAGE %s --match-full-lines
 # RUN: not llvm-strip -abcabc 2>&1 | FileCheck --check-prefix=UNKNOWN-ARG %s
 # RUN: not llvm-strip --abcabc 2>&1 | FileCheck --check-prefix=UNKNOWN-ARG %s
 # RUN: not llvm-strip --strip-debug 2>&1 | FileCheck %s --check-prefix=NO-INPUT-FILES
 
 # RUN: llvm-install-name-tool -h | FileCheck --check-prefix=INSTALL-NAME-TOOL-USAGE %s --match-full-lines
 # RUN: llvm-install-name-tool --help | FileCheck --check-prefix=INSTALL-NAME-TOOL-USAGE %s --match-full-lines
 # RUN: not llvm-install-name-tool 2>&1 | FileCheck --check-prefix=INSTALL-NAME-TOOL-USAGE %s --match-full-lines
 # RUN: not llvm-install-name-tool -abcabc 2>&1 | FileCheck --check-prefix=UNKNOWN-ARG %s

llvm/tools/llvm-objcopy/ConfigManager.cpp

parseBitcodeStripOptions(ArrayRef<const char *> ArgsArr) {
   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.
 Expected<DriverConfig>
-parseStripOptions(ArrayRef<const char *> ArgsArr,
+parseStripOptions(ArrayRef<const char *> RawArgsArr,
                   llvm::function_ref<Error(Error)> ErrorCallback) {
+  const char *const *DashDash =
+      std::find_if(RawArgsArr.begin(), RawArgsArr.end(),
+                   [](StringRef Str) { return Str == "--"; });
+  ArrayRef<const char *> ArgsArr = makeArrayRef(RawArgsArr.begin(), DashDash);
+  if (DashDash != RawArgsArr.end())
+    DashDash = std::next(DashDash);
+
   StripOptTable T;
   unsigned MissingArgumentIndex, MissingArgumentCount;
   llvm::opt::InputArgList InputArgs =
       T.ParseArgs(ArgsArr, MissingArgumentIndex, MissingArgumentCount);
 
-  if (InputArgs.size() == 0) {
+  if (InputArgs.size() == 0 && DashDash == RawArgsArr.end()) {
     printHelp(T, errs(), ToolType::Strip);
     exit(1);
   }
 
   if (InputArgs.hasArg(STRIP_help)) {
     printHelp(T, outs(), ToolType::Strip);
     exit(0);
   }
 
   if (InputArgs.hasArg(STRIP_version)) {
     outs() << "llvm-strip, compatible with GNU strip\n";
     cl::PrintVersionMessage();
     exit(0);
   }
 
   SmallVector<StringRef, 2> Positional;
   for (auto Arg : InputArgs.filtered(STRIP_UNKNOWN))
     return createStringError(errc::invalid_argument, "unknown argument '%s'",
                              Arg->getAsString(InputArgs).c_str());
   for (auto Arg : InputArgs.filtered(STRIP_INPUT))
     Positional.push_back(Arg->getValue());
+  std::copy(DashDash, RawArgsArr.end(), std::back_inserter(Positional));
 
   if (Positional.empty())
     return createStringError(errc::invalid_argument, "no input file specified");
 
   if (Positional.size() > 1 && InputArgs.hasArg(STRIP_output))
     return createStringError(
         errc::invalid_argument,
         "multiple input files cannot be used in combination with -o");