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

[Greg McGary via Phabricator via llvm-commits]

gkm marked 2 inline comments as done.
gkm added inline comments.
Herald added a reviewer: int3.


================
Comment at: lld/MachO/SyntheticSections.cpp:396
+          MachO::S_ATTR_PURE_INSTRUCTIONS;
+  align = 4;
   reserved2 = target->stubSize;
----------------
int3 wrote:
> gkm wrote:
> > 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.
> Maybe naive question: So this means arm64/x86_64 instructions are only 4-byte-aligned?
> 
> In any case, this still needs a comment :)
Yes. arm64 instructions are fixed-width 4 bytes, with natural 4-byte alignment. x86_64 instructions are variable width and have no alignment requirement, but ld64 aligns them to 4 bytes anyway.


================
Comment at: lld/MachO/UnwindInfoSection.cpp:90
     : SyntheticSection(segment_names::text, section_names::unwindInfo) {
-  align = WordSize; // TODO(gkm): make this 4 KiB ?
+  align = 4;
 }
----------------
int3 wrote:
> gkm wrote:
> > 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?
> Yeah it's worth mentioning IMO to explain why we are not using a larger value. Is this also the minimum alignment, i.e. do we get EBADMACHO if a smaller alignment is used? That's worth mentioning too...
> 
> Relatedly, I'm a bit confused by the commit title/message. The commit title mentions codesign, the message mentions dyld first, but later talks about execve and EBADMACHO, which leads me to believe that the error is coming from the kernel. (At least, it looks like EBADMACHO is generated here: https://github.com/apple/darwin-xnu/blob/8f02f2a044b9bb1ad951987ef5bab20ec9486310/bsd/kern/kern_exec.c#L6478).
Regarding whether we get `EBADMACHO` at smaller alignment: unknown. I corrected alignments to mimic ld64 until codesign & execve(2) began working and never experimented with underaligning anything to see if it still worked. That never seemed a worthy use of time.

I clarified the commit summary: codesign & kernel execve(2) failures are what guided my changes. dyld(1) is likely also picky, but never gets a chance to run until codesign and kernel are happy.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94935