[llvm-install-name-tool] Add `delete_rpath` option

[sameerarora101]

Summary

Add support for delete_rpath option under install-name-tools
in file MachOObjcopy.cpp. New option installed under
InstallNameToolOpts.td and CopyConfig.cpp.

[jhenderson]

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

2

Note to self: I still need to review the tests.

llvm/tools/llvm-objcopy/CopyConfig.cpp

833–836

This seems like an unnecessary limitation to me. I'd just expect it to ignore the later ones (you could use a set rather than a vector to store the set to remove in to ensure that).

llvm/tools/llvm-objcopy/InstallNameToolOpts.td

22

I'd get rid of "old" from the text. We don't say "old" for e.g. --remove-section in objcopy.

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

26–45

This whole block seems unnecessarily overcomplicated to me.

1. Make Config.RPathsToRemove a set and then you don't have to copy things from one set to another - you can just use the config variable directly.
2. There's no need for the if (!Config.RPathsToRemove.empty()) check, since it is safe to do everything here whether it's empty or not (the insert disappears, but would be safe, and defining the lambda is harmless and trivial.
3. By getting rid of the "if empty" check, you can also get rid of the return false lambda.
4. trim(0) - I assume you mean trim('\0')? If so, use the char directly rather than an integer literal, since you are dealing with an array of chars. This makes it easier to follow. I am assuming here that LC.Payload.size() includes a trailing null byte you want to get rid of. Do you also mean to get rid of any leading null bytes too? If not, you might be better off using rtrim.

50–51

I'd rephrase this comment slightly:

"Emit an error if the Mach-O binary does not contain an rpath path name specified in -delete_rpath."

52

In every other remove operation within objcopy, we don't emit an error if a switch had no effect because the named section/symbol etc wasn't present. Do we really need this error message? Getting rid of it would allow us to further simplify logic above.

[alexander-shaposhnikov]

llvm/tools/llvm-objcopy/CopyConfig.cpp

839

since we are already in the namespace llvm you can simply use is_contained

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

26–45

After some thinking it seems to me that having the original list of specified rpaths can be useful since it enables us to report the first option causing the issue. If we drop that error reporting then indeed it makes sense to simply use StringSet for storing RPATHs in CopyConfig and use that set directly.

2., 3. - I also think that !Config.RPathsToRemove.empty() is unnecessary

If I am not mistaken there is a tiny bug in buildRPathLoadCommand function (I've just spotted it) - would be good to fix separately.

RPathLC.cmdsize = alignTo(sizeof(MachO::rpath_command) + Path.size(), 8);

should be replaced with

RPathLC.cmdsize = alignTo(sizeof(MachO::rpath_command) + Path.size() + 1, 8);

this is what LD64 does (see ld64/src/ld/HeaderAndLoadCommands.hpp) :

template <typename A>
uint8_t* HeaderAndLoadCommandsAtom<A>::copyRPathLoadCommand(uint8_t* p, const char* path) const
{
    uint32_t sz = alignedSize(sizeof(macho_rpath_command<P>) + strlen(path) + 1);
    macho_rpath_command<P>* cmd = (macho_rpath_command<P>*)p;
    cmd->set_cmd(LC_RPATH);
    cmd->set_cmdsize(sz);
    cmd->set_path_offset();
    strcpy((char*)&p[sizeof(macho_rpath_command<P>)], path);
    return p + sz;
 }

52

I think the intention here is to catch the case when e.g. there is a typo in the paths specified by the command-line arguments and report an error rather than silently leave the binary untouched. I can see pros and cons with the both approaches, if I am not mistaken Xcode's install-name-tool returns an error (similarly to what's done here).

[sameerarora101]

llvm/tools/llvm-objcopy/CopyConfig.cpp

833–836

So currently, I have modeled the behavior according to Apples' install_name_tool and it throws this error in case the user specifies a delete_rpath entry twice. I can change it though if this is not we want?

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

52

yup, Apple's install_name_tool throws this error in case it can't find the rpath. It says in the specification too :

-delete_rpath old
	      ... If the Mach-O binary does not contains the old rpath path name specified in -delete_rpath it is an error.

[sameerarora101]

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

26–45

4 - for using trim(0), I was looking at RPath == StringRef(reinterpret_cast<char *>(LC.Payload.data()),LC.Payload.size().trim(0)) at line 264 under Config.RPathToAdd. Is that not how I should be doing it? Thanks

26–45

@alexshap - ya, I can push a fix for it separately.

[sameerarora101]

removing if (!Config.RPathsToRemove.empty()) check.

[sameerarora101]

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

26–45

pushed the fix at https://reviews.llvm.org/D81575

[smeenai]

llvm/tools/llvm-objcopy/CopyConfig.cpp

833–836

Yeah. There's some tension here between following standard objcopy conventions vs. emulating the behavior of Apple's install_name_tool (which this is an LLVM version of). In general, we thought it was best to match install_name_tool's behavior as much as possible, although I don't feel too strongly about this particular error case.

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

52

I do feel strongly about this case, however :) I feel like not erroring in this case would be a pretty major divergence from Apple install_name_tool's behavior. Our goal is for llvm-install-name-tool to be a drop-in replacement for Apple's install_name_tool, and having similar error conditions is an important part of that.

[jhenderson]

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

5–6

I could possibly be persuaded, but I'm not convinced you need this input validation check, if I'm honest. Presumably yaml2obj will have some testing that shows the output is as expected for this case, so all this is doing is showing you can write your input correctly. But even that is not necessary - your later test cases actually do this already, since llvm-install-name-tool will emit an error if an entry is missing in the input that is to be deleted, and FileCheck will emit an error if it is unexpectedly missing from the output.

14

It's generally more traditional for llvm-objcopy tests to use a separate output file rather than modifying in place. This means you don't have to repeatedly recreate the same input in later tests. In some cases, we even demonstrate that the input file hasn't been modified, though I'm not convinced by the necessity of that here.

Same applies for all test cases in this test.

16

FWIW, you don't really need to indent quite so far. Just an extra two spaces on top of the previous line's indentation would be sufficient.

18

You should validate that the other paths are still present.

19–20

I'm not going to push on this, but I don't think the double blank lines really help improve readability of the test.

27–29

It's worth remembering that FileCheck's CHECK-NOT patterns only check the region between the previous and next non-NOT matches (including the start/end of file as appropriate). This style of checking would therefore not prevent llvm-install-name-tool from reordering them - executable_d could appear before executable_a, executable_b and executable_c and this test would pass.

A better approach would be to use FileCheck's --implicit-check-not. This is equivalent to specifying CHECK-NOT in between every positive match. Anything that contains the pattern specified in implicit-check-not which is not explicitly matched by another check will cause the test to fail.

33

non-existing -> nonexistent

(according to my brief StackExchange search...)

48

Deleting -> deleting

llvm/tools/llvm-objcopy/CopyConfig.cpp

833–836

Not being an Apple developer, I don't feel like I'm best placed to say what the behaviour should be. I'd prefer @alexshap or other Apple developers to give their thoughts. How important is it to emulate error message behaviour? Are the errors actually useful?

One of the issues with prohibiting something being specified more than once is that people will occasionally be using response files or a build system or whatever where they can't easily change the existing commands that are specified, but can add to the end (I've run into this on one or two occasions). By following the more traditional approach of "last one wins", it's possible to workaround this difficulty, whereas this error makes it impossible.

My 2 pence are that this one is not particularly useful, whereas the other one I commented on might have some value for reasons you've already outlined.

llvm/tools/llvm-objcopy/InstallNameToolOpts.td

22

Actually, perhaps it should be "Delete specified rpath", since you have to explicitly specify the path that is being deleted.

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

26–45

I assume something got messed up when you copied in that line, since you obviously don't trim a size value!

I would suggest that the older code should be improved (not in this patch though). When I read this code originally, I thought it was trying to trim 0 bytes or something before I confirmed that there is no such interface.

[sameerarora101]

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

14

Unlike llvm-objcopy, it seems like install-name-tool doesn't have an option (like -o) where the user can specify the output binary. It only expects an input file and thus can only change the binary in place. (It also doesn't print the result on the terminal when it succeeds so > operator can't be used)

llvm/tools/llvm-objcopy/CopyConfig.cpp

833–836

Ok, so currently I have removed this error check and allowed for duplicate entries to be specified (Config.RPathsToRemove is a set). I can change it back if @alexshap or someone else feels that we should support it?

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

26–45

ok thanks. I have currently fixed it for above code in the current patch.

[sameerarora101]

Updates:

• Change Config.RPathsToRemove to a set.
• No error for duplicate rpaths
• incorporating above inline comments for test file

[jhenderson]

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

9

You could probably make this --implicit-check-not even simpler, for example simply @executable or path (but double-check the whole output for the second one, since it's possible it includes the file path somewhere else, and it's reasonable that somebody's file path will include "path" in a directory name somewhere).

Same goes below for the other cases.

Couple of other notes, but won't need dealing with, if you change as I request:

1. [a] is equivalent to simply a in the regex.
2. [a] is actually unnecessary entirely, since even without it, the check will still work - it will show that path @executable_ doesn't appear in the output anywhere other than the lines checked by RPATHS. Similarly [abc] and [abcd] below are unnecessary.

14

llvm-objcopy (unlike llvm-strip) doesn't have a -o option either. It uses the following syntax:

llvm-objcopy <input> [output]

In other words, the second positional argument is the output file the input file will be copied to. If not specified, the input will be modified in place.

According to the help text, llvm-install-name-tool uses the same syntax, although I don't know if that's the case for Apple's version. Either way, assuming the help text is accurate, I think we should follow the same approach.

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

26–29

I believe you can construct the set upfront with the right elements:

DenseSet<StringRef> RPathsToRemove(Config.RPathsToRemove.begin(), Config.RPathsToRemove.end());

[sameerarora101]

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

9

nice, thanks😊

14

Oh, it seems like the help text is wrong in case of llvm-install-name-tool. If I run something like ./bin/llvm-install-name-tool -add_rpath @ZZ Dependent Dep2, it throws the following error: error: llvm-install-name-tool expects a single input file.

the code too sets Config.InputFilename and Config.OutputFilename to the same file:

if (Positional.size() > 1)
  return createStringError(
      errc::invalid_argument,
      "llvm-install-name-tool expects a single input file");
Config.InputFilename = Positional[0];
Config.OutputFilename = Positional[0];

Nevertheless, the current llvm's version is compatible with Apple's version as it too throws an error when an output file is specified. I can create a patch for the help text in a separate diff? (or, if preferable, make install-name-tool accept a output file?)

[sameerarora101]

Simplifying --implicit-check-nots

[alexander-shaposhnikov]

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

42

Object stores indices of some special load commands which need to be updated (in this method).

for (const LoadCommand &LC : LoadCommand) {
     switch (LC.cmd) {
        ...
     }
}

[sameerarora101]

• Updating indices for special LCs (and test for it)

[sameerarora101]

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

14

currently resolved by updating the help text here https://reviews.llvm.org/D81907. (I can change it to accept a output file if people prefer that way?) Please lemme know.

[alexander-shaposhnikov]

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

42

i see, thanks, almost there. A few suggestions from me:

1. I think the quadratic behavior here is unnecessary, I would simply use smth like
   
   for (size_t Index = 0, Size = LoadCommands.size(); Index < Size; ++Index) { ... }

2. Let's factor out it into a helper method, e.g. updateLoadCommandsIndexes

[jhenderson]

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

9

You can also get rid of the {{ and }} from the pattern here and below, since the string is no longer a regex.

14

Thanks for the fix. It's up to you if you want to add an option in a separate patch for emitting an output file (it seems like a logical improvement), but I wouldn't make it the same as llvm-objcopy's positional argument, but rather I'd follow llvm-strip's separate option style. I don't think you need to do that for this change to land though.

46

Nit: missing full stop.

[sameerarora101]

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

14

ok, got it!

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

42

got it, thanks!

[sameerarora101]

• Factoring out updateLoadCommandIndices
• Updating test as per the suggestions

[smeenai]

llvm/tools/llvm-objcopy/CopyConfig.cpp

832

Super nit: spell out "and" instead of using "&"

[smeenai]

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

52–53

There's a bunch of special load commands whose indices are stored. Can we add tests for the other ones as well (in addition to LC_SYMTAB)?

[sameerarora101]

• Adding index update test for other special LCs
• & -> and

[sameerarora101]

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

52–53

Ok, to test this I have added a test remove-lc-index-update.test. The test uses the binary from strip-all.yaml (with additional modifications) as it already made use of all special Load Commands. Now, if the index is not updated properly, the binary either crashes or raises an assert error. However, if the indices are set right, the binary will run and the LC indices will be updated appropriately (aka the test passes).

[jhenderson]

LGTM, with a couple of small comments, but please wait for others to confirm they are happy with the latest test changes, as I don't know Mach-O well enough to follow the full logic.

llvm/test/tools/llvm-objcopy/MachO/remove-lc-index-update.test

1 ↗

(On Diff #271274)

updates index for -> updates the indexes of

7–8 ↗

(On Diff #271274)

To make this easier to read, add additional spaces to make the checked patterns line up:

# INDEX:      Load command 3
# INDEX-NEXT: cmd LC_DYLD_INFO_ONLY

Same goes below.

[sameerarora101]

Aligning LCs in test.

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

+## This test checks deleting a LC_RPATH load command from a MachO binary.
+
+# RUN: yaml2obj %s -o %t
+
+## Deleting a single RPath entry:
+# RUN: llvm-install-name-tool -delete_rpath @executable_a/. %t
+# RUN: llvm-objdump -p %t | \
+# RUN:   FileCheck %s --check-prefix=RPATHS --implicit-check-not={{@executable}}
+
+# RPATHS: path @executable_b/.
+# RPATHS: path @executable_c/.
+# RPATHS: path @executable_d/.
+# RPATHS: path @executable_e/.
+
+## Deleting multiple RPath entries:
+# RUN: llvm-install-name-tool -delete_rpath @executable_b/. \
+# RUN:                        -delete_rpath @executable_c/. %t
+# RUN: llvm-objdump -p %t | \
+# RUN:   FileCheck %s --check-prefix=RPATHS-MULTIPLE --implicit-check-not={{@executable}}
+
+# RPATHS-MULTIPLE: path @executable_d/.
+# RPATHS-MULTIPLE: path @executable_e/.
+
+## Duplicate delete_rpath commands:
+# RUN: llvm-install-name-tool -delete_rpath @executable_d/. \
+# RUN:                        -delete_rpath @executable_d/. %t
+# RUN: llvm-objdump -p %t | \
+# RUN:   FileCheck %s --check-prefix=DUPLICATE --implicit-check-not={{@executable}}
+
+# DUPLICATE: path @executable_e/.
+
+## Deleting a nonexistent RPath:
+# RUN: not llvm-install-name-tool -delete_rpath @executable_a/. %t 2>&1 | \
+# RUN:   FileCheck %s --check-prefix=ABSENT-RPATH
+
+# 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/.
+
+--- !mach-o
+FileHeader:
+  magic:           0xFEEDFACF
+  cputype:         0x01000007
+  cpusubtype:      0x00000003
+  filetype:        0x00000001
+  ncmds:           5
+  sizeofcmds:      160
+  flags:           0x00002000
+  reserved:        0x00000000
+LoadCommands:
+  - cmd:             LC_RPATH
+    cmdsize:         32
+    path:            12
+    PayloadString:   '@executable_a/.'
+  - cmd:             LC_RPATH
+    cmdsize:         32
+    path:            12
+    PayloadString:   '@executable_b/.'
+  - cmd:             LC_RPATH
+    cmdsize:         32
+    path:            12
+    PayloadString:   '@executable_c/.'
+  - cmd:             LC_RPATH
+    cmdsize:         32
+    path:            12
+    PayloadString:   '@executable_d/.'
+  - cmd:             LC_RPATH
+    cmdsize:         32
+    path:            12
+    PayloadString:   '@executable_e/.'

llvm/tools/llvm-objcopy/CopyConfig.h

 //===- CopyConfig.h -------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_TOOLS_LLVM_OBJCOPY_COPY_CONFIG_H
 #define LLVM_TOOLS_LLVM_OBJCOPY_COPY_CONFIG_H
 
 #include "ELF/ELFConfig.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/BitmaskEnum.h"
+#include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Object/ELFTypes.h"
 #include "llvm/Support/Allocator.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/GlobPattern.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;
+  DenseSet<StringRef> RPathsToRemove;
 
   // Section matchers
   NameMatcher KeepSection;
   NameMatcher OnlySection;
   NameMatcher ToRemove;
 
   // Symbol matchers
   NameMatcher SymbolsToGlobalize;

llvm/tools/llvm-objcopy/CopyConfig.cpp

outs() << "llvm-install-name-tool, compatible with cctools "
               "install_name_tool\n";
     cl::PrintVersionMessage();
     exit(0);
   }
 
   for (auto Arg : InputArgs.filtered(INSTALL_NAME_TOOL_add_rpath))
     Config.RPathToAdd.push_back(Arg->getValue());
 
+  for (auto Arg : InputArgs.filtered(INSTALL_NAME_TOOL_delete_rpath)) {
+    StringRef RPath = Arg->getValue();
+
+    // Cannot add & 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",
+          RPath.str().c_str(), RPath.str().c_str());
+
+    Config.RPathsToRemove.insert(RPath);
+  }
+
   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

 include "llvm/Option/OptParser.td"
 
 def help : Flag<["--"], "help">;
 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 version : Flag<["--"], "version">,
               HelpText<"Print the version and exit.">;

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

 //===- MachOObjcopy.cpp -----------------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "MachOObjcopy.h"
 #include "../CopyConfig.h"
 #include "MachOReader.h"
 #include "MachOWriter.h"
+#include "llvm/ADT/DenseSet.h"
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/Error.h"
 
 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))
+    return E;
+
+  // Emit an error if the Mach-O binary does not contain an rpath path name
+  // specified in -delete_rpath.
+  for (StringRef RPath : Config.RPathsToRemove) {
+    if (RPathsToRemove.count(RPath))
+      return createStringError(errc::invalid_argument,
+                               "no LC_RPATH load command with path: %s",
+                               RPath.str().c_str());
+  }
+  return Error::success();
+}
 
 static Error removeSections(const CopyConfig &Config, Object &Obj) {
   SectionPred RemovePred = [](const std::unique_ptr<Section> &) {
     return false;
   };
 
   if (!Config.ToRemove.empty()) {
     RemovePred = [&Config, RemovePred](const std::unique_ptr<Section> &Sec) {
for (const auto &Flag : Config.AddSection) {
     StringRef SecName = SecPair.first;
     StringRef File = SecPair.second;
     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 (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 +

llvm/tools/llvm-objcopy/MachO/Object.h

struct Object {
 
   BumpPtrAllocator Alloc;
   StringSaver NewSectionsContents;
 
   Object() : NewSectionsContents(Alloc) {}
 
   Error
   removeSections(function_ref<bool(const std::unique_ptr<Section> &)> ToRemove);
+
+  Error removeLoadCommands(function_ref<bool(const LoadCommand &)> ToRemove);
+
   void addLoadCommand(LoadCommand LC);
 
   /// Creates a new segment load command in the object and returns a reference
   /// to the newly created load command. The caller should verify that SegName
   /// is not too long (SegName.size() should be less than or equal to 16).
   LoadCommand &addSegment(StringRef SegName);
 
   bool is64Bit() const {

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

 
 void SymbolTable::removeSymbols(
     function_ref<bool(const std::unique_ptr<SymbolEntry> &)> ToRemove) {
   Symbols.erase(
       std::remove_if(std::begin(Symbols), std::end(Symbols), ToRemove),
       std::end(Symbols));
 }
 
+Error Object::removeLoadCommands(
+    function_ref<bool(const LoadCommand &)> ToRemove) {
+  auto It = std::stable_partition(
+      LoadCommands.begin(), LoadCommands.end(),
+      [&](const LoadCommand &LC) { return !ToRemove(LC); });
+  LoadCommands.erase(It, LoadCommands.end());
+  return Error::success();
+}
+
 Error Object::removeSections(
     function_ref<bool(const std::unique_ptr<Section> &)> ToRemove) {
   DenseMap<uint32_t, const Section *> OldIndexToSection;
   uint32_t NextSectionIndex = 1;
   for (LoadCommand &LC : LoadCommands) {
     auto It = std::stable_partition(
         std::begin(LC.Sections), std::end(LC.Sections),
         [&](const std::unique_ptr<Section> &Sec) { return !ToRemove(Sec); });