Commit 793c52ed authored by Andreas Haas's avatar Andreas Haas Committed by Commit Bot

[wasm] Improve stack check in the interpreter

The existing stack check only checked the number of stack frames on the
stack, not the actual size of the stack frames. In the test case, each
stack frame is huge, and the interpreter runs out of memory before the
stack check stops the execution. With this change we take the size of
the value stack and the size of the control stack and compare their sum
to the stack limit of V8. Note that this stack limit is kind of
arbitrary, because the stack space of the interpreter is not on the
actual runtime stack but allocated in zone memory, and the stack check
exists to simulate stack overflows in compiled code, not to prevent
actual stack overflows.

R=clemensh@chromium.org
TEST=mjsunit/regress/wasm/regress-778917

Bug: chromium:778917
Change-Id: Ife47631fcb1a178a68facab1e42c0069b12c0155
Reviewed-on: https://chromium-review.googlesource.com/744003
Commit-Queue: Andreas Haas <ahaas@chromium.org>
Reviewed-by: 's avatarClemens Hammacher <clemensh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#49071}
parent fd5b067f
......@@ -1523,8 +1523,15 @@ class ThreadImpl {
// 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)) {
// The goal of this stack check is not to prevent actual stack overflows,
// but to simulate stack overflows during the execution of compiled code.
// That is why this function uses FLAG_stack_size, even though the value
// stack actually lies in zone memory.
const size_t stack_size_limit = FLAG_stack_size * KB;
// Sum up the value stack size and the control stack size.
const size_t current_stack_size =
(sp_ - stack_start_) + frames_.size() * sizeof(Frame);
if (V8_LIKELY(current_stack_size <= stack_size_limit)) {
return true;
}
if (!codemap()->has_instance()) {
......
......@@ -48,9 +48,6 @@ constexpr uint64_t kWasmMaxHeapOffset =
std::numeric_limits<uint32_t>::max()) // maximum base value
+ std::numeric_limits<uint32_t>::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
......
// Copyright 2017 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.
// Flags: --expose-wasm --wasm-interpret-all
load("test/mjsunit/wasm/wasm-constants.js");
load("test/mjsunit/wasm/wasm-module-builder.js");
const builder = new WasmModuleBuilder();
const index = builder.addFunction("huge_frame", kSig_v_v)
.addBody([kExprCallFunction, 0])
.addLocals({f64_count: 49555}).exportFunc().index;
// We assume above that the function we added has index 0.
assertEquals(0, index);
const module = builder.instantiate();
assertThrows(module.exports.huge_frame, RangeError);
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment