From 2dd32644225dc993dcbb1dce1595cf5fd3da02d1 Mon Sep 17 00:00:00 2001 From: Michael Starzinger Date: Mon, 28 Jan 2019 14:59:04 +0100 Subject: [PATCH] [wasm] Preliminary interpreter support for exceptions. This adds preliminary support for exception handling to the interpreter. Note that due to missing reference type support, the exception object is not yet correctly put onto the operand stack. Also exceptions raised by call operations are not handled properly yet. R=clemensh@chromium.org TEST=cctest/test-run-wasm-exceptions BUG=v8:8091 Change-Id: Ie68ca9448c7beafe8967dff5bb5de6642edcc9e4 Reviewed-on: https://chromium-review.googlesource.com/c/1436017 Commit-Queue: Michael Starzinger Reviewed-by: Clemens Hammacher Cr-Commit-Position: refs/heads/master@{#59131} --- src/wasm/function-body-decoder-impl.h | 2 +- src/wasm/wasm-interpreter.cc | 113 +++++++++++++++++-- test/cctest/BUILD.gn | 1 + test/cctest/wasm/test-run-wasm-exceptions.cc | 37 ++++++ test/cctest/wasm/test-run-wasm-js.cc | 25 ---- test/cctest/wasm/wasm-run-utils.cc | 44 ++++++++ test/cctest/wasm/wasm-run-utils.h | 10 ++ test/common/wasm/wasm-macro-gen.h | 8 +- 8 files changed, 201 insertions(+), 39 deletions(-) create mode 100644 test/cctest/wasm/test-run-wasm-exceptions.cc diff --git a/src/wasm/function-body-decoder-impl.h b/src/wasm/function-body-decoder-impl.h index a76b3fbbfe..dab189d566 100644 --- a/src/wasm/function-body-decoder-impl.h +++ b/src/wasm/function-body-decoder-impl.h @@ -1295,7 +1295,6 @@ class WasmDecoder : public Decoder { case kExprIf: case kExprRethrow: return {1, 0}; - case kExprCatch: case kExprGetLocal: case kExprGetGlobal: case kExprI32Const: @@ -1329,6 +1328,7 @@ class WasmDecoder : public Decoder { case kExprEnd: case kExprElse: case kExprTry: + case kExprCatch: case kExprBrOnExn: case kExprNop: case kExprReturn: diff --git a/src/wasm/wasm-interpreter.cc b/src/wasm/wasm-interpreter.cc index a34e07db44..e47c492164 100644 --- a/src/wasm/wasm-interpreter.cc +++ b/src/wasm/wasm-interpreter.cc @@ -638,6 +638,8 @@ const char* OpcodeName(uint32_t val) { return WasmOpcodes::OpcodeName(static_cast(val)); } +constexpr uint32_t kCatchInArity = 1; + } // namespace class SideTable; @@ -754,6 +756,11 @@ class SideTable : public ZoneObject { // bytecodes with their target, as well as determining whether the current // bytecodes are within the true or false block of an else. ZoneVector control_stack(&control_transfer_zone); + // It also maintains a stack of all nested {try} blocks to resolve local + // handler targets for potentially throwing operations. These exceptional + // control transfers are treated just like other branches in the resulting + // map. This stack contains indices into the above control stack. + ZoneVector exception_stack(zone); uint32_t stack_height = 0; uint32_t func_arity = static_cast(code->function->sig->return_count()); @@ -839,6 +846,53 @@ class SideTable : public ZoneObject { stack_height = c->end_label->target_stack_height; break; } + case kExprTry: { + BlockTypeImmediate imm(kAllWasmFeatures, &i, + i.pc()); + if (imm.type == kWasmVar) { + imm.sig = module->signatures[imm.sig_index]; + } + TRACE("control @%u: Try, arity %d->%d\n", i.pc_offset(), + imm.in_arity(), imm.out_arity()); + CLabel* end_label = CLabel::New(&control_transfer_zone, stack_height, + imm.out_arity()); + CLabel* catch_label = + CLabel::New(&control_transfer_zone, stack_height, kCatchInArity); + control_stack.emplace_back(i.pc(), end_label, catch_label, + imm.out_arity()); + exception_stack.push_back(control_stack.size() - 1); + copy_unreachable(); + break; + } + case kExprCatch: { + DCHECK_EQ(control_stack.size() - 1, exception_stack.back()); + Control* c = &control_stack.back(); + exception_stack.pop_back(); + copy_unreachable(); + TRACE("control @%u: Catch\n", i.pc_offset()); + if (!control_parent().unreachable) { + c->end_label->Ref(i.pc(), stack_height); + } + DCHECK_NOT_NULL(c->else_label); + c->else_label->Bind(i.pc() + 1); + c->else_label->Finish(&map_, code->orig_start); + c->else_label = nullptr; + DCHECK_GE(stack_height, c->end_label->target_stack_height); + stack_height = c->end_label->target_stack_height + kCatchInArity; + break; + } + case kExprThrow: + case kExprRethrow: { + if (exception_stack.empty()) break; // Nothing to do here. + // TODO(mstarzinger): The same needs to be done for calls, not only + // for "throw" and "rethrow". Factor this logic out accordingly. + DCHECK_GE(control_stack.size() - 1, exception_stack.back()); + Control* c = &control_stack[exception_stack.back()]; + if (!unreachable) c->else_label->Ref(i.pc(), stack_height); + TRACE("handler @%u: %s -> try @%u\n", i.pc_offset(), + OpcodeName(opcode), static_cast(c->pc - code->start)); + break; + } case kExprEnd: { Control* c = &control_stack.back(); TRACE("control @%u: End\n", i.pc_offset()); @@ -894,6 +948,11 @@ class SideTable : public ZoneObject { DCHECK_EQ(func_arity, stack_height); } + bool HasEntryAt(pc_t from) { + auto result = map_.find(from); + return result != map_.end(); + } + ControlTransferEntry& Lookup(pc_t from) { auto result = map_.find(from); DCHECK(result != map_.end()); @@ -1211,8 +1270,16 @@ class ThreadImpl { WasmInterpreter::Thread::ExceptionHandlingResult HandleException( Isolate* isolate) { DCHECK(isolate->has_pending_exception()); - // TODO(wasm): Add wasm exception handling (would return HANDLED). - USE(isolate->pending_exception()); + InterpreterCode* code = frames_.back().code; + if (code->side_table->HasEntryAt(frames_.back().pc)) { + TRACE("----- HANDLE -----\n"); + // TODO(mstarzinger): Push a reference to the pending exception instead of + // the bogus {int32_t(0)} value here once the interpreter supports it. + USE(isolate->pending_exception()); + Push(WasmValue(int32_t{0})); + isolate->clear_pending_exception(); + return WasmInterpreter::Thread::HANDLED; + } TRACE("----- UNWIND -----\n"); DCHECK_LT(0, activations_.size()); Activation& act = activations_.back(); @@ -1326,6 +1393,13 @@ class ThreadImpl { return static_cast(code->side_table->Lookup(pc).pc_diff); } + int JumpToHandlerDelta(InterpreterCode* code, pc_t pc) { + ControlTransferEntry& control_transfer_entry = code->side_table->Lookup(pc); + DoStackTransfer(sp_ - (control_transfer_entry.sp_diff + kCatchInArity), + control_transfer_entry.target_arity); + return control_transfer_entry.pc_diff; + } + int DoBreak(InterpreterCode* code, pc_t pc, size_t depth) { ControlTransferEntry& control_transfer_entry = code->side_table->Lookup(pc); DoStackTransfer(sp_ - control_transfer_entry.sp_diff, @@ -2095,7 +2169,7 @@ class ThreadImpl { } // Allocate, initialize and throw a new exception. The exception values are - // being popped off the operand stack. Returns true of the exception is being + // being popped off the operand stack. Returns true if the exception is being // handled locally by the interpreter, false otherwise (interpreter exits). bool DoThrowException(const WasmException* exception, uint32_t index) V8_WARN_UNUSED_RESULT { @@ -2151,6 +2225,16 @@ class ThreadImpl { return HandleException(isolate) == WasmInterpreter::Thread::HANDLED; } + // Throw a given existing exception. Returns true if the exception is being + // handled locally by the interpreter, false otherwise (interpreter exits). + bool DoRethrowException(WasmValue* exception) { + Isolate* isolate = instance_object_->GetIsolate(); + // TODO(mstarzinger): Use the passed {exception} here once reference types + // as values on the operand stack are supported by the interpreter. + isolate->ReThrow(*isolate->factory()->undefined_value()); + return HandleException(isolate) == WasmInterpreter::Thread::HANDLED; + } + void Execute(InterpreterCode* code, pc_t pc, int max) { DCHECK_NOT_NULL(code->side_table); DCHECK(!frames_.empty()); @@ -2223,13 +2307,9 @@ class ThreadImpl { switch (orig) { case kExprNop: break; - case kExprBlock: { - BlockTypeImmediate imm(kAllWasmFeatures, - &decoder, code->at(pc)); - len = 1 + imm.length; - break; - } - case kExprLoop: { + case kExprBlock: + case kExprLoop: + case kExprTry: { BlockTypeImmediate imm(kAllWasmFeatures, &decoder, code->at(pc)); len = 1 + imm.length; @@ -2250,7 +2330,8 @@ class ThreadImpl { } break; } - case kExprElse: { + case kExprElse: + case kExprCatch: { len = LookupTargetDelta(code, pc); TRACE(" end => @%zu\n", pc + len); break; @@ -2258,9 +2339,17 @@ class ThreadImpl { case kExprThrow: { ExceptionIndexImmediate imm(&decoder, code->at(pc)); - len = 1 + imm.length; + CommitPc(pc); // Needed for local unwinding. const WasmException* exception = &module()->exceptions[imm.index]; if (!DoThrowException(exception, imm.index)) return; + len = JumpToHandlerDelta(code, pc); + break; + } + case kExprRethrow: { + WasmValue ex = Pop(); + CommitPc(pc); // Needed for local unwinding. + if (!DoRethrowException(&ex)) return; + len = JumpToHandlerDelta(code, pc); break; } case kExprSelect: { diff --git a/test/cctest/BUILD.gn b/test/cctest/BUILD.gn index 9c18ce5806..4daa0cdd94 100644 --- a/test/cctest/BUILD.gn +++ b/test/cctest/BUILD.gn @@ -248,6 +248,7 @@ v8_source_set("cctest_sources") { "wasm/test-run-wasm-asmjs.cc", "wasm/test-run-wasm-atomics.cc", "wasm/test-run-wasm-atomics64.cc", + "wasm/test-run-wasm-exceptions.cc", "wasm/test-run-wasm-interpreter.cc", "wasm/test-run-wasm-js.cc", "wasm/test-run-wasm-module.cc", diff --git a/test/cctest/wasm/test-run-wasm-exceptions.cc b/test/cctest/wasm/test-run-wasm-exceptions.cc new file mode 100644 index 0000000000..2ad16b679a --- /dev/null +++ b/test/cctest/wasm/test-run-wasm-exceptions.cc @@ -0,0 +1,37 @@ +// Copyright 2019 the V8 project authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "test/cctest/wasm/wasm-atomics-utils.h" +#include "test/common/wasm/test-signatures.h" +#include "test/common/wasm/wasm-macro-gen.h" + +namespace v8 { +namespace internal { +namespace wasm { +namespace test_run_wasm_exceptions { + +WASM_EXEC_TEST(TryCatchThrow) { + TestSignatures sigs; + EXPERIMENTAL_FLAG_SCOPE(eh); + WasmRunner r(execution_tier); + uint32_t except = r.builder().AddException(sigs.v_v()); + constexpr uint32_t kResult0 = 23; + constexpr uint32_t kResult1 = 42; + + BUILD(r, WASM_TRY_CATCH_T(kWasmI32, + WASM_STMTS(WASM_I32V(kResult1), + WASM_IF(WASM_I32_EQZ(WASM_GET_LOCAL(0)), + WASM_THROW(except))), + WASM_STMTS(WASM_DROP, WASM_I32V(kResult0)))); + + // Need to call through JS to allow for creation of stack traces. + Handle jsfunc = r.builder().WrapCode(r.function()->func_index); + EXPECT_CALL(kResult0, jsfunc, 0); + EXPECT_CALL(kResult1, jsfunc, 1); +} + +} // namespace test_run_wasm_exceptions +} // namespace wasm +} // namespace internal +} // namespace v8 diff --git a/test/cctest/wasm/test-run-wasm-js.cc b/test/cctest/wasm/test-run-wasm-js.cc index 499942464e..36b0b18db2 100644 --- a/test/cctest/wasm/test-run-wasm-js.cc +++ b/test/cctest/wasm/test-run-wasm-js.cc @@ -69,31 +69,6 @@ ManuallyImportedJSFunction CreateJSSelector(FunctionSig* sig, int which) { return import; } - -void EXPECT_CALL(double expected, Handle jsfunc, - Handle* buffer, int count) { - Isolate* isolate = jsfunc->GetIsolate(); - Handle global(isolate->context()->global_object(), isolate); - MaybeHandle retval = - Execution::Call(isolate, jsfunc, global, count, buffer); - - CHECK(!retval.is_null()); - Handle result = retval.ToHandleChecked(); - if (result->IsSmi()) { - CHECK_EQ(expected, Smi::ToInt(*result)); - } else { - CHECK(result->IsHeapNumber()); - CHECK_FLOAT_EQ(expected, HeapNumber::cast(*result)->value()); - } -} - -void EXPECT_CALL(double expected, Handle jsfunc, double a, - double b) { - Isolate* isolate = jsfunc->GetIsolate(); - Handle buffer[] = {isolate->factory()->NewNumber(a), - isolate->factory()->NewNumber(b)}; - EXPECT_CALL(expected, jsfunc, buffer, 2); -} } // namespace WASM_EXEC_TEST(Run_Int32Sub_jswrapped) { diff --git a/test/cctest/wasm/wasm-run-utils.cc b/test/cctest/wasm/wasm-run-utils.cc index 5212953b7c..475560490f 100644 --- a/test/cctest/wasm/wasm-run-utils.cc +++ b/test/cctest/wasm/wasm-run-utils.cc @@ -189,6 +189,18 @@ uint32_t TestingModuleBuilder::AddBytes(Vector bytes) { return bytes_offset; } +uint32_t TestingModuleBuilder::AddException(FunctionSig* sig) { + DCHECK_EQ(0, sig->return_count()); + uint32_t index = static_cast(test_module_->exceptions.size()); + test_module_->exceptions.push_back(WasmException{sig}); + Handle tag = WasmExceptionTag::New(isolate_, index); + Handle table(instance_object_->exceptions_table(), isolate_); + table = isolate_->factory()->CopyFixedArrayAndGrow(table, 1); + instance_object_->set_exceptions_table(*table); + table->set(index, *tag); + return index; +} + CompilationEnv TestingModuleBuilder::CreateCompilationEnv() { return { test_module_ptr_, @@ -221,6 +233,7 @@ Handle TestingModuleBuilder::InitInstanceObject() { native_module_->ReserveCodeTableForTesting(kMaxFunctions); auto instance = WasmInstanceObject::New(isolate_, module_object); + instance->set_exceptions_table(*isolate_->factory()->empty_fixed_array()); instance->set_globals_start(globals_data_); return instance; } @@ -472,6 +485,37 @@ FunctionSig* WasmRunnerBase::CreateSig(MachineType return_type, // static bool WasmRunnerBase::trap_happened; +void EXPECT_CALL(double expected, Handle jsfunc, + Handle* buffer, int count) { + Isolate* isolate = jsfunc->GetIsolate(); + Handle global(isolate->context()->global_object(), isolate); + MaybeHandle retval = + Execution::Call(isolate, jsfunc, global, count, buffer); + + CHECK(!retval.is_null()); + Handle result = retval.ToHandleChecked(); + if (result->IsSmi()) { + CHECK_EQ(expected, Smi::ToInt(*result)); + } else { + CHECK(result->IsHeapNumber()); + CHECK_FLOAT_EQ(expected, HeapNumber::cast(*result)->value()); + } +} + +void EXPECT_CALL(double expected, Handle jsfunc, double a, + double b) { + Isolate* isolate = jsfunc->GetIsolate(); + Handle buffer[] = {isolate->factory()->NewNumber(a), + isolate->factory()->NewNumber(b)}; + EXPECT_CALL(expected, jsfunc, buffer, 2); +} + +void EXPECT_CALL(double expected, Handle jsfunc, double a) { + Isolate* isolate = jsfunc->GetIsolate(); + Handle buffer[] = {isolate->factory()->NewNumber(a)}; + EXPECT_CALL(expected, jsfunc, buffer, 1); +} + } // namespace wasm } // namespace internal } // namespace v8 diff --git a/test/cctest/wasm/wasm-run-utils.h b/test/cctest/wasm/wasm-run-utils.h index af575fff77..55cd83d96f 100644 --- a/test/cctest/wasm/wasm-run-utils.h +++ b/test/cctest/wasm/wasm-run-utils.h @@ -39,6 +39,7 @@ #include "test/cctest/cctest.h" #include "test/cctest/compiler/call-tester.h" #include "test/cctest/compiler/graph-builder-tester.h" +#include "test/cctest/compiler/value-helper.h" #include "test/common/wasm/flag-utils.h" namespace v8 { @@ -184,6 +185,8 @@ class TestingModuleBuilder { uint32_t AddBytes(Vector bytes); + uint32_t AddException(FunctionSig* sig); + WasmFunction* GetFunctionAt(int index) { return &test_module_->functions[index]; } @@ -491,6 +494,13 @@ class WasmRunner : public WasmRunnerBase { Handle GetWrapperCode() { return wrapper_.GetWrapperCode(); } }; +// TODO(mstarzinger): Rename these since they are not macros but functions. +void EXPECT_CALL(double expected, Handle jsfunc, + Handle* buffer, int count); +void EXPECT_CALL(double expected, Handle jsfunc, double a, + double b); +void EXPECT_CALL(double expected, Handle jsfunc, double a); + // A macro to define tests that run in different engine configurations. #define WASM_EXEC_TEST(name) \ void RunWasm_##name(ExecutionTier execution_tier); \ diff --git a/test/common/wasm/wasm-macro-gen.h b/test/common/wasm/wasm-macro-gen.h index 17045ac325..d67553eb9e 100644 --- a/test/common/wasm/wasm-macro-gen.h +++ b/test/common/wasm/wasm-macro-gen.h @@ -135,6 +135,10 @@ #define WASM_IF_ELSE_X(index, cond, tstmt, fstmt) \ cond, kExprIf, static_cast(index), tstmt, kExprElse, fstmt, kExprEnd +#define WASM_TRY_CATCH_T(t, trystmt, catchstmt) \ + kExprTry, static_cast(ValueTypes::ValueTypeCodeFor(t)), trystmt, \ + kExprCatch, catchstmt, kExprEnd + #define WASM_SELECT(tval, fval, cond) tval, fval, cond, kExprSelect #define WASM_RETURN0 kExprReturn @@ -154,10 +158,12 @@ #define WASM_CASE(x) static_cast(x), static_cast(x >> 8) #define WASM_CASE_BR(x) static_cast(x), static_cast(0x80 | (x) >> 8) +#define WASM_THROW(index) kExprThrow, static_cast(index) + //------------------------------------------------------------------------------ // Misc expressions. //------------------------------------------------------------------------------ -#define WASM_ID(...) __VA_ARGS__ +#define WASM_STMTS(...) __VA_ARGS__ #define WASM_ZERO kExprI32Const, 0 #define WASM_ONE kExprI32Const, 1