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

[jhenderson]

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

3

in -> by

This test should also show that you can remove multiple sections at the same time with the switch.

[rupprecht]

LGTM, no concerns from me once the test is updated

[seiya]

• Addressed review comments.

[seiya]

• Added a comment in the test.

[jhenderson]

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

7

"Error case 1:" is meaningless without other error cases.

63

How easy would it be to use FileCheck --check-prefixes, and possibly -D<NAME> options to share this block with the previous test case? That would avoid duplicating a big chunk of the expected output. You could do something like:

# RUN: ... | FileCheck --check-prefixes=REMOVE-COMMON,REMOVE-TEXT-ONLY
# RUN: ... | FileCheck --check-prefixes=REMOVE-COMMON

# REMOVE-COMMON:
# REMOVE-COMMON-NEXT:
...
# REMOVE-TEXT-ONLY-NEXT:
# REMOVE-TEXT-ONLY-NEXT:
...
# REMOVE-COMMON-NEXT:

[seiya]

Address review comments.

[jhenderson]

LGTM, thanks.

[seiya]

Update CommandGuide.

[seiya]

CommandGuide: Removed remove "the" before "<section>".

[jhenderson]

llvm/docs/CommandGuide/llvm-objcopy.rst

84 ↗

(On Diff #216120)

Delete "the"

[seiya]

Rebased.

[rupprecht]

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

42–46

Not related to this patch, but looks like this discards RemovePred, e.g. --strip-all --only-section will effectively work like --only-section.

I found this after noticing that RemovePred is not referenced in the bit you added, which is actually fine there but error prone if the iterative construction of RemovePred is ever reordered. ELF objcopy works like this too, but correctly chains RemovePred for the --only-section switch.

[rupprecht]

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

42–46

Nevermind, that flag takes priority, so that's WAI. Ignore this comment.

[seiya]

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

42–46

I'll add a comment for it. It's worth mentioning in the code.

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

+## Show that if --remove-section is given, llvm-objcopy removes sections
+## specified by the option.
+
+# RUN: yaml2obj %s > %t
+
+# RUN: not llvm-objcopy --remove-section __text %t %t2 2>&1 \
+# RUN:   | FileCheck %s -DINPUT=%t --check-prefix=BAD-SECTION-NAME
+# BAD-SECTION-NAME: error: '[[INPUT]]': invalid section name '__text' (should be formatted as '<segment name>,<section name>')
+
+## Remove only __TEXT,__text section.
+# RUN: llvm-objcopy --remove-section __TEXT,__text %t %t2
+# RUN: llvm-readobj --sections --section-data %t2 \
+# RUN:   | FileCheck %s --check-prefix=REMOVE-TEXT-SECTION
+# REMOVE-TEXT-SECTION:      Sections [
+# REMOVE-TEXT-SECTION-NEXT:   Section {
+# REMOVE-TEXT-SECTION-NEXT:     Index: 0
+# REMOVE-TEXT-SECTION-NEXT:     Name: __data (5F 5F 64 61 74 61 00 00 00 00 00 00 00 00 00 00)
+# REMOVE-TEXT-SECTION-NEXT:     Segment: __DATA (5F 5F 44 41 54 41 00 00 00 00 00 00 00 00 00 00)
+# REMOVE-TEXT-SECTION-NEXT:     Address: 0x4
+# REMOVE-TEXT-SECTION-NEXT:     Size: 0x4
+# REMOVE-TEXT-SECTION-NEXT:     Offset: 264
+# REMOVE-TEXT-SECTION-NEXT:     Alignment: 0
+# REMOVE-TEXT-SECTION-NEXT:     RelocationOffset: 0x0
+# REMOVE-TEXT-SECTION-NEXT:     RelocationCount: 0
+# REMOVE-TEXT-SECTION-NEXT:     Type: Regular (0x0)
+# REMOVE-TEXT-SECTION-NEXT:     Attributes [ (0x0)
+# REMOVE-TEXT-SECTION-NEXT:     ]
+# REMOVE-TEXT-SECTION-NEXT:     Reserved1: 0x0
+# REMOVE-TEXT-SECTION-NEXT:     Reserved2: 0x0
+# REMOVE-TEXT-SECTION-NEXT:     Reserved3: 0x0
+# REMOVE-TEXT-SECTION-NEXT:     SectionData (
+# REMOVE-TEXT-SECTION-NEXT:       0000: DDAADDAA                             |....|
+# REMOVE-TEXT-SECTION-NEXT:     )
+# REMOVE-TEXT-SECTION-NEXT:   }
+# REMOVE-TEXT-SECTION-NEXT:   Section {
+# REMOVE-TEXT-SECTION-NEXT:     Index: 1
+# REMOVE-TEXT-SECTION-NEXT:     Name: __const (5F 5F 63 6F 6E 73 74 00 00 00 00 00 00 00 00 00)
+# REMOVE-TEXT-SECTION-NEXT:     Segment: __TEXT (5F 5F 54 45 58 54 00 00 00 00 00 00 00 00 00 00)
+# REMOVE-TEXT-SECTION-NEXT:     Address: 0x8
+# REMOVE-TEXT-SECTION-NEXT:     Size: 0x4
+# REMOVE-TEXT-SECTION-NEXT:     Offset: 268
+# REMOVE-TEXT-SECTION-NEXT:     Alignment: 0
+# REMOVE-TEXT-SECTION-NEXT:     RelocationOffset: 0x0
+# REMOVE-TEXT-SECTION-NEXT:     RelocationCount: 0
+# REMOVE-TEXT-SECTION-NEXT:     Type: Regular (0x0)
+# REMOVE-TEXT-SECTION-NEXT:     Attributes [ (0x0)
+# REMOVE-TEXT-SECTION-NEXT:     ]
+# REMOVE-TEXT-SECTION-NEXT:     Reserved1: 0x0
+# REMOVE-TEXT-SECTION-NEXT:     Reserved2: 0x0
+# REMOVE-TEXT-SECTION-NEXT:     Reserved3: 0x0
+# REMOVE-TEXT-SECTION-NEXT:     SectionData (
+# REMOVE-TEXT-SECTION-NEXT:       0000: EEFFEEFF                             |....|
+# REMOVE-TEXT-SECTION-NEXT:     )
+# REMOVE-TEXT-SECTION-NEXT:   }
+# REMOVE-TEXT-SECTION-NEXT: ]
+
+## Remove multiple sections.
+# RUN: llvm-objcopy --remove-section __TEXT,__text --remove-section __DATA,__data %t %t3
+# RUN: llvm-readobj --sections --section-data %t3 \
+# RUN:   | FileCheck %s --check-prefix=REMOVE-MULTIPLE-SECTIONS
+# REMOVE-MULTIPLE-SECTIONS:      Sections [
+# REMOVE-MULTIPLE-SECTIONS-NEXT:   Section {
+# REMOVE-MULTIPLE-SECTIONS-NEXT:     Index: 0
+# REMOVE-MULTIPLE-SECTIONS-NEXT:     Name: __const (5F 5F 63 6F 6E 73 74 00 00 00 00 00 00 00 00 00)
+# REMOVE-MULTIPLE-SECTIONS-NEXT:     Segment: __TEXT (5F 5F 54 45 58 54 00 00 00 00 00 00 00 00 00 00)
+# REMOVE-MULTIPLE-SECTIONS-NEXT:     Address: 0x8
+# REMOVE-MULTIPLE-SECTIONS-NEXT:     Size: 0x4
+# REMOVE-MULTIPLE-SECTIONS-NEXT:     Offset: 184
+# REMOVE-MULTIPLE-SECTIONS-NEXT:     Alignment: 0
+# REMOVE-MULTIPLE-SECTIONS-NEXT:     RelocationOffset: 0x0
+# REMOVE-MULTIPLE-SECTIONS-NEXT:     RelocationCount: 0
+# REMOVE-MULTIPLE-SECTIONS-NEXT:     Type: Regular (0x0)
+# REMOVE-MULTIPLE-SECTIONS-NEXT:     Attributes [ (0x0)
+# REMOVE-MULTIPLE-SECTIONS-NEXT:     ]
+# REMOVE-MULTIPLE-SECTIONS-NEXT:     Reserved1: 0x0
+# REMOVE-MULTIPLE-SECTIONS-NEXT:     Reserved2: 0x0
+# REMOVE-MULTIPLE-SECTIONS-NEXT:     Reserved3: 0x0
+# REMOVE-MULTIPLE-SECTIONS-NEXT:     SectionData (
+# REMOVE-MULTIPLE-SECTIONS-NEXT:       0000: EEFFEEFF                             |....|
+# REMOVE-MULTIPLE-SECTIONS-NEXT:     )
+# REMOVE-MULTIPLE-SECTIONS-NEXT:   }
+# REMOVE-MULTIPLE-SECTIONS-NEXT: ]
+
+## Keep all sections if the specified section name is not present in the
+## input. The output file should be the same as the input.
+# RUN: llvm-objcopy --remove-section __TEXT,__foo %t %t4
+# RUN: cmp %t %t4
+
+--- !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/MachO/MachOObjcopy.cpp

 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.ToRemove.empty()) {
+    RemovePred = [&Config, RemovePred](const Section &Sec) {
+      return is_contained(Config.ToRemove, Sec.CannonicalName);
+    };
+  }
+
   if (Config.StripAll) {
     // Remove all debug sections.
     RemovePred = [RemovePred](const Section &Sec) {
       if (Sec.Segname == "__DWARF")
         return true;
 
       return RemovePred(Sec);
     };
   }
 
   if (!Config.OnlySection.empty()) {
     RemovePred = [&Config, RemovePred](const Section &Sec) {
       return !is_contained(Config.OnlySection, Sec.CannonicalName);
     };
   }
 
   return Obj.removeSections(RemovePred);
 }
 
 static void markSymbols(const CopyConfig &Config, Object &Obj) {
   // Symbols referenced from the indirect symbol table must not be removed.
   for (IndirectSymbolEntry &ISE : Obj.IndirectSymTable.Symbols)
     if (ISE.Symbol)
static Error validateOptions(const CopyConfig &Config) {
   // 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;
   }
 
+  if (!Config.ToRemove.empty()) {
+    for (const NameOrRegex &NR : Config.ToRemove)
+      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.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.StripAllGNU || 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) {
+      !Config.SetSectionFlags.empty() || Config.ExtractDWO ||
+      Config.KeepFileSymbols || Config.LocalizeHidden || Config.PreserveDates ||
+      Config.StripAllGNU || 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);