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

[Jez Ng via Phabricator via llvm-commits]

int3 added a comment.

> I am uncertain how to structure a good test.

The output of `llvm-objdump --macho --all-headers` should allow you to verify the ordering of both load commands and segment/sections.



================
Comment at: lld/MachO/SyntheticSections.cpp:396
+          MachO::S_ATTR_PURE_INSTRUCTIONS;
+  align = 4;
   reserved2 = target->stubSize;
----------------
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 :)


================
Comment at: lld/MachO/UnwindInfoSection.cpp:90
     : SyntheticSection(segment_names::text, section_names::unwindInfo) {
-  align = WordSize; // TODO(gkm): make this 4 KiB ?
+  align = 4;
 }
----------------
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).


================
Comment at: lld/test/MachO/symtab.s:13
 # CHECK-NEXT:     Type: Section (0xE)
-# CHECK-NEXT:     Section: __data (0x4)
+# CHECK-NEXT:     Section: __data (0x{{[0-9a-f]}})
 # CHECK-NEXT:     RefType: UndefinedNonLazy (0x0)
----------------
FileCheck does substring matches by default, so we can just skip checking the value at the end

(ditto for the lines below)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94935