JIT/wasm: Pri-1 R2R codegen fixes (BBJ_CALLFINALLY back-edge, null-check loads)#129807
Open
AndyAyersMS wants to merge 2 commits into
Open
JIT/wasm: Pri-1 R2R codegen fixes (BBJ_CALLFINALLY back-edge, null-check loads)#129807AndyAyersMS wants to merge 2 commits into
AndyAyersMS wants to merge 2 commits into
Conversation
…llyret BBJ_CALLFINALLYRET emits no code via genCodeForBlock's early return, relying on fall-through to the continuation. On wasm the layout may place the continuation elsewhere (e.g., a loop header reached via back-edge from a catch handler that leaves to a label outside its enclosing try-finally), in which case fall-through skips past the target. Emit an explicit branch in genCallFinally when the CALLFINALLYRET's continuation is not the linear next block. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
On wasm a load from a null address does not fault, so the array-length read in a decoupled bounds check returned 0 and threw IndexOutOfRange instead of NullReference. Emit an explicit null check for faulting loads, matching GT_STOREIND (lower marks the address multiply-used, regalloc reserves the temp, codegen emits the check using the same predicate as the throw-helper reservation phase). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
github-actions
Bot
added
the
area-CodeGen-coreclr
Member
Author
|
@adamperlin PTAL |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the CoreCLR WASM JIT backend to address two correctness issues that arise from WASM’s different control-flow and faulting semantics compared to native targets (notably: call_indirect finally invocation and non-faulting loads from address 0).
Changes:
- Ensure
BBJ_CALLFINALLYcodegen emits an explicit branch to theBBJ_CALLFINALLYRETcontinuation when that continuation is not the next block in the linear (preorder) block order. - Add explicit null-check emission for faulting
GT_INDloads in WASM codegen, mirroring existing explicit null-check handling for faulting stores. - Plumb the required “multiply-used address” temp consumption through lowering and WASM regalloc so the address is available both for the null check and the subsequent load.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/coreclr/jit/regallocwasm.cpp | Consumes the temporary register for multiply-used GT_IND addresses so the null-check+load sequence can share the address value. |
| src/coreclr/jit/lowerwasm.cpp | Marks faulting GT_IND load addresses as multiply-used so codegen can both null-check and load using the same computed address. |
| src/coreclr/jit/codegenwasm.cpp | Emits explicit null checks for faulting loads and emits a corrective branch after call_indirect finally calls when the continuation is non-fallthrough. |
Contributor
|
Tagging subscribers to 'arch-wasm': @lewing, @pavelsavara |
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.
Two wasm JIT codegen correctness fixes surfaced by the Pri-1 R2R test pass.
BBJ_CALLFINALLY non-fallthrough back-edge
genCodeForBlockearly-returns forBBJ_CALLFINALLYRETassuming fall-through tothe next linear block. On wasm the finally is invoked via
call_indirect, so whenthe paired continuation is reached via a back-edge (e.g. a catch leave targeting a
loop header outside the enclosing try/finally) no branch was emitted and control
fell out of the wasm loop, running a cloned finally a second time. Emit a
brtothe continuation when it is not the next linear block. Fixes the double-finally in
strswitch_catchrettoinnertryand similar.Null-check faulting GT_IND loads
On wasm a load from a null address does not fault, so a decoupled array bounds
check read the length from
[null+lenOffset]as 0 and threwIndexOutOfRangeExceptioninstead ofNullReferenceException.genCodeForStoreIndalready emits an explicit null check for faulting stores; do the same for faulting
GT_INDloads (lower marks the address multiply-used, regalloc reserves the temp,codegen emits the check using the same
GTF_EXCEPT && OperMayThrowpredicate thethrow-helper reservation phase uses). Fixes
b42929,b41063,b31903,b091942, and theJitTest_catchfinally_*array-null tests.