[PATCH] D94935: [lld-macho] Fix alignment & layout to match ld64 and satisfy codesign

[Greg McGary via Phabricator via llvm-commits]

gkm added a comment.

@compnerd: I am uncertain how to structure a good test. A good test for satisfactory load-command order is already implicit: lld-linked programs don't run on ARM64 macOS otherwise.



================
Comment at: lld/MachO/SyntheticSections.cpp:396
+          MachO::S_ATTR_PURE_INSTRUCTIONS;
+  align = 4;
   reserved2 = target->stubSize;
----------------
compnerd wrote:
> Hmm, this is `4` and not `WordSize`?  It is plausible, but might be reasonable to add a comment explaining why `4` on 64-bit systems is needed/correct.
`__stubs` and `__stub_helper` are text sections populated with machine instruction, so 4-byte alignment is appropriate.


================
Comment at: lld/MachO/Target.h:30
   WordSize = 8,
-  PageSize = 4096,
+  PageSize = 0x4000,         // 16 KiB
   PageZeroSize = 1ull << 32, // XXX should be 4096 for 32-bit targets
----------------
compnerd wrote:
> Isn't the page size 4K on x64 and 16K on ARM64?
This belongs in the ARM64 target diff.


================
Comment at: lld/MachO/UnwindInfoSection.cpp:90
     : SyntheticSection(segment_names::text, section_names::unwindInfo) {
-  align = WordSize; // TODO(gkm): make this 4 KiB ?
+  align = 4;
 }
----------------
compnerd wrote:
> IIRC, 4 is correct for the `__unwind_info` section, but this probably deserves a comment (unless Im mixing up the `__compact_unwind_entries`).
In all cases, I choose alignment boundaries to mimic ld64. Do you want comment(s) to say that?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94935