Skip to content

test(relinker): execute the AMD64-only conversion stubs on macOS - #6524

Merged
oneandonlydean merged 2 commits into
boykopovar:mainfrom
javierUDP:test/macos-conversion-execution
Oct 17, 2026
Merged

oneandonlydean merged 2 commits into
boykopovar:mainfrom
javierUDP:test/macos-conversion-execution

Conversation

@javierUDP

@javierUDP javierUDP commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

What

Amd64OnlyConverterTests.cpp compiled its execution section for Linux x86-64 only, so the SSE4A, SHA-1, SHA-256, CLZERO and reciprocal stubs were never executed on a macOS x86-64 build, although AndnLoweringTests.cpp already builds its execution path on __APPLE__ and the Rosetta conversion limits record that the other conversions were checked on a Mac outside this repository. The section now builds on macOS x86-64, where it runs under Rosetta.

The CLZERO case with an address-size override needs a buffer below 4 GiB. A 64-bit Mach-O executable reserves the first 4 GiB as __PAGEZERO, so MAP_32BIT has no address to return; the same mmap succeeds when the executable is linked with a smaller zero page. The Apple x86-64 build links this test with a 64 KiB zero page and all four CLZERO cases run, so the mapping failure stays fatal on every platform. The arm64 build keeps the 4 GiB zero page, which it requires to start, and the section is not built there.

Tested

Apple M5, macOS 26.4, AppleClang 21.0.0, relinker-only build of 44df21632:

  • -DCMAKE_OSX_ARCHITECTURES=x86_64, executed under Rosetta: 173/173 ctest pass. amd64_only_converter runs the EXTRQ and INSERTQ register and immediate form stubs, SHA-1, SHA-256, all four CLZERO cases, the reciprocal stubs and the RIP-relative stub bodies.
  • The 67h CLZERO stub case maps its buffer at 0x4632000, below 4 GiB, and the linked image reports __PAGEZERO with vmsize 0x10000.
  • Native arm64: 173/173 ctest pass, __PAGEZERO stays at vmsize 0x100000000, and the section stays out of the build.
  • Control run: adding 1 to the result of extrqReference fails the x86-64 run with "EXTRQ register form stub computed the wrong field", so the section executes instead of being skipped.
  • python3 tools/check_conventions.py --base origin/main passes.

Zero page measurements on this host, matching the review below:

build MAP_32BIT MAP_FIXED at 0x10000000
-arch x86_64, default link ENOMEM ENOMEM
-arch x86_64, -Wl,-pagezero_size,0x10000 0x8c7000 0x10000000
-arch arm64, -Wl,-pagezero_size,0x10000 killed at exec —

No Linux or Windows host was available, so the Linux x86-64 execution path was not re-run; its flags and its mapping requirement are unchanged.

Thanks to @jotitadev for identifying __PAGEZERO as the cause and for the Linux coverage point; both are addressed in d67e7b738.

Checklist

  • Based on current main; no other open PR implements the same functions
  • One topic per PR; follow-ups go in a new PR
  • No comments in code except technical debt
  • Unimplemented paths throw (NotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file: n/a, test-only change
  • No notes, investigation .md files or images added to the repository
  • New third-party code is a submodule built from source: n/a
  • Generic behaviour, not specific to one title
  • Depends on: none
  • Unblocks: none
  • GPUs tested: n/a
  • AI-assisted: yes

The execution part of the AMD64-only converter tests was compiled for
Linux x86-64 only, so the SSE4A, SHA-1, SHA-256, CLZERO and reciprocal
stubs were never run on a macOS x86-64 build, unlike the ANDN lowering
tests. Rosetta maps no memory below 4 GiB, so the CLZERO case that needs
a 32-bit address is left out when that mapping is unavailable.

@jotitadev jotitadev left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Static review on macOS, head 6f100d3f6. Checked with a standalone snippet, not with this PR's test binary.

Machine: Apple M2 Max, macOS 27.2 (26B5091g), Xcode 27 clang.

The missing low mapping comes from __PAGEZERO, not from Rosetta. A 64-bit Mach-O executable gets a 4 GiB __PAGEZERO by default, so nothing below 4 GiB is free. Rosetta does map low memory when the executable leaves room for it.

A snippet doing the same mmap(..., MAP_PRIVATE | MAP_ANONYMOUS | MAP_32BIT) and a MAP_FIXED at 0x10000000:

Build MAP_32BIT MAP_FIXED 0x10000000
-arch x86_64 (Rosetta), default link ENOMEM ENOMEM
-arch arm64, default link ENOMEM ENOMEM
-arch x86_64 (Rosetta), -Wl,-pagezero_size,0x10000 0x7d7000 0x10000000
-arch arm64, -Wl,-pagezero_size,0x10000 killed at exec (137) —

Linking only amd64_only_converter_tests with -Wl,-pagezero_size,0x10000 on Apple x86_64 would run the fourth CLZERO case under Rosetta too, so the continue would not be needed on macOS. arm64 requires the 4 GiB zero page, but this section is x86_64-only anyway. Something like:

if(APPLE AND CMAKE_OSX_ARCHITECTURES STREQUAL "x86_64")
    target_link_options(amd64_only_converter_tests PRIVATE "LINKER:-pagezero_size,0x10000")
endif()

Side effect on Linux. if (buffer == nullptr && item.Low) continue; applies on every platform. If MAP_32BIT ever fails on Linux, the low case is now skipped without any message, where it used to fail with "cannot map the CLZERO buffer". Limiting the continue to __APPLE__, or failing on Linux as before, would keep the Linux coverage strict.

The rest of the description matches what I see: MAP_32BIT is defined in the macOS SDK (0x8000), so the section compiles on both architectures.

Written with AI assistance (Claude Code). I ran the check above on my machine.

@github-actions

github-actions Bot commented Oct 15, 2026 •

Copy link
Copy Markdown

Some comparisons could not be completed because a pull request conflicts with the base branch. Review these shared files manually before concluding that there is no overlap.

Pull request Shared files not compared
PR 5021 core/relinker/CMakeLists.txt
PR 6044 core/relinker/CMakeLists.txt
PR 6314 core/relinker/CMakeLists.txt

@javierUDP
javierUDP marked this pull request as draft October 15, 2026 20:10
The CLZERO case that needs a 32-bit address found no mapping because a
64-bit Mach-O executable reserves 4 GiB as `__PAGEZERO`, not because
Rosetta withholds low memory: the same `mmap` succeeds once the
executable is linked with a smaller zero page. The Apple x86-64 build
links the test that way and the case runs with the other three, so the
skip it had is gone and the Linux mapping failure stays fatal. The arm64
build keeps the 4 GiB zero page, which it requires to start.
@javierUDP

Copy link
Copy Markdown
Contributor Author

Confirmed on an M5 running macOS 26.4, AppleClang 21.0.0, with the same snippet:

build MAP_32BIT MAP_FIXED at 0x10000000
-arch x86_64, default link ENOMEM ENOMEM
-arch x86_64, -Wl,-pagezero_size,0x10000 0x8c7000 0x10000000
-arch arm64, -Wl,-pagezero_size,0x10000 killed at exec (137) —

You are right that it is __PAGEZERO and not Rosetta, and the fix removes the second problem as well. d67e7b738 links the test with a 64 KiB zero page on Apple x86-64, so the continue is gone and the mapping failure is fatal again on every platform, as it was before this pull request.

All four CLZERO cases now run on macOS: the 67h CLZERO stub case maps its buffer at 0x4632000 and otool -l reports __PAGEZERO with vmsize 0x10000. The arm64 build keeps vmsize 0x100000000, does not get the option, and still passes 173/173, as does the x86-64 build under Rosetta.

The condition also covers a native Intel Mac, where CMAKE_OSX_ARCHITECTURES is empty and CMAKE_SYSTEM_PROCESSOR is x86_64. I have no such host, so that path is untested.

The description no longer claims that Rosetta withholds low memory. Thanks for catching it.

@javierUDP
javierUDP marked this pull request as ready for review October 15, 2026 20:14

@jotitadev jotitadev left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-checked on macOS, head d67e7b738 merged onto main 82ddae75b.

Machine: Apple M2 Max, macOS 27.2 (26B5091g), Xcode 27 clang, relinker-only Release builds.

  • x86_64 under Rosetta (-DCMAKE_OSX_ARCHITECTURES=x86_64 -DPython3_EXECUTABLE=/usr/bin/python3): amd64_only_converter passes; otool -l reports __PAGEZERO vmsize 0x10000 for amd64_only_converter_tests. Full ctest 170/173, the same 3 Python 3.9 failures as main (pr_overlap_tools, release_package, linux_fixed_base), none related.
  • Control: adding 1 to extrqReference makes the x86_64 binary fail with "EXTRQ register form stub computed the wrong field", so the execution section runs here too.
  • arm64: 173/173, __PAGEZERO stays at vmsize 0x100000000.
  • Merged together with #6111 (which adds rdtscpExecution to the same file): merges cleanly and amd64_only_converter still passes on x86_64 under Rosetta.

The zero page change resolves what I reported earlier.

Written with AI assistance (Claude Code). I ran the builds and tests above on my machine.

@oneandonlydean oneandonlydean self-assigned this Oct 17, 2026
@oneandonlydean

Copy link
Copy Markdown
Collaborator

Thanks, this looks good. The branch is about 600 commits behind main, so please rebase onto current main and we'll run full CI (the macos-15-intel job is the one that proves the change).

@oneandonlydean

Copy link
Copy Markdown
Collaborator

Correction to my earlier comment: no rebase is needed (our CI merges the PR into current main before building), and macos-15-intel isn't part of the proof, because no CI lane runs the relinker tests on Intel macOS. Full CI is green on this head. The arm64 macOS job shows the CMake gate keeps -pagezero_size off arm64, and your Rosetta run (x86_64, 173/173) covers the x86-64 execution. Nothing more needed from you.

@oneandonlydean
oneandonlydean merged commit 1846719 into boykopovar:main Oct 17, 2026
2 checks passed
ycarogabriel-lgtm added a commit to ycarogabriel-lgtm/AnyPS5 that referenced this pull request Oct 18, 2026
…ute the AMD64-only conversion stubs on macOS (@javierUDP)

Taken from the open upstream pull request so the macOS integration branch carries every piece of the port that is already under review, instead of re-implementing it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants