Skip to content

test(relinker): check ADX guest state preservation - #7531

Open
MarcZX10 wants to merge 6 commits into
boykopovar:mainfrom
MarcZX10:codex/adx-state-preservation
Open

MarcZX10 wants to merge 6 commits into
boykopovar:mainfrom
MarcZX10:codex/adx-state-preservation

Conversation

@MarcZX10

Copy link
Copy Markdown
Contributor

What

Adds generated-stub regression checks for the ADCX/ADOX lowering in #7173. For both widths and instructions, the test covers every destination except RSP and every source register including RSP, with four flag images and zero, mixed, and all-ones register patterns. It checks the result, every unrelated guest GPR, the guest RSP, arithmetic flags, and the caller's 128-byte red zone.

This is test-only and depends on #7173. The preservation gap and independent validation matrix were reported by plokmklo in #7157; this follow-up credits that contribution and omits the VEX forms whose validity was not established in that discussion.

Tested

  • Linux x86_64, relinker-only build: ctest -R '^adx_lowering$' passes (1/1), including the new preservation matrix.
  • python3 tools/check_conventions.py --base upstream/main passes.
  • git diff --check passes.
  • The hosted relinker checks for fix(relinker): lower ADCX and ADOX for Rosetta #7173 passed on Linux, macOS, macOS x86_64, and ASan; Windows was still running when this follow-up was prepared.

Checklist

@github-actions

github-actions Bot commented Oct 18, 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 2780 core/relinker/CMakeLists.txt
core/relinker/codegen/src/x86/Amd64OnlyInstructionMatcher.cpp
PR 5021 core/relinker/CMakeLists.txt
docs/dev/debt/relinker.md
PR 5138 core/relinker/CMakeLists.txt
PR 6044 core/relinker/CMakeLists.txt
PR 6204 core/relinker/CMakeLists.txt
PR 6448 core/relinker/CMakeLists.txt
PR 6607 core/relinker/CMakeLists.txt
PR 6618 core/relinker/CMakeLists.txt
PR 6681 core/relinker/CMakeLists.txt
PR 6684 core/relinker/CMakeLists.txt
PR 6697 core/relinker/CMakeLists.txt
docs/dev/debt/relinker.md
PR 6702 core/relinker/CMakeLists.txt
PR 6713 core/relinker/CMakeLists.txt
PR 6802 core/relinker/CMakeLists.txt
PR 6879 core/relinker/CMakeLists.txt
PR 6903 core/relinker/CMakeLists.txt
PR 6977 core/relinker/CMakeLists.txt
docs/dev/debt/relinker.md
PR 7332 core/relinker/CMakeLists.txt
core/relinker/codegen/include/codegen/x86/AdxLowering.hpp
core/relinker/codegen/src/x86/AdxLowering.cpp
core/relinker/codegen/src/x86/Amd64OnlyInstructionMatcher.cpp
core/relinker/codegen/tests/AdxLoweringTests.cpp
docs/dev/debt/relinker.md
PR 7361 core/relinker/CMakeLists.txt
PR 7417 core/relinker/CMakeLists.txt
docs/dev/debt/relinker.md
PR 7450 core/relinker/CMakeLists.txt
docs/dev/debt/relinker.md
PR 7453 core/relinker/CMakeLists.txt
PR 7532 core/relinker/CMakeLists.txt
PR 7651 docs/dev/debt/relinker.md
PR 7660 core/relinker/CMakeLists.txt
docs/dev/debt/relinker.md

@jotitadev

Copy link
Copy Markdown
Contributor

Ran this under Rosetta, head 6d7eddffc merged into main a62c724c3.

Machine: Apple M2 Max, macOS 27.2, Xcode 27 clang, relinker-only Release builds, x86_64 (-DCMAKE_OSX_ARCHITECTURES=x86_64, Xcode's Python 3.9.6, run under Rosetta 2) and native arm64.

  • Full ctest: 189/189 on both. On x86_64, adx_lowering_tests runs the new preservation matrix under Rosetta and passes. On arm64 the execution part is compiled out, as expected.
  • Checked that the matrix adds coverage. I dropped the final mov scratch, [rsp] from EmitAdx's epilogue, so the scratch register is not restored. The new test fails (ADX changed an unrelated guest register or produced the wrong destination). fix(relinker): lower ADCX and ADOX for Rosetta #7173's tests on 010ce9b02 still pass with that change.

The red-zone check looks at the wrong side of RSP. The wrapper writes the sentinels at [rsp]..[rsp+120] after lea rsp, [rsp-128], then saves that RSP as the guest RSP (cell 72). So the 128 bytes it checks sit above the guest RSP, in the wrapper's own frame. The red zone that EmitAdx's 144-byte frame steps over is the 128 bytes below the guest RSP, and nothing checks those. To show it, I shrank the lowering's frame from 144 to 16 bytes (kFrameSize and both leas). The scratch and flags spills then land at guest RSP-16 and RSP-8, inside the red zone, and the test still passes.

A version that checks the bytes below the guest RSP works for me under Rosetta:

  • Load the input flags (push/popf) before the lea.
  • Write the sentinels at [rsp-128+8*i].
  • Right after the lowered body, copy those 16 qwords into RIP-relative cells, saving rax around the copy, before the wrapper pushes anything. Then fill RedZone from the cells.

With that version, the real lowering passes. A 16-byte frame and a 128-byte frame both fail with ADX changed the caller red zone. The dropped scratch restore still fails with the register message. The 19-line diff is below. I only ran it under Rosetta, not on a native Linux or Windows x86-64 host.

Test diff
@@ -149,14 +149,13 @@ public:
         rspToCell(48);
         statePtrToCell(0);
         loadCellTo(0, 256);
+        push(0);
+        bytes.push_back(0x9D);
         bytes.insert(bytes.end(), {0x48, 0x8D, 0xA4, 0x24});
         append32(-128);
         bytes.insert(bytes.end(), {0x48, 0xB8});
         append64(preservationSentinel);
-        for (int slot = 0; slot < 16; ++slot) storeAtStack(0, slot * 8);
-        loadCellTo(0, 256);
-        push(0);
-        bytes.push_back(0x9D);
+        for (int slot = 0; slot < 16; ++slot) storeAtStack(0, -128 + slot * 8);
         rspToCell(72);
@@ -170,6 +169,12 @@ public:
         rspToCell(24);
+        raxToCell(80);
+        for (int slot = 0; slot < 16; ++slot) {
+            stackLoad(0, -128 + slot * 8);
+            raxToCell(96 + slot * 8);
+        }
+        loadCellTo(0, 80);
         rspFromCell(72);
@@ -185,7 +190,7 @@ public:
         for (int slot = 0; slot < 16; ++slot) {
-            stackLoad(0, slot * 8);
+            loadCellTo(0, 96 + slot * 8);
             store(0, (34 + slot) * 8);
         }
@@ -255,6 +260,10 @@ private:
+    void raxToCell(const int slot) {
+        bytes.insert(bytes.end(), {0x48, 0x89, 0x05});
+        append32(static_cast<std::int32_t>(4096 + slot - (bytes.size() + 4)));
+    }

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

@MarcZX10

Copy link
Copy Markdown
Contributor Author

Fixed in f1fcb95f4. The fixture now plants and reads sentinels in the 128 bytes below the guest RSP. It captures those bytes immediately after the lowered instruction and before the wrapper pushes registers, then checks them after restoring the wrapper state. The focused Linux adx_lowering CTest passes (1/1); git diff --check and python3 tools/check_conventions.py --base upstream/main pass.

@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Oct 18, 2026
@MarcZX10

Copy link
Copy Markdown
Contributor Author

Thanks for the independent Rosetta check and the short-frame negative control. The updated head f1fcb95f4 now places and checks the sentinel region below the guest RSP, and the focused Linux test plus Linux/macOS/Rosetta/Windows relinker CI are green. I also see the PR is currently DIRTY against main; this looks like a base update/rebase item before merge.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

build docs needs rebase Conflicts with main, rebase needed relinker

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants