From bd863f2c8ca3e24d02ea0cba90c2ec35675890b8 Mon Sep 17 00:00:00 2001 From: Larry Ewing Date: Thu, 23 Jul 2026 12:13:49 -0500 Subject: [PATCH 1/3] [wasm] Restrict enclosing-try throw-helper attach to the same funclet VisitWasmSuccs (added in #130945) makes a throw helper a successor of a try side-entry whenever the helper's keyed try region lexically encloses the side-entry (bbInTryRegions). bbInTryRegions is a purely lexical try-nesting test: it does not check that the helper and the side-entry live in the same function region (funclet). As a result a main-method throw helper for an outer try can be attached to a catch-resumption side-entry that merely nests inside that try but lives in a handler funclet. RPO layout then places the main-method helper inside the funclet, interleaving it with funclet blocks. That corrupts the emitted wasm: the funclet's terminal `end` is dropped (module fails "function body must end with end opcode") and branch target depths are miscomputed. Gate the enclosing-try relaxation on same-function-region ownership: only attach the helper when funcRegionOf(block) == funcRegionOf(acd->acdDstBlk). funcRegionOf mirrors funGetFuncIdx but works for an arbitrary block and distinguishes a filter funclet from its filter-handler. The exact-match path (the block's own region) is unchanged, and #130945's intended case (an inner-try side-entry pulling its enclosing try's helper within the same funclet) still matches. Verified on a standalone System.Data.Common R2R wasm crossgen (release JIT, without the end-emission change from #131251): baseline emits an invalid module (func 597 fails to validate); with this fix the module validates. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2e627b94-2658-41ce-a880-d9c27b7febd9 --- src/coreclr/jit/fgwasm.h | 44 ++++++++++++++++++++++++++++++++++++---- 1 file changed, 40 insertions(+), 4 deletions(-) diff --git a/src/coreclr/jit/fgwasm.h b/src/coreclr/jit/fgwasm.h index 5070be0da26f61..95ba33f6e63a82 100644 --- a/src/coreclr/jit/fgwasm.h +++ b/src/coreclr/jit/fgwasm.h @@ -431,6 +431,29 @@ BasicBlockVisit FgWasm::VisitWasmSuccs(Compiler* comp, BasicBlock* block, TFunc return false; }; + // The function region (funclet) index that physically contains 'block'. + // 0 == the main method. This mirrors funGetFuncIdx() but works for an + // arbitrary block (not just a funclet entry), distinguishing a filter + // funclet (FUNC_FILTER) from its filter-handler. + // + auto funcRegionOf = [comp](BasicBlock* blk) -> unsigned { + if (!blk->hasHndIndex()) + { + return 0; + } + + EHblkDsc* const eh = comp->ehGetDsc(blk->getHndIndex()); + unsigned funcIdx = eh->ebdFuncIndex; + + if (eh->HasFilter() && eh->InFilterRegionBBRange(blk)) + { + // The filter is the funclet immediately preceding its filter-handler. + funcIdx--; + } + + return funcIdx; + }; + // Special case throw helper blocks that are not yet connected in the flow graph. // Compiler::AddCodeDscMap* const acdMap = comp->fgGetAddCodeDscMap(); @@ -447,13 +470,15 @@ BasicBlockVisit FgWasm::VisitWasmSuccs(Compiler* comp, BasicBlock* block, TFunc for (const Compiler::AddCodeDscKey& key : Compiler::AddCodeDscMap::KeyIteration(acdMap)) { - const unsigned acdData = key.Data(); - bool matches = (acdData == blockData); + const unsigned acdData = key.Data(); + bool matches = (acdData == blockData); + bool viaEnclosingTry = false; if (!matches && isTrySideEntry && (key.Designator() == Compiler::AcdKeyDesignator::KD_TRY)) { // Also add edges from all enclosing try regions - matches = comp->bbInTryRegions(key.RegionIndex(), block); + matches = comp->bbInTryRegions(key.RegionIndex(), block); + viaEnclosingTry = matches; } if (matches) @@ -468,7 +493,18 @@ BasicBlockVisit FgWasm::VisitWasmSuccs(Compiler* comp, BasicBlock* block, TFunc // if (acd->acdUsed) { - RETURN_ON_ABORT(func(acd->acdDstBlk)); + // bbInTryRegions is a purely lexical try-nesting test, so an enclosing-try + // match can name a throw helper that lives in a different function region + // (funclet) than this side-entry -- e.g. a main-method helper for an outer + // try whose handler funclet merely nests inside that try. Attaching it here + // would pull the helper into this funclet's layout, interleaving it with + // funclet blocks and corrupting the emitted wasm. Only attach when the helper + // and the side-entry share the same function region. + // + if (!viaEnclosingTry || (funcRegionOf(block) == funcRegionOf(acd->acdDstBlk))) + { + RETURN_ON_ABORT(func(acd->acdDstBlk)); + } } } } From 53b73163e27040d62a814aa101d8fd6f2a01800e Mon Sep 17 00:00:00 2001 From: Larry Ewing Date: Thu, 23 Jul 2026 14:27:26 -0500 Subject: [PATCH 2/3] [wasm] Address review: extract bbIsInSameFunclet predicate, drop viaEnclosingTry Andy Ayers' review on #131279: 1. Extract the local funcRegionOf lambda in VisitWasmSuccs into a reusable Compiler::bbIsInSameFunclet(BasicBlock*, BasicBlock*) predicate (declared in compiler.h beside funGetFuncIdx, inline definition in compiler.hpp). It mirrors funGetFuncIdx but is valid for an arbitrary (non-entry) block and distinguishes a filter funclet from its filter-handler. 2. Drop the viaEnclosingTry guard and gate every match on bbIsInSameFunclet unconditionally. viaEnclosingTry restricted the same-funclet check to the enclosing-try relaxation only, out of blast-radius conservatism. A 194-assembly R2R-wasm crossgen A/B (guarded vs. unconditional) produced byte-identical output on all 193 emitted modules with zero new invalid modules or crossgen failures, so the guard never changed a direct-match outcome. Gating direct matches on same-funclet too is strictly more correct: a direct match naming a cross-funclet helper is exactly the mis-layout this fix prevents. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2e627b94-2658-41ce-a880-d9c27b7febd9 --- src/coreclr/jit/compiler.h | 1 + src/coreclr/jit/compiler.hpp | 30 +++++++++++++++++++++++ src/coreclr/jit/fgwasm.h | 47 +++++++++--------------------------- 3 files changed, 42 insertions(+), 36 deletions(-) diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 9894611a963450..127e3ec23e8237 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -9917,6 +9917,7 @@ class Compiler void funSetCurrentFunc(unsigned funcIdx); FuncInfoDsc* funGetFunc(unsigned funcIdx); unsigned int funGetFuncIdx(BasicBlock* block); + bool bbIsInSameFunclet(BasicBlock* block1, BasicBlock* block2); // LIVENESS diff --git a/src/coreclr/jit/compiler.hpp b/src/coreclr/jit/compiler.hpp index 8aad4186d05faf..7be92e22ab12fd 100644 --- a/src/coreclr/jit/compiler.hpp +++ b/src/coreclr/jit/compiler.hpp @@ -859,6 +859,36 @@ inline unsigned Compiler::funGetFuncIdx(BasicBlock* block) return funcIdx; } +/***************************************************************************** + * Are two blocks physically contained in the same function region (funclet)? + * The main method is region 0. Unlike funGetFuncIdx, this works for an + * arbitrary block (not just a funclet entry), distinguishing a filter + * funclet (FUNC_FILTER) from its filter-handler. Only valid after funclets + * are created. + * + */ +inline bool Compiler::bbIsInSameFunclet(BasicBlock* block1, BasicBlock* block2) +{ + auto funcRegionOf = [this](BasicBlock* blk) -> unsigned { + if (!blk->hasHndIndex()) + { + return 0; + } + + EHblkDsc* const eh = ehGetDsc(blk->getHndIndex()); + unsigned funcIdx = eh->ebdFuncIndex; + + if (eh->HasFilter() && eh->InFilterRegionBBRange(blk)) + { + // The filter is the funclet immediately preceding its filter-handler. + funcIdx--; + } + + return funcIdx; + }; + + return funcRegionOf(block1) == funcRegionOf(block2); +} #if HAS_FIXED_REGISTER_SET //------------------------------------------------------------------------------ // genRegNumFromMask : Maps a single register mask to a register number. diff --git a/src/coreclr/jit/fgwasm.h b/src/coreclr/jit/fgwasm.h index 95ba33f6e63a82..2832967bc0bfc3 100644 --- a/src/coreclr/jit/fgwasm.h +++ b/src/coreclr/jit/fgwasm.h @@ -431,29 +431,6 @@ BasicBlockVisit FgWasm::VisitWasmSuccs(Compiler* comp, BasicBlock* block, TFunc return false; }; - // The function region (funclet) index that physically contains 'block'. - // 0 == the main method. This mirrors funGetFuncIdx() but works for an - // arbitrary block (not just a funclet entry), distinguishing a filter - // funclet (FUNC_FILTER) from its filter-handler. - // - auto funcRegionOf = [comp](BasicBlock* blk) -> unsigned { - if (!blk->hasHndIndex()) - { - return 0; - } - - EHblkDsc* const eh = comp->ehGetDsc(blk->getHndIndex()); - unsigned funcIdx = eh->ebdFuncIndex; - - if (eh->HasFilter() && eh->InFilterRegionBBRange(blk)) - { - // The filter is the funclet immediately preceding its filter-handler. - funcIdx--; - } - - return funcIdx; - }; - // Special case throw helper blocks that are not yet connected in the flow graph. // Compiler::AddCodeDscMap* const acdMap = comp->fgGetAddCodeDscMap(); @@ -470,15 +447,13 @@ BasicBlockVisit FgWasm::VisitWasmSuccs(Compiler* comp, BasicBlock* block, TFunc for (const Compiler::AddCodeDscKey& key : Compiler::AddCodeDscMap::KeyIteration(acdMap)) { - const unsigned acdData = key.Data(); - bool matches = (acdData == blockData); - bool viaEnclosingTry = false; + const unsigned acdData = key.Data(); + bool matches = (acdData == blockData); if (!matches && isTrySideEntry && (key.Designator() == Compiler::AcdKeyDesignator::KD_TRY)) { // Also add edges from all enclosing try regions - matches = comp->bbInTryRegions(key.RegionIndex(), block); - viaEnclosingTry = matches; + matches = comp->bbInTryRegions(key.RegionIndex(), block); } if (matches) @@ -493,15 +468,15 @@ BasicBlockVisit FgWasm::VisitWasmSuccs(Compiler* comp, BasicBlock* block, TFunc // if (acd->acdUsed) { - // bbInTryRegions is a purely lexical try-nesting test, so an enclosing-try - // match can name a throw helper that lives in a different function region - // (funclet) than this side-entry -- e.g. a main-method helper for an outer - // try whose handler funclet merely nests inside that try. Attaching it here - // would pull the helper into this funclet's layout, interleaving it with - // funclet blocks and corrupting the emitted wasm. Only attach when the helper - // and the side-entry share the same function region. + // bbThrowIndex / bbInTryRegions are purely lexical tests, so a match can + // name a throw helper that lives in a different function region (funclet) + // than this side-entry -- e.g. a main-method helper for an outer try whose + // handler funclet merely nests inside that try. Attaching it here would pull + // the helper into this funclet's layout, interleaving it with funclet blocks + // and corrupting the emitted wasm. Only attach when the helper and the + // side-entry share the same function region. // - if (!viaEnclosingTry || (funcRegionOf(block) == funcRegionOf(acd->acdDstBlk))) + if (comp->bbIsInSameFunclet(block, acd->acdDstBlk)) { RETURN_ON_ABORT(func(acd->acdDstBlk)); } From db3450fb64fdefeabddb270f476e766fb308b015 Mon Sep 17 00:00:00 2001 From: Larry Ewing Date: Thu, 23 Jul 2026 14:42:25 -0500 Subject: [PATCH 3/3] [wasm] Assert fgFuncletsCreated in bbIsInSameFunclet The helper is documented as valid only after funclets are created; assert it like funGetFunc/funGetFuncIdx do, to catch misuse early instead of silently returning region 0 for blocks with unset funclet metadata. The sole caller (VisitWasmSuccs, in PHASE_WASM_CONTROL_FLOW) runs long after PHASE_CREATE_FUNCLETS, so the invariant always holds. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2e627b94-2658-41ce-a880-d9c27b7febd9 --- src/coreclr/jit/compiler.hpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/coreclr/jit/compiler.hpp b/src/coreclr/jit/compiler.hpp index 7be92e22ab12fd..ecd0a3eeb94024 100644 --- a/src/coreclr/jit/compiler.hpp +++ b/src/coreclr/jit/compiler.hpp @@ -869,6 +869,8 @@ inline unsigned Compiler::funGetFuncIdx(BasicBlock* block) */ inline bool Compiler::bbIsInSameFunclet(BasicBlock* block1, BasicBlock* block2) { + assert(fgFuncletsCreated); + auto funcRegionOf = [this](BasicBlock* blk) -> unsigned { if (!blk->hasHndIndex()) {