Skip to content

Commit

Permalink
Merge pull request #669 from wasmx/correct_max_call_depth
Browse files Browse the repository at this point in the history 
Correct max call depth
  • Loading branch information
@chfast
chfast authored Dec 29, 2020
2 parents 879829b + 4e411f0 commit f8f26c5
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 26 deletions.
2 changes: 1 addition & 1 deletion lib/fizzy/execute.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -504,7 +504,7 @@ inline bool invoke_function(const FuncType& func_type, uint32_t func_idx, Instan
ExecutionResult execute(Instance& instance, FuncIdx func_idx, const Value* args, int depth)
{
assert(depth >= 0);
if (depth > CallStackLimit)
if (depth >= CallStackLimit)
return Trap;

const auto& func_type = instance.module->get_function_type(func_idx);
Expand Down
8 changes: 4 additions & 4 deletions lib/fizzy/limits.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@ static_assert(MemoryPagesValidationLimit == 65536);
/// The default hard limit of the memory size (256MB) as number of pages.
constexpr uint32_t DefaultMemoryPagesLimit = (256 * 1024 * 1024ULL) / PageSize;

/// Call depth limit is set to default limit in wabt.
/// See
/// https://github.com/WebAssembly/wabt/blob/ae2140ddc6969ef53599fe2fab81818de65db875/src/interp/interp.h#L1007
// TODO: review this
/// The limit of the size of the call stack, i.e. how many calls are allowed to be stacked up
/// in a single execution thread. Allowed values for call depth levels are [0, CallStackLimit-1].
/// The current value is the same as the default limit in WABT:
/// https://github.com/WebAssembly/wabt/blob/1.0.20/src/interp/interp.h#L1027
constexpr int CallStackLimit = 2048;
} // namespace fizzy
42 changes: 21 additions & 21 deletions test/unittests/execute_call_test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -605,8 +605,8 @@ TEST(execute_call, call_indirect_infinite_recursion)
EXPECT_TRUE(execute(module, 0, {}).trapped);
}

constexpr int MaxDepth = 2048;
static_assert(MaxDepth == CallStackLimit);
constexpr auto TestCallStackLimit = 2048;
static_assert(TestCallStackLimit == CallStackLimit);

TEST(execute_call, call_initial_depth)
{
Expand Down Expand Up @@ -639,8 +639,8 @@ TEST(execute_call, call_max_depth)

auto instance = instantiate(parse(bin));

EXPECT_THAT(execute(*instance, 0, {}, MaxDepth), Result(42));
EXPECT_THAT(execute(*instance, 1, {}, MaxDepth), Traps());
EXPECT_THAT(execute(*instance, 0, {}, TestCallStackLimit - 1), Result(42));
EXPECT_THAT(execute(*instance, 1, {}, TestCallStackLimit - 1), Traps());
}

TEST(execute_call, execute_imported_max_depth)
Expand All @@ -655,17 +655,17 @@ TEST(execute_call, execute_imported_max_depth)

auto module = parse(wasm);
auto host_foo = [](Instance& /*instance*/, const Value*, int depth) -> ExecutionResult {
EXPECT_LE(depth, MaxDepth);
EXPECT_LE(depth, TestCallStackLimit - 1);
return Void;
};
const auto host_foo_type = module->typesec[0];

auto instance = instantiate(std::move(module), {{host_foo, host_foo_type}});

EXPECT_THAT(execute(*instance, 0, {}, MaxDepth), Result());
EXPECT_THAT(execute(*instance, 1, {}, MaxDepth), Result());
EXPECT_THAT(execute(*instance, 0, {}, MaxDepth + 1), Traps());
EXPECT_THAT(execute(*instance, 1, {}, MaxDepth + 1), Traps());
EXPECT_THAT(execute(*instance, 0, {}, TestCallStackLimit - 1), Result());
EXPECT_THAT(execute(*instance, 1, {}, TestCallStackLimit - 1), Result());
EXPECT_THAT(execute(*instance, 0, {}, TestCallStackLimit), Traps());
EXPECT_THAT(execute(*instance, 1, {}, TestCallStackLimit), Traps());
}

TEST(execute_call, imported_function_from_another_module_max_depth)
Expand Down Expand Up @@ -702,8 +702,8 @@ TEST(execute_call, imported_function_from_another_module_max_depth)

auto instance2 = instantiate(std::move(module2), {{sub, instance1->module->typesec[0]}});

EXPECT_THAT(execute(*instance2, 2, {}, MaxDepth - 1), Traps());
EXPECT_THAT(execute(*instance2, 3, {}, MaxDepth - 1), Result());
EXPECT_THAT(execute(*instance2, 2, {}, TestCallStackLimit - 1 - 1), Traps());
EXPECT_THAT(execute(*instance2, 3, {}, TestCallStackLimit - 1 - 1), Result());
}

TEST(execute_call, count_calls_to_imported_function)
Expand All @@ -724,7 +724,7 @@ TEST(execute_call, count_calls_to_imported_function)
auto i1 = instantiate(parse(wasm1), {}, {}, {}, {{&counter, {ValType::i64, true}}});
counter.i64 = 0;
EXPECT_THAT(execute(*i1, 0, {}), Traps());
EXPECT_EQ(counter.i64, MaxDepth + 1);
EXPECT_EQ(counter.i64, TestCallStackLimit);


/* wat2wasm
Expand All @@ -734,7 +734,7 @@ TEST(execute_call, count_calls_to_imported_function)
auto i2 = instantiate(parse(wasm2), {*find_exported_function(*i1, "infinite")});
counter.i64 = 0;
EXPECT_THAT(execute(*i2, 0, {}), Traps());
EXPECT_EQ(counter.i64, MaxDepth + 1);
EXPECT_EQ(counter.i64, TestCallStackLimit);
}

// A regression test for incorrect number of arguments passed to a call.
Expand Down Expand Up @@ -773,7 +773,7 @@ TEST(execute_call, call_imported_infinite_recursion)
const auto module = parse(wasm);
int counter = 0;
auto host_foo = [&counter](Instance& instance, const Value* args, int depth) {
EXPECT_LE(depth, MaxDepth);
EXPECT_LE(depth, TestCallStackLimit - 1);
++counter;
return execute(instance, 0, args, depth + 1);
};
Expand All @@ -783,11 +783,11 @@ TEST(execute_call, call_imported_infinite_recursion)

counter = 0;
EXPECT_THAT(execute(*instance, 0, {}), Traps());
EXPECT_EQ(counter, MaxDepth + 1);
EXPECT_EQ(counter, TestCallStackLimit);

counter = 0;
EXPECT_THAT(execute(*instance, 1, {}), Traps());
EXPECT_EQ(counter, MaxDepth);
EXPECT_EQ(counter, TestCallStackLimit - 1);
}

TEST(execute_call, call_imported_interleaved_infinite_recursion)
Expand All @@ -805,7 +805,7 @@ TEST(execute_call, call_imported_interleaved_infinite_recursion)
int counter = 0;
auto host_foo = [&counter](Instance& instance, const Value* args, int depth) {
// Function $f will increase depth. This means each iteration goes 2 steps deeper.
EXPECT_LE(depth, MaxDepth);
EXPECT_LT(depth, CallStackLimit);
++counter;
return execute(instance, 1, args, depth + 1);
};
Expand All @@ -816,12 +816,12 @@ TEST(execute_call, call_imported_interleaved_infinite_recursion)
// Start with the imported host function.
counter = 0;
EXPECT_THAT(execute(*instance, 0, {}), Traps());
EXPECT_EQ(counter, MaxDepth / 2 + 1);
EXPECT_EQ(counter, CallStackLimit / 2);

// Start with the wasm function.
counter = 0;
EXPECT_THAT(execute(*instance, 1, {}), Traps());
EXPECT_EQ(counter, MaxDepth / 2);
EXPECT_EQ(counter, CallStackLimit / 2);
}

TEST(execute_call, call_imported_max_depth_recursion)
Expand All @@ -833,7 +833,7 @@ TEST(execute_call, call_imported_max_depth_recursion)

const auto module = parse(wasm);
auto host_foo = [](Instance& instance, const Value* args, int depth) -> ExecutionResult {
if (depth == MaxDepth)
if (depth == TestCallStackLimit - 1)
return Value{uint32_t{1}}; // Terminate recursion on the max depth.
return execute(instance, 0, args, depth + 1);
};
Expand All @@ -858,7 +858,7 @@ TEST(execute_call, call_via_imported_max_depth_recursion)
const auto module = parse(wasm);
auto host_foo = [](Instance& instance, const Value* args, int depth) -> ExecutionResult {
// Function $f will increase depth. This means each iteration goes 2 steps deeper.
if (depth == (MaxDepth - 1))
if (depth == TestCallStackLimit - 1)
return Value{uint32_t{1}}; // Terminate recursion on the max depth.
return execute(instance, 1, args, depth + 1);
};
Expand Down

0 comments on commit f8f26c5

Please sign in to comment.