[llvm] remove empty __LLVM segment in llvm-bitcode-strip

[rmaz]

Summary

When running llvm-bitcode-strip we want to remove the __LLVM
segment as well as the __bundle section when there are no other
sections in the segment.

[rmaz]

update test comment

[jhenderson]

llvm/test/tools/llvm-objcopy/MachO/bitcode-strip-remove-nonempty-segment.test

4–6

Ideally, we'd use the same invocation to check everything at once. I believe llvm-readobj has an option to dump Mach-O segments. Does that suffice?

llvm/test/tools/llvm-objcopy/MachO/bitcode-strip-remove.test

12

It's usually a bad idea to have a purely negative check like this, because they can rot too easily. Better would be to check all segments and show the one you want isn't in that set.

llvm/tools/llvm-objcopy/ObjcopyOptions.cpp

1224

its - belonging to it
it's - it is

[rmaz]

Single pass tests

[rmaz]

Use llvm-readobj --macho-segment

[jhenderson]

LGTM, with one change.

By the way, if you haven't already, you've probably done enough to request commit access.

llvm/test/tools/llvm-objcopy/MachO/bitcode-strip-remove.test

15

No need for this line: 1) the --implicit-check-not covers it. 2) This won't catch __LLVM appearing before or between the other segments.

[rmaz]

Remove unnecessary CHECK-NOT

[rmaz]

With linter

llvm/include/llvm/ObjCopy/MachO/MachOConfig.h

struct MachOConfig {
   std::vector<StringRef> RPathToPrepend;
   DenseMap<StringRef, StringRef> RPathsToUpdate;
   DenseMap<StringRef, StringRef> InstallNamesToUpdate;
   DenseSet<StringRef> RPathsToRemove;
 
   // install-name-tool's id option
   Optional<StringRef> SharedLibId;
 
+  // Segments to remove if they are empty
+  DenseSet<StringRef> EmptySegmentsToRemove;
+
   // Boolean options
   bool StripSwiftSymbols = false;
   bool KeepUndefined = false;
   bool RemoveCodesignLoadCommands = false;
 
   // install-name-tool's --delete_all_rpaths
   bool RemoveAllRpaths = false;
 };
 
 } // namespace objcopy
 } // namespace llvm
 
 #endif // LLVM_OBJCOPY_MACHO_MACHOCONFIG_H

llvm/lib/ObjCopy/MachO/MachOObjcopy.cpp

if (MachOConfig.RemoveCodesignLoadCommands) {
     auto RemovePred = [](const LoadCommand &LC) {
       return LC.MachOLoadCommand.load_command_data.cmd ==
              MachO::LC_CODE_SIGNATURE;
     };
     if (Error E = Obj.removeLoadCommands(RemovePred))
       return E;
   }
 
+  // Remove any empty segments if required.
+  if (!MachOConfig.EmptySegmentsToRemove.empty()) {
+    auto RemovePred = [&MachOConfig](const LoadCommand &LC) {
+      if (LC.MachOLoadCommand.load_command_data.cmd == MachO::LC_SEGMENT_64 ||
+          LC.MachOLoadCommand.load_command_data.cmd == MachO::LC_SEGMENT) {
+        return LC.Sections.empty() &&
+               MachOConfig.EmptySegmentsToRemove.contains(
+                   LC.getSegmentName().getValue());
+      }
+      return false;
+    };
+    if (Error E = Obj.removeLoadCommands(RemovePred))
+      return E;
+  }
+
   return Error::success();
 }
 
 static Error dumpSectionToFile(StringRef SecName, StringRef Filename,
                                Object &Obj) {
   for (LoadCommand &LC : Obj.LoadCommands)
     for (const std::unique_ptr<Section> &Sec : LC.Sections) {
       if (Sec->CanonicalName == SecName) {

llvm/test/tools/llvm-objcopy/MachO/bitcode-strip-remove-nonempty-segment.test

-## Test bitcode section removal.
+## Test bitcode section and segment removal.
 # RUN: yaml2obj %s -o %t
 # RUN: llvm-bitcode-strip -r %t -o %t2
 # RUN: llvm-readobj --sections %t2 | FileCheck --implicit-check-not=Name %s
+# RUN: llvm-otool -l %t2 | FileCheck --check-prefix=SEGMENTS %s
 
 # CHECK:      Name: __text
 # CHECK-NEXT: Segment: __TEXT
 # CHECK:      Name: __bundle
 # CHECK-NEXT: Segment: __DATA
 # CHECK:      Name: __notbundle
 # CHECK-NEXT: Segment: __LLVM
 
+# SEGMENTS: segname __LLVM
+
 --- !mach-o
 FileHeader:
   magic:           0xFEEDFACF
   cputype:         0x01000007
   cpusubtype:      0x00000003
   filetype:        0x00000001
-  ncmds:           1
-  sizeofcmds:      392
+  ncmds:           3
+  sizeofcmds:      536
   flags:           0x00002000
   reserved:        0x00000000
 LoadCommands:
   - cmd:             LC_SEGMENT_64
-    cmdsize:         392
-    segname:         ''
+    cmdsize:         152
+    segname:         __TEXT
     vmaddr:          0
-    vmsize:          16
-    fileoff:         424
-    filesize:        16
+    vmsize:          4
+    fileoff:         568
+    filesize:        4
     maxprot:         7
     initprot:        7
-    nsects:          4
+    nsects:          1
     flags:           0
     Sections:
       - sectname:        __text
         segname:         __TEXT
         addr:            0x0000000000000000
         content:         'AABBCCDD'
         size:            4
-        offset:          424
+        offset:          568
         align:           0
         reloff:          0x00000000
         nreloc:          0
         flags:           0x80000400
         reserved1:       0x00000000
         reserved2:       0x00000000
         reserved3:       0x00000000
+  - cmd:             LC_SEGMENT_64
+    cmdsize:         152
+    segname:         __DATA
+    vmaddr:          4
+    vmsize:          4
+    fileoff:         572
+    filesize:        4
+    maxprot:         7
+    initprot:        7
+    nsects:          1
+    flags:           0
+    Sections:
       - sectname:        __bundle
         segname:         __DATA
         addr:            0x0000000000000004
         content:         'DDAADDAA'
         size:            4
-        offset:          428
+        offset:          572
         align:           0
         reloff:          0x00000000
         nreloc:          0
         flags:           0x00000000
         reserved1:       0x00000000
         reserved2:       0x00000000
         reserved3:       0x00000000
+  - cmd:             LC_SEGMENT_64
+    cmdsize:         232
+    segname:         __LLVM
+    vmaddr:          8
+    vmsize:          8
+    fileoff:         576
+    filesize:        8
+    maxprot:         7
+    initprot:        7
+    nsects:          2
+    flags:           0
+    Sections:
       - sectname:        __bundle
         segname:         __LLVM
         addr:            0x0000000000000008
         content:         'EEFFEEFF'
         size:            4
-        offset:          432
+        offset:          576
         align:           0
         reloff:          0x00000000
         nreloc:          0
         flags:           0x00000000
         reserved1:       0x00000000
         reserved2:       0x00000000
         reserved3:       0x00000000
       - sectname:        __notbundle
         segname:         __LLVM
         addr:            0x0000000000000008
         content:         'EEFFEEFF'
         size:            4
-        offset:          436
+        offset:          580
         align:           0
         reloff:          0x00000000
         nreloc:          0
         flags:           0x00000000
         reserved1:       0x00000000
         reserved2:       0x00000000
         reserved3:       0x00000000

llvm/test/tools/llvm-objcopy/MachO/bitcode-strip-remove.test

-## Test bitcode section removal.
+## Test bitcode section and segment removal.
 # RUN: yaml2obj %s -o %t
 # RUN: llvm-bitcode-strip -r %t -o %t2
 # RUN: llvm-readobj --sections %t2 | FileCheck --implicit-check-not=Name %s
+# RUN: llvm-otool -l %t2 | FileCheck --check-prefix=SEGMENTS %s
 
 # CHECK:      Name: __text
 # CHECK-NEXT: Segment: __TEXT
 # CHECK:      Name: __bundle
 # CHECK-NEXT: Segment: __DATA
-# CHECK:      Name: __notbundle
-# CHECK-NEXT: Segment: __LLVM
+
+# SEGMENTS-NOT: segname __LLVM
 
 --- !mach-o
 FileHeader:
   magic:           0xFEEDFACF
   cputype:         0x01000007
   cpusubtype:      0x00000003
   filetype:        0x00000001
-  ncmds:           1
-  sizeofcmds:      392
+  ncmds:           3
+  sizeofcmds:      456
   flags:           0x00002000
   reserved:        0x00000000
 LoadCommands:
   - cmd:             LC_SEGMENT_64
-    cmdsize:         392
-    segname:         ''
+    cmdsize:         152
+    segname:         __TEXT
     vmaddr:          0
-    vmsize:          16
-    fileoff:         424
-    filesize:        16
+    vmsize:          4
+    fileoff:         488
+    filesize:        4
     maxprot:         7
     initprot:        7
-    nsects:          4
+    nsects:          1
     flags:           0
     Sections:
       - sectname:        __text
         segname:         __TEXT
         addr:            0x0000000000000000
         content:         'AABBCCDD'
         size:            4
-        offset:          424
+        offset:          488
         align:           0
         reloff:          0x00000000
         nreloc:          0
         flags:           0x80000400
         reserved1:       0x00000000
         reserved2:       0x00000000
         reserved3:       0x00000000
+  - cmd:             LC_SEGMENT_64
+    cmdsize:         152
+    segname:         __DATA
+    vmaddr:          4
+    vmsize:          4
+    fileoff:         492
+    filesize:        4
+    maxprot:         7
+    initprot:        7
+    nsects:          1
+    flags:           0
+    Sections:
       - sectname:        __bundle
         segname:         __DATA
         addr:            0x0000000000000004
         content:         'DDAADDAA'
         size:            4
-        offset:          428
+        offset:          492
         align:           0
         reloff:          0x00000000
         nreloc:          0
         flags:           0x00000000
         reserved1:       0x00000000
         reserved2:       0x00000000
         reserved3:       0x00000000
-      - sectname:        __bundle
-        segname:         __LLVM
-        addr:            0x0000000000000008
-        content:         'EEFFEEFF'
-        size:            4
-        offset:          432
-        align:           0
-        reloff:          0x00000000
-        nreloc:          0
-        flags:           0x00000000
-        reserved1:       0x00000000
-        reserved2:       0x00000000
-        reserved3:       0x00000000
-      - sectname:        __notbundle
+  - cmd:             LC_SEGMENT_64
+    cmdsize:         152
+    vmaddr:          8
+    vmsize:          4
+    fileoff:         496
+    filesize:        4
+    maxprot:         7
+    initprot:        7
+    nsects:          1
+    flags:           0
+    Sections:
+      - sectname:        __bundle
         segname:         __LLVM
         addr:            0x0000000000000008
         content:         'EEFFEEFF'
         size:            4
-        offset:          436
+        offset:          496
         align:           0
         reloff:          0x00000000
         nreloc:          0
         flags:           0x00000000
         reserved1:       0x00000000
         reserved2:       0x00000000
         reserved3:       0x00000000

llvm/tools/llvm-objcopy/ObjcopyOptions.cpp

if (!InputArgs.hasArg(BITCODE_STRIP_output)) {
     return createStringError(errc::invalid_argument,
                              "-o is a required argument");
   }
   Config.OutputFilename = InputArgs.getLastArgValue(BITCODE_STRIP_output);
 
   if (!InputArgs.hasArg(BITCODE_STRIP_remove))
     return createStringError(errc::invalid_argument, "no action specified");
 
-  // We only support -r for now, which removes all bitcode sections.
+  // We only support -r for now, which removes all bitcode sections and
+  // the __LLVM segment if its now empty.
   cantFail(Config.ToRemove.addMatcher(NameOrPattern::create(
       "__LLVM,__bundle", MatchStyle::Literal, ErrorCallback)));
+  MachOConfig.EmptySegmentsToRemove.insert("__LLVM");
 
   // By default remove codesign load commands as they will be invalid after
   // stripping.
   MachOConfig.RemoveCodesignLoadCommands =
       !InputArgs.hasArg(BITCODE_STRIP_keep_codesign);
 
   DC.CopyConfigs.push_back(std::move(ConfigMgr));
   return std::move(DC);