[llvm-objcopy][MachO] Implement -Obinary

[jhenderson]

llvm/test/tools/llvm-objcopy/MachO/binary-output.test

1

How about "Shows that for binary output, llvm-objcopy extracts ..."?

3

Remove gap -> It removes any gap

5

Preserve gaps -> It preserves any gaps

[seiya]

Fixed review comments.

[grimar]

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

503

LC -> Cmd?

518

This can be:

// Remove ....
for (Section *Sec : OrderedSections)
  Sec->Addr -= OrderedSections[0]->Addr;

531

Does it make sense to remove this in favor of speed?
I.e. instead of filling the whole buffer with zeroes you can fill only gaps.
This might be better for large binaries.

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

72

What do you think of using probably more traditional/clear Obj and Buf names?

[jhenderson]

llvm/test/tools/llvm-objcopy/MachO/binary-output.test

3

MachO -> Mach-O (I believe)

[rupprecht]

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

524–527

ELF/binary-output-empty.test would be a good test to add for this

[seiya]

Addressed review comments.

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

503

LC is the common abbreviation for LoadCommand under llvm-objcopy/MachO.

518

I don't think it works. This for loop subtracts the original value of the first section's address (OrderedSections[0]->Addr). In the code you suggested, in the first iteration, Sec points to OrderedSections[0] and as a result OrderedSections[0]->Addr becomes 0 in remaining iterations.

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

72

I'd like to keep them because O and B are widely used under llvm-objcopy/MachO. That said, Obj and Buf looks better to me. I'll submit another NFC patch to rename them.

[seiya]

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

531

Updated the patch to fill only gaps instead of the whole buffer.

[seiya]

Addressed a review comment.

[jhenderson]

llvm/test/tools/llvm-objcopy/MachO/binary-output.test

36

Nit: delete extra blank line here.

llvm/test/tools/llvm-objcopy/MachO/binary-output.test

+## Show that llvm-objcopy extracts section contents in MachO properly, that is:
+##
+##   - Remove gap before the first section: __bar section content is at the
+##     beginning of the output.
+##   - Preserve gaps between sections: 12 bytes padding (zeroes) before __foo
+##     section content.
+##
+# RUN: yaml2obj %s > %t
+# RUN: llvm-objcopy -Obinary %t %t.bin
+# RUN: od -t x1 %t.bin | FileCheck %s
+
+# CHECK:      0000000    aa  bb  cc  dd  00  00  00  00  00  00  00  00  00  00  00  00
+# CHECK-NEXT: 0000020    ee  ff
+# CHECK-NEXT: 0000022{{$}}
+
+--- !mach-o
+FileHeader:
+  magic:           0xFEEDFACF
+  cputype:         0x01000007
+  cpusubtype:      0x80000003
+  filetype:        0x00000002
+  ncmds:           1
+  sizeofcmds:      232
+  flags:           0x00002000
+  reserved:        0x00000000
+LoadCommands:
+  - cmd:             LC_SEGMENT_64
+    cmdsize:         232
+    segname:         '__DATA'
+    vmaddr:          0x0000000000001000
+    vmsize:          32
+    fileoff:         472
+    filesize:        6
+    maxprot:         7
+    initprot:        7
+    nsects:          2
+    flags:           0
+    Sections:
+      - sectname:        __bar
+        segname:         __DATA
+        addr:            0x0000000000001000
+        content:         'AABBCCDD'
+        size:            4
+        offset:          0x0000010c
+        align:           0
+        reloff:          0x00000000
+        nreloc:          0
+        flags:           0x00000000
+        reserved1:       0x00000000
+        reserved2:       0x00000000
+        reserved3:       0x00000000
+      - sectname:        __foo
+        segname:         __DATA
+        addr:            0x0000000000001010
+        size:            2
+        content:         'EEFF'
+        offset:          0x00000110
+        align:           0
+        reloff:          0x00000000
+        nreloc:          0
+        flags:           0x00000000
+        reserved1:       0x00000000
+        reserved2:       0x00000000
+        reserved3:       0x00000000

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

if (Error E = isValidMachOCannonicalName(SecName))
       return E;
     if (Error E = dumpSectionToFile(SecName, File, Obj))
       return E;
   }
 
   return Error::success();
 }
 
-Error writeOutput(Object &O, Buffer &Out, bool Is64Bit, bool IsLittleEndian) {
+static std::unique_ptr<Writer> createWriter(const CopyConfig &Config, Object &O,
+                                            Buffer &Out, bool Is64Bit,
+                                            bool IsLittleEndian) {
   // TODO: Support 16KB pages which are employed in iOS arm64 binaries:
   //       https://github.com/llvm/llvm-project/commit/1bebb2832ee312d3b0316dacff457a7a29435edb
   const uint64_t PageSize = 4096;
 
-  MachOWriter Writer(O, Is64Bit, IsLittleEndian, PageSize, Out);
-  if (auto E = Writer.finalize())
+  switch (Config.OutputFormat) {
+  case FileFormat::Binary:
+    return std::make_unique<BinaryWriter>(
+        BinaryWriter(O, Is64Bit, IsLittleEndian, PageSize, Out));
+  default:
+    return std::make_unique<MachOWriter>(
+        MachOWriter(O, Is64Bit, IsLittleEndian, PageSize, Out));
+  }
+}
+
+Error writeOutput(const CopyConfig &Config, Object &O, Buffer &Out,
+                  bool Is64Bit, bool IsLittleEndian) {
+  std::unique_ptr<Writer> W =
+      createWriter(Config, O, Out, Is64Bit, IsLittleEndian);
+  if (auto E = W->finalize())
     return E;
-  return Writer.write();
+  return W->write();
 }
 
 Error executeObjcopyOnRawBinary(const CopyConfig &Config, MemoryBuffer &In,
                                 Buffer &Out) {
   const MachineInfo &MI = Config.BinaryArch;
   BinaryReader Reader(MI, In);
   std::unique_ptr<Object> O = Reader.create();
 
   if (Error E = handleArgs(Config, *O))
     return createFileError(Config.InputFilename, std::move(E));
 
-  return writeOutput(*O, Out, MI.Is64Bit, MI.IsLittleEndian);
+  return writeOutput(Config, *O, Out, MI.Is64Bit, MI.IsLittleEndian);
 }
 
 Error executeObjcopyOnBinary(const CopyConfig &Config,
                              object::MachOObjectFile &In, Buffer &Out) {
   MachOReader Reader(In);
   std::unique_ptr<Object> O = Reader.create();
   if (!O)
     return createFileError(
         Config.InputFilename,
         createStringError(object_error::parse_failed,
                           "unable to deserialize MachO object"));
 
   if (Error E = handleArgs(Config, *O))
     return createFileError(Config.InputFilename, std::move(E));
 
-  return writeOutput(*O, Out, In.is64Bit(), In.isLittleEndian());
+  return writeOutput(Config, *O, Out, In.is64Bit(), In.isLittleEndian());
 }
 
 } // end namespace macho
 } // end namespace objcopy
 } // end namespace llvm

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

 #include "llvm/Object/MachO.h"
 
 namespace llvm {
 class Error;
 
 namespace objcopy {
 namespace macho {
 
-class MachOWriter {
+class Writer {
+public:
+  virtual ~Writer() {}
+  virtual Error finalize() = 0;
+  virtual Error write() = 0;
+};
+
+class MachOWriter : public Writer {
   Object &O;
   bool Is64Bit;
   bool IsLittleEndian;
   uint64_t PageSize;
   Buffer &B;
   MachOLayoutBuilder LayoutBuilder;
 
   size_t headerSize() const;
class MachOWriter : public Writer {
   void writeFunctionStartsData();
   void writeTail();
 
 public:
   MachOWriter(Object &O, bool Is64Bit, bool IsLittleEndian, uint64_t PageSize,
               Buffer &B)
       : O(O), Is64Bit(Is64Bit), IsLittleEndian(IsLittleEndian),
         PageSize(PageSize), B(B), LayoutBuilder(O, Is64Bit, PageSize) {}
-
+  ~MachOWriter() {}
   size_t totalSize() const;
-  Error finalize();
-  Error write();
+  Error finalize() override;
+  Error write() override;
+};
+
+class BinaryWriter : public Writer {
+  Object &O;
+  Buffer &B;
+  MachOLayoutBuilder LayoutBuilder;
+  std::vector<Section *> OrderedSections;
+
+public:
+  BinaryWriter(Object &O, bool Is64Bit, bool IsLittleEndian, uint64_t PageSize,
+               Buffer &B)
+      : O(O), B(B), LayoutBuilder(O, Is64Bit, PageSize) {}
+  ~BinaryWriter() {}
+  Error finalize() override;
+  Error write() override;
 };
 
 } // end namespace macho
 } // end namespace objcopy
 } // end namespace llvm

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

Error MachOWriter::write() {
   memset(B.getBufferStart(), 0, totalSize());
   writeHeader();
   writeLoadCommands();
   writeSections();
   writeTail();
   return B.commit();
 }
 
+Error BinaryWriter::finalize() {
+  if (Error E = LayoutBuilder.layout())
+    return E;
+
+  // Sort sections by their addresses.
+  for (LoadCommand &LC : O.LoadCommands)
+    for (Section &Sec : LC.Sections)
+      if (!Sec.isVirtualSection())
+        OrderedSections.push_back(&Sec);
+
+  llvm::stable_sort(OrderedSections, [](const Section *A, const Section *B) {
+    return A->Addr < B->Addr;
+  });
+
+  // Remove gap at the start. `Addr` is used as the file offset in the
+  // output file.
+  if (!OrderedSections.empty()) {
+    uint64_t AddrBase = OrderedSections[0]->Addr;
+    for (Section *Sec : OrderedSections)
+      Sec->Addr -= AddrBase;
+  }
+
+  return Error::success();
+}
+
+Error BinaryWriter::write() {
+  size_t TotalSize =
+      OrderedSections.empty()
+          ? 0
+          : (OrderedSections.back()->Addr + OrderedSections.back()->Size);
+
+  if (Error E = B.allocate(TotalSize))
+    return E;
+  memset(B.getBufferStart(), 0, TotalSize);
+
+  for (const Section *Sec : OrderedSections)
+    memcpy(B.getBufferStart() + Sec->Addr, Sec->getContent().data(), Sec->Size);
+
+  return B.commit();
+}
+
 } // end namespace macho
 } // end namespace objcopy
 } // end namespace llvm