From bb187a106edf9d1d5fb23b567bb457b260289ff6 Mon Sep 17 00:00:00 2001 From: ahaas Date: Fri, 19 Aug 2016 01:54:22 -0700 Subject: [PATCH] [wasm] Add stack checks at the beginning of each function. TEST=mjsunit/wasm/stack.js:testStackOverflow R=titzer@chromium.org Review-Url: https://codereview.chromium.org/2256603002 Cr-Commit-Position: refs/heads/master@{#38742} --- src/compiler/wasm-compiler.cc | 42 +++++++++++++++++++++++++++++++---- src/compiler/wasm-compiler.h | 2 ++ src/wasm/ast-decoder.cc | 3 +++ test/mjsunit/wasm/stack.js | 22 ++++++++++++++++++ 4 files changed, 65 insertions(+), 4 deletions(-) diff --git a/src/compiler/wasm-compiler.cc b/src/compiler/wasm-compiler.cc index 07df3857d3..cd2cd81a70 100644 --- a/src/compiler/wasm-compiler.cc +++ b/src/compiler/wasm-compiler.cc @@ -66,7 +66,7 @@ void MergeControlToEnd(JSGraph* jsgraph, Node* node) { Node* BuildCallToRuntime(Runtime::FunctionId f, JSGraph* jsgraph, Handle context, Node** parameters, int parameter_count, Node** effect_ptr, - Node** control_ptr) { + Node* control) { // At the moment we only allow 2 parameters. If more parameters are needed, // then the size of {inputs} below has to be increased accordingly. DCHECK(parameter_count <= 2); @@ -88,7 +88,7 @@ Node* BuildCallToRuntime(Runtime::FunctionId f, JSGraph* jsgraph, inputs[count++] = jsgraph->Int32Constant(fun->nargs); // arity inputs[count++] = jsgraph->HeapConstant(context); // context inputs[count++] = *effect_ptr; - inputs[count++] = *control_ptr; + inputs[count++] = control; Node* node = jsgraph->graph()->NewNode(jsgraph->common()->Call(desc), count, inputs); @@ -263,7 +263,7 @@ class WasmTrapHelper : public ZoneObject { trap_position_smi}; // byte position BuildCallToRuntime(Runtime::kThrowWasmError, jsgraph(), module->instance->context, parameters, - arraysize(parameters), effect_ptr, control_ptr); + arraysize(parameters), effect_ptr, *control_ptr); } if (false) { // End the control flow with a throw @@ -388,6 +388,40 @@ Node* WasmGraphBuilder::Int64Constant(int64_t value) { return jsgraph()->Int64Constant(value); } +void WasmGraphBuilder::StackCheck(wasm::WasmCodePosition position) { + // We do not generate stack checks for cctests. + if (module_ && !module_->instance->context.is_null()) { + Node* limit = graph()->NewNode( + jsgraph()->machine()->Load(MachineType::Pointer()), + jsgraph()->ExternalConstant( + ExternalReference::address_of_stack_limit(jsgraph()->isolate())), + jsgraph()->IntPtrConstant(0), *effect_, *control_); + Node* pointer = graph()->NewNode(jsgraph()->machine()->LoadStackPointer()); + + Node* check = + graph()->NewNode(jsgraph()->machine()->UintLessThan(), limit, pointer); + + Diamond stack_check(graph(), jsgraph()->common(), check, BranchHint::kTrue); + + Node* effect_true = *effect_; + + Node* effect_false; + // Generate a call to the runtime if there is a stack check failure. + { + Node* node = BuildCallToRuntime(Runtime::kStackGuard, jsgraph(), + module_->instance->context, nullptr, 0, + effect_, stack_check.if_false); + effect_false = node; + } + + Node* ephi = graph()->NewNode(jsgraph()->common()->EffectPhi(2), + effect_true, effect_false, stack_check.merge); + + *control_ = stack_check.merge; + *effect_ = ephi; + } +} + Node* WasmGraphBuilder::Binop(wasm::WasmOpcode opcode, Node* left, Node* right, wasm::WasmCodePosition position) { const Operator* op; @@ -2205,7 +2239,7 @@ Node* WasmGraphBuilder::ToJS(Node* node, wasm::LocalType type) { // Throw a TypeError. return BuildCallToRuntime(Runtime::kWasmThrowTypeError, jsgraph(), module_->instance->context, nullptr, 0, effect_, - control_); + *control_); case wasm::kAstF32: node = graph()->NewNode(jsgraph()->machine()->ChangeFloat32ToFloat64(), node); diff --git a/src/compiler/wasm-compiler.h b/src/compiler/wasm-compiler.h index a764f30256..aaa51f27ec 100644 --- a/src/compiler/wasm-compiler.h +++ b/src/compiler/wasm-compiler.h @@ -137,6 +137,8 @@ class WasmGraphBuilder { void AppendToMerge(Node* merge, Node* from); void AppendToPhi(Node* phi, Node* from); + void StackCheck(wasm::WasmCodePosition position); + //----------------------------------------------------------------------- // Operations that read and/or write {control} and {effect}. //----------------------------------------------------------------------- diff --git a/src/wasm/ast-decoder.cc b/src/wasm/ast-decoder.cc index 740199c298..0f192508ba 100644 --- a/src/wasm/ast-decoder.cc +++ b/src/wasm/ast-decoder.cc @@ -559,6 +559,9 @@ class WasmFullDecoder : public WasmDecoder { ssa_env->control = start; ssa_env->effect = start; SetEnv("initial", ssa_env); + if (builder_) { + builder_->StackCheck(position()); + } } TFNode* DefaultValue(LocalType type) { diff --git a/test/mjsunit/wasm/stack.js b/test/mjsunit/wasm/stack.js index 4ff0d1d4fd..0197b77caf 100644 --- a/test/mjsunit/wasm/stack.js +++ b/test/mjsunit/wasm/stack.js @@ -125,3 +125,25 @@ Error.prepareStackTrace = function(error, frames) { ]); } })(); + + +(function testStackOverflow() { + print("testStackOverflow"); + var builder = new WasmModuleBuilder(); + + var sig_index = builder.addType(kSig_v_v); + builder.addFunction("recursion", sig_index) + .addBody([ + kExprI32Const, 0, + kExprCallIndirect, kArity0, sig_index + ]) + .exportFunc() + builder.appendToTable([0]); + + try { + builder.instantiate().exports.recursion(); + fail("expected wasm exception"); + } catch (e) { + assertEquals("Maximum call stack size exceeded", e.message, "trap reason"); + } +})();