From c4568e43b9e67fc36a4c9dc01d92e4517fac5033 Mon Sep 17 00:00:00 2001 From: Jakob Kummerow Date: Wed, 17 Mar 2021 13:01:57 +0100 Subject: [PATCH] [wasm][liftoff][eh] Fix locals in FinishTryCatch When dropping the exception from the stack, we have to take locals into account when computing the right stack slot. Fixed: chromium:1187836 Change-Id: I76acb1e4dc50992524123cc369dea8e51242164c Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2764749 Commit-Queue: Jakob Kummerow Reviewed-by: Clemens Backes Reviewed-by: Thibaud Michaud Cr-Commit-Position: refs/heads/master@{#73469} --- src/wasm/baseline/liftoff-assembler.h | 2 ++ src/wasm/baseline/liftoff-compiler.cc | 2 +- test/mjsunit/wasm/exceptions.js | 28 +++++++++++++++++++++++++-- 3 files changed, 29 insertions(+), 3 deletions(-) diff --git a/src/wasm/baseline/liftoff-assembler.h b/src/wasm/baseline/liftoff-assembler.h index 8fb2a0bcb9..b07f0cd1bd 100644 --- a/src/wasm/baseline/liftoff-assembler.h +++ b/src/wasm/baseline/liftoff-assembler.h @@ -422,6 +422,8 @@ class LiftoffAssembler : public TurboAssembler { void DropValues(int count); + // Careful: this indexes "from the other end", i.e. depth=0 is the value + // at the bottom of the stack! void DropValue(int depth); // Ensure that the loop inputs are either in a register or spilled to the diff --git a/src/wasm/baseline/liftoff-compiler.cc b/src/wasm/baseline/liftoff-compiler.cc index 0511fec402..1f554e0093 100644 --- a/src/wasm/baseline/liftoff-compiler.cc +++ b/src/wasm/baseline/liftoff-compiler.cc @@ -1302,7 +1302,7 @@ class LiftoffCompiler { if (!c->end_merge.reached) { if (c->try_info->catch_reached) { // Drop the implicit exception ref. - __ DropValue(c->stack_depth + c->num_exceptions); + __ DropValue(__ num_locals() + c->stack_depth + c->num_exceptions); } // Else we did not enter the catch state, continue with the current state. } else { diff --git a/test/mjsunit/wasm/exceptions.js b/test/mjsunit/wasm/exceptions.js index 36f470ac0b..7115c54a1f 100644 --- a/test/mjsunit/wasm/exceptions.js +++ b/test/mjsunit/wasm/exceptions.js @@ -440,6 +440,7 @@ load("test/mjsunit/wasm/exceptions-utils.js"); let builder = new WasmModuleBuilder(); let except = builder.addException(kSig_v_l); builder.addFunction("throw_catch_param", kSig_i_i) + .addLocals(kWasmI64, 1) .addBody([ kExprLocalGet, 0, kExprI64UConvertI32, @@ -457,7 +458,7 @@ load("test/mjsunit/wasm/exceptions-utils.js"); kExprI32Const, 0, kExprEnd, kExprEnd, - ]).addLocals(kWasmI64, 1).exportFunc(); + ]).exportFunc(); let instance = builder.instantiate(); assertEquals(1, instance.exports.throw_catch_param(5)); @@ -663,6 +664,7 @@ load("test/mjsunit/wasm/exceptions-utils.js"); // p == 2 -> path == 298 // p == 3 -> path == 338 // else -> path == 146 + .addLocals(kWasmI32, 1) .addBody([ kExprTry, kWasmI32, kExprTry, kWasmI32, @@ -719,7 +721,6 @@ load("test/mjsunit/wasm/exceptions-utils.js"); kExprI32Ior, kExprEnd, ]) - .addLocals(kWasmI32, 1) .exportFunc(); // Scenario 2: Catches an exception raised from the direct callee. @@ -1067,3 +1068,26 @@ load("test/mjsunit/wasm/exceptions-utils.js"); let instance = builder.instantiate(); })(); + +(function TestThrowWithLocal() { + print(arguments.callee.name); + let builder = new WasmModuleBuilder(); + let except = builder.addException(kSig_v_v); + builder.addFunction('throw_with_local', kSig_i_v) + .addLocals(kWasmI32, 4) + .addBody([ + kExprI32Const, 42, + kExprF64Const, 0, 0, 0, 0, 0, 0, 0, 0, + kExprTry, kWasmF32, + kExprThrow, except, + kExprCatchAll, + kExprF32Const, 0, 0, 0, 0, + kExprEnd, + kExprDrop, // Drop the f32. + kExprDrop, // Drop the f64. + // Leave the '42' on the stack. + ]).exportFunc(); + + let instance = builder.instantiate(); + assertEquals(42, instance.exports.throw_with_local()); +})();