Save all volatile argument registers in GC probe hijack frames on AMD64 and ARM64 - #126848

Merged
MichalStrehovsky merged 16 commits into
mainfrom
copilot/fix-fast-tail-call-candidates-test
Apr 17, 2026
Merged

Save all volatile argument registers in GC probe hijack frames on AMD64 and ARM64#126848
MichalStrehovsky merged 16 commits into
mainfrom
copilot/fix-fast-tail-call-candidates-test

Conversation

CopilotAI commented Apr 13, 2026

Copy link
Copy Markdown
Contributor
  • Save volatile argument registers (RDX/R8/R9) in Windows AMD64 GC probe hijack frames
  • Save volatile argument registers (RSI/RDI/R8/R9) in Unix AMD64 GC probe hijack frames
  • Use CFI-aware push_register/pop_register macros for proper unwind directives
  • Save volatile argument registers (x3-x7) in ARM64 GC probe hijack frames
  • Update RBM_INTERFACELOOKUP_FOR_SLOT_TRASH to trash all non-argument volatile registers
  • Save all FP argument registers in GC probe hijack frames
  • Merge main and apply targeted JIT PHI jump threading fix for JIT: fix issue in ssa-aware jump threading #126976
  • Add project exclusions for arm64 Windows tests
  • Fix JITDUMP formatting in redundantbranchopts.cpp for jitformat checker

… frames
On AMD64, when the GC hijacks a thread via RhpGcProbeHijack, the hijack stub
creates a PInvokeTransitionFrame. Previously on Windows, only RAX and RCX were
saved, and on Unix only RAX, RCX, and RDX were saved. If the GC info for the
managed frame at the hijack point reports other volatile registers (RDX, R8, R9)
as live GC references, the StackFrameIterator would find NULL save locations
and crash (AV in SVR::GCHeap::Promote).
Windows changes:
- FixupHijackedCallstack now uses R10/R11 for thread pointer instead of RDX/R8,
preserving all volatile argument registers (RAX, RCX, RDX, R8, R9)
- PUSH_PROBE_FRAME/POP_PROBE_FRAME extended to save/restore RDX, R8, R9
- RhpGcProbeHijack flags include PTFF_SAVE_RDX + PTFF_SAVE_R8 + PTFF_SAVE_R9
- AsmMacros.inc gains PTFF_SAVE_RDX, PTFF_SAVE_R8, PTFF_SAVE_R9 definitions
Unix changes:
- FixupHijackedCallstack now also saves/restores R8 and R9 across GETTHREAD
- Uses R10 instead of R8 for scratch in hijack fixup and bitmask passing
- PUSH_PROBE_FRAME/POP_PROBE_FRAME extended to save/restore R8, R9
- RhpGcProbeHijack flags include PTFF_SAVE_R8 + PTFF_SAVE_R9
- unixasmmacrosamd64.inc gains PTFF_SAVE_R8, PTFF_SAVE_R9 definitions
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/411be3e7-43a1-4bc3-b896-e27d92fe37c3
Co-authored-by: mangod9 <61718172+mangod9@users.noreply.github.com>
CopilotAI requested review from Copilot and removed request for CopilotApril 13, 2026 19:18
CopilotAI requested review from Copilot and removed request for CopilotApril 13, 2026 19:25
CopilotAI changed the title [WIP] Fix test failure for FastTailCallCandidatesSave all volatile argument registers (RDX, R8, R9) in GC probe hijack frames on AMD64Apr 13, 2026
CopilotAI requested a review from mangod9April 13, 2026 19:26
Comment threadsrc/coreclr/nativeaot/Runtime/amd64/GcProbe.S
On Unix System V ABI, RSI and RDI are volatile argument-passing registers
(1st and 2nd integer args) that could contain live GC references at hijack
points. On Windows, they are already saved as callee-saved registers in
PTFF_SAVE_ALL_PRESERVED.
Unix changes:
- FixupHijackedCallstack now saves/restores RSI and RDI across GETTHREAD
- PUSH_PROBE_FRAME saves RSI and RDI between R12 and RBX, matching the
StackFrameIterator's flag processing order (RBX, RSI, RDI, R12...)
- POP_PROBE_FRAME restores RSI and RDI in correct order
- RhpGcProbeHijack flags include PTFF_SAVE_RSI + PTFF_SAVE_RDI
- unixasmmacrosamd64.inc gains PTFF_SAVE_RSI and PTFF_SAVE_RDI definitions
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/85ca9ec1-057b-4a3c-8182-e6da274d44b6
Co-authored-by: mangod9 <61718172+mangod9@users.noreply.github.com>
CopilotAI requested review from Copilot and removed request for CopilotApril 13, 2026 21:27
CopilotAI changed the title Save all volatile argument registers (RDX, R8, R9) in GC probe hijack frames on AMD64Save all volatile argument registers in GC probe hijack frames on AMD64Apr 13, 2026
@jkotas
jkotas marked this pull request as ready for review April 13, 2026 21:43
CopilotAI review requested due to automatic review settings April 13, 2026 21:43

CopilotAI commented Apr 16, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot cherry picking is going to cause conflicts. You need to merge main into this PR and then apply the fix on top of that to keep things clean.

Reverted the cherry-pick, merged main, and applied the targeted fix on top in 7442a6d. The HasGlobalUse() path now returns CannotThread immediately, matching the diff from the comment.

@mangod9

Copy link
Copy Markdown
Member

/azp run runtime-nativeaot-outerloop

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@jkotasjkotas left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM once CI is green

@mangod9mangod9 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good, the outerloop failures look similar to main, there are more failures around pulling images. :

  • linux_musl-x64: Docker image pull failure on Helix (alpine-edge-helix-amd64) — infrastructure issue
  • windows-arm64: Also fails on main (build 1378947, Apr 14) — pre-existing

Added project exclusions for LibraryImportGenerator and ComInterfaceGenerator tests on arm64 Windows.
CopilotAI review requested due to automatic review settings April 16, 2026 07:10
@MichalStrehovsky

Copy link
Copy Markdown
Member

windows-arm64: Also fails on main (build 1378947, Apr 14) — pre-existing

I've added an exclusion for this, this is taking longer to fix than I'd like and we probably want to run all testing on this PR that we can.

@MichalStrehovsky

Copy link
Copy Markdown
Member

/azp run runtime-nativeaot-outerloop

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

CopilotAI 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.

Pull request overview

Fixes NativeAOT GC probe hijack correctness by ensuring volatile argument registers (and relevant FP argument registers) are preserved in hijack/probe frames on AMD64 and ARM64, preventing GC stack walking from observing missing/NULL save locations when GC info reports registers as live. Also updates JIT helper kill sets for interface lookup helpers and applies a targeted JIT jump-threading safety bailout for a pre-existing regression.

Changes:

  • NativeAOT: Expand GC probe hijack frame saving/restoring to include volatile argument registers (and more FP arg regs) on AMD64/ARM64; adjust hijack stubs to avoid clobbering argument registers.
  • JIT: Update RBM_INTERFACELOOKUP_FOR_SLOT_TRASH on AMD64/ARM64 to conservatively model all non-argument volatile regs as trashed.
  • JIT: Disable PHI-based jump threading when SSA defs have global uses (targeted workaround for #126976).

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.

Show a summary per file
FileDescription
src/libraries/tests.projExcludes specific NativeAOT Windows ARM64 test projects per linked issue.
src/coreclr/nativeaot/Runtime/unix/unixasmmacrosarm64.incAdds PTFF flag definitions for saving x3–x7 on Unix ARM64.
src/coreclr/nativeaot/Runtime/unix/unixasmmacrosamd64.incAdds PTFF flag definitions for saving RSI/RDI/R8/R9 on Unix AMD64.
src/coreclr/nativeaot/Runtime/arm64/GcProbe.asmUpdates Windows ARM64 probe frame layout and hijack paths to preserve x0–x7 and q0–q7; avoids clobbering arg regs.
src/coreclr/nativeaot/Runtime/arm64/GcProbe.SUpdates Unix ARM64 probe frame layout and hijack paths to preserve x0–x7 and q0–q7; adjusts FixupHijackedCallstack.
src/coreclr/nativeaot/Runtime/amd64/GcProbe.asmUpdates Windows AMD64 probe frame to preserve RDX/R8/R9 and xmm0–xmm3; avoids clobbering volatile arg regs in hijack fixup.
src/coreclr/nativeaot/Runtime/amd64/GcProbe.SUpdates Unix AMD64 probe frame to preserve RSI/RDI/RDX/R8/R9 and xmm0–xmm7; adds CFI-aware save/restore around INLINE_GETTHREAD.
src/coreclr/nativeaot/Runtime/amd64/AsmMacros.incAdds PTFF flag definitions for saving RDX/R8/R9 on Windows AMD64.
src/coreclr/jit/targetarm64.hBroadens interface lookup helper trash set to all non-argument volatile regs.
src/coreclr/jit/targetamd64.hBroadens interface lookup helper trash set to all non-argument volatile regs (incl. FP/mask as applicable).
src/coreclr/jit/redundantbranchopts.cppBail out immediately from PHI-based jump threading when SSA defs have global uses.

Comment threadsrc/libraries/tests.proj
Comment threadsrc/coreclr/nativeaot/Runtime/arm64/GcProbe.S
Comment threadsrc/coreclr/jit/redundantbranchopts.cpp
Comment threadsrc/coreclr/nativeaot/Runtime/arm64/GcProbe.S
Comment threadsrc/coreclr/jit/redundantbranchopts.cpp Outdated
@mangod9

Copy link
Copy Markdown
Member

Overall this is now looking good, one Build Analysis failure which is not a known issue -- timeout in one of the agents.

@MichalStrehovsky

Copy link
Copy Markdown
Member

/ba-g devops timed out while waiting for helix but inspecting https://helix.dot.net/api/jobs/5e8abde5-94fb-4abf-9b8a-f7cbb124f091/workitems/nativeaot/files/console.0c33c2fb.log?api-version=2019-06-17 shows all test eventually passed

@MichalStrehovsky
MichalStrehovsky merged commit 31f66d2 into mainApr 17, 2026
148 of 160 checks passed
@MichalStrehovsky
MichalStrehovsky deleted the copilot/fix-fast-tail-call-candidates-test branch April 17, 2026 00:54
@github-actionsgithub-actionsBot locked and limited conversation to collaborators May 17, 2026
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Test failure: JIT/opt/FastTailCall/FastTailCallCandidates/FastTailCallCandidates.cmd

8 participants

@jkotas@VSadov@mangod9@jakobbotsch@MichalStrehovsky@AndyAyersMS
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

Save all volatile argument registers in GC probe hijack frames on AMD64 and ARM64 - #126848

Merged
MichalStrehovsky merged 16 commits into
mainfrom
copilot/fix-fast-tail-call-candidates-test
Apr 17, 2026
Merged

Save all volatile argument registers in GC probe hijack frames on AMD64 and ARM64#126848
MichalStrehovsky merged 16 commits into
mainfrom
copilot/fix-fast-tail-call-candidates-test

Conversation

CopilotAI commented Apr 13, 2026

Copy link
Copy Markdown
Contributor
  • Save volatile argument registers (RDX/R8/R9) in Windows AMD64 GC probe hijack frames
  • Save volatile argument registers (RSI/RDI/R8/R9) in Unix AMD64 GC probe hijack frames
  • Use CFI-aware push_register/pop_register macros for proper unwind directives
  • Save volatile argument registers (x3-x7) in ARM64 GC probe hijack frames
  • Update RBM_INTERFACELOOKUP_FOR_SLOT_TRASH to trash all non-argument volatile registers
  • Save all FP argument registers in GC probe hijack frames
  • Merge main and apply targeted JIT PHI jump threading fix for JIT: fix issue in ssa-aware jump threading #126976
  • Add project exclusions for arm64 Windows tests
  • Fix JITDUMP formatting in redundantbranchopts.cpp for jitformat checker

… frames
On AMD64, when the GC hijacks a thread via RhpGcProbeHijack, the hijack stub
creates a PInvokeTransitionFrame. Previously on Windows, only RAX and RCX were
saved, and on Unix only RAX, RCX, and RDX were saved. If the GC info for the
managed frame at the hijack point reports other volatile registers (RDX, R8, R9)
as live GC references, the StackFrameIterator would find NULL save locations
and crash (AV in SVR::GCHeap::Promote).
Windows changes:
- FixupHijackedCallstack now uses R10/R11 for thread pointer instead of RDX/R8,
preserving all volatile argument registers (RAX, RCX, RDX, R8, R9)
- PUSH_PROBE_FRAME/POP_PROBE_FRAME extended to save/restore RDX, R8, R9
- RhpGcProbeHijack flags include PTFF_SAVE_RDX + PTFF_SAVE_R8 + PTFF_SAVE_R9
- AsmMacros.inc gains PTFF_SAVE_RDX, PTFF_SAVE_R8, PTFF_SAVE_R9 definitions
Unix changes:
- FixupHijackedCallstack now also saves/restores R8 and R9 across GETTHREAD
- Uses R10 instead of R8 for scratch in hijack fixup and bitmask passing
- PUSH_PROBE_FRAME/POP_PROBE_FRAME extended to save/restore R8, R9
- RhpGcProbeHijack flags include PTFF_SAVE_R8 + PTFF_SAVE_R9
- unixasmmacrosamd64.inc gains PTFF_SAVE_R8, PTFF_SAVE_R9 definitions
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/411be3e7-43a1-4bc3-b896-e27d92fe37c3
Co-authored-by: mangod9 <61718172+mangod9@users.noreply.github.com>
CopilotAI requested review from Copilot and removed request for CopilotApril 13, 2026 19:18
CopilotAI requested review from Copilot and removed request for CopilotApril 13, 2026 19:25
CopilotAI changed the title [WIP] Fix test failure for FastTailCallCandidatesSave all volatile argument registers (RDX, R8, R9) in GC probe hijack frames on AMD64Apr 13, 2026
CopilotAI requested a review from mangod9April 13, 2026 19:26
Comment threadsrc/coreclr/nativeaot/Runtime/amd64/GcProbe.S
On Unix System V ABI, RSI and RDI are volatile argument-passing registers
(1st and 2nd integer args) that could contain live GC references at hijack
points. On Windows, they are already saved as callee-saved registers in
PTFF_SAVE_ALL_PRESERVED.
Unix changes:
- FixupHijackedCallstack now saves/restores RSI and RDI across GETTHREAD
- PUSH_PROBE_FRAME saves RSI and RDI between R12 and RBX, matching the
StackFrameIterator's flag processing order (RBX, RSI, RDI, R12...)
- POP_PROBE_FRAME restores RSI and RDI in correct order
- RhpGcProbeHijack flags include PTFF_SAVE_RSI + PTFF_SAVE_RDI
- unixasmmacrosamd64.inc gains PTFF_SAVE_RSI and PTFF_SAVE_RDI definitions
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/85ca9ec1-057b-4a3c-8182-e6da274d44b6
Co-authored-by: mangod9 <61718172+mangod9@users.noreply.github.com>
CopilotAI requested review from Copilot and removed request for CopilotApril 13, 2026 21:27
CopilotAI changed the title Save all volatile argument registers (RDX, R8, R9) in GC probe hijack frames on AMD64Save all volatile argument registers in GC probe hijack frames on AMD64Apr 13, 2026
@jkotas
jkotas marked this pull request as ready for review April 13, 2026 21:43
CopilotAI review requested due to automatic review settings April 13, 2026 21:43

CopilotAI commented Apr 16, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot cherry picking is going to cause conflicts. You need to merge main into this PR and then apply the fix on top of that to keep things clean.

Reverted the cherry-pick, merged main, and applied the targeted fix on top in 7442a6d. The HasGlobalUse() path now returns CannotThread immediately, matching the diff from the comment.

@mangod9

Copy link
Copy Markdown
Member

/azp run runtime-nativeaot-outerloop

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@jkotasjkotas left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM once CI is green

@mangod9mangod9 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good, the outerloop failures look similar to main, there are more failures around pulling images. :

  • linux_musl-x64: Docker image pull failure on Helix (alpine-edge-helix-amd64) — infrastructure issue
  • windows-arm64: Also fails on main (build 1378947, Apr 14) — pre-existing

Added project exclusions for LibraryImportGenerator and ComInterfaceGenerator tests on arm64 Windows.
CopilotAI review requested due to automatic review settings April 16, 2026 07:10
@MichalStrehovsky

Copy link
Copy Markdown
Member

windows-arm64: Also fails on main (build 1378947, Apr 14) — pre-existing

I've added an exclusion for this, this is taking longer to fix than I'd like and we probably want to run all testing on this PR that we can.

@MichalStrehovsky

Copy link
Copy Markdown
Member

/azp run runtime-nativeaot-outerloop

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

CopilotAI 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.

Pull request overview

Fixes NativeAOT GC probe hijack correctness by ensuring volatile argument registers (and relevant FP argument registers) are preserved in hijack/probe frames on AMD64 and ARM64, preventing GC stack walking from observing missing/NULL save locations when GC info reports registers as live. Also updates JIT helper kill sets for interface lookup helpers and applies a targeted JIT jump-threading safety bailout for a pre-existing regression.

Changes:

  • NativeAOT: Expand GC probe hijack frame saving/restoring to include volatile argument registers (and more FP arg regs) on AMD64/ARM64; adjust hijack stubs to avoid clobbering argument registers.
  • JIT: Update RBM_INTERFACELOOKUP_FOR_SLOT_TRASH on AMD64/ARM64 to conservatively model all non-argument volatile regs as trashed.
  • JIT: Disable PHI-based jump threading when SSA defs have global uses (targeted workaround for #126976).

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.

Show a summary per file
FileDescription
src/libraries/tests.projExcludes specific NativeAOT Windows ARM64 test projects per linked issue.
src/coreclr/nativeaot/Runtime/unix/unixasmmacrosarm64.incAdds PTFF flag definitions for saving x3–x7 on Unix ARM64.
src/coreclr/nativeaot/Runtime/unix/unixasmmacrosamd64.incAdds PTFF flag definitions for saving RSI/RDI/R8/R9 on Unix AMD64.
src/coreclr/nativeaot/Runtime/arm64/GcProbe.asmUpdates Windows ARM64 probe frame layout and hijack paths to preserve x0–x7 and q0–q7; avoids clobbering arg regs.
src/coreclr/nativeaot/Runtime/arm64/GcProbe.SUpdates Unix ARM64 probe frame layout and hijack paths to preserve x0–x7 and q0–q7; adjusts FixupHijackedCallstack.
src/coreclr/nativeaot/Runtime/amd64/GcProbe.asmUpdates Windows AMD64 probe frame to preserve RDX/R8/R9 and xmm0–xmm3; avoids clobbering volatile arg regs in hijack fixup.
src/coreclr/nativeaot/Runtime/amd64/GcProbe.SUpdates Unix AMD64 probe frame to preserve RSI/RDI/RDX/R8/R9 and xmm0–xmm7; adds CFI-aware save/restore around INLINE_GETTHREAD.
src/coreclr/nativeaot/Runtime/amd64/AsmMacros.incAdds PTFF flag definitions for saving RDX/R8/R9 on Windows AMD64.
src/coreclr/jit/targetarm64.hBroadens interface lookup helper trash set to all non-argument volatile regs.
src/coreclr/jit/targetamd64.hBroadens interface lookup helper trash set to all non-argument volatile regs (incl. FP/mask as applicable).
src/coreclr/jit/redundantbranchopts.cppBail out immediately from PHI-based jump threading when SSA defs have global uses.

Comment threadsrc/libraries/tests.proj
Comment threadsrc/coreclr/nativeaot/Runtime/arm64/GcProbe.S
Comment threadsrc/coreclr/jit/redundantbranchopts.cpp
Comment threadsrc/coreclr/nativeaot/Runtime/arm64/GcProbe.S
Comment threadsrc/coreclr/jit/redundantbranchopts.cpp Outdated
@mangod9

Copy link
Copy Markdown
Member

Overall this is now looking good, one Build Analysis failure which is not a known issue -- timeout in one of the agents.

@MichalStrehovsky

Copy link
Copy Markdown
Member

/ba-g devops timed out while waiting for helix but inspecting https://helix.dot.net/api/jobs/5e8abde5-94fb-4abf-9b8a-f7cbb124f091/workitems/nativeaot/files/console.0c33c2fb.log?api-version=2019-06-17 shows all test eventually passed

@MichalStrehovsky
MichalStrehovsky merged commit 31f66d2 into mainApr 17, 2026
148 of 160 checks passed
@MichalStrehovsky
MichalStrehovsky deleted the copilot/fix-fast-tail-call-candidates-test branch April 17, 2026 00:54
@github-actionsgithub-actionsBot locked and limited conversation to collaborators May 17, 2026
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Test failure: JIT/opt/FastTailCall/FastTailCallCandidates/FastTailCallCandidates.cmd

8 participants

@jkotas@VSadov@mangod9@jakobbotsch@MichalStrehovsky@AndyAyersMS
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Save all volatile argument registers in GC probe hijack frames on AMD64 and ARM64 - #126848

Merged
MichalStrehovsky merged 16 commits into
mainfrom
copilot/fix-fast-tail-call-candidates-test
Apr 17, 2026
Merged

Save all volatile argument registers in GC probe hijack frames on AMD64 and ARM64#126848
MichalStrehovsky merged 16 commits into
mainfrom
copilot/fix-fast-tail-call-candidates-test

Conversation

CopilotAI commented Apr 13, 2026

Copy link
Copy Markdown
Contributor
  • Save volatile argument registers (RDX/R8/R9) in Windows AMD64 GC probe hijack frames
  • Save volatile argument registers (RSI/RDI/R8/R9) in Unix AMD64 GC probe hijack frames
  • Use CFI-aware push_register/pop_register macros for proper unwind directives
  • Save volatile argument registers (x3-x7) in ARM64 GC probe hijack frames
  • Update RBM_INTERFACELOOKUP_FOR_SLOT_TRASH to trash all non-argument volatile registers
  • Save all FP argument registers in GC probe hijack frames
  • Merge main and apply targeted JIT PHI jump threading fix for JIT: fix issue in ssa-aware jump threading #126976
  • Add project exclusions for arm64 Windows tests
  • Fix JITDUMP formatting in redundantbranchopts.cpp for jitformat checker

… frames
On AMD64, when the GC hijacks a thread via RhpGcProbeHijack, the hijack stub
creates a PInvokeTransitionFrame. Previously on Windows, only RAX and RCX were
saved, and on Unix only RAX, RCX, and RDX were saved. If the GC info for the
managed frame at the hijack point reports other volatile registers (RDX, R8, R9)
as live GC references, the StackFrameIterator would find NULL save locations
and crash (AV in SVR::GCHeap::Promote).
Windows changes:
- FixupHijackedCallstack now uses R10/R11 for thread pointer instead of RDX/R8,
preserving all volatile argument registers (RAX, RCX, RDX, R8, R9)
- PUSH_PROBE_FRAME/POP_PROBE_FRAME extended to save/restore RDX, R8, R9
- RhpGcProbeHijack flags include PTFF_SAVE_RDX + PTFF_SAVE_R8 + PTFF_SAVE_R9
- AsmMacros.inc gains PTFF_SAVE_RDX, PTFF_SAVE_R8, PTFF_SAVE_R9 definitions
Unix changes:
- FixupHijackedCallstack now also saves/restores R8 and R9 across GETTHREAD
- Uses R10 instead of R8 for scratch in hijack fixup and bitmask passing
- PUSH_PROBE_FRAME/POP_PROBE_FRAME extended to save/restore R8, R9
- RhpGcProbeHijack flags include PTFF_SAVE_R8 + PTFF_SAVE_R9
- unixasmmacrosamd64.inc gains PTFF_SAVE_R8, PTFF_SAVE_R9 definitions
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/411be3e7-43a1-4bc3-b896-e27d92fe37c3
Co-authored-by: mangod9 <61718172+mangod9@users.noreply.github.com>
CopilotAI requested review from Copilot and removed request for CopilotApril 13, 2026 19:18
CopilotAI requested review from Copilot and removed request for CopilotApril 13, 2026 19:25
CopilotAI changed the title [WIP] Fix test failure for FastTailCallCandidatesSave all volatile argument registers (RDX, R8, R9) in GC probe hijack frames on AMD64Apr 13, 2026
CopilotAI requested a review from mangod9April 13, 2026 19:26
Comment threadsrc/coreclr/nativeaot/Runtime/amd64/GcProbe.S
On Unix System V ABI, RSI and RDI are volatile argument-passing registers
(1st and 2nd integer args) that could contain live GC references at hijack
points. On Windows, they are already saved as callee-saved registers in
PTFF_SAVE_ALL_PRESERVED.
Unix changes:
- FixupHijackedCallstack now saves/restores RSI and RDI across GETTHREAD
- PUSH_PROBE_FRAME saves RSI and RDI between R12 and RBX, matching the
StackFrameIterator's flag processing order (RBX, RSI, RDI, R12...)
- POP_PROBE_FRAME restores RSI and RDI in correct order
- RhpGcProbeHijack flags include PTFF_SAVE_RSI + PTFF_SAVE_RDI
- unixasmmacrosamd64.inc gains PTFF_SAVE_RSI and PTFF_SAVE_RDI definitions
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/85ca9ec1-057b-4a3c-8182-e6da274d44b6
Co-authored-by: mangod9 <61718172+mangod9@users.noreply.github.com>
CopilotAI requested review from Copilot and removed request for CopilotApril 13, 2026 21:27
CopilotAI changed the title Save all volatile argument registers (RDX, R8, R9) in GC probe hijack frames on AMD64Save all volatile argument registers in GC probe hijack frames on AMD64Apr 13, 2026
@jkotas
jkotas marked this pull request as ready for review April 13, 2026 21:43
CopilotAI review requested due to automatic review settings April 13, 2026 21:43

CopilotAI commented Apr 16, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot cherry picking is going to cause conflicts. You need to merge main into this PR and then apply the fix on top of that to keep things clean.

Reverted the cherry-pick, merged main, and applied the targeted fix on top in 7442a6d. The HasGlobalUse() path now returns CannotThread immediately, matching the diff from the comment.

@mangod9

Copy link
Copy Markdown
Member

/azp run runtime-nativeaot-outerloop

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@jkotasjkotas left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM once CI is green

@mangod9mangod9 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good, the outerloop failures look similar to main, there are more failures around pulling images. :

  • linux_musl-x64: Docker image pull failure on Helix (alpine-edge-helix-amd64) — infrastructure issue
  • windows-arm64: Also fails on main (build 1378947, Apr 14) — pre-existing

Added project exclusions for LibraryImportGenerator and ComInterfaceGenerator tests on arm64 Windows.
CopilotAI review requested due to automatic review settings April 16, 2026 07:10
@MichalStrehovsky

Copy link
Copy Markdown
Member

windows-arm64: Also fails on main (build 1378947, Apr 14) — pre-existing

I've added an exclusion for this, this is taking longer to fix than I'd like and we probably want to run all testing on this PR that we can.

@MichalStrehovsky

Copy link
Copy Markdown
Member

/azp run runtime-nativeaot-outerloop

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

CopilotAI 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.

Pull request overview

Fixes NativeAOT GC probe hijack correctness by ensuring volatile argument registers (and relevant FP argument registers) are preserved in hijack/probe frames on AMD64 and ARM64, preventing GC stack walking from observing missing/NULL save locations when GC info reports registers as live. Also updates JIT helper kill sets for interface lookup helpers and applies a targeted JIT jump-threading safety bailout for a pre-existing regression.

Changes:

  • NativeAOT: Expand GC probe hijack frame saving/restoring to include volatile argument registers (and more FP arg regs) on AMD64/ARM64; adjust hijack stubs to avoid clobbering argument registers.
  • JIT: Update RBM_INTERFACELOOKUP_FOR_SLOT_TRASH on AMD64/ARM64 to conservatively model all non-argument volatile regs as trashed.
  • JIT: Disable PHI-based jump threading when SSA defs have global uses (targeted workaround for #126976).

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.

Show a summary per file
FileDescription
src/libraries/tests.projExcludes specific NativeAOT Windows ARM64 test projects per linked issue.
src/coreclr/nativeaot/Runtime/unix/unixasmmacrosarm64.incAdds PTFF flag definitions for saving x3–x7 on Unix ARM64.
src/coreclr/nativeaot/Runtime/unix/unixasmmacrosamd64.incAdds PTFF flag definitions for saving RSI/RDI/R8/R9 on Unix AMD64.
src/coreclr/nativeaot/Runtime/arm64/GcProbe.asmUpdates Windows ARM64 probe frame layout and hijack paths to preserve x0–x7 and q0–q7; avoids clobbering arg regs.
src/coreclr/nativeaot/Runtime/arm64/GcProbe.SUpdates Unix ARM64 probe frame layout and hijack paths to preserve x0–x7 and q0–q7; adjusts FixupHijackedCallstack.
src/coreclr/nativeaot/Runtime/amd64/GcProbe.asmUpdates Windows AMD64 probe frame to preserve RDX/R8/R9 and xmm0–xmm3; avoids clobbering volatile arg regs in hijack fixup.
src/coreclr/nativeaot/Runtime/amd64/GcProbe.SUpdates Unix AMD64 probe frame to preserve RSI/RDI/RDX/R8/R9 and xmm0–xmm7; adds CFI-aware save/restore around INLINE_GETTHREAD.
src/coreclr/nativeaot/Runtime/amd64/AsmMacros.incAdds PTFF flag definitions for saving RDX/R8/R9 on Windows AMD64.
src/coreclr/jit/targetarm64.hBroadens interface lookup helper trash set to all non-argument volatile regs.
src/coreclr/jit/targetamd64.hBroadens interface lookup helper trash set to all non-argument volatile regs (incl. FP/mask as applicable).
src/coreclr/jit/redundantbranchopts.cppBail out immediately from PHI-based jump threading when SSA defs have global uses.

Comment threadsrc/libraries/tests.proj
Comment threadsrc/coreclr/nativeaot/Runtime/arm64/GcProbe.S
Comment threadsrc/coreclr/jit/redundantbranchopts.cpp
Comment threadsrc/coreclr/nativeaot/Runtime/arm64/GcProbe.S
Comment threadsrc/coreclr/jit/redundantbranchopts.cpp Outdated
@mangod9

Copy link
Copy Markdown
Member

Overall this is now looking good, one Build Analysis failure which is not a known issue -- timeout in one of the agents.

@MichalStrehovsky

Copy link
Copy Markdown
Member

/ba-g devops timed out while waiting for helix but inspecting https://helix.dot.net/api/jobs/5e8abde5-94fb-4abf-9b8a-f7cbb124f091/workitems/nativeaot/files/console.0c33c2fb.log?api-version=2019-06-17 shows all test eventually passed

@MichalStrehovsky
MichalStrehovsky merged commit 31f66d2 into mainApr 17, 2026
148 of 160 checks passed
@MichalStrehovsky
MichalStrehovsky deleted the copilot/fix-fast-tail-call-candidates-test branch April 17, 2026 00:54
@github-actionsgithub-actionsBot locked and limited conversation to collaborators May 17, 2026
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Test failure: JIT/opt/FastTailCall/FastTailCallCandidates/FastTailCallCandidates.cmd

8 participants

@jkotas@VSadov@mangod9@jakobbotsch@MichalStrehovsky@AndyAyersMS
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Save all volatile argument registers in GC probe hijack frames on AMD64 and ARM64 - #126848

Merged
MichalStrehovsky merged 16 commits into
mainfrom
copilot/fix-fast-tail-call-candidates-test
Apr 17, 2026
Merged

Save all volatile argument registers in GC probe hijack frames on AMD64 and ARM64#126848
MichalStrehovsky merged 16 commits into
mainfrom
copilot/fix-fast-tail-call-candidates-test

Conversation

CopilotAI commented Apr 13, 2026

Copy link
Copy Markdown
Contributor
  • Save volatile argument registers (RDX/R8/R9) in Windows AMD64 GC probe hijack frames
  • Save volatile argument registers (RSI/RDI/R8/R9) in Unix AMD64 GC probe hijack frames
  • Use CFI-aware push_register/pop_register macros for proper unwind directives
  • Save volatile argument registers (x3-x7) in ARM64 GC probe hijack frames
  • Update RBM_INTERFACELOOKUP_FOR_SLOT_TRASH to trash all non-argument volatile registers
  • Save all FP argument registers in GC probe hijack frames
  • Merge main and apply targeted JIT PHI jump threading fix for JIT: fix issue in ssa-aware jump threading #126976
  • Add project exclusions for arm64 Windows tests
  • Fix JITDUMP formatting in redundantbranchopts.cpp for jitformat checker

… frames
On AMD64, when the GC hijacks a thread via RhpGcProbeHijack, the hijack stub
creates a PInvokeTransitionFrame. Previously on Windows, only RAX and RCX were
saved, and on Unix only RAX, RCX, and RDX were saved. If the GC info for the
managed frame at the hijack point reports other volatile registers (RDX, R8, R9)
as live GC references, the StackFrameIterator would find NULL save locations
and crash (AV in SVR::GCHeap::Promote).
Windows changes:
- FixupHijackedCallstack now uses R10/R11 for thread pointer instead of RDX/R8,
preserving all volatile argument registers (RAX, RCX, RDX, R8, R9)
- PUSH_PROBE_FRAME/POP_PROBE_FRAME extended to save/restore RDX, R8, R9
- RhpGcProbeHijack flags include PTFF_SAVE_RDX + PTFF_SAVE_R8 + PTFF_SAVE_R9
- AsmMacros.inc gains PTFF_SAVE_RDX, PTFF_SAVE_R8, PTFF_SAVE_R9 definitions
Unix changes:
- FixupHijackedCallstack now also saves/restores R8 and R9 across GETTHREAD
- Uses R10 instead of R8 for scratch in hijack fixup and bitmask passing
- PUSH_PROBE_FRAME/POP_PROBE_FRAME extended to save/restore R8, R9
- RhpGcProbeHijack flags include PTFF_SAVE_R8 + PTFF_SAVE_R9
- unixasmmacrosamd64.inc gains PTFF_SAVE_R8, PTFF_SAVE_R9 definitions
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/411be3e7-43a1-4bc3-b896-e27d92fe37c3
Co-authored-by: mangod9 <61718172+mangod9@users.noreply.github.com>
CopilotAI requested review from Copilot and removed request for CopilotApril 13, 2026 19:18
CopilotAI requested review from Copilot and removed request for CopilotApril 13, 2026 19:25
CopilotAI changed the title [WIP] Fix test failure for FastTailCallCandidatesSave all volatile argument registers (RDX, R8, R9) in GC probe hijack frames on AMD64Apr 13, 2026
CopilotAI requested a review from mangod9April 13, 2026 19:26
Comment threadsrc/coreclr/nativeaot/Runtime/amd64/GcProbe.S
On Unix System V ABI, RSI and RDI are volatile argument-passing registers
(1st and 2nd integer args) that could contain live GC references at hijack
points. On Windows, they are already saved as callee-saved registers in
PTFF_SAVE_ALL_PRESERVED.
Unix changes:
- FixupHijackedCallstack now saves/restores RSI and RDI across GETTHREAD
- PUSH_PROBE_FRAME saves RSI and RDI between R12 and RBX, matching the
StackFrameIterator's flag processing order (RBX, RSI, RDI, R12...)
- POP_PROBE_FRAME restores RSI and RDI in correct order
- RhpGcProbeHijack flags include PTFF_SAVE_RSI + PTFF_SAVE_RDI
- unixasmmacrosamd64.inc gains PTFF_SAVE_RSI and PTFF_SAVE_RDI definitions
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/85ca9ec1-057b-4a3c-8182-e6da274d44b6
Co-authored-by: mangod9 <61718172+mangod9@users.noreply.github.com>
CopilotAI requested review from Copilot and removed request for CopilotApril 13, 2026 21:27
CopilotAI changed the title Save all volatile argument registers (RDX, R8, R9) in GC probe hijack frames on AMD64Save all volatile argument registers in GC probe hijack frames on AMD64Apr 13, 2026
@jkotas
jkotas marked this pull request as ready for review April 13, 2026 21:43
CopilotAI review requested due to automatic review settings April 13, 2026 21:43

CopilotAI commented Apr 16, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot cherry picking is going to cause conflicts. You need to merge main into this PR and then apply the fix on top of that to keep things clean.

Reverted the cherry-pick, merged main, and applied the targeted fix on top in 7442a6d. The HasGlobalUse() path now returns CannotThread immediately, matching the diff from the comment.

@mangod9

Copy link
Copy Markdown
Member

/azp run runtime-nativeaot-outerloop

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@jkotasjkotas left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM once CI is green

@mangod9mangod9 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good, the outerloop failures look similar to main, there are more failures around pulling images. :

  • linux_musl-x64: Docker image pull failure on Helix (alpine-edge-helix-amd64) — infrastructure issue
  • windows-arm64: Also fails on main (build 1378947, Apr 14) — pre-existing

Added project exclusions for LibraryImportGenerator and ComInterfaceGenerator tests on arm64 Windows.
CopilotAI review requested due to automatic review settings April 16, 2026 07:10
@MichalStrehovsky

Copy link
Copy Markdown
Member

windows-arm64: Also fails on main (build 1378947, Apr 14) — pre-existing

I've added an exclusion for this, this is taking longer to fix than I'd like and we probably want to run all testing on this PR that we can.

@MichalStrehovsky

Copy link
Copy Markdown
Member

/azp run runtime-nativeaot-outerloop

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

CopilotAI 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.

Pull request overview

Fixes NativeAOT GC probe hijack correctness by ensuring volatile argument registers (and relevant FP argument registers) are preserved in hijack/probe frames on AMD64 and ARM64, preventing GC stack walking from observing missing/NULL save locations when GC info reports registers as live. Also updates JIT helper kill sets for interface lookup helpers and applies a targeted JIT jump-threading safety bailout for a pre-existing regression.

Changes:

  • NativeAOT: Expand GC probe hijack frame saving/restoring to include volatile argument registers (and more FP arg regs) on AMD64/ARM64; adjust hijack stubs to avoid clobbering argument registers.
  • JIT: Update RBM_INTERFACELOOKUP_FOR_SLOT_TRASH on AMD64/ARM64 to conservatively model all non-argument volatile regs as trashed.
  • JIT: Disable PHI-based jump threading when SSA defs have global uses (targeted workaround for #126976).

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.

Show a summary per file
FileDescription
src/libraries/tests.projExcludes specific NativeAOT Windows ARM64 test projects per linked issue.
src/coreclr/nativeaot/Runtime/unix/unixasmmacrosarm64.incAdds PTFF flag definitions for saving x3–x7 on Unix ARM64.
src/coreclr/nativeaot/Runtime/unix/unixasmmacrosamd64.incAdds PTFF flag definitions for saving RSI/RDI/R8/R9 on Unix AMD64.
src/coreclr/nativeaot/Runtime/arm64/GcProbe.asmUpdates Windows ARM64 probe frame layout and hijack paths to preserve x0–x7 and q0–q7; avoids clobbering arg regs.
src/coreclr/nativeaot/Runtime/arm64/GcProbe.SUpdates Unix ARM64 probe frame layout and hijack paths to preserve x0–x7 and q0–q7; adjusts FixupHijackedCallstack.
src/coreclr/nativeaot/Runtime/amd64/GcProbe.asmUpdates Windows AMD64 probe frame to preserve RDX/R8/R9 and xmm0–xmm3; avoids clobbering volatile arg regs in hijack fixup.
src/coreclr/nativeaot/Runtime/amd64/GcProbe.SUpdates Unix AMD64 probe frame to preserve RSI/RDI/RDX/R8/R9 and xmm0–xmm7; adds CFI-aware save/restore around INLINE_GETTHREAD.
src/coreclr/nativeaot/Runtime/amd64/AsmMacros.incAdds PTFF flag definitions for saving RDX/R8/R9 on Windows AMD64.
src/coreclr/jit/targetarm64.hBroadens interface lookup helper trash set to all non-argument volatile regs.
src/coreclr/jit/targetamd64.hBroadens interface lookup helper trash set to all non-argument volatile regs (incl. FP/mask as applicable).
src/coreclr/jit/redundantbranchopts.cppBail out immediately from PHI-based jump threading when SSA defs have global uses.

Comment threadsrc/libraries/tests.proj
Comment threadsrc/coreclr/nativeaot/Runtime/arm64/GcProbe.S
Comment threadsrc/coreclr/jit/redundantbranchopts.cpp
Comment threadsrc/coreclr/nativeaot/Runtime/arm64/GcProbe.S
Comment threadsrc/coreclr/jit/redundantbranchopts.cpp Outdated
@mangod9

Copy link
Copy Markdown
Member

Overall this is now looking good, one Build Analysis failure which is not a known issue -- timeout in one of the agents.

@MichalStrehovsky

Copy link
Copy Markdown
Member

/ba-g devops timed out while waiting for helix but inspecting https://helix.dot.net/api/jobs/5e8abde5-94fb-4abf-9b8a-f7cbb124f091/workitems/nativeaot/files/console.0c33c2fb.log?api-version=2019-06-17 shows all test eventually passed

@MichalStrehovsky
MichalStrehovsky merged commit 31f66d2 into mainApr 17, 2026
148 of 160 checks passed
@MichalStrehovsky
MichalStrehovsky deleted the copilot/fix-fast-tail-call-candidates-test branch April 17, 2026 00:54
@github-actionsgithub-actionsBot locked and limited conversation to collaborators May 17, 2026
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Test failure: JIT/opt/FastTailCall/FastTailCallCandidates/FastTailCallCandidates.cmd

8 participants

@jkotas@VSadov@mangod9@jakobbotsch@MichalStrehovsky@AndyAyersMS
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

Save all volatile argument registers in GC probe hijack frames on AMD64 and ARM64 - #126848

Merged
MichalStrehovsky merged 16 commits into
mainfrom
copilot/fix-fast-tail-call-candidates-test
Apr 17, 2026
Merged

Save all volatile argument registers in GC probe hijack frames on AMD64 and ARM64#126848
MichalStrehovsky merged 16 commits into
mainfrom
copilot/fix-fast-tail-call-candidates-test

Conversation

CopilotAI commented Apr 13, 2026

Copy link
Copy Markdown
Contributor
  • Save volatile argument registers (RDX/R8/R9) in Windows AMD64 GC probe hijack frames
  • Save volatile argument registers (RSI/RDI/R8/R9) in Unix AMD64 GC probe hijack frames
  • Use CFI-aware push_register/pop_register macros for proper unwind directives
  • Save volatile argument registers (x3-x7) in ARM64 GC probe hijack frames
  • Update RBM_INTERFACELOOKUP_FOR_SLOT_TRASH to trash all non-argument volatile registers
  • Save all FP argument registers in GC probe hijack frames
  • Merge main and apply targeted JIT PHI jump threading fix for JIT: fix issue in ssa-aware jump threading #126976
  • Add project exclusions for arm64 Windows tests
  • Fix JITDUMP formatting in redundantbranchopts.cpp for jitformat checker

… frames
On AMD64, when the GC hijacks a thread via RhpGcProbeHijack, the hijack stub
creates a PInvokeTransitionFrame. Previously on Windows, only RAX and RCX were
saved, and on Unix only RAX, RCX, and RDX were saved. If the GC info for the
managed frame at the hijack point reports other volatile registers (RDX, R8, R9)
as live GC references, the StackFrameIterator would find NULL save locations
and crash (AV in SVR::GCHeap::Promote).
Windows changes:
- FixupHijackedCallstack now uses R10/R11 for thread pointer instead of RDX/R8,
preserving all volatile argument registers (RAX, RCX, RDX, R8, R9)
- PUSH_PROBE_FRAME/POP_PROBE_FRAME extended to save/restore RDX, R8, R9
- RhpGcProbeHijack flags include PTFF_SAVE_RDX + PTFF_SAVE_R8 + PTFF_SAVE_R9
- AsmMacros.inc gains PTFF_SAVE_RDX, PTFF_SAVE_R8, PTFF_SAVE_R9 definitions
Unix changes:
- FixupHijackedCallstack now also saves/restores R8 and R9 across GETTHREAD
- Uses R10 instead of R8 for scratch in hijack fixup and bitmask passing
- PUSH_PROBE_FRAME/POP_PROBE_FRAME extended to save/restore R8, R9
- RhpGcProbeHijack flags include PTFF_SAVE_R8 + PTFF_SAVE_R9
- unixasmmacrosamd64.inc gains PTFF_SAVE_R8, PTFF_SAVE_R9 definitions
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/411be3e7-43a1-4bc3-b896-e27d92fe37c3
Co-authored-by: mangod9 <61718172+mangod9@users.noreply.github.com>
CopilotAI requested review from Copilot and removed request for CopilotApril 13, 2026 19:18
CopilotAI requested review from Copilot and removed request for CopilotApril 13, 2026 19:25
CopilotAI changed the title [WIP] Fix test failure for FastTailCallCandidatesSave all volatile argument registers (RDX, R8, R9) in GC probe hijack frames on AMD64Apr 13, 2026
CopilotAI requested a review from mangod9April 13, 2026 19:26
Comment threadsrc/coreclr/nativeaot/Runtime/amd64/GcProbe.S
On Unix System V ABI, RSI and RDI are volatile argument-passing registers
(1st and 2nd integer args) that could contain live GC references at hijack
points. On Windows, they are already saved as callee-saved registers in
PTFF_SAVE_ALL_PRESERVED.
Unix changes:
- FixupHijackedCallstack now saves/restores RSI and RDI across GETTHREAD
- PUSH_PROBE_FRAME saves RSI and RDI between R12 and RBX, matching the
StackFrameIterator's flag processing order (RBX, RSI, RDI, R12...)
- POP_PROBE_FRAME restores RSI and RDI in correct order
- RhpGcProbeHijack flags include PTFF_SAVE_RSI + PTFF_SAVE_RDI
- unixasmmacrosamd64.inc gains PTFF_SAVE_RSI and PTFF_SAVE_RDI definitions
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/85ca9ec1-057b-4a3c-8182-e6da274d44b6
Co-authored-by: mangod9 <61718172+mangod9@users.noreply.github.com>
CopilotAI requested review from Copilot and removed request for CopilotApril 13, 2026 21:27
CopilotAI changed the title Save all volatile argument registers (RDX, R8, R9) in GC probe hijack frames on AMD64Save all volatile argument registers in GC probe hijack frames on AMD64Apr 13, 2026
@jkotas
jkotas marked this pull request as ready for review April 13, 2026 21:43
CopilotAI review requested due to automatic review settings April 13, 2026 21:43

CopilotAI commented Apr 16, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot cherry picking is going to cause conflicts. You need to merge main into this PR and then apply the fix on top of that to keep things clean.

Reverted the cherry-pick, merged main, and applied the targeted fix on top in 7442a6d. The HasGlobalUse() path now returns CannotThread immediately, matching the diff from the comment.

@mangod9

Copy link
Copy Markdown
Member

/azp run runtime-nativeaot-outerloop

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@jkotasjkotas left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM once CI is green

@mangod9mangod9 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good, the outerloop failures look similar to main, there are more failures around pulling images. :

  • linux_musl-x64: Docker image pull failure on Helix (alpine-edge-helix-amd64) — infrastructure issue
  • windows-arm64: Also fails on main (build 1378947, Apr 14) — pre-existing

Added project exclusions for LibraryImportGenerator and ComInterfaceGenerator tests on arm64 Windows.
CopilotAI review requested due to automatic review settings April 16, 2026 07:10
@MichalStrehovsky

Copy link
Copy Markdown
Member

windows-arm64: Also fails on main (build 1378947, Apr 14) — pre-existing

I've added an exclusion for this, this is taking longer to fix than I'd like and we probably want to run all testing on this PR that we can.

@MichalStrehovsky

Copy link
Copy Markdown
Member

/azp run runtime-nativeaot-outerloop

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

CopilotAI 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.

Pull request overview

Fixes NativeAOT GC probe hijack correctness by ensuring volatile argument registers (and relevant FP argument registers) are preserved in hijack/probe frames on AMD64 and ARM64, preventing GC stack walking from observing missing/NULL save locations when GC info reports registers as live. Also updates JIT helper kill sets for interface lookup helpers and applies a targeted JIT jump-threading safety bailout for a pre-existing regression.

Changes:

  • NativeAOT: Expand GC probe hijack frame saving/restoring to include volatile argument registers (and more FP arg regs) on AMD64/ARM64; adjust hijack stubs to avoid clobbering argument registers.
  • JIT: Update RBM_INTERFACELOOKUP_FOR_SLOT_TRASH on AMD64/ARM64 to conservatively model all non-argument volatile regs as trashed.
  • JIT: Disable PHI-based jump threading when SSA defs have global uses (targeted workaround for #126976).

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.

Show a summary per file
FileDescription
src/libraries/tests.projExcludes specific NativeAOT Windows ARM64 test projects per linked issue.
src/coreclr/nativeaot/Runtime/unix/unixasmmacrosarm64.incAdds PTFF flag definitions for saving x3–x7 on Unix ARM64.
src/coreclr/nativeaot/Runtime/unix/unixasmmacrosamd64.incAdds PTFF flag definitions for saving RSI/RDI/R8/R9 on Unix AMD64.
src/coreclr/nativeaot/Runtime/arm64/GcProbe.asmUpdates Windows ARM64 probe frame layout and hijack paths to preserve x0–x7 and q0–q7; avoids clobbering arg regs.
src/coreclr/nativeaot/Runtime/arm64/GcProbe.SUpdates Unix ARM64 probe frame layout and hijack paths to preserve x0–x7 and q0–q7; adjusts FixupHijackedCallstack.
src/coreclr/nativeaot/Runtime/amd64/GcProbe.asmUpdates Windows AMD64 probe frame to preserve RDX/R8/R9 and xmm0–xmm3; avoids clobbering volatile arg regs in hijack fixup.
src/coreclr/nativeaot/Runtime/amd64/GcProbe.SUpdates Unix AMD64 probe frame to preserve RSI/RDI/RDX/R8/R9 and xmm0–xmm7; adds CFI-aware save/restore around INLINE_GETTHREAD.
src/coreclr/nativeaot/Runtime/amd64/AsmMacros.incAdds PTFF flag definitions for saving RDX/R8/R9 on Windows AMD64.
src/coreclr/jit/targetarm64.hBroadens interface lookup helper trash set to all non-argument volatile regs.
src/coreclr/jit/targetamd64.hBroadens interface lookup helper trash set to all non-argument volatile regs (incl. FP/mask as applicable).
src/coreclr/jit/redundantbranchopts.cppBail out immediately from PHI-based jump threading when SSA defs have global uses.

Comment threadsrc/libraries/tests.proj
Comment threadsrc/coreclr/nativeaot/Runtime/arm64/GcProbe.S
Comment threadsrc/coreclr/jit/redundantbranchopts.cpp
Comment threadsrc/coreclr/nativeaot/Runtime/arm64/GcProbe.S
Comment threadsrc/coreclr/jit/redundantbranchopts.cpp Outdated
@mangod9

Copy link
Copy Markdown
Member

Overall this is now looking good, one Build Analysis failure which is not a known issue -- timeout in one of the agents.

@MichalStrehovsky

Copy link
Copy Markdown
Member

/ba-g devops timed out while waiting for helix but inspecting https://helix.dot.net/api/jobs/5e8abde5-94fb-4abf-9b8a-f7cbb124f091/workitems/nativeaot/files/console.0c33c2fb.log?api-version=2019-06-17 shows all test eventually passed

@MichalStrehovsky
MichalStrehovsky merged commit 31f66d2 into mainApr 17, 2026
148 of 160 checks passed
@MichalStrehovsky
MichalStrehovsky deleted the copilot/fix-fast-tail-call-candidates-test branch April 17, 2026 00:54
@github-actionsgithub-actionsBot locked and limited conversation to collaborators May 17, 2026
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Test failure: JIT/opt/FastTailCall/FastTailCallCandidates/FastTailCallCandidates.cmd

8 participants

@jkotas@VSadov@mangod9@jakobbotsch@MichalStrehovsky@AndyAyersMS
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Save all volatile argument registers in GC probe hijack frames on AMD64 and ARM64 - #126848

Merged
MichalStrehovsky merged 16 commits into
mainfrom
copilot/fix-fast-tail-call-candidates-test
Apr 17, 2026
Merged

Save all volatile argument registers in GC probe hijack frames on AMD64 and ARM64#126848
MichalStrehovsky merged 16 commits into
mainfrom
copilot/fix-fast-tail-call-candidates-test

Conversation

CopilotAI commented Apr 13, 2026

Copy link
Copy Markdown
Contributor
  • Save volatile argument registers (RDX/R8/R9) in Windows AMD64 GC probe hijack frames
  • Save volatile argument registers (RSI/RDI/R8/R9) in Unix AMD64 GC probe hijack frames
  • Use CFI-aware push_register/pop_register macros for proper unwind directives
  • Save volatile argument registers (x3-x7) in ARM64 GC probe hijack frames
  • Update RBM_INTERFACELOOKUP_FOR_SLOT_TRASH to trash all non-argument volatile registers
  • Save all FP argument registers in GC probe hijack frames
  • Merge main and apply targeted JIT PHI jump threading fix for JIT: fix issue in ssa-aware jump threading #126976
  • Add project exclusions for arm64 Windows tests
  • Fix JITDUMP formatting in redundantbranchopts.cpp for jitformat checker

… frames
On AMD64, when the GC hijacks a thread via RhpGcProbeHijack, the hijack stub
creates a PInvokeTransitionFrame. Previously on Windows, only RAX and RCX were
saved, and on Unix only RAX, RCX, and RDX were saved. If the GC info for the
managed frame at the hijack point reports other volatile registers (RDX, R8, R9)
as live GC references, the StackFrameIterator would find NULL save locations
and crash (AV in SVR::GCHeap::Promote).
Windows changes:
- FixupHijackedCallstack now uses R10/R11 for thread pointer instead of RDX/R8,
preserving all volatile argument registers (RAX, RCX, RDX, R8, R9)
- PUSH_PROBE_FRAME/POP_PROBE_FRAME extended to save/restore RDX, R8, R9
- RhpGcProbeHijack flags include PTFF_SAVE_RDX + PTFF_SAVE_R8 + PTFF_SAVE_R9
- AsmMacros.inc gains PTFF_SAVE_RDX, PTFF_SAVE_R8, PTFF_SAVE_R9 definitions
Unix changes:
- FixupHijackedCallstack now also saves/restores R8 and R9 across GETTHREAD
- Uses R10 instead of R8 for scratch in hijack fixup and bitmask passing
- PUSH_PROBE_FRAME/POP_PROBE_FRAME extended to save/restore R8, R9
- RhpGcProbeHijack flags include PTFF_SAVE_R8 + PTFF_SAVE_R9
- unixasmmacrosamd64.inc gains PTFF_SAVE_R8, PTFF_SAVE_R9 definitions
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/411be3e7-43a1-4bc3-b896-e27d92fe37c3
Co-authored-by: mangod9 <61718172+mangod9@users.noreply.github.com>
CopilotAI requested review from Copilot and removed request for CopilotApril 13, 2026 19:18
CopilotAI requested review from Copilot and removed request for CopilotApril 13, 2026 19:25
CopilotAI changed the title [WIP] Fix test failure for FastTailCallCandidatesSave all volatile argument registers (RDX, R8, R9) in GC probe hijack frames on AMD64Apr 13, 2026
CopilotAI requested a review from mangod9April 13, 2026 19:26
Comment threadsrc/coreclr/nativeaot/Runtime/amd64/GcProbe.S
On Unix System V ABI, RSI and RDI are volatile argument-passing registers
(1st and 2nd integer args) that could contain live GC references at hijack
points. On Windows, they are already saved as callee-saved registers in
PTFF_SAVE_ALL_PRESERVED.
Unix changes:
- FixupHijackedCallstack now saves/restores RSI and RDI across GETTHREAD
- PUSH_PROBE_FRAME saves RSI and RDI between R12 and RBX, matching the
StackFrameIterator's flag processing order (RBX, RSI, RDI, R12...)
- POP_PROBE_FRAME restores RSI and RDI in correct order
- RhpGcProbeHijack flags include PTFF_SAVE_RSI + PTFF_SAVE_RDI
- unixasmmacrosamd64.inc gains PTFF_SAVE_RSI and PTFF_SAVE_RDI definitions
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/85ca9ec1-057b-4a3c-8182-e6da274d44b6
Co-authored-by: mangod9 <61718172+mangod9@users.noreply.github.com>
CopilotAI requested review from Copilot and removed request for CopilotApril 13, 2026 21:27
CopilotAI changed the title Save all volatile argument registers (RDX, R8, R9) in GC probe hijack frames on AMD64Save all volatile argument registers in GC probe hijack frames on AMD64Apr 13, 2026
@jkotas
jkotas marked this pull request as ready for review April 13, 2026 21:43
CopilotAI review requested due to automatic review settings April 13, 2026 21:43

CopilotAI commented Apr 16, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot cherry picking is going to cause conflicts. You need to merge main into this PR and then apply the fix on top of that to keep things clean.

Reverted the cherry-pick, merged main, and applied the targeted fix on top in 7442a6d. The HasGlobalUse() path now returns CannotThread immediately, matching the diff from the comment.

@mangod9

Copy link
Copy Markdown
Member

/azp run runtime-nativeaot-outerloop

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@jkotasjkotas left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM once CI is green

@mangod9mangod9 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good, the outerloop failures look similar to main, there are more failures around pulling images. :

  • linux_musl-x64: Docker image pull failure on Helix (alpine-edge-helix-amd64) — infrastructure issue
  • windows-arm64: Also fails on main (build 1378947, Apr 14) — pre-existing

Added project exclusions for LibraryImportGenerator and ComInterfaceGenerator tests on arm64 Windows.
CopilotAI review requested due to automatic review settings April 16, 2026 07:10
@MichalStrehovsky

Copy link
Copy Markdown
Member

windows-arm64: Also fails on main (build 1378947, Apr 14) — pre-existing

I've added an exclusion for this, this is taking longer to fix than I'd like and we probably want to run all testing on this PR that we can.

@MichalStrehovsky

Copy link
Copy Markdown
Member

/azp run runtime-nativeaot-outerloop

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

CopilotAI 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.

Pull request overview

Fixes NativeAOT GC probe hijack correctness by ensuring volatile argument registers (and relevant FP argument registers) are preserved in hijack/probe frames on AMD64 and ARM64, preventing GC stack walking from observing missing/NULL save locations when GC info reports registers as live. Also updates JIT helper kill sets for interface lookup helpers and applies a targeted JIT jump-threading safety bailout for a pre-existing regression.

Changes:

  • NativeAOT: Expand GC probe hijack frame saving/restoring to include volatile argument registers (and more FP arg regs) on AMD64/ARM64; adjust hijack stubs to avoid clobbering argument registers.
  • JIT: Update RBM_INTERFACELOOKUP_FOR_SLOT_TRASH on AMD64/ARM64 to conservatively model all non-argument volatile regs as trashed.
  • JIT: Disable PHI-based jump threading when SSA defs have global uses (targeted workaround for #126976).

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.

Show a summary per file
FileDescription
src/libraries/tests.projExcludes specific NativeAOT Windows ARM64 test projects per linked issue.
src/coreclr/nativeaot/Runtime/unix/unixasmmacrosarm64.incAdds PTFF flag definitions for saving x3–x7 on Unix ARM64.
src/coreclr/nativeaot/Runtime/unix/unixasmmacrosamd64.incAdds PTFF flag definitions for saving RSI/RDI/R8/R9 on Unix AMD64.
src/coreclr/nativeaot/Runtime/arm64/GcProbe.asmUpdates Windows ARM64 probe frame layout and hijack paths to preserve x0–x7 and q0–q7; avoids clobbering arg regs.
src/coreclr/nativeaot/Runtime/arm64/GcProbe.SUpdates Unix ARM64 probe frame layout and hijack paths to preserve x0–x7 and q0–q7; adjusts FixupHijackedCallstack.
src/coreclr/nativeaot/Runtime/amd64/GcProbe.asmUpdates Windows AMD64 probe frame to preserve RDX/R8/R9 and xmm0–xmm3; avoids clobbering volatile arg regs in hijack fixup.
src/coreclr/nativeaot/Runtime/amd64/GcProbe.SUpdates Unix AMD64 probe frame to preserve RSI/RDI/RDX/R8/R9 and xmm0–xmm7; adds CFI-aware save/restore around INLINE_GETTHREAD.
src/coreclr/nativeaot/Runtime/amd64/AsmMacros.incAdds PTFF flag definitions for saving RDX/R8/R9 on Windows AMD64.
src/coreclr/jit/targetarm64.hBroadens interface lookup helper trash set to all non-argument volatile regs.
src/coreclr/jit/targetamd64.hBroadens interface lookup helper trash set to all non-argument volatile regs (incl. FP/mask as applicable).
src/coreclr/jit/redundantbranchopts.cppBail out immediately from PHI-based jump threading when SSA defs have global uses.

Comment threadsrc/libraries/tests.proj
Comment threadsrc/coreclr/nativeaot/Runtime/arm64/GcProbe.S
Comment threadsrc/coreclr/jit/redundantbranchopts.cpp
Comment threadsrc/coreclr/nativeaot/Runtime/arm64/GcProbe.S
Comment threadsrc/coreclr/jit/redundantbranchopts.cpp Outdated
@mangod9

Copy link
Copy Markdown
Member

Overall this is now looking good, one Build Analysis failure which is not a known issue -- timeout in one of the agents.

@MichalStrehovsky

Copy link
Copy Markdown
Member

/ba-g devops timed out while waiting for helix but inspecting https://helix.dot.net/api/jobs/5e8abde5-94fb-4abf-9b8a-f7cbb124f091/workitems/nativeaot/files/console.0c33c2fb.log?api-version=2019-06-17 shows all test eventually passed

@MichalStrehovsky
MichalStrehovsky merged commit 31f66d2 into mainApr 17, 2026
148 of 160 checks passed
@MichalStrehovsky
MichalStrehovsky deleted the copilot/fix-fast-tail-call-candidates-test branch April 17, 2026 00:54
@github-actionsgithub-actionsBot locked and limited conversation to collaborators May 17, 2026
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Test failure: JIT/opt/FastTailCall/FastTailCallCandidates/FastTailCallCandidates.cmd

8 participants

@jkotas@VSadov@mangod9@jakobbotsch@MichalStrehovsky@AndyAyersMS
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Save all volatile argument registers in GC probe hijack frames on AMD64 and ARM64 - #126848

Merged
MichalStrehovsky merged 16 commits into
mainfrom
copilot/fix-fast-tail-call-candidates-test
Apr 17, 2026
Merged

Save all volatile argument registers in GC probe hijack frames on AMD64 and ARM64#126848
MichalStrehovsky merged 16 commits into
mainfrom
copilot/fix-fast-tail-call-candidates-test

Conversation

CopilotAI commented Apr 13, 2026

Copy link
Copy Markdown
Contributor
  • Save volatile argument registers (RDX/R8/R9) in Windows AMD64 GC probe hijack frames
  • Save volatile argument registers (RSI/RDI/R8/R9) in Unix AMD64 GC probe hijack frames
  • Use CFI-aware push_register/pop_register macros for proper unwind directives
  • Save volatile argument registers (x3-x7) in ARM64 GC probe hijack frames
  • Update RBM_INTERFACELOOKUP_FOR_SLOT_TRASH to trash all non-argument volatile registers
  • Save all FP argument registers in GC probe hijack frames
  • Merge main and apply targeted JIT PHI jump threading fix for JIT: fix issue in ssa-aware jump threading #126976
  • Add project exclusions for arm64 Windows tests
  • Fix JITDUMP formatting in redundantbranchopts.cpp for jitformat checker

… frames
On AMD64, when the GC hijacks a thread via RhpGcProbeHijack, the hijack stub
creates a PInvokeTransitionFrame. Previously on Windows, only RAX and RCX were
saved, and on Unix only RAX, RCX, and RDX were saved. If the GC info for the
managed frame at the hijack point reports other volatile registers (RDX, R8, R9)
as live GC references, the StackFrameIterator would find NULL save locations
and crash (AV in SVR::GCHeap::Promote).
Windows changes:
- FixupHijackedCallstack now uses R10/R11 for thread pointer instead of RDX/R8,
preserving all volatile argument registers (RAX, RCX, RDX, R8, R9)
- PUSH_PROBE_FRAME/POP_PROBE_FRAME extended to save/restore RDX, R8, R9
- RhpGcProbeHijack flags include PTFF_SAVE_RDX + PTFF_SAVE_R8 + PTFF_SAVE_R9
- AsmMacros.inc gains PTFF_SAVE_RDX, PTFF_SAVE_R8, PTFF_SAVE_R9 definitions
Unix changes:
- FixupHijackedCallstack now also saves/restores R8 and R9 across GETTHREAD
- Uses R10 instead of R8 for scratch in hijack fixup and bitmask passing
- PUSH_PROBE_FRAME/POP_PROBE_FRAME extended to save/restore R8, R9
- RhpGcProbeHijack flags include PTFF_SAVE_R8 + PTFF_SAVE_R9
- unixasmmacrosamd64.inc gains PTFF_SAVE_R8, PTFF_SAVE_R9 definitions
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/411be3e7-43a1-4bc3-b896-e27d92fe37c3
Co-authored-by: mangod9 <61718172+mangod9@users.noreply.github.com>
CopilotAI requested review from Copilot and removed request for CopilotApril 13, 2026 19:18
CopilotAI requested review from Copilot and removed request for CopilotApril 13, 2026 19:25
CopilotAI changed the title [WIP] Fix test failure for FastTailCallCandidatesSave all volatile argument registers (RDX, R8, R9) in GC probe hijack frames on AMD64Apr 13, 2026
CopilotAI requested a review from mangod9April 13, 2026 19:26
Comment threadsrc/coreclr/nativeaot/Runtime/amd64/GcProbe.S
On Unix System V ABI, RSI and RDI are volatile argument-passing registers
(1st and 2nd integer args) that could contain live GC references at hijack
points. On Windows, they are already saved as callee-saved registers in
PTFF_SAVE_ALL_PRESERVED.
Unix changes:
- FixupHijackedCallstack now saves/restores RSI and RDI across GETTHREAD
- PUSH_PROBE_FRAME saves RSI and RDI between R12 and RBX, matching the
StackFrameIterator's flag processing order (RBX, RSI, RDI, R12...)
- POP_PROBE_FRAME restores RSI and RDI in correct order
- RhpGcProbeHijack flags include PTFF_SAVE_RSI + PTFF_SAVE_RDI
- unixasmmacrosamd64.inc gains PTFF_SAVE_RSI and PTFF_SAVE_RDI definitions
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/85ca9ec1-057b-4a3c-8182-e6da274d44b6
Co-authored-by: mangod9 <61718172+mangod9@users.noreply.github.com>
CopilotAI requested review from Copilot and removed request for CopilotApril 13, 2026 21:27
CopilotAI changed the title Save all volatile argument registers (RDX, R8, R9) in GC probe hijack frames on AMD64Save all volatile argument registers in GC probe hijack frames on AMD64Apr 13, 2026
@jkotas
jkotas marked this pull request as ready for review April 13, 2026 21:43
CopilotAI review requested due to automatic review settings April 13, 2026 21:43

CopilotAI commented Apr 16, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot cherry picking is going to cause conflicts. You need to merge main into this PR and then apply the fix on top of that to keep things clean.

Reverted the cherry-pick, merged main, and applied the targeted fix on top in 7442a6d. The HasGlobalUse() path now returns CannotThread immediately, matching the diff from the comment.

@mangod9

Copy link
Copy Markdown
Member

/azp run runtime-nativeaot-outerloop

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@jkotasjkotas left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM once CI is green

@mangod9mangod9 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good, the outerloop failures look similar to main, there are more failures around pulling images. :

  • linux_musl-x64: Docker image pull failure on Helix (alpine-edge-helix-amd64) — infrastructure issue
  • windows-arm64: Also fails on main (build 1378947, Apr 14) — pre-existing

Added project exclusions for LibraryImportGenerator and ComInterfaceGenerator tests on arm64 Windows.
CopilotAI review requested due to automatic review settings April 16, 2026 07:10
@MichalStrehovsky

Copy link
Copy Markdown
Member

windows-arm64: Also fails on main (build 1378947, Apr 14) — pre-existing

I've added an exclusion for this, this is taking longer to fix than I'd like and we probably want to run all testing on this PR that we can.

@MichalStrehovsky

Copy link
Copy Markdown
Member

/azp run runtime-nativeaot-outerloop

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

CopilotAI 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.

Pull request overview

Fixes NativeAOT GC probe hijack correctness by ensuring volatile argument registers (and relevant FP argument registers) are preserved in hijack/probe frames on AMD64 and ARM64, preventing GC stack walking from observing missing/NULL save locations when GC info reports registers as live. Also updates JIT helper kill sets for interface lookup helpers and applies a targeted JIT jump-threading safety bailout for a pre-existing regression.

Changes:

  • NativeAOT: Expand GC probe hijack frame saving/restoring to include volatile argument registers (and more FP arg regs) on AMD64/ARM64; adjust hijack stubs to avoid clobbering argument registers.
  • JIT: Update RBM_INTERFACELOOKUP_FOR_SLOT_TRASH on AMD64/ARM64 to conservatively model all non-argument volatile regs as trashed.
  • JIT: Disable PHI-based jump threading when SSA defs have global uses (targeted workaround for #126976).

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.

Show a summary per file
FileDescription
src/libraries/tests.projExcludes specific NativeAOT Windows ARM64 test projects per linked issue.
src/coreclr/nativeaot/Runtime/unix/unixasmmacrosarm64.incAdds PTFF flag definitions for saving x3–x7 on Unix ARM64.
src/coreclr/nativeaot/Runtime/unix/unixasmmacrosamd64.incAdds PTFF flag definitions for saving RSI/RDI/R8/R9 on Unix AMD64.
src/coreclr/nativeaot/Runtime/arm64/GcProbe.asmUpdates Windows ARM64 probe frame layout and hijack paths to preserve x0–x7 and q0–q7; avoids clobbering arg regs.
src/coreclr/nativeaot/Runtime/arm64/GcProbe.SUpdates Unix ARM64 probe frame layout and hijack paths to preserve x0–x7 and q0–q7; adjusts FixupHijackedCallstack.
src/coreclr/nativeaot/Runtime/amd64/GcProbe.asmUpdates Windows AMD64 probe frame to preserve RDX/R8/R9 and xmm0–xmm3; avoids clobbering volatile arg regs in hijack fixup.
src/coreclr/nativeaot/Runtime/amd64/GcProbe.SUpdates Unix AMD64 probe frame to preserve RSI/RDI/RDX/R8/R9 and xmm0–xmm7; adds CFI-aware save/restore around INLINE_GETTHREAD.
src/coreclr/nativeaot/Runtime/amd64/AsmMacros.incAdds PTFF flag definitions for saving RDX/R8/R9 on Windows AMD64.
src/coreclr/jit/targetarm64.hBroadens interface lookup helper trash set to all non-argument volatile regs.
src/coreclr/jit/targetamd64.hBroadens interface lookup helper trash set to all non-argument volatile regs (incl. FP/mask as applicable).
src/coreclr/jit/redundantbranchopts.cppBail out immediately from PHI-based jump threading when SSA defs have global uses.

Comment threadsrc/libraries/tests.proj
Comment threadsrc/coreclr/nativeaot/Runtime/arm64/GcProbe.S
Comment threadsrc/coreclr/jit/redundantbranchopts.cpp
Comment threadsrc/coreclr/nativeaot/Runtime/arm64/GcProbe.S
Comment threadsrc/coreclr/jit/redundantbranchopts.cpp Outdated
@mangod9

Copy link
Copy Markdown
Member

Overall this is now looking good, one Build Analysis failure which is not a known issue -- timeout in one of the agents.

@MichalStrehovsky

Copy link
Copy Markdown
Member

/ba-g devops timed out while waiting for helix but inspecting https://helix.dot.net/api/jobs/5e8abde5-94fb-4abf-9b8a-f7cbb124f091/workitems/nativeaot/files/console.0c33c2fb.log?api-version=2019-06-17 shows all test eventually passed

@MichalStrehovsky
MichalStrehovsky merged commit 31f66d2 into mainApr 17, 2026
148 of 160 checks passed
@MichalStrehovsky
MichalStrehovsky deleted the copilot/fix-fast-tail-call-candidates-test branch April 17, 2026 00:54
@github-actionsgithub-actionsBot locked and limited conversation to collaborators May 17, 2026
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Test failure: JIT/opt/FastTailCall/FastTailCallCandidates/FastTailCallCandidates.cmd

8 participants

@jkotas@VSadov@mangod9@jakobbotsch@MichalStrehovsky@AndyAyersMS
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

Save all volatile argument registers in GC probe hijack frames on AMD64 and ARM64 - #126848

Merged
MichalStrehovsky merged 16 commits into
mainfrom
copilot/fix-fast-tail-call-candidates-test
Apr 17, 2026
Merged

Save all volatile argument registers in GC probe hijack frames on AMD64 and ARM64#126848
MichalStrehovsky merged 16 commits into
mainfrom
copilot/fix-fast-tail-call-candidates-test

Conversation

CopilotAI commented Apr 13, 2026

Copy link
Copy Markdown
Contributor
  • Save volatile argument registers (RDX/R8/R9) in Windows AMD64 GC probe hijack frames
  • Save volatile argument registers (RSI/RDI/R8/R9) in Unix AMD64 GC probe hijack frames
  • Use CFI-aware push_register/pop_register macros for proper unwind directives
  • Save volatile argument registers (x3-x7) in ARM64 GC probe hijack frames
  • Update RBM_INTERFACELOOKUP_FOR_SLOT_TRASH to trash all non-argument volatile registers
  • Save all FP argument registers in GC probe hijack frames
  • Merge main and apply targeted JIT PHI jump threading fix for JIT: fix issue in ssa-aware jump threading #126976
  • Add project exclusions for arm64 Windows tests
  • Fix JITDUMP formatting in redundantbranchopts.cpp for jitformat checker

… frames
On AMD64, when the GC hijacks a thread via RhpGcProbeHijack, the hijack stub
creates a PInvokeTransitionFrame. Previously on Windows, only RAX and RCX were
saved, and on Unix only RAX, RCX, and RDX were saved. If the GC info for the
managed frame at the hijack point reports other volatile registers (RDX, R8, R9)
as live GC references, the StackFrameIterator would find NULL save locations
and crash (AV in SVR::GCHeap::Promote).
Windows changes:
- FixupHijackedCallstack now uses R10/R11 for thread pointer instead of RDX/R8,
preserving all volatile argument registers (RAX, RCX, RDX, R8, R9)
- PUSH_PROBE_FRAME/POP_PROBE_FRAME extended to save/restore RDX, R8, R9
- RhpGcProbeHijack flags include PTFF_SAVE_RDX + PTFF_SAVE_R8 + PTFF_SAVE_R9
- AsmMacros.inc gains PTFF_SAVE_RDX, PTFF_SAVE_R8, PTFF_SAVE_R9 definitions
Unix changes:
- FixupHijackedCallstack now also saves/restores R8 and R9 across GETTHREAD
- Uses R10 instead of R8 for scratch in hijack fixup and bitmask passing
- PUSH_PROBE_FRAME/POP_PROBE_FRAME extended to save/restore R8, R9
- RhpGcProbeHijack flags include PTFF_SAVE_R8 + PTFF_SAVE_R9
- unixasmmacrosamd64.inc gains PTFF_SAVE_R8, PTFF_SAVE_R9 definitions
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/411be3e7-43a1-4bc3-b896-e27d92fe37c3
Co-authored-by: mangod9 <61718172+mangod9@users.noreply.github.com>
CopilotAI requested review from Copilot and removed request for CopilotApril 13, 2026 19:18
CopilotAI requested review from Copilot and removed request for CopilotApril 13, 2026 19:25
CopilotAI changed the title [WIP] Fix test failure for FastTailCallCandidatesSave all volatile argument registers (RDX, R8, R9) in GC probe hijack frames on AMD64Apr 13, 2026
CopilotAI requested a review from mangod9April 13, 2026 19:26
Comment threadsrc/coreclr/nativeaot/Runtime/amd64/GcProbe.S
On Unix System V ABI, RSI and RDI are volatile argument-passing registers
(1st and 2nd integer args) that could contain live GC references at hijack
points. On Windows, they are already saved as callee-saved registers in
PTFF_SAVE_ALL_PRESERVED.
Unix changes:
- FixupHijackedCallstack now saves/restores RSI and RDI across GETTHREAD
- PUSH_PROBE_FRAME saves RSI and RDI between R12 and RBX, matching the
StackFrameIterator's flag processing order (RBX, RSI, RDI, R12...)
- POP_PROBE_FRAME restores RSI and RDI in correct order
- RhpGcProbeHijack flags include PTFF_SAVE_RSI + PTFF_SAVE_RDI
- unixasmmacrosamd64.inc gains PTFF_SAVE_RSI and PTFF_SAVE_RDI definitions
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/85ca9ec1-057b-4a3c-8182-e6da274d44b6
Co-authored-by: mangod9 <61718172+mangod9@users.noreply.github.com>
CopilotAI requested review from Copilot and removed request for CopilotApril 13, 2026 21:27
CopilotAI changed the title Save all volatile argument registers (RDX, R8, R9) in GC probe hijack frames on AMD64Save all volatile argument registers in GC probe hijack frames on AMD64Apr 13, 2026
@jkotas
jkotas marked this pull request as ready for review April 13, 2026 21:43
CopilotAI review requested due to automatic review settings April 13, 2026 21:43

CopilotAI commented Apr 16, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot cherry picking is going to cause conflicts. You need to merge main into this PR and then apply the fix on top of that to keep things clean.

Reverted the cherry-pick, merged main, and applied the targeted fix on top in 7442a6d. The HasGlobalUse() path now returns CannotThread immediately, matching the diff from the comment.

@mangod9

Copy link
Copy Markdown
Member

/azp run runtime-nativeaot-outerloop

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@jkotasjkotas left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM once CI is green

@mangod9mangod9 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good, the outerloop failures look similar to main, there are more failures around pulling images. :

  • linux_musl-x64: Docker image pull failure on Helix (alpine-edge-helix-amd64) — infrastructure issue
  • windows-arm64: Also fails on main (build 1378947, Apr 14) — pre-existing

Added project exclusions for LibraryImportGenerator and ComInterfaceGenerator tests on arm64 Windows.
CopilotAI review requested due to automatic review settings April 16, 2026 07:10
@MichalStrehovsky

Copy link
Copy Markdown
Member

windows-arm64: Also fails on main (build 1378947, Apr 14) — pre-existing

I've added an exclusion for this, this is taking longer to fix than I'd like and we probably want to run all testing on this PR that we can.

@MichalStrehovsky

Copy link
Copy Markdown
Member

/azp run runtime-nativeaot-outerloop

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

CopilotAI 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.

Pull request overview

Fixes NativeAOT GC probe hijack correctness by ensuring volatile argument registers (and relevant FP argument registers) are preserved in hijack/probe frames on AMD64 and ARM64, preventing GC stack walking from observing missing/NULL save locations when GC info reports registers as live. Also updates JIT helper kill sets for interface lookup helpers and applies a targeted JIT jump-threading safety bailout for a pre-existing regression.

Changes:

  • NativeAOT: Expand GC probe hijack frame saving/restoring to include volatile argument registers (and more FP arg regs) on AMD64/ARM64; adjust hijack stubs to avoid clobbering argument registers.
  • JIT: Update RBM_INTERFACELOOKUP_FOR_SLOT_TRASH on AMD64/ARM64 to conservatively model all non-argument volatile regs as trashed.
  • JIT: Disable PHI-based jump threading when SSA defs have global uses (targeted workaround for #126976).

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.

Show a summary per file
FileDescription
src/libraries/tests.projExcludes specific NativeAOT Windows ARM64 test projects per linked issue.
src/coreclr/nativeaot/Runtime/unix/unixasmmacrosarm64.incAdds PTFF flag definitions for saving x3–x7 on Unix ARM64.
src/coreclr/nativeaot/Runtime/unix/unixasmmacrosamd64.incAdds PTFF flag definitions for saving RSI/RDI/R8/R9 on Unix AMD64.
src/coreclr/nativeaot/Runtime/arm64/GcProbe.asmUpdates Windows ARM64 probe frame layout and hijack paths to preserve x0–x7 and q0–q7; avoids clobbering arg regs.
src/coreclr/nativeaot/Runtime/arm64/GcProbe.SUpdates Unix ARM64 probe frame layout and hijack paths to preserve x0–x7 and q0–q7; adjusts FixupHijackedCallstack.
src/coreclr/nativeaot/Runtime/amd64/GcProbe.asmUpdates Windows AMD64 probe frame to preserve RDX/R8/R9 and xmm0–xmm3; avoids clobbering volatile arg regs in hijack fixup.
src/coreclr/nativeaot/Runtime/amd64/GcProbe.SUpdates Unix AMD64 probe frame to preserve RSI/RDI/RDX/R8/R9 and xmm0–xmm7; adds CFI-aware save/restore around INLINE_GETTHREAD.
src/coreclr/nativeaot/Runtime/amd64/AsmMacros.incAdds PTFF flag definitions for saving RDX/R8/R9 on Windows AMD64.
src/coreclr/jit/targetarm64.hBroadens interface lookup helper trash set to all non-argument volatile regs.
src/coreclr/jit/targetamd64.hBroadens interface lookup helper trash set to all non-argument volatile regs (incl. FP/mask as applicable).
src/coreclr/jit/redundantbranchopts.cppBail out immediately from PHI-based jump threading when SSA defs have global uses.

Comment threadsrc/libraries/tests.proj
Comment threadsrc/coreclr/nativeaot/Runtime/arm64/GcProbe.S
Comment threadsrc/coreclr/jit/redundantbranchopts.cpp
Comment threadsrc/coreclr/nativeaot/Runtime/arm64/GcProbe.S
Comment threadsrc/coreclr/jit/redundantbranchopts.cpp Outdated
@mangod9

Copy link
Copy Markdown
Member

Overall this is now looking good, one Build Analysis failure which is not a known issue -- timeout in one of the agents.

@MichalStrehovsky

Copy link
Copy Markdown
Member

/ba-g devops timed out while waiting for helix but inspecting https://helix.dot.net/api/jobs/5e8abde5-94fb-4abf-9b8a-f7cbb124f091/workitems/nativeaot/files/console.0c33c2fb.log?api-version=2019-06-17 shows all test eventually passed

@MichalStrehovsky
MichalStrehovsky merged commit 31f66d2 into mainApr 17, 2026
148 of 160 checks passed
@MichalStrehovsky
MichalStrehovsky deleted the copilot/fix-fast-tail-call-candidates-test branch April 17, 2026 00:54
@github-actionsgithub-actionsBot locked and limited conversation to collaborators May 17, 2026
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Test failure: JIT/opt/FastTailCall/FastTailCallCandidates/FastTailCallCandidates.cmd

8 participants

@jkotas@VSadov@mangod9@jakobbotsch@MichalStrehovsky@AndyAyersMS