[wasm] Restore TARGET_WASM short-circuit in VirtualUnwindCallFrame#129551
Closed
lewing wants to merge 3 commits into
Closed
[wasm] Restore TARGET_WASM short-circuit in VirtualUnwindCallFrame#129551lewing wants to merge 3 commits into
lewing wants to merge 3 commits into
Conversation
PR dotnet#128423 (Use Virtual IPs in R2R format) removed the TARGET_WASM short-circuit at the top of Thread::VirtualUnwindCallFrame(T_CONTEXT*) on the assumption that all WASM builds now ship R2R unwind info that the function can walk. That is not the case for the interpreter-only WASI corerun: there is no R2R, and the function has no RtlVirtualUnwind equivalent to fall back to. Restore the assert + early-return-0 (matching the original code from PR dotnet#113429) so the function is a hard error instead of corrupting the register display by calling RtlVirtualUnwind on an unrelated context. On WASM EH stack-walking proceeds through the explicit Frame chain (Thread::GetFrame()), so this overload is not reached on the working path; restoring the short-circuit only protects against future bugs that would otherwise show up as silent register-display corruption. This is defensive and not a fix for the wider interp+EH integration gap on WASI (managed try/catch still aborts in SfiNextWorker at the 'unhandled by runtime' branch; that needs separate work). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR reintroduces a WebAssembly-specific guard in Thread::VirtualUnwindCallFrame(T_CONTEXT*) to avoid attempting to unwind call frames on WASM in unsupported scenarios.
Changes:
- Adds a
TARGET_WASMearly-exit path inThread::VirtualUnwindCallFrame(T_CONTEXT*). - Wraps the existing non-WASM implementation under
#elseand closes it with a trailing#endif.
jkotas
reviewed
Jun 18, 2026
jkotas
reviewed
Jun 18, 2026
| ARM64_ARG(TADDR * pSpForPacSign /*= NULL*/)) | ||
| { | ||
| #ifdef TARGET_WASM | ||
| // Interpreter-only WASM has no R2R unwind info to walk and no |
Member
There was a problem hiding this comment.
This is talking about Interpreter-only WASM, but ifdef is TARGET_WASM that includes R2R.
Member
Author
There was a problem hiding this comment.
Good catch — fixed in 3c1e28b. Narrowed the guard with if (!ExecutionManager::IsVirtualIP(GetIP(pContext))) so the short-circuit only fires for non-virtual-IP contexts (the interpreter-only case), and R2R-WASM frames flow through to the WASM RtlVirtualUnwind in vm/wasm/helpers.cpp as intended.
Note
AI-generated content by GitHub Copilot CLI.
Co-authored-by: Jan Kotas <jkotas@microsoft.com>
Addresses review feedback from @jkotas: the previous guard short- circuited the entire method for all TARGET_WASM, but CoreCLR has a working WASM RtlVirtualUnwind in vm/wasm/helpers.cpp that handles R2R-WASM frames keyed by virtual IPs. Blanket-disabling this method prevented unwinding in the valid R2R path as well. Check ExecutionManager::IsVirtualIP(GetIP(pContext)) at entry and only fail fast when the context's control PC is not a virtual IP — that is the interpreter-only case (e.g., WASI corerun without R2R) where the RtlVirtualUnwind body would assert anyway and continuing past it would corrupt the register display. R2R-WASM contexts now flow through the regular path and reach the WASM RtlVirtualUnwind as intended. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Member
Author
|
Pushed 3c1e28b addressing the review:
Smoke-tested locally:
Note AI-generated content by GitHub Copilot CLI. |
This was referenced Jun 18, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR #128423 (Use Virtual IPs in R2R format) removed the TARGET_WASM short-circuit at the top of Thread::VirtualUnwindCallFrame(T_CONTEXT*) on the assumption that all WASM builds now ship R2R unwind info that the function can walk. That is not the case for the interpreter-only WASI corerun: there is no R2R, and the function has no RtlVirtualUnwind equivalent to fall back to.
Restore the assert + early-return-0 (matching the original code from PR #113429) so the function is a hard error instead of corrupting the register display by calling RtlVirtualUnwind on an unrelated context. On WASM EH stack-walking proceeds through the explicit Frame chain (Thread::GetFrame()), so this overload is not reached on the working path; restoring the short-circuit only protects against future bugs that would otherwise show up as silent register-display corruption.
This is defensive and not a fix for the wider interp+EH integration gap on WASI (managed try/catch still aborts in SfiNextWorker at the 'unhandled by runtime' branch; that needs separate work).