From 701124db95b1267af189746bb24854dbe0258e77 Mon Sep 17 00:00:00 2001 From: Clemens Hammacher Date: Fri, 31 Mar 2017 10:29:02 +0200 Subject: [PATCH] [wasm] [interpreter] Add stack overflow checks Add a limit to the number of nested call frames in the C++ wasm interpreter. Both the size of the value stack as well as the size of the block stack are limited per call frame. Thus, a limit on only the call frame stack is enough to limit the overall memory consumption of one interpreter instance. R=ahaas@chromium.org BUG=v8:5822 Change-Id: If9f7e547cd1d003bc2ae3c7586ece6b3cf3be587 Reviewed-on: https://chromium-review.googlesource.com/463486 Commit-Queue: Clemens Hammacher Reviewed-by: Andreas Haas Cr-Commit-Position: refs/heads/master@{#44296} --- src/isolate.cc | 13 +++++---- src/wasm/wasm-interpreter.cc | 37 +++++++++++++++++++++--- src/wasm/wasm-limits.h | 49 ++++++++++++++++++-------------- test/mjsunit/wasm/interpreter.js | 20 +++++++++++++ 4 files changed, 88 insertions(+), 31 deletions(-) diff --git a/src/isolate.cc b/src/isolate.cc index 73c15a4e1e..ce6d8d09b7 100644 --- a/src/isolate.cc +++ b/src/isolate.cc @@ -12,6 +12,7 @@ #include "src/assembler-inl.h" #include "src/ast/ast-value-factory.h" #include "src/ast/context-slot-cache.h" +#include "src/base/adapters.h" #include "src/base/hashmap.h" #include "src/base/platform/platform.h" #include "src/base/sys-info.h" @@ -464,7 +465,8 @@ Handle Isolate::CaptureSimpleStackTrace(Handle error_object, // function. List frames(FLAG_max_inlining_levels + 1); js_frame->Summarize(&frames); - for (int i = frames.length() - 1; i >= 0; i--) { + for (int i = frames.length() - 1; + i >= 0 && elements->FrameCount() < limit; i--) { const auto& summ = frames[i].AsJavaScript(); Handle fun = summ.function(); @@ -551,12 +553,11 @@ Handle Isolate::CaptureSimpleStackTrace(Handle error_object, // interpreted_stack is bottom-up, i.e. caller before callee. We need it // the other way around. - for (auto it = interpreted_stack.rbegin(), - end = interpreted_stack.rend(); - it != end; ++it) { + for (auto pair : base::Reversed(interpreted_stack)) { elements = FrameArray::AppendWasmFrame( - elements, instance, it->first, Handle::null(), - it->second, FrameArray::kIsWasmInterpretedFrame); + elements, instance, pair.first, Handle::null(), + pair.second, FrameArray::kIsWasmInterpretedFrame); + if (elements->FrameCount() >= limit) break; } } break; diff --git a/src/wasm/wasm-interpreter.cc b/src/wasm/wasm-interpreter.cc index a9b3b2212f..a72d1bf528 100644 --- a/src/wasm/wasm-interpreter.cc +++ b/src/wasm/wasm-interpreter.cc @@ -1406,13 +1406,17 @@ class ThreadImpl { } } - void DoCall(Decoder* decoder, InterpreterCode* target, pc_t* pc, - pc_t* limit) { + // Returns true if the call was successful, false if the stack check failed + // and the current activation was fully unwound. + bool DoCall(Decoder* decoder, InterpreterCode* target, pc_t* pc, + pc_t* limit) WARN_UNUSED_RESULT { frames_.back().pc = *pc; PushFrame(target); + if (!DoStackCheck()) return false; *pc = frames_.back().pc; *limit = target->end - target->start; decoder->Reset(target->start, target->end); + return true; } // Copies {arity} values on the top of the stack down the stack to {dest}, @@ -1475,6 +1479,30 @@ class ThreadImpl { return true; } + // Check if our control stack (frames_) exceeds the limit. Trigger stack + // overflow if it does, and unwinding the current frame. + // Returns true if execution can continue, false if the current activation was + // fully unwound. + // Do call this function immediately *after* pushing a new frame. The pc of + // the top frame will be reset to 0 if the stack check fails. + bool DoStackCheck() WARN_UNUSED_RESULT { + // Sum up the size of all dynamically growing structures. + if (V8_LIKELY(frames_.size() <= kV8MaxWasmInterpretedStackSize)) { + return true; + } + if (!codemap()->has_instance()) { + // In test mode: Just abort. + FATAL("wasm interpreter: stack overflow"); + } + // The pc of the top frame is initialized to the first instruction. We reset + // it to 0 here such that we report the same position as in compiled code. + frames_.back().pc = 0; + Isolate* isolate = codemap()->instance()->GetIsolate(); + HandleScope handle_scope(isolate); + isolate->StackOverflow(); + return HandleException(isolate) == WasmInterpreter::Thread::HANDLED; + } + void Execute(InterpreterCode* code, pc_t pc, int max) { Decoder decoder(code->start, code->end); pc_t limit = code->end - code->start; @@ -1683,7 +1711,7 @@ class ThreadImpl { if (result.type != ExternalCallResult::INTERNAL) break; } // Execute an internal call. - DoCall(&decoder, target, &pc, &limit); + if (!DoCall(&decoder, target, &pc, &limit)) return; code = target; PAUSE_IF_BREAK_FLAG(AfterCall); continue; // don't bump pc @@ -1698,7 +1726,8 @@ class ThreadImpl { switch (result.type) { case ExternalCallResult::INTERNAL: // The import is a function of this instance. Call it directly. - DoCall(&decoder, result.interpreter_code, &pc, &limit); + if (!DoCall(&decoder, result.interpreter_code, &pc, &limit)) + return; code = result.interpreter_code; PAUSE_IF_BREAK_FLAG(AfterCall); continue; // don't bump pc diff --git a/src/wasm/wasm-limits.h b/src/wasm/wasm-limits.h index bf657a8c5b..c2ecf61657 100644 --- a/src/wasm/wasm-limits.h +++ b/src/wasm/wasm-limits.h @@ -5,41 +5,48 @@ #ifndef V8_WASM_WASM_LIMITS_H_ #define V8_WASM_WASM_LIMITS_H_ +#include +#include +#include + namespace v8 { namespace internal { namespace wasm { // The following limits are imposed by V8 on WebAssembly modules. // The limits are agreed upon with other engines for consistency. -const size_t kV8MaxWasmTypes = 1000000; -const size_t kV8MaxWasmFunctions = 1000000; -const size_t kV8MaxWasmImports = 100000; -const size_t kV8MaxWasmExports = 100000; -const size_t kV8MaxWasmGlobals = 1000000; -const size_t kV8MaxWasmDataSegments = 100000; +constexpr size_t kV8MaxWasmTypes = 1000000; +constexpr size_t kV8MaxWasmFunctions = 1000000; +constexpr size_t kV8MaxWasmImports = 100000; +constexpr size_t kV8MaxWasmExports = 100000; +constexpr size_t kV8MaxWasmGlobals = 1000000; +constexpr size_t kV8MaxWasmDataSegments = 100000; // Don't use this limit directly, but use the value of FLAG_wasm_max_mem_pages. -const size_t kV8MaxWasmMemoryPages = 16384; // = 1 GiB -const size_t kV8MaxWasmStringSize = 100000; -const size_t kV8MaxWasmModuleSize = 1024 * 1024 * 1024; // = 1 GiB -const size_t kV8MaxWasmFunctionSize = 128 * 1024; -const size_t kV8MaxWasmFunctionLocals = 50000; -const size_t kV8MaxWasmFunctionParams = 1000; -const size_t kV8MaxWasmFunctionMultiReturns = 1000; -const size_t kV8MaxWasmFunctionReturns = 1; +constexpr size_t kV8MaxWasmMemoryPages = 16384; // = 1 GiB +constexpr size_t kV8MaxWasmStringSize = 100000; +constexpr size_t kV8MaxWasmModuleSize = 1024 * 1024 * 1024; // = 1 GiB +constexpr size_t kV8MaxWasmFunctionSize = 128 * 1024; +constexpr size_t kV8MaxWasmFunctionLocals = 50000; +constexpr size_t kV8MaxWasmFunctionParams = 1000; +constexpr size_t kV8MaxWasmFunctionMultiReturns = 1000; +constexpr size_t kV8MaxWasmFunctionReturns = 1; // Don't use this limit directly, but use the value of FLAG_wasm_max_table_size. -const size_t kV8MaxWasmTableSize = 10000000; -const size_t kV8MaxWasmTableEntries = 10000000; -const size_t kV8MaxWasmTables = 1; -const size_t kV8MaxWasmMemories = 1; +constexpr size_t kV8MaxWasmTableSize = 10000000; +constexpr size_t kV8MaxWasmTableEntries = 10000000; +constexpr size_t kV8MaxWasmTables = 1; +constexpr size_t kV8MaxWasmMemories = 1; -const size_t kSpecMaxWasmMemoryPages = 65536; -const size_t kSpecMaxWasmTableSize = 0xFFFFFFFFu; +constexpr size_t kSpecMaxWasmMemoryPages = 65536; +constexpr size_t kSpecMaxWasmTableSize = 0xFFFFFFFFu; -const uint64_t kWasmMaxHeapOffset = +constexpr uint64_t kWasmMaxHeapOffset = static_cast( std::numeric_limits::max()) // maximum base value + std::numeric_limits::max(); // maximum index value +// Limit the control stack size of the C++ wasm interpreter. +constexpr size_t kV8MaxWasmInterpretedStackSize = 64 * 1024; + } // namespace wasm } // namespace internal } // namespace v8 diff --git a/test/mjsunit/wasm/interpreter.js b/test/mjsunit/wasm/interpreter.js index 40d06e6dd5..c445f64558 100644 --- a/test/mjsunit/wasm/interpreter.js +++ b/test/mjsunit/wasm/interpreter.js @@ -309,3 +309,23 @@ function checkStack(stack, expected_lines) { ]); } })(); + +(function testInfiniteRecursion() { + var builder = new WasmModuleBuilder(); + + var direct = builder.addFunction('main', kSig_v_v) + .addBody([kExprNop, kExprCallFunction, 0]) + .exportFunc(); + var instance = builder.instantiate(); + + try { + instance.exports.main(); + assertUnreachable("should throw"); + } catch (e) { + if (!(e instanceof RangeError)) throw e; + checkStack(stripPath(e.stack), [ + 'RangeError: Maximum call stack size exceeded', + ' at main ([0]+0)' + ].concat(Array(9).fill(' at main ([0]+2)'))); + } +})();