[PATCH] D118724: [lld-macho] Add -pagezero_size

[Jez Ng via Phabricator via llvm-commits]

int3 added inline comments.


================
Comment at: lld/MachO/Driver.cpp:641-642
+
+    // __PAGEZERO should be aligned to 4KB, not to the page alignment like ld64
+    // says.
+    if ((pagezeroSize % 0x1000) != 0) {
----------------
Hm, this seems like an ld64 implementation error. Personally I think it makes more sense to round it to the page size...


================
Comment at: lld/MachO/Driver.cpp:643
+    // says.
+    if ((pagezeroSize % 0x1000) != 0) {
+      warn("__PAGEZERO size is misaligned, rounding down");
----------------
nit: would be nice to use the `isAligned` helper for a bit more explicitness -- `isAligned(Align(target->getPageSize()), pagezeroSize)`


================
Comment at: lld/test/MachO/pagesize-zero.s:1
+# REQUIRES: x86
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %s -o %t.o
----------------
oontvoo wrote:
> could you group all three of these files into the one test file?
> 
+1


================
Comment at: lld/test/MachO/pagezero-misaligned.s:3
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %s -o %t.o
+# RUN: %no_fatal_warnings_lld -lSystem -arch x86_64 -o %t %t.o -pagezero_size 100001 2>&1 | FileCheck %s --check-prefix=LINK
+
----------------
we should test the 32-bit case here too (regardless of whether we use 4KiB or the actual page size to round down).

the only 32-bit target we properly support is arm64_32, so that's what I would use here. (You can use `%lld-watchos` instead of `%lld` to link for that target.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118724