[PATCH] D65539: [llvm-objcopy][MachO] Implement a layout algorithm for executables

[Alexander Shaposhnikov via Phabricator via llvm-commits]

alexshap added a comment.

in general i like the approach, i have added a few minor comments and one question (maybe I'm missing smth).



================
Comment at: llvm/tools/llvm-objcopy/MachO/MachOLayoutBuilder.cpp:20
   for (const auto &LC : O.LoadCommands) {
     auto &MLC = LC.MachOLoadCommand;
     auto cmd = MLC.load_command_data.cmd;
----------------
const auto &MLC


================
Comment at: llvm/tools/llvm-objcopy/MachO/MachOLayoutBuilder.cpp:104
       Is64Bit ? sizeof(MachO::mach_header_64) : sizeof(MachO::mach_header);
-  auto Offset = HeaderSize + O.Header.SizeOfCmds;
-
-  // Lay out sections.
+  bool IsObjectFile = O.Header.FileType == MachO::HeaderFileType::MH_OBJECT;
+  uint64_t Offset = IsObjectFile ? (HeaderSize + O.Header.SizeOfCmds) : 0;
----------------
const bool


================
Comment at: llvm/tools/llvm-objcopy/MachO/MachOLayoutBuilder.cpp:151
+        }
+        VMSize = std::max(VMSize, Sec.Addr + Sec.Size);
+      } else {
----------------
khm, maybe I'm missing smth, but why do we have Sec.Addr + Sec.Size here ?



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65539/new/

https://reviews.llvm.org/D65539