[llvm-objcopy][MachO] Recompute and update offset/size fields in the writer

[seiya]

Summary

Recompute and update offset/size fields so that we can implement llvm-objcopy options like --only-section.

This patch is the first step and focuses on supporting load commands that covered by existing tests: executable files and
dynamic libraries are not supported.

[alexander-shaposhnikov]

great start! I will take a closer look at the diff tomorrow + over the weekend

[compnerd]

llvm/test/tools/llvm-objcopy/MachO/Inputs/various-symbols.s

3

NIT: common is misspelt

5

global3 is unreferenced. Is that intended?

llvm/test/tools/llvm-objcopy/MachO/real-world-input-copy.test

13

The test input seems oddly complicated for testing that. It seems that anything that contains a LC_DYNSYMTAB which means anything with an externally visible symbol should be sufficient. The BSS section would be emitted by any static. Something like this seems much more concise which is better for testing the cases:

static int i;
int f(void) { return i; }

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

22

Isn't this just strnlen?

34

I believe that the 16 here is supposed to be the maximum length of the section/segment name in MachO. Can you use sizeof(llvm::MachO::section::sectname) and sizeof(llvm::MachO::section::segname) instead please?

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

139

I wish that we had the structs built using packed_endian types

143

Can the Sec be const?

146

I think that you can drop copyStringWithPadding. Instead zero initialise the section that you are constructing, which also gives you more reproducible output.

462

Can't Sec be const?

470

VM is more appropriate - it is an initialism for Virtual Memory.

501

Can't LC be const?

502

Can't Sec be const?

510

Can't LC be const?

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

32

Might be more readable as updateDySymTab

[alexander-shaposhnikov]

llvm/test/tools/llvm-objcopy/MachO/real-world-input-copy.test

13

+1

[seiya]

• Simplified various-symbols.s.
• Replaced strlenOrMaxLen (introduced by this patch) with strnlen.
• Replaced the hard-coded lengths of segment/section name with sizeof().
• Renamed updateDysymtab to updateDySymTab .
• Replaced copyStringWithPadding (introduced by this patch) with memset-ing by zero and memcpy.
• Add const to a variable.

[seiya]

llvm/test/tools/llvm-objcopy/MachO/real-world-input-copy.test

13

Indeed. Simplified the test case.

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

22

Quite so :)

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

139

I wonder if we could do this better too. Could you tell me what packed_endian is? You mean llvm/Support/Endian.h? I'd like to take a look at it.

462

No. It's mutated in the loop.

471

I've replaced the calculation of VMSize by the algorithm used in MachObjectWriter::writeObject

501

It can't be const since its sections are mutated in the loop.

502

No. It's mutated in the loop.

510

No. LC. MachOLoadCommand is mutated in the loop.

[seiya]

llvm/test/tools/llvm-objcopy/MachO/Inputs/various-symbols.s

5

That was a mistake. I've updated this file so it no longer matters btw.

[seiya]

• add some consts

[alexander-shaposhnikov]

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

135

nit: I'd use the explicit type here (for readability)

147

khm, here and in some other similar places, i'd use sizeof(...)
or create a named constant for this number (16).

174

here and the lines above (143 - 170) are very similar,
let's create a helper function for this and avoid code duplication.

422

explicit type + I think naming here doesn't follow our style guide)

432

see the comment on the line 438

465

   
1 << Sec.Align

P.S. pow works with floating point numbers, I think we don't really need these conversions here

[seiya]

Address review comments.

[seiya]

Address remaining review comments.

[alexander-shaposhnikov]

so in general this looks very promising to me,
however test coverage needs improvements imo, though given that at the moment we don't expose any non-trivial options here, we can add more tests incrementally. Btw - for DYSYMTAB, SYMTAB - is it possible to add YAML-based tests ? or yaml2obj's support is not sufficient for this ?

I'd also wait for @jhenderson and/or @rupprecht to take a look at this code as well.

[jhenderson]

I'm struggling for time at the moment, and am not really familiar with MachO at all, so it would take me quite some time to review this. If @rupprecht is happy, I am happy.

[seiya]

In D62652#1530262, @alexshap wrote:

so in general this looks very promising to me,
however test coverage needs improvements imo, though given that at the moment we don't expose any non-trivial options here, we can add more tests incrementally. Btw - for DYSYMTAB, SYMTAB - is it possible to add YAML-based tests ? or yaml2obj's support is not sufficient for this ?

I'd also wait for @jhenderson and/or @rupprecht to take a look at this code as well.

It seems that yaml2obj supports DYSYMTAB and SYMTAB. I'll add YAML-based tests in another patch which implements symbol table rebuilding.

[alexander-shaposhnikov]

@seiya, ok, i think if nobody comments on this diff in the coming days it's safe to proceed and commit it, I kinda don't want to block it and I'm looking forward to more tests in the follow-up patches .

[rupprecht]

Also not a Mach-O expert, but I looked through it and what I can understand looks fine (+ it passes existing tests). More tests in a future patch are always welcome though.

[seiya]

Thank you all. I'll commit this later.

llvm/test/tools/llvm-objcopy/MachO/Inputs/various-symbols.s

+# static int i; // A local symbol. 
+# int f(void) { return i; } // An external symbol.
+
+  .section  __TEXT,__text,regular,pure_instructions
+  .build_version macos, 10, 14
+  .globl  _f                      ## -- Begin function f
+  .p2align  4, 0x90
+_f:                                     ## @f
+  .cfi_startproc
+## %bb.0:
+  pushq %rbp
+  .cfi_def_cfa_offset 16
+  .cfi_offset %rbp, -16
+  movq  %rsp, %rbp
+  .cfi_def_cfa_register %rbp
+  movl  _i(%rip), %eax
+  popq  %rbp
+  retq
+  .cfi_endproc
+                                        ## -- End function
+.zerofill __DATA,__bss,_i,4,2           ## @i
+
+.subsections_via_symbols

llvm/test/tools/llvm-objcopy/MachO/real-world-input-copy.test

 # RUN: llvm-mc -assemble -triple i386-apple-darwin9 -filetype=obj %p/Inputs/macho.32.s -o %t.32.o
 # RUN: llvm-objcopy %t.32.o %t.32.copy.o
 # RUN: cmp %t.32.o %t.32.copy.o
 
 # RUN: llvm-mc -assemble -triple x86_64-apple-darwin9 -filetype=obj %p/Inputs/macho.64.s -o %t.64.o
 # RUN: llvm-objcopy %t.64.o %t.64.copy.o
 # RUN: cmp %t.64.o %t.64.copy.o
 
+# Make sure that it properly constructs LC_DYSYMTAB and handles virtual sections
+# (the bss section for example).
+# RUN: llvm-mc -assemble -triple x86_64-apple-darwin9 -filetype=obj %p/Inputs/various-symbols.s -o %t.various-symbols.o
+# RUN: llvm-objcopy %t.various-symbols.o %t.various-symbols.copy.o
+# RUN: cmp %t.various-symbols.o %t.various-symbols.copy.o
+
 # REQUIRES: x86-registered-target

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

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));
 
   MachOWriter Writer(*O, In.is64Bit(), In.isLittleEndian(), Out);
+  if (auto E = Writer.finalize())
+    return E;
   return Writer.write();
 }
 
 } // end namespace macho
 } // end namespace objcopy
 } // end namespace llvm

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

 #include <memory>
 
 namespace llvm {
 namespace objcopy {
 namespace macho {
 
 void MachOReader::readHeader(Object &O) const {
   O.Header.Magic = MachOObj.getHeader().magic;
   O.Header.CPUType = MachOObj.getHeader().cputype;
   O.Header.CPUSubType = MachOObj.getHeader().cpusubtype;
   O.Header.FileType = MachOObj.getHeader().filetype;
   O.Header.NCmds = MachOObj.getHeader().ncmds;
   O.Header.SizeOfCmds = MachOObj.getHeader().sizeofcmds;
   O.Header.Flags = MachOObj.getHeader().flags;
 }
 
 template <typename SectionType>
 Section constructSectionCommon(SectionType Sec) {
   Section S;
-  memcpy(S.Sectname, Sec.sectname, sizeof(Sec.sectname));
-  memcpy(S.Segname, Sec.segname, sizeof(Sec.segname));
+  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.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;
if (MachOObj.isLittleEndian() != sys::IsLittleEndianHost) {
       MachO::swapStruct(Sec);
       Sections.push_back(constructSection(Sec));
     } else {
       Sections.push_back(constructSection(*Curr));
     }
 
     Section &S = Sections.back();
 
-    StringRef SectName(S.Sectname);
     Expected<object::SectionRef> SecRef =
         MachOObj.getSection(NextSectionIndex++);
     if (!SecRef)
       reportError(MachOObj.getFileName(), SecRef.takeError());
 
     if (Expected<ArrayRef<uint8_t>> E =
             MachOObj.getSectionContents(SecRef->getRawDataRefImpl()))
       S.Content =

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

class MachOWriter {
   bool IsLittleEndian;
   Buffer &B;
 
   size_t headerSize() const;
   size_t loadCommandsSize() const;
   size_t symTableSize() const;
   size_t strTableSize() const;
 
+  void updateDySymTab(MachO::macho_load_command &MLC);
+  void updateSizeOfCmds();
+  Error layout();
+
   void writeHeader();
   void writeLoadCommands();
+  template <typename StructType>
+  void writeSectionInLoadCommand(const Section &Sec, uint8_t *&Out);
   void writeSections();
   void writeSymbolTable();
   void writeStringTable();
   void writeRebaseInfo();
   void writeBindInfo();
   void writeWeakBindInfo();
   void writeLazyBindInfo();
   void writeExportInfo();
   void writeTail();
 
 public:
   MachOWriter(Object &O, bool Is64Bit, bool IsLittleEndian, Buffer &B)
       : O(O), Is64Bit(Is64Bit), IsLittleEndian(IsLittleEndian), B(B) {}
 
   size_t totalSize() const;
+  Error finalize();
   Error write();
 };
 
 } // end namespace macho
 } // end namespace objcopy
 } // end namespace llvm

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

 //===- MachOWriter.cpp ------------------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "MachOWriter.h"
 #include "Object.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/BinaryFormat/MachO.h"
 #include "llvm/Object/MachO.h"
+#include "llvm/Support/Errc.h"
+#include "llvm/Support/ErrorHandling.h"
 #include <memory>
 
 namespace llvm {
 namespace objcopy {
 namespace macho {
 
 size_t MachOWriter::headerSize() const {
   return Is64Bit ? sizeof(MachO::mach_header_64) : sizeof(MachO::mach_header);
void MachOWriter::writeHeader() {
 
   auto HeaderSize =
       Is64Bit ? sizeof(MachO::mach_header_64) : sizeof(MachO::mach_header);
   memcpy(B.getBufferStart(), &Header, HeaderSize);
 }
 
 void MachOWriter::writeLoadCommands() {
   uint8_t *Begin = B.getBufferStart() + headerSize();
-  MachO::macho_load_command MLC;
   for (const auto &LC : O.LoadCommands) {
+    // Construct a load command.
+    MachO::macho_load_command MLC = LC.MachOLoadCommand;
+    switch (MLC.load_command_data.cmd) {
+    case MachO::LC_SEGMENT:
+      if (IsLittleEndian != sys::IsLittleEndianHost)
+        MachO::swapStruct(MLC.segment_command_data);
+      memcpy(Begin, &MLC.segment_command_data, sizeof(MachO::segment_command));
+      Begin += sizeof(MachO::segment_command);
+
+      for (const auto &Sec : LC.Sections)
+        writeSectionInLoadCommand<MachO::section>(Sec, Begin);
+      continue;
+    case MachO::LC_SEGMENT_64:
+      if (IsLittleEndian != sys::IsLittleEndianHost)
+        MachO::swapStruct(MLC.segment_command_64_data);
+      memcpy(Begin, &MLC.segment_command_64_data,
+             sizeof(MachO::segment_command_64));
+      Begin += sizeof(MachO::segment_command_64);
+
+      for (const auto &Sec : LC.Sections)
+        writeSectionInLoadCommand<MachO::section_64>(Sec, Begin);
+      continue;
+    }
+
 #define HANDLE_LOAD_COMMAND(LCName, LCValue, LCStruct)                         \
   case MachO::LCName:                                                          \
     assert(sizeof(MachO::LCStruct) + LC.Payload.size() ==                      \
-           LC.MachOLoadCommand.load_command_data.cmdsize);                     \
-    MLC = LC.MachOLoadCommand;                                                 \
+           MLC.load_command_data.cmdsize);                                     \
     if (IsLittleEndian != sys::IsLittleEndianHost)                             \
       MachO::swapStruct(MLC.LCStruct##_data);                                  \
     memcpy(Begin, &MLC.LCStruct##_data, sizeof(MachO::LCStruct));              \
     Begin += sizeof(MachO::LCStruct);                                          \
     memcpy(Begin, LC.Payload.data(), LC.Payload.size());                       \
     Begin += LC.Payload.size();                                                \
     break;
 
-    switch (LC.MachOLoadCommand.load_command_data.cmd) {
+    // Copy the load command as it is.
+    switch (MLC.load_command_data.cmd) {
     default:
       assert(sizeof(MachO::load_command) + LC.Payload.size() ==
-             LC.MachOLoadCommand.load_command_data.cmdsize);
-      MLC = LC.MachOLoadCommand;
+             MLC.load_command_data.cmdsize);
       if (IsLittleEndian != sys::IsLittleEndianHost)
         MachO::swapStruct(MLC.load_command_data);
       memcpy(Begin, &MLC.load_command_data, sizeof(MachO::load_command));
       Begin += sizeof(MachO::load_command);
       memcpy(Begin, LC.Payload.data(), LC.Payload.size());
       Begin += LC.Payload.size();
       break;
 #include "llvm/BinaryFormat/MachO.def"
     }
   }
 }
 
+template <typename StructType>
+void MachOWriter::writeSectionInLoadCommand(const Section &Sec, uint8_t *&Out) {
+  StructType Temp;
+  assert(Sec.Segname.size() <= sizeof(Temp.segname) && "too long segment name");
+  assert(Sec.Sectname.size() <= sizeof(Temp.sectname) &&
+         "too long section name");
+  memset(&Temp, 0, sizeof(StructType));
+  memcpy(Temp.segname, Sec.Segname.data(), Sec.Segname.size());
+  memcpy(Temp.sectname, Sec.Sectname.data(), Sec.Sectname.size());
+  Temp.addr = Sec.Addr;
+  Temp.size = Sec.Size;
+  Temp.offset = Sec.Offset;
+  Temp.align = Sec.Align;
+  Temp.reloff = Sec.RelOff;
+  Temp.nreloc = Sec.NReloc;
+  Temp.flags = Sec.Flags;
+  Temp.reserved1 = Sec.Reserved1;
+  Temp.reserved2 = Sec.Reserved2;
+
+  if (IsLittleEndian != sys::IsLittleEndianHost)
+    MachO::swapStruct(Temp);
+  memcpy(Out, &Temp, sizeof(StructType));
+  Out += sizeof(StructType);
+}
+
 void MachOWriter::writeSections() {
   for (const auto &LC : O.LoadCommands)
     for (const auto &Sec : LC.Sections) {
+      if (Sec.isVirtualSection())
+        continue;
+
       assert(Sec.Offset && "Section offset can not be zero");
       assert((Sec.Size == Sec.Content.size()) && "Incorrect section size");
       memcpy(B.getBufferStart() + Sec.Offset, Sec.Content.data(),
              Sec.Content.size());
       for (size_t Index = 0; Index < Sec.Relocations.size(); ++Index) {
         MachO::any_relocation_info R = Sec.Relocations[Index];
         if (IsLittleEndian != sys::IsLittleEndianHost)
           MachO::swapStruct(R);
void MachOWriter::writeTail() {
   llvm::sort(Queue, [](const WriteOperation &LHS, const WriteOperation &RHS) {
     return LHS.first < RHS.first;
   });
 
   for (auto WriteOp : Queue)
     (this->*WriteOp.second)();
 }
 
+void MachOWriter::updateSizeOfCmds() {
+  auto Size = 0;
+  for (const auto &LC : O.LoadCommands) {
+    auto &MLC = LC.MachOLoadCommand;
+    auto cmd = MLC.load_command_data.cmd;
+
+    switch (cmd) {
+    case MachO::LC_SEGMENT:
+      Size += sizeof(MachO::segment_command) +
+              sizeof(MachO::section) * LC.Sections.size();
+      continue;
+    case MachO::LC_SEGMENT_64:
+      Size += sizeof(MachO::segment_command_64) +
+              sizeof(MachO::section_64) * LC.Sections.size();
+      continue;
+    }
+
+    switch (cmd) {
+#define HANDLE_LOAD_COMMAND(LCName, LCValue, LCStruct)                         \
+  case MachO::LCName:                                                          \
+    Size += sizeof(MachO::LCStruct);                                           \
+    break;
+#include "llvm/BinaryFormat/MachO.def"
+#undef HANDLE_LOAD_COMMAND
+    }
+  }
+
+  O.Header.SizeOfCmds = Size;
+}
+
+// Updates the index and the number of local/external/undefined symbols. Here we
+// assume that MLC is a LC_DYSYMTAB and the nlist entries in the symbol table
+// are already sorted by the those types.
+void MachOWriter::updateDySymTab(MachO::macho_load_command &MLC) {
+  uint32_t NumLocalSymbols = 0;
+  auto Iter = O.SymTable.NameList.begin();
+  auto End = O.SymTable.NameList.end();
+  for (; Iter != End; Iter++) {
+    if (Iter->n_type & (MachO::N_EXT | MachO::N_PEXT))
+      break;
+
+    NumLocalSymbols++;
+  }
+
+  uint32_t NumExtDefSymbols = 0;
+  for (; Iter != End; Iter++) {
+    if ((Iter->n_type & MachO::N_TYPE) == MachO::N_UNDF)
+      break;
+
+    NumExtDefSymbols++;
+  }
+
+  MLC.dysymtab_command_data.ilocalsym = 0;
+  MLC.dysymtab_command_data.nlocalsym = NumLocalSymbols;
+  MLC.dysymtab_command_data.iextdefsym = NumLocalSymbols;
+  MLC.dysymtab_command_data.nextdefsym = NumExtDefSymbols;
+  MLC.dysymtab_command_data.iundefsym = NumLocalSymbols + NumExtDefSymbols;
+  MLC.dysymtab_command_data.nundefsym =
+      O.SymTable.NameList.size() - (NumLocalSymbols + NumExtDefSymbols);
+}
+
+// Recomputes and updates offset and size fields in load commands and sections
+// since they could be modified.
+Error MachOWriter::layout() {
+  auto SizeOfCmds = loadCommandsSize();
+  auto Offset = headerSize() + SizeOfCmds;
+  O.Header.NCmds = O.LoadCommands.size();
+  O.Header.SizeOfCmds = SizeOfCmds;
+
+  // Lay out sections.
+  for (auto &LC : O.LoadCommands) {
+    uint64_t FileOff = Offset;
+    uint64_t VMSize = 0;
+    uint64_t FileOffsetInSegment = 0;
+    for (auto &Sec : LC.Sections) {
+      if (!Sec.isVirtualSection()) {
+        auto FilePaddingSize =
+            OffsetToAlignment(FileOffsetInSegment, 1 << Sec.Align);
+        Sec.Offset = Offset + FileOffsetInSegment + FilePaddingSize;
+        Sec.Size = Sec.Content.size();
+        FileOffsetInSegment += FilePaddingSize + Sec.Size;
+      }
+
+      VMSize = std::max(VMSize, Sec.Addr + Sec.Size);
+    }
+
+    // TODO: Handle the __PAGEZERO segment.
+    auto &MLC = LC.MachOLoadCommand;
+    switch (MLC.load_command_data.cmd) {
+    case MachO::LC_SEGMENT:
+      MLC.segment_command_data.cmdsize =
+          sizeof(MachO::segment_command) +
+          sizeof(MachO::section) * LC.Sections.size();
+      MLC.segment_command_data.nsects = LC.Sections.size();
+      MLC.segment_command_data.fileoff = FileOff;
+      MLC.segment_command_data.vmsize = VMSize;
+      MLC.segment_command_data.filesize = FileOffsetInSegment;
+      break;
+    case MachO::LC_SEGMENT_64:
+      MLC.segment_command_64_data.cmdsize =
+          sizeof(MachO::segment_command_64) +
+          sizeof(MachO::section_64) * LC.Sections.size();
+      MLC.segment_command_64_data.nsects = LC.Sections.size();
+      MLC.segment_command_64_data.fileoff = FileOff;
+      MLC.segment_command_64_data.vmsize = VMSize;
+      MLC.segment_command_64_data.filesize = FileOffsetInSegment;
+      break;
+    }
+
+    Offset += FileOffsetInSegment;
+  }
+
+  // Lay out relocations.
+  for (auto &LC : O.LoadCommands)
+    for (auto &Sec : LC.Sections) {
+      Sec.RelOff = Sec.Relocations.empty() ? 0 : Offset;
+      Sec.NReloc = Sec.Relocations.size();
+      Offset += sizeof(MachO::any_relocation_info) * Sec.NReloc;
+    }
+
+  // Lay out tail stuff.
+  auto NListSize = Is64Bit ? sizeof(MachO::nlist_64) : sizeof(MachO::nlist);
+  for (auto &LC : O.LoadCommands) {
+    auto &MLC = LC.MachOLoadCommand;
+    auto cmd = MLC.load_command_data.cmd;
+    switch (cmd) {
+    case MachO::LC_SYMTAB:
+      MLC.symtab_command_data.symoff = Offset;
+      MLC.symtab_command_data.nsyms = O.SymTable.NameList.size();
+      Offset += NListSize * MLC.symtab_command_data.nsyms;
+      MLC.symtab_command_data.stroff = Offset;
+      Offset += MLC.symtab_command_data.strsize;
+      break;
+    case MachO::LC_DYSYMTAB: {
+      if (MLC.dysymtab_command_data.ntoc != 0 ||
+          MLC.dysymtab_command_data.nmodtab != 0 ||
+          MLC.dysymtab_command_data.nextrefsyms != 0 ||
+          MLC.dysymtab_command_data.nlocrel != 0 ||
+          MLC.dysymtab_command_data.nextrel != 0)
+        return createStringError(llvm::errc::not_supported,
+                                 "shared library is not yet supported");
+
+      if (MLC.dysymtab_command_data.nindirectsyms != 0)
+        return createStringError(llvm::errc::not_supported,
+                                 "indirect symbol table is not yet supported");
+
+      updateDySymTab(MLC);
+      break;
+    }
+    case MachO::LC_SEGMENT:
+    case MachO::LC_SEGMENT_64:
+    case MachO::LC_VERSION_MIN_MACOSX:
+    case MachO::LC_BUILD_VERSION:
+    case MachO::LC_ID_DYLIB:
+    case MachO::LC_LOAD_DYLIB:
+    case MachO::LC_UUID:
+    case MachO::LC_SOURCE_VERSION:
+      // Nothing to update.
+      break;
+    default:
+      // Abort if it's unsupported in order to prevent corrupting the object.
+      return createStringError(llvm::errc::not_supported,
+                               "unsupported load command (cmd=0x%x)", cmd);
+    }
+  }
+
+  return Error::success();
+}
+
+Error MachOWriter::finalize() {
+  updateSizeOfCmds();
+
+  if (auto E = layout())
+    return E;
+
+  return Error::success();
+}
+
 Error MachOWriter::write() {
   if (Error E = B.allocate(totalSize()))
     return E;
   memset(B.getBufferStart(), 0, totalSize());
   writeHeader();
   writeLoadCommands();
   writeSections();
   writeTail();
   return B.commit();
 }
 
 } // end namespace macho
 } // end namespace objcopy
 } // end namespace llvm

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

struct MachHeader {
   uint32_t FileType;
   uint32_t NCmds;
   uint32_t SizeOfCmds;
   uint32_t Flags;
   uint32_t Reserved = 0;
 };
 
 struct Section {
-  char Sectname[16];
-  char Segname[16];
+  std::string Sectname;
+  std::string Segname;
   uint64_t Addr;
   uint64_t Size;
   uint32_t Offset;
   uint32_t Align;
   uint32_t RelOff;
   uint32_t NReloc;
   uint32_t Flags;
   uint32_t Reserved1;
   uint32_t Reserved2;
   uint32_t Reserved3;
 
   StringRef Content;
   std::vector<MachO::any_relocation_info> Relocations;
+
+  MachO::SectionType getType() const {
+    return static_cast<MachO::SectionType>(Flags & MachO::SECTION_TYPE);
+  }
+
+  bool isVirtualSection() const {
+    return (getType() == MachO::S_ZEROFILL ||
+            getType() == MachO::S_GB_ZEROFILL ||
+            getType() == MachO::S_THREAD_LOCAL_ZEROFILL);
+  }
 };
 
 struct LoadCommand {
   // The type MachO::macho_load_command is defined in llvm/BinaryFormat/MachO.h
   // and it is a union of all the structs corresponding to various load
   // commands.
   MachO::macho_load_command MachOLoadCommand;