[llvm-objcopy] Quote passed rpath args in errors

[keith]

Summary

This makes it more clear in error messages what arguments were passed to
llvm-install-name-tool.

[jhenderson]

LGTM, thanks!

[keith]

Thanks all! Can someone land this for me again?

llvm/test/tools/llvm-objcopy/MachO/install-name-tool-add-rpath.test

 
 # NEW-RPATH: cmd LC_RPATH
 # NEW-RPATH-NEXT: cmdsize
 # NEW-RPATH-NEXT: @executable_path/.
 
 # RUN: not llvm-install-name-tool -add_rpath @executable_path/. %t.i386 2>&1 \
 # RUN: | FileCheck --check-prefix=DUPLICATE-RPATH %s
 
-# DUPLICATE-RPATH: duplicate load command
+# DUPLICATE-RPATH: rpath '@executable_path/.' would create a duplicate load command
 
 # RUN: not llvm-install-name-tool -add_rpath @executable_path/. 2>&1 \
 # RUN: | FileCheck --check-prefix=NO-INPUT %s
 
 # NO-INPUT: no input file specified
 
 ## Add same RPATH twice:
 # RUN: not llvm-install-name-tool -add_rpath @executable_X \
 # RUN:                            -add_rpath @executable_X %t.i386 2>&1 \
 # RUN: | FileCheck --check-prefix=DOUBLE %s
 
-# DOUBLE: duplicate load command
+# DOUBLE: rpath '@executable_X' would create a duplicate load command
 
 ## Check that cmdsize accounts for NULL terminator.
 # RUN: yaml2obj %p/Inputs/x86_64.yaml -o %t.x86_64
 # RUN: llvm-install-name-tool -add_rpath abcd %t.x86_64
 # RUN: llvm-objdump -p %t.x86_64 | FileCheck %s --check-prefix=RPATH-SIZE
 
 # RPATH-SIZE: cmd LC_RPATH
 # RPATH-SIZE-NEXT: cmdsize 24
 # RPATH-SIZE-NEXT: path abcd

llvm/test/tools/llvm-objcopy/MachO/install-name-tool-delete-rpath.test

 
 # ABSENT-RPATH: no LC_RPATH load command with path: @executable_a/.
 
 ## Adding and deleting RPATH at the same time:
 # RUN: not llvm-install-name-tool -add_rpath @executable_b/. \
 # RUN:                            -delete_rpath @executable_b/. %t 2>&1 | \
 # RUN:   FileCheck %s --check-prefix=COMBINED
 
-# COMBINED: cannot specify both -add_rpath @executable_b/. and -delete_rpath @executable_b/.
+# COMBINED: cannot specify both -add_rpath '@executable_b/.' and -delete_rpath '@executable_b/.'
 
 ## Remove all RPATHS
 # RUN: yaml2obj %s -o %t2
 # RUN: llvm-install-name-tool -delete_all_rpaths %t2
 # RUN: llvm-objdump -p %t2 | FileCheck %s
 
 # CHECK-NOT: LC_RPATH
 

llvm/test/tools/llvm-objcopy/MachO/install-name-tool-prepend-rpath.test

 
 # DYLIB:      cmd LC_RPATH
 # DYLIB-NEXT: cmdsize
 # DYLIB-NEXT: first_rpath
 
 # RUN: not llvm-install-name-tool -prepend_rpath first_rpath %t.i386 2>&1 | \
 # RUN:   FileCheck --check-prefix=DUPLICATE-RPATH %s
 
-# DUPLICATE-RPATH: rpath first_rpath would create a duplicate load command
+# DUPLICATE-RPATH: rpath 'first_rpath' would create a duplicate load command
 
 ## Prepend same RPATH twice:
 # RUN: not llvm-install-name-tool -prepend_rpath @executable_X \
 # RUN:                            -prepend_rpath @executable_X %t.i386 2>&1 | \
 # RUN:   FileCheck --check-prefix=DOUBLE %s
 
-# DOUBLE: rpath @executable_X would create a duplicate load command
+# DOUBLE: rpath '@executable_X' would create a duplicate load command
 
 ## Prepend and delete RPATH:
 # RUN: not llvm-install-name-tool -prepend_rpath foo \
 # RUN:                            -delete_rpath foo %t.i386 2>&1 | \
 # RUN:   FileCheck --check-prefix=DELETE %s
 
-# DELETE: cannot specify both -prepend_rpath foo and -delete_rpath foo
+# DELETE: cannot specify both -prepend_rpath 'foo' and -delete_rpath 'foo'
 
 ## Prepend and replace RPATH:
 # RUN: not llvm-install-name-tool -prepend_rpath foo \
 # RUN:                            -rpath foo bar %t.i386 2>&1 | \
 # RUN:   FileCheck --check-prefix=REPLACE %s
 
-# REPLACE: cannot specify both -prepend_rpath foo and -rpath foo bar
+# REPLACE: cannot specify both -prepend_rpath 'foo' and -rpath 'foo' 'bar'
 
 ## Check that cmdsize accounts for NULL terminator:
 # RUN: yaml2obj %p/Inputs/x86_64.yaml -o %t.x86_64
 # RUN: llvm-install-name-tool -prepend_rpath abcd %t.x86_64
 # RUN: llvm-objdump -p %t.x86_64 | FileCheck %s --check-prefix=RPATH-SIZE
 
 # RPATH-SIZE:      cmd LC_RPATH
 # RPATH-SIZE-NEXT: cmdsize 24
 # RPATH-SIZE-NEXT: path abcd

llvm/test/tools/llvm-objcopy/MachO/install-name-tool-rpath.test

 
 # ABSENT-RPATH: no LC_RPATH load command with path: @executable_a/.
 
 ## Updating to an existing RPath:
 # RUN: not llvm-install-name-tool -rpath @executable_d/. ABCD %t 2>&1 | \
 # RUN:   FileCheck %s --check-prefix=EXISTING
 # RUN: cmp %t %t1
 
-# EXISTING: rpath ABCD would create a duplicate load command
+# EXISTING: rpath 'ABCD' would create a duplicate load command
 
 ## Duplicate RPath entries:
 # RUN: not llvm-install-name-tool -rpath DDD1/. @exec_d/. \
 # RUN:                            -rpath @exec_d/. DDD2/. %t 2>&1 | \
 # RUN:   FileCheck %s --check-prefix=DUPLICATE
 # RUN: cmp %t %t1
 
-# DUPLICATE: cannot specify both -rpath DDD1/. @exec_d/. and -rpath @exec_d/. DDD2/.
+# DUPLICATE: cannot specify both -rpath 'DDD1/.' '@exec_d/.' and -rpath '@exec_d/.' 'DDD2/.'
 
 ## Updating and deleting RPath at the same time:
 # RUN: not llvm-install-name-tool -rpath @executable_d/. DD/. \
 # RUN:                            -delete_rpath @executable_d/. %t 2>&1 | \
 # RUN:   FileCheck %s --check-prefix=COMBINED-DELETE
 # RUN: cmp %t %t1
 
-# COMBINED-DELETE: cannot specify both -delete_rpath @executable_d/. and -rpath @executable_d/. DD/.
+# COMBINED-DELETE: cannot specify both -delete_rpath '@executable_d/.' and -rpath '@executable_d/.' 'DD/.'
 
 ## Updating and adding RPath at the same time:
 # RUN: not llvm-install-name-tool -rpath @executable_e/. EE/. \
 # RUN:                            -add_rpath @executable_e/. %t 2>&1 | \
 # RUN:   FileCheck %s --check-prefix=COMBINED-ADD
 # RUN: cmp %t %t1
 
-# COMBINED-ADD: cannot specify both -add_rpath @executable_e/. and -rpath @executable_e/. EE/.
+# COMBINED-ADD: cannot specify both -add_rpath '@executable_e/.' and -rpath '@executable_e/.' 'EE/.'
 
 ## Missing an RPath argument:
 # RUN: not llvm-install-name-tool %t -rpath @executable_e/. 2>&1 | \
 # RUN:   FileCheck %s --check-prefix=MISSING
 # RUN: cmp %t %t1
 
 ## Missing both RPath arguments:
 # RUN: not llvm-install-name-tool %t -rpath 2>&1 | \

llvm/tools/llvm-objcopy/CopyConfig.cpp

parseInstallNameToolOptions(ArrayRef<const char *> ArgsArr) {
 
   for (auto Arg : InputArgs.filtered(INSTALL_NAME_TOOL_delete_rpath)) {
     StringRef RPath = Arg->getValue();
 
     // Cannot add and delete the same rpath at the same time.
     if (is_contained(Config.RPathToAdd, RPath))
       return createStringError(
           errc::invalid_argument,
-          "cannot specify both -add_rpath %s and -delete_rpath %s",
+          "cannot specify both -add_rpath '%s' and -delete_rpath '%s'",
           RPath.str().c_str(), RPath.str().c_str());
     if (is_contained(Config.RPathToPrepend, RPath))
       return createStringError(
           errc::invalid_argument,
-          "cannot specify both -prepend_rpath %s and -delete_rpath %s",
+          "cannot specify both -prepend_rpath '%s' and -delete_rpath '%s'",
           RPath.str().c_str(), RPath.str().c_str());
 
     Config.RPathsToRemove.insert(RPath);
   }
 
   for (auto *Arg : InputArgs.filtered(INSTALL_NAME_TOOL_rpath)) {
     StringRef Old = Arg->getValue(0);
     StringRef New = Arg->getValue(1);
 
     auto Match = [=](StringRef RPath) { return RPath == Old || RPath == New; };
 
     // Cannot specify duplicate -rpath entries
     auto It1 = find_if(
         Config.RPathsToUpdate,
         [&Match](const DenseMap<StringRef, StringRef>::value_type &OldNew) {
           return Match(OldNew.getFirst()) || Match(OldNew.getSecond());
         });
     if (It1 != Config.RPathsToUpdate.end())
       return createStringError(errc::invalid_argument,
-                               "cannot specify both -rpath " + It1->getFirst() +
-                                   " " + It1->getSecond() + " and -rpath " +
-                                   Old + " " + New);
+                               "cannot specify both -rpath '" +
+                                   It1->getFirst() + "' '" + It1->getSecond() +
+                                   "' and -rpath '" + Old + "' '" + New + "'");
 
     // Cannot specify the same rpath under both -delete_rpath and -rpath
     auto It2 = find_if(Config.RPathsToRemove, Match);
     if (It2 != Config.RPathsToRemove.end())
       return createStringError(errc::invalid_argument,
-                               "cannot specify both -delete_rpath " + *It2 +
-                                   " and -rpath " + Old + " " + New);
+                               "cannot specify both -delete_rpath '" + *It2 +
+                                   "' and -rpath '" + Old + "' '" + New + "'");
 
     // Cannot specify the same rpath under both -add_rpath and -rpath
     auto It3 = find_if(Config.RPathToAdd, Match);
     if (It3 != Config.RPathToAdd.end())
       return createStringError(errc::invalid_argument,
-                               "cannot specify both -add_rpath " + *It3 +
-                                   " and -rpath " + Old + " " + New);
+                               "cannot specify both -add_rpath '" + *It3 +
+                                   "' and -rpath '" + Old + "' '" + New + "'");
 
     // Cannot specify the same rpath under both -prepend_rpath and -rpath.
     auto It4 = find_if(Config.RPathToPrepend, Match);
     if (It4 != Config.RPathToPrepend.end())
       return createStringError(errc::invalid_argument,
-                               "cannot specify both -prepend_rpath " + *It4 +
-                                   " and -rpath " + Old + " " + New);
+                               "cannot specify both -prepend_rpath '" + *It4 +
+                                   "' and -rpath '" + Old + "' '" + New + "'");
 
     Config.RPathsToUpdate.insert({Old, New});
   }
 
   if (auto *Arg = InputArgs.getLastArg(INSTALL_NAME_TOOL_id)) {
     Config.SharedLibId = Arg->getValue();
     if (Config.SharedLibId->empty())
       return createStringError(errc::invalid_argument,

llvm/tools/llvm-objcopy/MachO/MachOObjcopy.cpp

static Error processLoadCommands(const CopyConfig &Config, Object &Obj) {
   for (const auto &OldNew : Config.RPathsToUpdate) {
     StringRef Old = OldNew.getFirst();
     StringRef New = OldNew.getSecond();
     if (RPaths.count(Old) == 0)
       return createStringError(errc::invalid_argument,
                                "no LC_RPATH load command with path: " + Old);
     if (RPaths.count(New) != 0)
       return createStringError(errc::invalid_argument,
-                               "rpath " + New +
-                                   " would create a duplicate load command");
+                               "rpath '" + New +
+                                   "' would create a duplicate load command");
   }
 
   // Update load commands.
   for (LoadCommand &LC : Obj.LoadCommands) {
     switch (LC.MachOLoadCommand.load_command_data.cmd) {
     case MachO::LC_ID_DYLIB:
       if (Config.SharedLibId)
         updateLoadCommandPayloadString<MachO::dylib_command>(
case MachO::LC_LOAD_WEAK_DYLIB:
       break;
     }
   }
 
   // Add new RPaths.
   for (StringRef RPath : Config.RPathToAdd) {
     if (RPaths.count(RPath) != 0)
       return createStringError(errc::invalid_argument,
-                               "rpath " + RPath +
-                                   " would create a duplicate load command");
+                               "rpath '" + RPath +
+                                   "' would create a duplicate load command");
     RPaths.insert(RPath);
     Obj.LoadCommands.push_back(buildRPathLoadCommand(RPath));
   }
 
   for (StringRef RPath : Config.RPathToPrepend) {
     if (RPaths.count(RPath) != 0)
       return createStringError(errc::invalid_argument,
-                               "rpath " + RPath +
-                                   " would create a duplicate load command");
+                               "rpath '" + RPath +
+                                   "' would create a duplicate load command");
 
     RPaths.insert(RPath);
     Obj.LoadCommands.insert(Obj.LoadCommands.begin(),
                             buildRPathLoadCommand(RPath));
   }
 
   // Unlike appending rpaths, the indexes of subsequent load commands must
   // be recalculated after prepending one.