Skip to content

Commit

Permalink
Correct the meaning of CallStackLimit
Browse files Browse the repository at this point in the history
Previously there were 2049 number of recursive calls allowed (depth
levels from 0 to 2048). With this change the number is reduced to 2048
(depth levels from 0 to 2047).
  • Loading branch information
chfast committed Dec 28, 2020
1 parent 879829b commit 4e411f0
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
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
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
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 4e411f0

Please sign in to comment.