[llvm-install-name-tool] Add -rpath option

[sameerarora101]

Summary

Implement -rpath option for install-name-tool via replacing the old
rpath entry with the new one in place.

[sameerarora101]

cmdsize update

[sameerarora101]

llvm/tools/llvm-objcopy/CopyConfig.cpp

853

Unlike -delete_rpath, we thought it was necessary to check duplication in case of -rpath. This is because it would not be useful to do something like -rpath A B .... -rpath A C or something like -rpath A B ... -rpath B C. Additionally, this also matches XCode's -rpath behavior.

867

This highlights a warning :
warning: lambda templates are only available with -std=c++2a or -std=gnu++2a [-Wpedantic]

Is this fine? Not sure what I should do about it?

[sameerarora101]

llvm/tools/llvm-objcopy/CopyConfig.cpp

853

I can remove this check if people feel the need?

[jhenderson]

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

38–39

Nit: indent the FileCheck line by an additional space, matching e.g. lines 8 and 19.

83

It probably is best to do this after each individual error, rather than once at the end (just to make sure that one doesn't change the binary and the next change it back).

[jhenderson]

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

34

Use cp rather than llvm-objcopy to copy the file.

67

You probably want to show the behaviour when different values are used too.

74

Ditto.

76–80

How do you say "RPath" - "ah-path" or something else? If "ah-path", (which is how I read it), the "a" before should be "an".

Also might be worth having another case here where -rpath has no arguments at all.

llvm/tools/llvm-objcopy/CopyConfig.cpp

853

No, it makes reasonable sense to me. I guess if people ware used to XCode's behaviour, it's okay to follow it.

854–855

There's an llvm::find_if which just takes the container. There's an is_contained method too which does std::find + the end check, which might be worth extending to allow it to take a lambda (something like is_contained_if). You might want to do that (in a prerequisite patch), to simplify a lot of this code.

860

adl_end(Config.RPathsToUpdate) -> Config.RPathsToUpdate.end() is more common syntax in LLVM.

867

LLVM is built with C++14 so this will fail the build on most build bots.

I'm actually struggling to follow the logic of this whole section, if I'm honest! If I follow it right, I think you could simplify it to something like below:

if (is_contained(Config.RPathsToRemove), OldRPath)
  return creatStringError(...);
if (is_contained(Config.RPathsToRemove), NewRPath)
  return creatStringError(...);
if (is_contained(Config.RPathsToAdd), OldRPath)
  return creatStringError(...);
if (is_contained(Config.RPathsToAdd), NewRPath)
  return creatStringError(...);

Alternatively, with an is_contained_if you could fold it down to just two cases, one for each of the two existing Config variables.

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

124–125

These two lines can be folded together I believe using std::back_insertor in the std::copy line, right?

273

NULL -> nullptr

Same below.

[sameerarora101]

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

67

With different values it works fine - added the test now, thanks!

76–80

yup, it should be "an", thanks 😄(added the test too)

llvm/tools/llvm-objcopy/CopyConfig.cpp

854–855

oh thanks, llvm::find_if helps. But, I don't want is_contained because it just returns true or false. I also need access to the element itself in case true is returned (error msg use the element). Therefore, I am explicitly checking whether It != RPaths.end() and using the iterator to access the element if the above condition is true.

If I use is_contained, I would have to again iterate through the vector to find the element, right?

867

ok, i got rid of the lambda. No warnings are thrown (the code is little cleaner too perhaps).

For the pattern,

if (is_contained(Config.RPathsToRemove), OldRPath)
  return creatStringError(...);

the issue is again that I need access to the element (if is_contained is true) to construct the error msg which means iterating the vector again. Instead I am following this pattern:

It = find_if(Config.RPathsToRemove, StringChecker)
if (It != Config.RPathsToRemove.end())
  return createStringError(..<use It here>..);

It = find_if(Config.RPathsToAdd, StringChecker)
if (It != Config.RPathsToAdd.end())
  return createStringError(..<use It here>..);

Is this fine?

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

124–125

So I tried folding the 2 lines into std::copy(NewName.begin(), NewName.end(), std::back_insertor(LC->Payload)); but it fails the rpath test giving the following error: truncated or malformed object (load command 1 extends past end of file).

Surprisingly, even LC->Payload.assign(NewName.begin(), NewName.end()); didn't work (giving the same error as above).

However, the initial approach seems to work lol, not sure what's the difference in behavior from these new approaches?

[sameerarora101]

• Updating test as per comments
• Cleaning CopyConfig.cpp

[sameerarora101]

auto Arg -> auto *Arg (as per clang-tidy, fails build otherwise).

[jhenderson]

llvm/tools/llvm-objcopy/CopyConfig.cpp

853–855

Move this down to where it's used for the first time.

854–855

Yeah, that's right. I didn't realise you wanted to use the element, so find_if looks good.

867

Thanks. I find the new code much easier to follow.

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

124–125

That error message doesn't really give me enough info to understand what's going on. Did you perhaps write one too many bytes somewhere/not write the null byte? Regarding the copy, you'd also need to clear beforehand, to avoid it just appending the data to the end, so actually assign is probably want you want anyway, but I believe you may need to push a null byte to the end too, i.e.

LC->Payload.assign(NewName.begin(), NewName.end());
LC->Payload.push_back('\0');

[sameerarora101]

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

124–125

Oh I see now why it wasn't working - I didn't align the Payload to a multiple of 8. NewCmd has been aligned to a multiple of 8 as

uint32_t NewCmdsize = alignTo(sizeof(T) + NewName.size() + 1, 8);

So using the following approach

LC->Payload.assign(NewName.begin(), NewName.end());
LC->Payload.push_back('\0');

still won't be sufficient as we need to align it to multiple of 8. (Something like pushing '\0' for NewCmdSize - sizeof(T) - NewName.size() times)

I couldn't find a cleaner way of doing it beside the current implementation itself, what do you think?

[sameerarora101]

Moving StringChecker down before its first use.

[jhenderson]

LGTM, but I would like @smeenai or @alexshap (or someone else with Mach-O knowledge) to be happy with the logic too.

[alexander-shaposhnikov]

llvm/tools/llvm-objcopy/CopyConfig.cpp

852

1. llvm::find_if - since the code is already inside the namespace "llvm" the prefix is unnecessary.
2. StringRef can be passed by value
3. You can reuse your code a tiny bit further + I would probably rename some things here:

auto Match  = [=](StringRef RPath) { return RPath == Old || RPath == New; };
    
auto It1 = find_if(Config.RPathsToUpdate, [](const std::pair<StringRef, StringRef> & OldNew) { 
                                      return Match(OldNew.first) || Match(OldNew.second); });

888

Config.RPathsToUpdate.emplace_back(Old, New);

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

120

Different load commands have very different payloads, so i think this function is a bit misleading.
What about this:

static void updateRPathLoadCommand(LoadCommand &LC, StringRef NewRPath) {
    assert( ....  ); // make sure that LC is indeed LC_RPATH;
    ...
}

269

StringRef Old, New;
for (std::tie(Old, New) : Config.RPathsToUpdate) {
    ...
}

276

If I'm not mistaken there are multiple places where this construction is used, so i think we need to factor out it into a helper function.

StringRef getRPath(const LoadCommand &LC) {
    assert(LC.MachOLoadCommand.load_command_data.cmd == MachO::LC_RPATH &&
              "LC_RPATH is expected");
    return StringRef(reinterpret_cast<char *>(LC.Payload.data()),
                              LC.Payload.size()).rtrim('\0');
}

Plus I would slightly reorganize the code (see below):

StringRef Old, New;
for (std::tie(Old, New) : Config.RPathsToUpdate) {
     auto It = find_if(Obj.LoadCommands, [](const LoadCommand &LC) { 
                              return LC.MachOLoadCommand.load_command_data.cmd == MachO::LC_RPATH &&
                                         getRPath(LC) == Old); });
     if (It == Obj.LoadCommands.end())
         ...
     if (getRPath(*It) == New)
        ...
     updateRPathLoadCommand(*It, New);
}

[sameerarora101]

llvm/tools/llvm-objcopy/CopyConfig.cpp

852

nice, thanks! 😊

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

120

So it turns out the way to update LC name follows the same logic (as above) for all the options -rpath, -id, and -change for llvm-install-name-tool. Here is how I was thinking about using it :

-rpath : updateLoadCommandName<MachO::rpath_command>(&LC, NewRPath);,

-id : updateLoadCommandName<MachO::dylib_command>(&LC, NewID);

-change : updateLoadCommandName<MachO::dylib_command>(&LC, NewInstallName);

This allowed me to refactor the common logic of updating the name payload of any load command (instead of making it specific for rpath). Would this not be ok?

269

for (std::tie(Old, New) : Config.RPathsToUpdate) is perhaps not allowed : it says for range declaration must declare a variable.

I have updated it to

StringRef OldRPath, NewRPath;
for (auto &RPair : Config.RPathsToUpdate) {
   std::tie(OldRPath, NewRPath) = RPair;
   ...
}

I hope this works?

276

Ok, I refactored getting the RPath construction into a separate function and updated it to be used by add_rpath, delete_rpath and rpath options.

For the reorganization, we need to check that New doesn't exists for any of the LC_RPATH commands. Please correct me in case I am wrong, but it seems that the above suggested code finds an LC_RPATH that matches Old. However, above, we throw an error only when either we can't find such a command (It == Obj.LoadCommands.end()) or if the command's rpath equals the New value(getRPath(*It) == New). But we need to throw the latter error in case any command's value equals New. Here is what I have done now:

StringRef Old, New;
  for (auto &RPair : Config.RPathsToUpdate) {
    std::tie(Old, New) = RPair;

    auto Match = [&Obj] (StringRef RPath) {
      return find_if(Obj.LoadCommands, [=](const LoadCommand &LC) {
        return LC.MachOLoadCommand.load_command_data.cmd == MachO::LC_RPATH &&
               getRPath(LC) == RPath;
      });
    };

    auto NewIt = Match(New);
    if (NewIt != Obj.LoadCommands.end())
      return Error

    auto OldIt = Match(Old);
    if (OldIt == Obj.LoadCommands.end())
        return Error

    updateLoadCommandName<MachO::rpath_command>(&(*OldIt), New);
  }

Would this work? Thanks.

[sameerarora101]

Refactoring code under CopyConfig and MachOObjcopy.

[alexander-shaposhnikov]

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

25

nit: static

P.S. but, please, take a look at my comment below (line 278), let's try to understand the broader picture.
It doesn't mean that something is wrong now but it's necessary to think about the best way to organize this code
(to improve readability and avoid code duplication in the future).

120

alright, let's give it a try.

A few nits:

1. since in this function LC can't be null, it would be better to pass a reference (from the readability perspective)
2. maybe there is a better name for this function ? it actually updates the payload,

so I'd either use something like updateLoadCommand or updateLoadCommandPayload.

1. This code requires assert (that the type of the load command is one of those which you listed above)
2. T should be replaced with something more specific & informative, e.g. LCType

273

I'm not sure that Match is the best possible name here (it's more like findLoadCommand, but this requires more thinking),
but, more importantly, it's necessary to find out how the case of dylib_command differs from rpath_command.
Based on this information one will be able to refactor this code / make an informed decision - should it be generalized or not.

[sameerarora101]

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

25

ok thanks 😊 I have refactored this too we'll be needing it for -change option as well. I have renamed it to getLoadCommandPayload. Is this fine?

273

So it seems that this should not be generalized as -id and -change would not be making use of it. This is because:

• this current Match function takes in a StringRef RPath and checks for LC such that LC is of type MachO::LC_RPATH and matches this RPath passed in as argument.

• for -id: I need to search for a LC such that LC.MachOLoadCommand.load_command_data.cmd == MachO::LC_ID_DYLIB. Since there can only be one such LC, we don't need to pass in another StringRef Name as an argument. And so we wouldn't be using the Match function as we don't wish to pass in an argument to it.

• for -change: (currently I am matching the behavior of this option with that of XCode's). To match XCode's behavior, implementation of this option involves iterating over LCs before iterating over provided (old, new) install name pairs. More precisely,

here is what we are doing here in case of -rpath:

for (RPair : Config.RPathsToUpdate) {.                     // iterate over provided (old,new) pairs
  ...
  find_if( Obj.LoadCommands, ...)                          // iterate over all LCs to find the right LC
  ...

However, the iteration would be reversed in case of -change option as follows:

for (LoadCommand &LC : Obj.LoadCommands) {.                       // iterate over all LCs first
  ...
  for (auto InstallNamePair : Config.InstallNamesToUpdate)        // for each LC now, find the (old,new) pair
  ...

You see how the order of for loops is reversed. This reversed iteration is necessary in case of -change option to match the behavior of XCode's install_name_tool. And because of this, we wouldn't be able to use the Match function above (as that requires taking in a name argument and iterating over LCs after).

In light of this I have currently renamed Match to FindRPathLC (as it is rpath specific). What do you think?

Also. in case we do end up changing -change option's behavior in a later diff, I can then perhaps refactor the common code out later? (For now though, it doesn't seems to be needing refactoring.)

[sameerarora101]

Updating MachOObjcopy

[sameerarora101]

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

25

I haven't added an assert for LC_REEXPORT_DYLIB , LC_LOAD_UPWARD_DYLIB or LC_LAZY_LOAD_DYLIB as llvm-objcopy doesn't support them yet. I can add a TODO comment here if needed?

[sameerarora101]

auto &RPair -> const auto &RPair (for clang-tidy)

[sameerarora101]

@alexshap Please also take a brief look at how updateLoadCommandPayload function is being used again for -id option here: https://reviews.llvm.org/D82410. What do you think?

[jhenderson]

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

25

TODO comments for missing functionality is usually reasonable, though I wonder whether we should just have an unsupported error somewhere for such cases.

121–129

Since this assert is common between the two functions, it might be worth pulling it into a simple helper. Something like "isLoadCommandWithPayload" and just call it instead:

assert(isLoadCommandWithPayload(LC && "unsupported load command encountered");

[sameerarora101]

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

25

so there is currently an unsupported error thrown by MachO/MachOLayoutBuilder.cpp for such cases:

...
 case MachO::LC_SOURCE_VERSION:
      // Nothing to update.
      break;
    default:
      // Abort if it's unsupported in order to prevent corrupting the object.
      return createStringError(llvm::errc::not_supported,
                               "unsupported load command (cmd=0x%x)", cmd);
    }
...

But yeah I can add a TODO above as well. Thanks!

121–129

nice, thanks 😊

[sameerarora101]

refactoring assert and adding a TODO

[jhenderson]

LGTM again, but please wait for @alexshap.

[sameerarora101]

Rebasing (for harbormaster)

[sameerarora101]

Here is the diff for -change options as well: https://reviews.llvm.org/D82613. Please take a look at how I am using updateLoadCommandPayload for -id and -change. Thanks!

[sameerarora101]

changing name ...Payload(...) -> ...PayloadString(...)

[gchatelet]

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

35

In release mode warning: unused function 'isLoadCommandWithPayloadString'. Maybe you want to guard the function definition?

#ifndef NDEBUG
static bool isLoadCommandWithPayloadString(const LoadCommand &LC) { ... }
#endif

[sameerarora101]

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

35

Thanks for catching this! I was running my tests with asserts on. I have pushed a fix for the issue here: https://reviews.llvm.org/D82768 Please review.

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

+## This test checks updating a LC_RPATH load command in a MachO binary.
+
+# RUN: yaml2obj %s -o %t
+
+## Updating a single RPath entry:
+# RUN: llvm-install-name-tool -rpath @executable_a/. @executable_A/. %t
+# RUN: llvm-objdump -p %t | \
+# RUN:   FileCheck %s --check-prefix=RPATHS --implicit-check-not=@executable
+
+# RPATHS: @executable_A/.
+# RPATHS: @executable_short_test
+# RPATHS: @executable_long_test/.
+# RPATHS: @executable_d/.
+
+## Updating multiple RPath entries:
+# RUN: llvm-install-name-tool -rpath @executable_short_test/. @executable_test/. \
+# RUN:                        -rpath @executable_long_test/. @executable_long_long_test/. %t
+# RUN: llvm-objdump -p %t | \
+# RUN:   FileCheck %s --check-prefix=RPATHS-MULTIPLE --implicit-check-not=@executable
+
+# RPATHS-MULTIPLE: @executable_A/.
+# RPATHS-MULTIPLE: @executable_test/.
+# RPATHS-MULTIPLE: @executable_long_long_test/.
+# RPATHS-MULTIPLE: @executable_d/.
+
+## Check that cmdsize accounts for NULL terminator:
+# RUN: llvm-install-name-tool -rpath @executable_A/. ABCD %t
+# RUN: llvm-objdump -p %t | FileCheck %s --check-prefix=RPATH-SIZE
+
+# RPATH-SIZE:      cmd LC_RPATH
+# RPATH-SIZE-NEXT: cmdsize 24
+# RPATH-SIZE-NEXT: path ABCD
+
+# RUN: llvm-objcopy %t %t1
+
+## Updating multiple RPath entries where one exists and the other doesn't:
+# RUN: not llvm-install-name-tool -rpath @executable_test/. @executable/. \
+# RUN:                            -rpath @executable_long_test/. @executable_long_longest/. %t 2>&1 | \
+# RUN:  FileCheck %s --check-prefix=RPATHS-FAIL
+
+# RPATHS-FAIL: no LC_RPATH load command with path: @executable_long_test/.
+
+## Updating a nonexistent RPath:
+# RUN: not llvm-install-name-tool -rpath @executable_a/. @executable_AA/. %t 2>&1 | \
+# RUN:   FileCheck %s --check-prefix=ABSENT-RPATH
+
+# 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
+
+# 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
+
+# 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
+
+# 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
+
+# COMBINED-ADD: cannot specify both -add_rpath @executable_e/. and -rpath @executable_e/. EE/.
+
+## Missing a RPath argument:
+# RUN: not llvm-install-name-tool %t -rpath @executable_e/. 2>&1 | \
+# RUN:   FileCheck %s --check-prefix=MISSING
+
+# MISSING: missing argument to -rpath option
+
+## Check that binary is not modified after errors:
+# RUN: cmp %t %t1
+
+--- !mach-o
+FileHeader:
+  magic:           0xFEEDFACF
+  cputype:         0x01000007
+  cpusubtype:      0x00000003
+  filetype:        0x00000001
+  ncmds:           4
+  sizeofcmds:      144
+  flags:           0x00002000
+  reserved:        0x00000000
+LoadCommands:
+  - cmd:             LC_RPATH
+    cmdsize:         32
+    path:            12
+    PayloadString:   '@executable_a/.'
+  - cmd:             LC_RPATH
+    cmdsize:         40
+    path:            12
+    PayloadString:   '@executable_short_test/.'
+  - cmd:             LC_RPATH
+    cmdsize:         40
+    path:            12
+    PayloadString:   '@executable_long_test/.'
+  - cmd:             LC_RPATH
+    cmdsize:         32
+    path:            12
+    PayloadString:   '@executable_d/.'

llvm/tools/llvm-objcopy/CopyConfig.h

struct CopyConfig {
   DiscardType DiscardMode = DiscardType::None;
   Optional<StringRef> NewSymbolVisibility;
 
   // Repeated options
   std::vector<StringRef> AddSection;
   std::vector<StringRef> DumpSection;
   std::vector<StringRef> SymbolsToAdd;
   std::vector<StringRef> RPathToAdd;
+  std::vector<std::pair<StringRef, StringRef>> RPathsToUpdate;
   DenseSet<StringRef> RPathsToRemove;
 
   // Section matchers
   NameMatcher KeepSection;
   NameMatcher OnlySection;
   NameMatcher ToRemove;
 
   // Symbol matchers

llvm/tools/llvm-objcopy/CopyConfig.cpp

 parseInstallNameToolOptions(ArrayRef<const char *> ArgsArr) {
   DriverConfig DC;
   CopyConfig Config;
   InstallNameToolOptTable T;
   unsigned MissingArgumentIndex, MissingArgumentCount;
   llvm::opt::InputArgList InputArgs =
       T.ParseArgs(ArgsArr, MissingArgumentIndex, MissingArgumentCount);
 
+  if (MissingArgumentCount)
+    return createStringError(
+        errc::invalid_argument,
+        "missing argument to " +
+            StringRef(InputArgs.getArgString(MissingArgumentIndex)) +
+            " option");
+
   if (InputArgs.size() == 0) {
     printHelp(T, errs(), "llvm-install-name-tool");
     exit(1);
   }
 
   if (InputArgs.hasArg(INSTALL_NAME_TOOL_help)) {
     printHelp(T, outs(), "llvm-install-name-tool");
     exit(0);
if (is_contained(Config.RPathToAdd, RPath))
       return createStringError(
           errc::invalid_argument,
           "cannot specify both -add_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 OldRPath = Arg->getValue(0);
+    StringRef NewRPath = Arg->getValue(1);
+
+    // Cannot specify duplicate -rpath entries
+    auto It1 = std::find_if(
+        adl_begin(Config.RPathsToUpdate), adl_end(Config.RPathsToUpdate),
+        [&](const std::pair<StringRef, StringRef> &RPair) {
+          return RPair.first == OldRPath || RPair.first == NewRPath ||
+                 RPair.second == OldRPath || RPair.second == NewRPath;
+        });
+    if (It1 != adl_end(Config.RPathsToUpdate))
+      return createStringError(
+          errc::invalid_argument,
+          "cannot specify both -rpath %s %s and -rpath %s %s",
+          It1->first.str().c_str(), It1->second.str().c_str(),
+          OldRPath.str().c_str(), NewRPath.str().c_str());
+
+    auto IsValidWith = [&OldRPath, &NewRPath ]<class Iterator>(
+                           Iterator First, Iterator Last, const char *ErrorMsg)
+                           ->llvm::Error {
+      auto It2 = std::find_if(First, Last, [&](const StringRef &Str) {
+        return Str == OldRPath || Str == NewRPath;
+      });
+      if (It2 != Last)
+        return createStringError(errc::invalid_argument, ErrorMsg,
+                                 It2->str().c_str(), OldRPath.str().c_str(),
+                                 NewRPath.str().c_str());
+
+      return Error::success();
+    };
+
+    // Cannot specify the same rpath under both -delete_rpath and -rpath
+    if (Error E = IsValidWith(
+            adl_begin(Config.RPathsToRemove), adl_end(Config.RPathsToRemove),
+            "cannot specify both -delete_rpath %s and -rpath %s %s"))
+      return std::move(E);
+
+    // Cannot specify the same rpath under both -add_rpath and -rpath
+    if (Error E = IsValidWith(
+            adl_begin(Config.RPathToAdd), adl_end(Config.RPathToAdd),
+            "cannot specify both -add_rpath %s and -rpath %s %s"))
+      return std::move(E);
+
+    Config.RPathsToUpdate.push_back(std::make_pair(OldRPath, NewRPath));
+  }
+
   SmallVector<StringRef, 2> Positional;
   for (auto Arg : InputArgs.filtered(INSTALL_NAME_TOOL_UNKNOWN))
     return createStringError(errc::invalid_argument, "unknown argument '%s'",
                              Arg->getAsString(InputArgs).c_str());
   for (auto Arg : InputArgs.filtered(INSTALL_NAME_TOOL_INPUT))
     Positional.push_back(Arg->getValue());
   if (Positional.empty())
     return createStringError(errc::invalid_argument, "no input file specified");

llvm/tools/llvm-objcopy/InstallNameToolOpts.td

 def h : Flag<["-"], "h">, Alias<help>;
 
 def add_rpath : Option<["-", "--"], "add_rpath", KIND_SEPARATE>,
                 HelpText<"Add new rpath">;
 
 def delete_rpath: Option<["-", "--"], "delete_rpath", KIND_SEPARATE>,
                   HelpText<"Delete specified rpath">;
 
+def rpath: MultiArg<["-", "--"], "rpath", 2>,
+           HelpText<"Change rpath path name">;
+
 def version : Flag<["--"], "version">,
               HelpText<"Print the version and exit.">;

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

 namespace llvm {
 namespace objcopy {
 namespace macho {
 
 using namespace object;
 using SectionPred = std::function<bool(const std::unique_ptr<Section> &Sec)>;
 using LoadCommandPred = std::function<bool(const LoadCommand &LC)>;
 
 static Error removeLoadCommands(const CopyConfig &Config, Object &Obj) {
   DenseSet<StringRef> RPathsToRemove(Config.RPathsToRemove.begin(),
                                      Config.RPathsToRemove.end());
 
   LoadCommandPred RemovePred = [&RPathsToRemove](const LoadCommand &LC) {
     if (LC.MachOLoadCommand.load_command_data.cmd == MachO::LC_RPATH) {
       StringRef RPath =
           StringRef(reinterpret_cast<const char *>(LC.Payload.data()),
                     LC.Payload.size())
               .rtrim('\0');
       if (RPathsToRemove.count(RPath)) {
         RPathsToRemove.erase(RPath);
         return true;
       }
     }
     return false;
   };
 
   if (Error E = Obj.removeLoadCommands(RemovePred))
if (Config.StripSwiftSymbols && (Obj.Header.Flags & MachO::MH_DYLDLINK) &&
         Obj.SwiftVersion && *Obj.SwiftVersion && N->isSwiftSymbol())
       return true;
     return false;
   };
 
   Obj.SymTable.removeSymbols(RemovePred);
 }
 
+template <typename T>
+static void updateLoadCommandName(LoadCommand *LC, StringRef NewName) {
+  uint32_t NewCmdsize = alignTo(sizeof(T) + NewName.size() + 1, 8);
+
+  LC->MachOLoadCommand.load_command_data.cmdsize = NewCmdsize;
+  LC->Payload.assign(NewCmdsize - sizeof(T), 0);
+  std::copy(NewName.begin(), NewName.end(), LC->Payload.begin());
+}
+
 static LoadCommand buildRPathLoadCommand(StringRef Path) {
   LoadCommand LC;
   MachO::rpath_command RPathLC;
   RPathLC.cmd = MachO::LC_RPATH;
   RPathLC.path = sizeof(MachO::rpath_command);
   RPathLC.cmdsize = alignTo(sizeof(MachO::rpath_command) + Path.size(), 8);
   LC.MachOLoadCommand.rpath_command_data = RPathLC;
   LC.Payload.assign(RPathLC.cmdsize - sizeof(MachO::rpath_command), 0);
   std::copy(Path.begin(), Path.end(), LC.Payload.begin());
   return LC;
if (Error E = isValidMachOCannonicalName(SecName))
       return E;
     if (Error E = addSection(SecName, File, Obj))
       return E;
   }
 
   if (Error E = removeLoadCommands(Config, Obj))
     return E;
 
+  for (std::pair<StringRef, StringRef> RPair : Config.RPathsToUpdate) {
+    StringRef OldRPath = RPair.first;
+    StringRef NewRPath = RPair.second;
+
+    LoadCommand *ToBeModifiedLC = NULL;
+    for (LoadCommand &LC : Obj.LoadCommands) {
+      if (LC.MachOLoadCommand.load_command_data.cmd == MachO::LC_RPATH) {
+        StringRef CurrentRPath =
+            StringRef(reinterpret_cast<char *>(LC.Payload.data()),
+                      LC.Payload.size())
+                .rtrim('\0');
+
+        if (NewRPath == CurrentRPath)
+          return createStringError(
+              errc::invalid_argument,
+              "rpath " + NewRPath + " would create a duplicate load command");
+
+        if (OldRPath == CurrentRPath)
+          ToBeModifiedLC = &LC;
+      }
+    }
+    if (ToBeModifiedLC == NULL)
+      return createStringError(errc::invalid_argument,
+                               "no LC_RPATH load command with path: " +
+                                   OldRPath);
+
+    updateLoadCommandName<MachO::rpath_command>(ToBeModifiedLC, NewRPath);
+  }
+
   for (StringRef RPath : Config.RPathToAdd) {
     for (LoadCommand &LC : Obj.LoadCommands) {
       if (LC.MachOLoadCommand.load_command_data.cmd == MachO::LC_RPATH &&
           RPath == StringRef(reinterpret_cast<char *>(LC.Payload.data()),
                              LC.Payload.size())
                        .trim(0)) {
         return createStringError(errc::invalid_argument,
                                  "rpath " + RPath +