Skip to content

Commit 231862a

Browse files
committed
fast-interp: reject br/br_if/br_table to loop entry from inside try-region
When a br skips over a try-region's END, the runtime br doesn't pop eh-stack entries. For a one-shot br to a block / function-end / catch, the leaked entry is absorbed by the static `exception_handler_count * EH_ENTRY_CELLS` reservation and dies at frame teardown — a load-time `LOG_WARNING` surfaces the shape for embedders. If the br target is a LOOP entry, however, every iteration's TRY push adds one more entry to the eh-stack. After more iterations than the function's `exception_handler_count`, the next TRY push writes past the static reservation. `bh_assert(eh_count < count)` catches this in debug builds, but is a no-op without `BH_DEBUG` — release builds silently corrupt whatever sat past the reservation in the frame allocation. This commit changes that pathological shape from "log a warning and accept" to "fail load with an explicit error". The check sits next to the existing `count_try_blocks_crossed > 0` warning at all three branch sites (BR, BR_IF, BR_TABLE) and only fires when `frame_csp_tmp->label_type == LABEL_TYPE_LOOP`. The error message is identical at each site modulo opcode name: "br[_if|_table] to loop entry from inside try-region not supported in fast interpreter (would leak eh-stack entries per iteration)" Emitting a synthetic eh-stack pop at the br site would be the other fix and would let valid modules with this shape run, but it complicates the rewritten IR's br-info layout (the br dispatch currently emits a single uint32 depth; a pop-count immediate would need a per-target lookup) and the shape is rare in practice. Rejecting at load is the conservative, App-Store-safe choice — embedders see a deterministic error rather than silent memory corruption. Test added in the external integration suite: the previously- ignored `br_out_of_try_inside_loop` became `br_out_of_try_inside_loop_rejected`, which asserts the loader fails with the expected error string. Codex P1 review feedback on both PRs ("Reject branches that leak EH entries" / "Reject branches that leak EH stack entries").
1 parent 54bac97 commit 231862a

1 file changed

Lines changed: 51 additions & 16 deletions

File tree

core/iwasm/interpreter/wasm_loader.c

Lines changed: 51 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -13433,20 +13433,37 @@ wasm_loader_prepare_bytecode(WASMModule *module, WASMFunction *func,
1343313433
goto fail;
1343413434

1343513435
#if WASM_ENABLE_EXCE_HANDLING != 0 && WASM_ENABLE_FAST_INTERP != 0
13436-
/* Warn (pass 1 only — once per br site) when a br
13437-
* skips over a try-region's END. The runtime br
13438-
* doesn't pop eh-stack entries, so each leaked entry
13439-
* relies on the static
13440-
* `exception_handler_count * EH_ENTRY_CELLS` cell
13441-
* reservation per frame to absorb it. Pathological
13442-
* shape: `loop { try { br_to_loop_top } catch }`
13443-
* leaks one entry per iteration and eventually writes
13444-
* past that reservation. See `count_try_blocks_
13445-
* crossed` for the full mechanism. */
13446-
if (loader_ctx->p_code_compiled == NULL) {
13436+
/* When a br skips over a try-region's END, the
13437+
* runtime br doesn't pop eh-stack entries. For a
13438+
* one-shot br to a block / function-end / catch,
13439+
* the leaked entry is absorbed by the static
13440+
* `exception_handler_count * EH_ENTRY_CELLS`
13441+
* reservation and dies at frame teardown — log
13442+
* a warning so the shape shows up in load-time
13443+
* diagnostics, but accept the module.
13444+
*
13445+
* If the br target is a LOOP entry, however,
13446+
* every iteration's TRY push adds one more entry
13447+
* to the eh-stack and eventually overwrites past
13448+
* the static reservation (silently in release
13449+
* builds since `bh_assert` is a no-op without
13450+
* `BH_DEBUG`). Reject those modules at load time
13451+
* — emitting cleanup at the br site would be the
13452+
* other fix, but it complicates the hot dispatch
13453+
* loop and the shape is rare in practice. */
13454+
{
1344713455
uint32 leaked = count_try_blocks_crossed(
1344813456
loader_ctx->frame_csp - 1, frame_csp_tmp);
13449-
if (leaked > 0) {
13457+
if (leaked > 0
13458+
&& frame_csp_tmp->label_type == LABEL_TYPE_LOOP) {
13459+
set_error_buf(error_buf, error_buf_size,
13460+
"br to loop entry from inside "
13461+
"try-region not supported in fast "
13462+
"interpreter (would leak eh-stack "
13463+
"entries per iteration)");
13464+
goto fail;
13465+
}
13466+
if (leaked > 0 && loader_ctx->p_code_compiled == NULL) {
1345013467
LOG_WARNING("wasm fast-interp: br at func[%u] crosses "
1345113468
"%u try-region(s); each leaks one "
1345213469
"eh-stack entry until frame teardown",
@@ -13470,10 +13487,19 @@ wasm_loader_prepare_bytecode(WASMModule *module, WASMFunction *func,
1347013487
goto fail;
1347113488

1347213489
#if WASM_ENABLE_EXCE_HANDLING != 0 && WASM_ENABLE_FAST_INTERP != 0
13473-
if (loader_ctx->p_code_compiled == NULL) {
13490+
{
1347413491
uint32 leaked = count_try_blocks_crossed(
1347513492
loader_ctx->frame_csp - 1, frame_csp_tmp);
13476-
if (leaked > 0) {
13493+
if (leaked > 0
13494+
&& frame_csp_tmp->label_type == LABEL_TYPE_LOOP) {
13495+
set_error_buf(error_buf, error_buf_size,
13496+
"br_if to loop entry from inside "
13497+
"try-region not supported in fast "
13498+
"interpreter (would leak eh-stack "
13499+
"entries per iteration)");
13500+
goto fail;
13501+
}
13502+
if (leaked > 0 && loader_ctx->p_code_compiled == NULL) {
1347713503
LOG_WARNING(
1347813504
"wasm fast-interp: br_if at func[%u] crosses "
1347913505
"%u try-region(s); each leaks one "
@@ -13550,10 +13576,19 @@ wasm_loader_prepare_bytecode(WASMModule *module, WASMFunction *func,
1355013576
}
1355113577

1355213578
#if WASM_ENABLE_EXCE_HANDLING != 0 && WASM_ENABLE_FAST_INTERP != 0
13553-
if (loader_ctx->p_code_compiled == NULL) {
13579+
{
1355413580
uint32 leaked = count_try_blocks_crossed(
1355513581
loader_ctx->frame_csp - 1, frame_csp_tmp);
13556-
if (leaked > 0) {
13582+
if (leaked > 0
13583+
&& frame_csp_tmp->label_type == LABEL_TYPE_LOOP) {
13584+
set_error_buf(error_buf, error_buf_size,
13585+
"br_table to loop entry from inside "
13586+
"try-region not supported in fast "
13587+
"interpreter (would leak eh-stack "
13588+
"entries per iteration)");
13589+
goto fail;
13590+
}
13591+
if (leaked > 0 && loader_ctx->p_code_compiled == NULL) {
1355713592
LOG_WARNING(
1355813593
"wasm fast-interp: br_table[%u] at "
1355913594
"func[%u] crosses %u try-region(s); each "

0 commit comments

Comments
 (0)