From 47b53b9c055d89ca7631569720675f6af6142c3c Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Tue, 23 Jun 2026 19:23:17 -0700 Subject: [PATCH 1/3] JIT/wasm: emit branch in BBJ_CALLFINALLY for non-fallthrough callfinallyret 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> --- src/coreclr/jit/codegenwasm.cpp | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/src/coreclr/jit/codegenwasm.cpp b/src/coreclr/jit/codegenwasm.cpp index 7a0bbb5efeb440..610c84f30db879 100644 --- a/src/coreclr/jit/codegenwasm.cpp +++ b/src/coreclr/jit/codegenwasm.cpp @@ -3587,6 +3587,29 @@ void CodeGen::genCallFinally(BasicBlock* block) { GetEmitter()->emitIns(INS_end); } + + return; + } + + // After call_indirect returns, control falls through to the next wasm instruction. + // The paired BBJ_CALLFINALLYRET has no code emitted (see genCodeForBlock), so the + // CALLFINALLYRET's continuation must be reached either by fall-through (when it's + // the next block in linear order) or by an explicit branch emitted here. + // + // This is wasm-specific: other targets rely on the native `call`/`ret` to land at + // the instruction after the call, and the JIT layout places the CALLFINALLYRET's + // continuation immediately after. On wasm the layout may place the continuation + // elsewhere (e.g. a loop header reached via back-edge, as produced by a leave from + // a catch handler that targets a label outside its enclosing try-finally). + // + assert(block->isBBCallFinallyPair()); + BasicBlock* const callFinallyRet = block->Next(); + assert(callFinallyRet->KindIs(BBJ_CALLFINALLYRET)); + BasicBlock* const continuation = callFinallyRet->GetTarget(); + + if (continuation->bbPreorderNum != callFinallyRet->bbPreorderNum + 1) + { + inst_JMP(EJ_jmp, continuation); } } From 40434fb630e11eb8bf88c408a4db4b9532e43ceb Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Wed, 24 Jun 2026 09:42:51 -0700 Subject: [PATCH 2/3] JIT/wasm: null check faulting GT_IND loads 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> --- src/coreclr/jit/codegenwasm.cpp | 10 ++++++++++ src/coreclr/jit/lowerwasm.cpp | 7 +++++++ src/coreclr/jit/regallocwasm.cpp | 9 +++++++++ 3 files changed, 26 insertions(+) diff --git a/src/coreclr/jit/codegenwasm.cpp b/src/coreclr/jit/codegenwasm.cpp index 610c84f30db879..22badc840cac9c 100644 --- a/src/coreclr/jit/codegenwasm.cpp +++ b/src/coreclr/jit/codegenwasm.cpp @@ -2652,6 +2652,16 @@ void CodeGen::genCodeForIndir(GenTreeIndir* tree) genConsumeAddress(tree->Addr()); + // On wasm a load from a null address does not fault, so emit an explicit null check + // for faulting loads (mirrors genCodeForStoreInd). Use the same predicate the throw- + // helper reservation phase uses (StackLevelSetter), so we never demand an unreserved + // throw helper block here. + if (((tree->gtFlags & GTF_EXCEPT) != 0) && tree->OperMayThrow(m_compiler)) + { + regNumber addrReg = GetMultiUseOperandReg(tree->Addr()); + genEmitNullCheck(addrReg); + } + // TODO-WASM: Memory barriers GetEmitter()->emitIns_I(ins, emitActualTypeSize(type), 0); diff --git a/src/coreclr/jit/lowerwasm.cpp b/src/coreclr/jit/lowerwasm.cpp index 78cda8482f64d6..1932e39b99aa42 100644 --- a/src/coreclr/jit/lowerwasm.cpp +++ b/src/coreclr/jit/lowerwasm.cpp @@ -458,6 +458,13 @@ void Lowering::ContainCheckIndir(GenTreeIndir* indirNode) return; } + if (indirNode->OperIs(GT_IND) && ((indirNode->gtFlags & GTF_IND_NONFAULTING) == 0)) + { + // On wasm a load from a null address does not fault, so a faulting load needs an + // explicit null check, which requires multiple uses of the address operand. + SetMultiplyUsed(indirNode->Addr() DEBUGARG("ContainCheckIndir faulting load Addr")); + } + // TODO-WASM-CQ: contain suitable LEAs here. Take note of the fact that for this to be correct we must prove the // LEA doesn't overflow. It will involve creating a new frontend node to represent "nuw" (offset) addition. } diff --git a/src/coreclr/jit/regallocwasm.cpp b/src/coreclr/jit/regallocwasm.cpp index f8cac164887670..485d2c63cfe841 100644 --- a/src/coreclr/jit/regallocwasm.cpp +++ b/src/coreclr/jit/regallocwasm.cpp @@ -490,6 +490,15 @@ void WasmRegAlloc::CollectReferencesForNode(GenTree* node) CollectReferencesForStoreInd(node->AsStoreInd()); break; + case GT_IND: + if (node->AsIndir()->Addr()->gtLIRFlags & LIR::Flags::MultiplyUsed) + { + // Faulting loads are null checked on wasm; consume the temporary register + // holding the address so it is available for both the check and the load. + ConsumeTemporaryRegForOperand(node->AsIndir()->Addr() DEBUGARG("indir load null check")); + } + break; + case GT_STORE_BLK: CollectReferencesForBlockStore(node->AsBlk()); break; From bec8b9fb966191b91ddd0f9ae0e9f96961b5f17d Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Thu, 25 Jun 2026 14:19:24 -0700 Subject: [PATCH 3/3] JIT/wasm: address codegen review feedback Align wasm null-check helper reservation with codegen and fold the indirection regalloc handling shared by loads and stores. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/coreclr/jit/codegenwasm.cpp | 20 +++----------- src/coreclr/jit/lowerwasm.cpp | 2 -- src/coreclr/jit/regallocwasm.cpp | 20 +++++--------- src/coreclr/jit/regallocwasm.h | 2 +- src/coreclr/jit/stacklevelsetter.cpp | 40 ++++++++++++++++++++++++++-- src/coreclr/jit/stacklevelsetter.h | 1 + 6 files changed, 49 insertions(+), 36 deletions(-) diff --git a/src/coreclr/jit/codegenwasm.cpp b/src/coreclr/jit/codegenwasm.cpp index 22badc840cac9c..c5207235cd3f07 100644 --- a/src/coreclr/jit/codegenwasm.cpp +++ b/src/coreclr/jit/codegenwasm.cpp @@ -2652,11 +2652,7 @@ void CodeGen::genCodeForIndir(GenTreeIndir* tree) genConsumeAddress(tree->Addr()); - // On wasm a load from a null address does not fault, so emit an explicit null check - // for faulting loads (mirrors genCodeForStoreInd). Use the same predicate the throw- - // helper reservation phase uses (StackLevelSetter), so we never demand an unreserved - // throw helper block here. - if (((tree->gtFlags & GTF_EXCEPT) != 0) && tree->OperMayThrow(m_compiler)) + if ((tree->gtFlags & GTF_IND_NONFAULTING) == 0) { regNumber addrReg = GetMultiUseOperandReg(tree->Addr()); genEmitNullCheck(addrReg); @@ -3601,23 +3597,13 @@ void CodeGen::genCallFinally(BasicBlock* block) return; } - // After call_indirect returns, control falls through to the next wasm instruction. - // The paired BBJ_CALLFINALLYRET has no code emitted (see genCodeForBlock), so the - // CALLFINALLYRET's continuation must be reached either by fall-through (when it's - // the next block in linear order) or by an explicit branch emitted here. - // - // This is wasm-specific: other targets rely on the native `call`/`ret` to land at - // the instruction after the call, and the JIT layout places the CALLFINALLYRET's - // continuation immediately after. On wasm the layout may place the continuation - // elsewhere (e.g. a loop header reached via back-edge, as produced by a leave from - // a catch handler that targets a label outside its enclosing try-finally). - // + // Branch to the continuation block if it's not the next block. assert(block->isBBCallFinallyPair()); BasicBlock* const callFinallyRet = block->Next(); assert(callFinallyRet->KindIs(BBJ_CALLFINALLYRET)); BasicBlock* const continuation = callFinallyRet->GetTarget(); - if (continuation->bbPreorderNum != callFinallyRet->bbPreorderNum + 1) + if (continuation != callFinallyRet->Next()) { inst_JMP(EJ_jmp, continuation); } diff --git a/src/coreclr/jit/lowerwasm.cpp b/src/coreclr/jit/lowerwasm.cpp index 1932e39b99aa42..6818460dfa94f0 100644 --- a/src/coreclr/jit/lowerwasm.cpp +++ b/src/coreclr/jit/lowerwasm.cpp @@ -460,8 +460,6 @@ void Lowering::ContainCheckIndir(GenTreeIndir* indirNode) if (indirNode->OperIs(GT_IND) && ((indirNode->gtFlags & GTF_IND_NONFAULTING) == 0)) { - // On wasm a load from a null address does not fault, so a faulting load needs an - // explicit null check, which requires multiple uses of the address operand. SetMultiplyUsed(indirNode->Addr() DEBUGARG("ContainCheckIndir faulting load Addr")); } diff --git a/src/coreclr/jit/regallocwasm.cpp b/src/coreclr/jit/regallocwasm.cpp index 485d2c63cfe841..accca547daf71f 100644 --- a/src/coreclr/jit/regallocwasm.cpp +++ b/src/coreclr/jit/regallocwasm.cpp @@ -486,17 +486,9 @@ void WasmRegAlloc::CollectReferencesForNode(GenTree* node) CollectReferencesForBinop(node->AsOp()); break; - case GT_STOREIND: - CollectReferencesForStoreInd(node->AsStoreInd()); - break; - case GT_IND: - if (node->AsIndir()->Addr()->gtLIRFlags & LIR::Flags::MultiplyUsed) - { - // Faulting loads are null checked on wasm; consume the temporary register - // holding the address so it is available for both the check and the load. - ConsumeTemporaryRegForOperand(node->AsIndir()->Addr() DEBUGARG("indir load null check")); - } + case GT_STOREIND: + CollectReferencesForIndir(node->AsIndir()); break; case GT_STORE_BLK: @@ -657,15 +649,15 @@ void WasmRegAlloc::CollectReferencesForBinop(GenTreeOp* binopNode) } //------------------------------------------------------------------------ -// CollectReferencesForStoreInd: Collect virtual register references for an indirect store +// CollectReferencesForIndir: Collect virtual register references for an indirection. // // Arguments: -// node - The GT_STOREIND node +// node - The indirection node. // -void WasmRegAlloc::CollectReferencesForStoreInd(GenTreeStoreInd* node) +void WasmRegAlloc::CollectReferencesForIndir(GenTreeIndir* node) { GenTree* const addr = node->Addr(); - ConsumeTemporaryRegForOperand(addr DEBUGARG("storeind null check")); + ConsumeTemporaryRegForOperand(addr DEBUGARG("indirection address")); } //------------------------------------------------------------------------ diff --git a/src/coreclr/jit/regallocwasm.h b/src/coreclr/jit/regallocwasm.h index bc0f399b07e5ae..344a51260955fc 100644 --- a/src/coreclr/jit/regallocwasm.h +++ b/src/coreclr/jit/regallocwasm.h @@ -155,7 +155,7 @@ class WasmRegAlloc : public RegAllocInterface void CollectReferencesForCall(GenTreeCall* callNode); void CollectReferencesForCast(GenTreeOp* castNode); void CollectReferencesForBinop(GenTreeOp* binOpNode); - void CollectReferencesForStoreInd(GenTreeStoreInd* node); + void CollectReferencesForIndir(GenTreeIndir* node); void CollectReferencesForBlockStore(GenTreeBlk* node); void CollectReferencesForLclVar(GenTreeLclVar* lclVar); void CollectReferencesForIndexAddr(GenTreeIndexAddr* indexAddrNode); diff --git a/src/coreclr/jit/stacklevelsetter.cpp b/src/coreclr/jit/stacklevelsetter.cpp index 32f7fcf73dc244..91ae68a7e35b0a 100644 --- a/src/coreclr/jit/stacklevelsetter.cpp +++ b/src/coreclr/jit/stacklevelsetter.cpp @@ -190,7 +190,7 @@ void StackLevelSetter::ProcessBlock(BasicBlock* block) if (checkForHelpers) { - if (((node->gtFlags & GTF_EXCEPT) != 0) && node->OperMayThrow(m_compiler)) + if (MayUseThrowHelperBlock(node)) { SetThrowHelperBlocks(node, block); } @@ -199,6 +199,42 @@ void StackLevelSetter::ProcessBlock(BasicBlock* block) assert(currentStackLevel == 0); } +//------------------------------------------------------------------------ +// MayUseThrowHelperBlock: Check whether codegen may branch to a throw helper block. +// +// Arguments: +// node - The node to process. +// +// Return Value: +// True if the node may branch to a throw helper block. +// +bool StackLevelSetter::MayUseThrowHelperBlock(GenTree* node) +{ + if (((node->gtFlags & GTF_EXCEPT) != 0) && node->OperMayThrow(m_compiler)) + { + return true; + } + +#if defined(TARGET_WASM) + switch (node->OperGet()) + { + case GT_NULLCHECK: + case GT_IND: + case GT_STORE_BLK: + case GT_STOREIND: + return (node->gtFlags & GTF_IND_NONFAULTING) == 0; + + case GT_CALL: + return node->AsCall()->NeedsNullCheck(); + + default: + break; + } +#endif // defined(TARGET_WASM) + + return false; +} + //------------------------------------------------------------------------ // SetThrowHelperBlocks: Set throw helper blocks incoming stack levels targeted // from the node. @@ -213,7 +249,7 @@ void StackLevelSetter::ProcessBlock(BasicBlock* block) // void StackLevelSetter::SetThrowHelperBlocks(GenTree* node, BasicBlock* block) { - assert(node->OperMayThrow(m_compiler)); + assert(MayUseThrowHelperBlock(node)); // Check that it uses throw block, find its kind, find the block, set level. switch (node->OperGet()) diff --git a/src/coreclr/jit/stacklevelsetter.h b/src/coreclr/jit/stacklevelsetter.h index 55fdeb8428a274..339ae10fdc443a 100644 --- a/src/coreclr/jit/stacklevelsetter.h +++ b/src/coreclr/jit/stacklevelsetter.h @@ -17,6 +17,7 @@ class StackLevelSetter final : public Phase void ProcessBlocks(); void ProcessBlock(BasicBlock* block); + bool MayUseThrowHelperBlock(GenTree* node); void SetThrowHelperBlocks(GenTree* node, BasicBlock* block); void SetThrowHelperBlock(SpecialCodeKind kind, BasicBlock* block);