Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Correct max call depth #669

Merged
merged 1 commit into from
Dec 29, 2020
Merged

Correct max call depth #669

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/fizzy/execute.cpp
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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