[llvm-objcopy][MachO] Implement --only-section

[alexander-shaposhnikov]

llvm/tools/llvm-objcopy/CopyConfig.h

102

Name.conains(',')

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

28

return !is_contained(Config.OnlySection, Sec.CanonicalName)

[alexander-shaposhnikov]

llvm/test/tools/llvm-objcopy/MachO/only-section.test

3

0. I think we need to make this test more robust and verify that the content of those sections is correct as well as the load commands, etc
we also need tests:

1. where the specified arguments are invalid (to test error messages)
2. what happens when the specified section name is not present in the input binary
3. where all the sections should stay in place (i.e. --only-section lists all the available sections) -> the output binary should be "the same" (unless there is some ambiguity, i.e. the values of the padding bytes might be different, but I'd add a small test (as small as possible))

[seiya]

• Addressed some review comments.

[seiya]

llvm/test/tools/llvm-objcopy/MachO/only-section.test

3

0. I think we need to make this test more robust and verify that the content of those sections is correct as well as the load commands, etc

AFAIK, yaml2macho.cpp fills sections with dummy data (0xDEADBEEFu) so I'll write another patch to allow specifying arbitrary binary data in a MachO YAML.

[seiya]

• Addressed a review comment.

[seiya]

Re-upload the diff: the previous update was mistake.

[seiya]

• Improved the test cases.

[seiya]

• Added some newlines for readability.

[jhenderson]

This goes for all the new functionality you are adding, but make sure to review the llvm-strip and llvm-objcopy documentation (llvm/docs/CommandGuide) for any changes that need to be made. For example, if any of the switches are currently listed as "ELF specific" they need to be moved into the general switch section.

[rupprecht]

llvm/tools/llvm-objcopy/CopyConfig.cpp

419

These errors should also include the full Name string so the user knows specifically which flag is the culprit.

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

35–46

Aside: we still have a lot of ELF-specific stuff in CopyConfig, which should theoretically be relatively platform-agnostic -- for example, the --add-symbol parsing code creates functors that assign the symbol binding to ELF::STB_* values. We should think about how to properly separate ELF/MachO/COFF specific things there.

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

15–17

SymTable/StrTable are explicitly referenced in MachO/Object.h, so if either of those are matched by the predicate, those tables should be cleared out too. (And there should be test cases to verify this).

Also, you might want to use stable_sort to remove sections -- see ELF's implementation.

Although specific to --only-section (see replaceAndRemoveSections, ELFObjcopy.cpp), StrTable/SymTab should not be implicitly removed, so it's not really observable with just this patch.

[seiya]

• Updated CommandGuide.
• Addressed a review comment.

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

35–46

I think so too. What complicates the problem is that we don't know the file format until we parse the command line options and decode the input file. How about splitting the command-line parsing into two stages?

CopyConfig.h

struct UnparsedNewSymbolInfo {
  StringRef Bind; // Don't parse it for now.
  // ...
};

class CopyConfig {
  // Format-independent options.
  StringRef InputFilename;
  FileFormat InputFormat;
  MachineInfo BinaryArch;

  // ... 

  // Format-specific options.
  UnparsedNewSymbolInfo NewSymbolInfo;
};

ELFCopyConfig.h

struct NewSymbolInfo {
  uint8_t Type = ELF::STB_GLOBAL;
  // ...
};

class ELFCopyConfig : public CopyConfig {
  ELFCopyConfig(CopyConfig &Config) {
    // We should cache the result in case there're many input files to process.
    parse(Config);
  }
};

ELFObjcopy.cpp

Error executeObjcopyOnBinary(const CopyConfig &UnparsedConfig, ...) {
  ELFCopyConfig Config(UnparsedConfig);
  // ...
}

In the first stage (CopyConfig), we parse only format-independent options. In second stage (ELFCopyConfig), we parse format-specific options (the Bind field, in this example).

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

15–17

SymTable/StrTable are explicitly referenced in MachO/Object.h, so if either of those are matched by the predicate, those tables should be cleared out too. (And there should be test cases to verify this).

Unlike ELF, in MachO, SymTable/StrTable are not sections, but are load commands. Thus, we never remove them by this removeSections.

Also, you might want to use stable_sort to remove sections -- see ELF's implementation.

Perhaps you meant stable_partition?

[seiya]

Refactored a bit.

[jhenderson]

llvm/test/tools/llvm-objcopy/MachO/only-section.test

7

Error case 1? Where are error case 2, 3...?

[seiya]

Fix a review comment.

llvm/test/tools/llvm-objcopy/MachO/only-section.test

7

It's certainly confusing. Removed 1.

[jhenderson]

llvm/test/tools/llvm-objcopy/MachO/only-section.test

16

"Specify __TEXT,__text" -> "Specify one section."
"only the" -> "only that"

59–60

You say "without warning" here, but don't actually check for a warning, which you probably should do, if you're going to mention it in the comment.

[seiya]

Updated CommandGuide.

[rupprecht]

LGTM, once a MachO reviewer signs off. No action needed for my two comments.

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

35–46

Aside: woah, didn't realize phab could display cool tabbed codeblocks like that.

Yes, that's a decent sketch for how it might work. CopyConfig itself should be returning uninterpreted StringRefs instead of ELF/MachO-specific things.

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

15–17

Yeah, stable_partition, good catch.

In retrospect, I think the use of ELF/removeSections actually may not need to use stable_partition, and switching to remove_if may be faster. IIUC the difference is that stable_partition guarantees the things past the partition point are also in stable order, whereas remove_if does not (and is faster for not having to). So disregard my original suggestion.

[seiya]

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

35–46

OK, I'll propose it in a separate patch.

(I've been wanting to try this feature. It's neat.)

[seiya]

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

15–17

As you say, it seems that we should use remove_if instead. According to this blog post (lacks the details of the experiment environment though), remove_if is faster than stable_partiton. I think normally the number of sections are small, so using remove_if improve the performance not so much though.

[seiya]

I'll submit another patch to refactor CopyConfig as described in https://reviews.llvm.org/D65541#inline-594247

[seiya]

Add MachOConfig.cpp to isolate command-line validation logic from MachOObjCopy.cpp.

[jhenderson]

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

22–23 ↗

(On Diff #222342)

I know this was there before, but I'm not sure I understand this TODO. I think it's saying that support should be added to GNU objcopy for something? If so, I don't think you want this TODO in the code. TODOs in LLVM code should be about LLVM changes that should be made at some point.

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

12–13 ↗

(On Diff #222342)

I don't think you need these two includes here.

22–23 ↗

(On Diff #222342)

Is this clang-formatted? I thought it should be struct MachOCopyConfig {}; but could be wrong easily.

It might be worth a simple comment saying why there's an empty struct. I initially thought it was an incorrect attempt to forward declare it!

[seiya]

Addressed review comments.

[seiya]

llvm/test/tools/llvm-objcopy/MachO/only-section.test

59–60

Removed "without warning" from the comment since the behavior is not so surprising.

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

22–23 ↗

(On Diff #222342)

Now I noticed that comment does not make sense. This TODO is for LLVM code, not GNU objcopy. Rephrased the comment.

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

22–23 ↗

(On Diff #222342)

It might be worth a simple comment saying why there's an empty struct. I initially thought it was an incorrect attempt to forward declare it!

Good point! Added a comment.

[jhenderson]

I don't know about any Mach-O specific details, but everything else looks good to me. Best get somebody else with Mach-O expertise to confirm.

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

42

CannonicalName -> CanonicalName

[seiya]

Addressed a review comment.

[seiya]

Friendly ping: I'd like someone who is familiar with Mach-O to review this patch.

[seiya]

@alexshap Could you review this patch? It's accepted but I'd like someone who's familiar with Mach-O to confirm (a ping in case you missed changes in this patch).

[alexander-shaposhnikov]

llvm/tools/llvm-objcopy/CopyConfig.cpp

429

khm, maybe there is a named constant for this ? (somewhere in llvm::MachO:: )

[alexander-shaposhnikov]

@seiya - thanks for all your work, I'm wondering - can you commit this change / is there anything blocking this diff ?

[seiya]

Thank you for your review!

In D65541#1711938, @alexshap wrote:

@seiya - thanks for all your work, I'm wondering - can you commit this change / is there anything blocking this diff ?

There're no diffs that blocks this patch. I'll commit this patch along with D66280 and D66281.

Other Mach-O support patches for llvm-objcopy needs addressing some review comments. I'll let you know once I update them.

llvm/tools/llvm-objcopy/CopyConfig.cpp

429

I've git-grep'ed through llvm::MachO and lld but there were no constant for it (lld hard-codes the value).

[seiya]

Before committing this patch, I noticed that we need to consider how llvm-objcopy should handle wildcards in Mach-O support.

First I thought that validating the segment/section names would be helpful for users, for example:

$ llvm-objcopy llvm-objcopy --only-section __text foo
error: invalid section name '__text' (should be formatted as '<segment name>,<section name>')

However, the validation (check that the option value is formatted as <segment name>,<section name>) does not make sense when taking into account wildcards. In GNU objcopy, it seems it does not care about the segment/section name separator (.) at all. In the following example, GNU objcopy accepts both __TEXT* and __TEXT.* and outputs identical outputs.

$ gobjcopy llvm-objcopy --only-section '__TEXT*' foo foo2
$ gobjcopy llvm-objcopy --only-section '__TEXT.*' foo foo3
$ cmp foo2 foo3
$ echo $?
0

In this change, I've removed the validation to support the GNU objcopy's wildcard behavior for Mach-O objects as described above and added a test case for wildcard.

[seiya]

Ping @alexshap (and also @jhenderson and @rupprecht would want to confirm before landing). After this patch got accepted, in order to merge it, I added wildcard support and a test in this diff.

[seiya]

Updated a test.

[rupprecht]

LGTM with the wildcard changes, thanks!

llvm/test/tools/llvm-objcopy/MachO/only-section.test

+## Show that if --only-section is given, llvm-objcopy removes all sections
+## except ones specified in the option.
+
+# RUN: yaml2obj %s > %t
+
+# RUN: not llvm-objcopy --only-section __text %t %t2 2>&1 \
+# RUN:   | FileCheck %s -DINPUT=%t --check-prefix=BAD-SECTION-NAME
+# BAD-SECTION-NAME: error: '[[INPUT]]': invalid section name (should be formatted as '<segment name>,<section name>')
+
+## Specify all sections. The output file should be the same as the input.
+# RUN: llvm-objcopy -j __TEXT,__text -j __DATA,__data -j __TEXT,__const %t %t3
+# RUN: cmp %t %t3
+
+## Specify __TEXT,__text. The output file should contain only the section.
+# RUN: llvm-objcopy --only-section __TEXT,__text %t %t4
+# RUN: llvm-readobj --sections --section-data --macho-segment %t4 \
+# RUN:   | FileCheck %s --check-prefix=ONLY-TEXT-SECTION
+# ONLY-TEXT-SECTION:      Sections [
+# ONLY-TEXT-SECTION-NEXT:   Section {
+# ONLY-TEXT-SECTION-NEXT:     Index: 0
+# ONLY-TEXT-SECTION-NEXT:     Name: __text (5F 5F 74 65 78 74 00 00 00 00 00 00 00 00 00 00)
+# ONLY-TEXT-SECTION-NEXT:     Segment: __TEXT (5F 5F 54 45 58 54 00 00 00 00 00 00 00 00 00 00)
+# ONLY-TEXT-SECTION-NEXT:     Address: 0x0
+# ONLY-TEXT-SECTION-NEXT:     Size: 0x4
+# ONLY-TEXT-SECTION-NEXT:     Offset: 184
+# ONLY-TEXT-SECTION-NEXT:     Alignment: 0
+# ONLY-TEXT-SECTION-NEXT:     RelocationOffset: 0x0
+# ONLY-TEXT-SECTION-NEXT:     RelocationCount: 0
+# ONLY-TEXT-SECTION-NEXT:     Type: Regular (0x0)
+# ONLY-TEXT-SECTION-NEXT:     Attributes [ (0x800004)
+# ONLY-TEXT-SECTION-NEXT:       PureInstructions (0x800000)
+# ONLY-TEXT-SECTION-NEXT:       SomeInstructions (0x4)
+# ONLY-TEXT-SECTION-NEXT:     ]
+# ONLY-TEXT-SECTION-NEXT:     Reserved1: 0x0
+# ONLY-TEXT-SECTION-NEXT:     Reserved2: 0x0
+# ONLY-TEXT-SECTION-NEXT:     Reserved3: 0x0
+# ONLY-TEXT-SECTION-NEXT:     SectionData (
+# ONLY-TEXT-SECTION-NEXT:       0000: AABBCCDD                             |....|
+# ONLY-TEXT-SECTION-NEXT:     )
+# ONLY-TEXT-SECTION-NEXT:   }
+# ONLY-TEXT-SECTION-NEXT: ]
+# ONLY-TEXT-SECTION-NEXT: Segment {
+# ONLY-TEXT-SECTION-NEXT:   Cmd: LC_SEGMENT_64
+# ONLY-TEXT-SECTION-NEXT:   Name:
+# ONLY-TEXT-SECTION-NEXT:   Size: 152
+# ONLY-TEXT-SECTION-NEXT:   vmaddr: 0x0
+# ONLY-TEXT-SECTION-NEXT:   vmsize: 0x4
+# ONLY-TEXT-SECTION-NEXT:   fileoff: 184
+# ONLY-TEXT-SECTION-NEXT:   filesize: 4
+# ONLY-TEXT-SECTION-NEXT:   maxprot: rwx
+# ONLY-TEXT-SECTION-NEXT:   initprot: rwx
+# ONLY-TEXT-SECTION-NEXT:   nsects: 1
+# ONLY-TEXT-SECTION-NEXT:   flags: 0x0
+# ONLY-TEXT-SECTION-NEXT: }
+
+## Remove all sections without warning if the specified section name is not
+## present in the input.
+# RUN: llvm-objcopy --only-section __TEXT,__nonexistent %t %t5
+# RUN: llvm-readobj --sections --section-data --macho-segment %t5 \
+# RUN:   | FileCheck %s --check-prefix=NONEXISTENT-SECTION
+# NONEXISTENT-SECTION:      Sections [
+# NONEXISTENT-SECTION-NEXT: ]
+# NONEXISTENT-SECTION-NEXT: Segment {
+# NONEXISTENT-SECTION-NEXT:   Cmd: LC_SEGMENT_64
+# NONEXISTENT-SECTION-NEXT:   Name:
+# NONEXISTENT-SECTION-NEXT:   Size: 72
+# NONEXISTENT-SECTION-NEXT:   vmaddr: 0x0
+# NONEXISTENT-SECTION-NEXT:   vmsize: 0x0
+# NONEXISTENT-SECTION-NEXT:   fileoff: 104
+# NONEXISTENT-SECTION-NEXT:   filesize: 0
+# NONEXISTENT-SECTION-NEXT:   maxprot: rwx
+# NONEXISTENT-SECTION-NEXT:   initprot: rwx
+# NONEXISTENT-SECTION-NEXT:   nsects: 0
+# NONEXISTENT-SECTION-NEXT:   flags: 0x0
+# NONEXISTENT-SECTION-NEXT: }
+
+--- !mach-o
+FileHeader:
+  magic:           0xFEEDFACF
+  cputype:         0x01000007
+  cpusubtype:      0x00000003
+  filetype:        0x00000001
+  ncmds:           1
+  sizeofcmds:      312
+  flags:           0x00002000
+  reserved:        0x00000000
+LoadCommands:
+  - cmd:             LC_SEGMENT_64
+    cmdsize:         312
+    segname:         ''
+    vmaddr:          0
+    vmsize:          12
+    fileoff:         344
+    filesize:        12
+    maxprot:         7
+    initprot:        7
+    nsects:          3
+    flags:           0
+    Sections:
+      - sectname:        __text
+        segname:         __TEXT
+        addr:            0x0000000000000000
+        content:         'AABBCCDD'
+        size:            4
+        offset:          344
+        align:           0
+        reloff:          0x00000000
+        nreloc:          0
+        flags:           0x80000400
+        reserved1:       0x00000000
+        reserved2:       0x00000000
+        reserved3:       0x00000000
+      - sectname:        __data
+        segname:         __DATA
+        addr:            0x0000000000000004
+        content:         'DDAADDAA'
+        size:            4
+        offset:          348
+        align:           0
+        reloff:          0x00000000
+        nreloc:          0
+        flags:           0x00000000
+        reserved1:       0x00000000
+        reserved2:       0x00000000
+        reserved3:       0x00000000
+      - sectname:        __const
+        segname:         __TEXT
+        addr:            0x0000000000000008
+        content:         'EEFFEEFF'
+        size:            4
+        offset:          352
+        align:           0
+        reloff:          0x00000000
+        nreloc:          0
+        flags:           0x00000000
+        reserved1:       0x00000000
+        reserved2:       0x00000000
+        reserved3:       0x00000000

llvm/tools/llvm-objcopy/CopyConfig.h

class NameOrRegex {
   StringRef Name;
   // Regex is shared between multiple CopyConfig instances.
   std::shared_ptr<Regex> R;
 
 public:
   NameOrRegex(StringRef Pattern, bool IsRegex);
   bool operator==(StringRef S) const { return R ? R->match(S) : Name == S; }
   bool operator!=(StringRef S) const { return !operator==(S); }
+  Error isMachOCannonicalName() const;
 };
 
 struct NewSymbolInfo {
   StringRef SymbolName;
   StringRef SectionName;
   uint64_t Value = 0;
   uint8_t Type = ELF::STT_NOTYPE;
   uint8_t Bind = ELF::STB_GLOBAL;
   uint8_t Visibility = ELF::STV_DEFAULT;
 };
 

 // Configuration for the overall invocation of this tool. When invoked as
 // objcopy, will always contain exactly one CopyConfig. When invoked as strip,
 // will contain one or more CopyConfigs.
 struct DriverConfig {
   SmallVector<CopyConfig, 1> CopyConfigs;
   BumpPtrAllocator Alloc;
 };
 
+// isValidMachOCannonicalName returns success if Name is a MachO cannonical name
+// ("<segment>,<section>") and lengths of both segment and section names are
+// valid.
+Error isValidMachOCannonicalName(StringRef Name);
+
 // ParseObjcopyOptions returns the config and sets the input arguments. If a
 // help flag is set then ParseObjcopyOptions will print the help messege and
 // exit.
 Expected<DriverConfig> parseObjcopyOptions(ArrayRef<const char *> ArgsArr);
 
 // ParseStripOptions returns the config and sets the input arguments. If a
 // help flag is set then ParseStripOptions will print the help messege and
 // exit. ErrorCallback is used to handle recoverable errors. An Error returned

llvm/tools/llvm-objcopy/CopyConfig.cpp

if (!IsRegex) {
     return;
   }
 
   SmallVector<char, 32> Data;
   R = std::make_shared<Regex>(
       ("^" + Pattern.ltrim('^').rtrim('$') + "$").toStringRef(Data));
 }
 
+Error NameOrRegex::isMachOCannonicalName() const {
+  return R ? Error::success() : isValidMachOCannonicalName(Name);
+}
+
 static Error addSymbolsToRenameFromFile(StringMap<StringRef> &SymbolsToRename,
                                         BumpPtrAllocator &Alloc,
                                         StringRef Filename) {
   StringSaver Saver(Alloc);
   SmallVector<StringRef, 16> Lines;
   auto BufOrErr = MemoryBuffer::getFile(Filename);
   if (!BufOrErr)
     return createFileError(Filename, BufOrErr.getError());

 
 template <class T> static ErrorOr<T> getAsInteger(StringRef Val) {
   T Result;
   if (Val.getAsInteger(0, Result))
     return errc::invalid_argument;
   return Result;
 }
 
+// isValidMachOCannonicalName returns success if Name is a MachO cannonical name
+// ("<segment>,<section>") and lengths of both segment and section names are
+// valid.
+Error isValidMachOCannonicalName(StringRef Name) {
+  if (Name.count(',') != 1)
+    return createStringError(errc::invalid_argument,
+                             "invalid section name (should be formatted as "
+                             "'<segment name>,<section name>')");
+
+  std::pair<StringRef, StringRef> Pair = Name.split(',');
+  if (Pair.first.size() > 16)
+    return createStringError(errc::invalid_argument,
+                             "too long segment name: '%s'",
+                             Pair.first.str().c_str());
+  if (Pair.second.size() > 16)
+    return createStringError(errc::invalid_argument,
+                             "too long section name: '%s'",
+                             Pair.second.str().c_str());
+  return Error::success();
+}
+
 // ParseObjcopyOptions returns the config and sets the input arguments. If a
 // help flag is set then ParseObjcopyOptions will print the help messege and
 // exit.
 Expected<DriverConfig> parseObjcopyOptions(ArrayRef<const char *> ArgsArr) {
   DriverConfig DC;
   ObjcopyOptTable T;
   unsigned MissingArgumentIndex, MissingArgumentCount;
   llvm::opt::InputArgList InputArgs =

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

 #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 Section &Sec)>;
+
+static void removeSections(const CopyConfig &Config, Object &Obj) {
+  SectionPred RemovePred = [](const Section &) { return false; };
+
+  if (!Config.OnlySection.empty()) {
+    RemovePred = [&Config, RemovePred](const Section &Sec) {
+      return !is_contained(Config.OnlySection, Sec.CannonicalName);
+    };
+  }
+
+  return Obj.removeSections(RemovePred);
+}
+
+static Error validateOptions(const CopyConfig &Config) {
+  // TODO: Support section renaming in GNU objcopy for compatibility (see
+  // http://lists.llvm.org/pipermail/llvm-dev/2019-May/132570.html).
+
+  if (!Config.OnlySection.empty()) {
+    for (const NameOrRegex &NR : Config.OnlySection)
+      if (Error E = NR.isMachOCannonicalName())
+        return E;
+  }
+
+  return Error::success();
+}
 
 static Error handleArgs(const CopyConfig &Config, Object &Obj) {
   if (Config.AllowBrokenLinks || !Config.BuildIdLinkDir.empty() ||
       Config.BuildIdLinkInput || Config.BuildIdLinkOutput ||
       !Config.SplitDWO.empty() || !Config.SymbolsPrefix.empty() ||
       !Config.AllocSectionsPrefix.empty() || !Config.AddSection.empty() ||
       !Config.DumpSection.empty() || !Config.KeepSection.empty() ||
-      !Config.OnlySection.empty() || !Config.SymbolsToGlobalize.empty() ||
-      !Config.SymbolsToKeep.empty() || !Config.SymbolsToLocalize.empty() ||
-      !Config.SymbolsToWeaken.empty() || !Config.SymbolsToKeepGlobal.empty() ||
-      !Config.SectionsToRename.empty() || !Config.SymbolsToRename.empty() ||
+      !Config.SymbolsToGlobalize.empty() || !Config.SymbolsToKeep.empty() ||
+      !Config.SymbolsToLocalize.empty() || !Config.SymbolsToWeaken.empty() ||
+      !Config.SymbolsToKeepGlobal.empty() || !Config.SectionsToRename.empty() ||
+      !Config.SymbolsToRename.empty() ||
       !Config.UnneededSymbolsToRemove.empty() ||
       !Config.SetSectionFlags.empty() || !Config.ToRemove.empty() ||
       Config.ExtractDWO || Config.KeepFileSymbols || Config.LocalizeHidden ||
       Config.PreserveDates || Config.StripDWO || Config.StripNonAlloc ||
       Config.StripSections || Config.Weaken || Config.DecompressDebugSections ||
       Config.StripDebug || Config.StripNonAlloc || Config.StripSections ||
       Config.StripUnneeded || Config.DiscardMode != DiscardType::None ||
       !Config.SymbolsToAdd.empty() || Config.EntryExpr) {
     return createStringError(llvm::errc::invalid_argument,
                              "option not supported by llvm-objcopy for MachO");
   }
 
+  if (auto E = validateOptions(Config))
+    return E;
+
+  removeSections(Config, Obj);
   return Error::success();
 }
 
 Error executeObjcopyOnBinary(const CopyConfig &Config,
                              object::MachOObjectFile &In, Buffer &Out) {
   MachOReader Reader(In);
   std::unique_ptr<Object> O = Reader.create();
   if (!O)

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

 template <typename SectionType>
 Section constructSectionCommon(SectionType Sec) {
   Section S;
   S.Sectname =
       StringRef(Sec.sectname, strnlen(Sec.sectname, sizeof(Sec.sectname)))
           .str();
   S.Segname =
       StringRef(Sec.segname, strnlen(Sec.segname, sizeof(Sec.sectname))).str();
+  S.CannonicalName = (Twine(S.Segname) + "," + S.Sectname).str();
   S.Addr = Sec.addr;
   S.Size = Sec.size;
   S.Offset = Sec.offset;
   S.Align = Sec.align;
   S.RelOff = Sec.reloff;
   S.NReloc = Sec.nreloc;
   S.Flags = Sec.flags;
   S.Reserved1 = Sec.reserved1;

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

struct MachHeader {
   uint32_t Flags;
   uint32_t Reserved = 0;
 };
 
 struct RelocationInfo;
 struct Section {
   std::string Sectname;
   std::string Segname;
+  // CannonicalName is a string formatted as “<Segname>,<Sectname>".
+  std::string CannonicalName;
   uint64_t Addr;
   uint64_t Size;
   uint32_t Offset;
   uint32_t Align;
   uint32_t RelOff;
   uint32_t NReloc;
   uint32_t Flags;
   uint32_t Reserved1;
struct Object {
   /// The index of LC_DYLD_INFO or LC_DYLD_INFO_ONLY load command if present.
   Optional<size_t> DyLdInfoCommandIndex;
   /// The index LC_DYSYMTAB load comamnd if present.
   Optional<size_t> DySymTabCommandIndex;
   /// The index LC_DATA_IN_CODE load comamnd if present.
   Optional<size_t> DataInCodeCommandIndex;
   /// The index LC_FUNCTION_STARTS load comamnd if present.
   Optional<size_t> FunctionStartsCommandIndex;
+
+  void removeSections(function_ref<bool(const Section &)> ToRemove);
 };
 
 } // end namespace macho
 } // end namespace objcopy
 } // end namespace llvm
 
 #endif // LLVM_OBJCOPY_MACHO_OBJECT_H

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

 #include "Object.h"
 #include "../llvm-objcopy.h"
 
 namespace llvm {
 namespace objcopy {
 namespace macho {
 
 const SymbolEntry *SymbolTable::getSymbolByIndex(uint32_t Index) const {
   assert(Index < Symbols.size() && "invalid symbol index");
   return Symbols[Index].get();
 }
 
+void Object::removeSections(function_ref<bool(const Section &)> ToRemove) {
+  for (LoadCommand &LC : LoadCommands)
+    LC.Sections.erase(std::remove_if(std::begin(LC.Sections),
+                                     std::end(LC.Sections), ToRemove),
+                      std::end(LC.Sections));
+}
+
 } // end namespace macho
 } // end namespace objcopy
 } // end namespace llvm