[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

88

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/docs/CommandGuide/llvm-objcopy.rst

  If specified, symbol and section names specified by other switches are treated
  as extended POSIX regular expression patterns.
 
 .. option:: --remove-section <section>, -R
 
  Remove the specified section from the output. Can be specified multiple times
  to remove multiple sections simultaneously.
 
+ For MachO objects, ``<section>`` must be formatted as
+ ``<segment name>,<section name>``.
+
 .. option:: --set-section-alignment <section>=<align>
 
  Set the alignment of section ``<section>`` to `<align>``. Can be specified
  multiple times to update multiple sections.
 
 .. option:: --strip-all-gnu
 
  Remove all symbols, debug sections and relocations from the output. This option

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
+
+## Error case: Invalid section name.
+# 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: 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-prefixes=COMMON,REMOVE-TEXT-ONLY
+
+## 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-prefixes=COMMON,REMOVE-TEXT-AND-DATA
+
+# COMMON:                    Sections [
+# REMOVE-TEXT-ONLY-NEXT:       Section {
+# REMOVE-TEXT-ONLY-NEXT:         Index: 0
+# REMOVE-TEXT-ONLY-NEXT:         Name: __data (5F 5F 64 61 74 61 00 00 00 00 00 00 00 00 00 00)
+# REMOVE-TEXT-ONLY-NEXT:         Segment: __DATA (5F 5F 44 41 54 41 00 00 00 00 00 00 00 00 00 00)
+# REMOVE-TEXT-ONLY-NEXT:         Address: 0x4
+# REMOVE-TEXT-ONLY-NEXT:         Size: 0x4
+# REMOVE-TEXT-ONLY-NEXT:         Offset: 264
+# REMOVE-TEXT-ONLY-NEXT:         Alignment: 0
+# REMOVE-TEXT-ONLY-NEXT:         RelocationOffset: 0x0
+# REMOVE-TEXT-ONLY-NEXT:         RelocationCount: 0
+# REMOVE-TEXT-ONLY-NEXT:         Type: Regular (0x0)
+# REMOVE-TEXT-ONLY-NEXT:         Attributes [ (0x0)
+# REMOVE-TEXT-ONLY-NEXT:         ]
+# REMOVE-TEXT-ONLY-NEXT:         Reserved1: 0x0
+# REMOVE-TEXT-ONLY-NEXT:         Reserved2: 0x0
+# REMOVE-TEXT-ONLY-NEXT:         Reserved3: 0x0
+# REMOVE-TEXT-ONLY-NEXT:         SectionData (
+# REMOVE-TEXT-ONLY-NEXT:           0000: DDAADDAA                             |....|
+# REMOVE-TEXT-ONLY-NEXT:         )
+# REMOVE-TEXT-ONLY-NEXT:       }
+# COMMON-NEXT:                 Section {
+# REMOVE-TEXT-ONLY-NEXT:         Index: 1
+# REMOVE-TEXT-AND-DATA-NEXT:     Index: 0
+# COMMON-NEXT:                   Name: __const (5F 5F 63 6F 6E 73 74 00 00 00 00 00 00 00 00 00)
+# COMMON-NEXT:                   Segment: __TEXT (5F 5F 54 45 58 54 00 00 00 00 00 00 00 00 00 00)
+# COMMON-NEXT:                   Address: 0x8
+# COMMON-NEXT:                   Size: 0x4
+# REMOVE-TEXT-ONLY-NEXT:         Offset: 268
+# REMOVE-TEXT-AND-DATA-NEXT:     Offset: 184
+# COMMON-NEXT:                   Alignment: 0
+# COMMON-NEXT:                   RelocationOffset: 0x0
+# COMMON-NEXT:                   RelocationCount: 0
+# COMMON-NEXT:                   Type: Regular (0x0)
+# COMMON-NEXT:                   Attributes [ (0x0)
+# COMMON-NEXT:                   ]
+# COMMON-NEXT:                   Reserved1: 0x0
+# COMMON-NEXT:                   Reserved2: 0x0
+# COMMON-NEXT:                   Reserved3: 0x0
+# COMMON-NEXT:                   SectionData (
+# COMMON-NEXT:                     0000: EEFFEEFF                             |....|
+# COMMON-NEXT:                   )
+# COMMON-NEXT:                 }
+# COMMON-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/MachOConfig.cpp

Expected<MachOCopyConfig> parseConfig(const CopyConfig &Config) {
   //
   // TODO: Support GNU objcopy's section renaming rules for compatibility (see
   // http://lists.llvm.org/pipermail/llvm-dev/2019-May/132570.html).
   if (!Config.OnlySection.empty()) {
     for (const NameOrRegex &NR : Config.OnlySection.matchers())
       if (Error E = NR.isMachOCanonicalName())
         return std::move(E);
   }
+  if (!Config.ToRemove.empty()) {
+    for (const NameOrRegex &NR : Config.ToRemove.matchers())
+      if (Error E = NR.isMachOCanonicalName())
+        return std::move(E);
+  }
 
   return MachOConfig;
 }
 
 } // end namespace macho
 } // end namespace objcopy
 } // end namespace llvm

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 Config.ToRemove.matches(Sec.CanonicalName);
+    };
+  }
+
   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 !Config.OnlySection.matches(Sec.CanonicalName);
     };
   }
 
   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)
if (Config.AllowBrokenLinks || !Config.BuildIdLinkDir.empty() ||
       !Config.AllocSectionsPrefix.empty() || !Config.AddSection.empty() ||
       !Config.DumpSection.empty() || !Config.KeepSection.empty() ||
       Config.NewSymbolVisibility || !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.SetSectionAlignment.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.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");
   }
 
   removeSections(Config, Obj);