Extend call depth tests #580
Conversation
Codecov Report
@@ Coverage Diff @@
## master #580 +/- ##
=======================================
Coverage 98.23% 98.23%
=======================================
Files 62 62
Lines 8990 9023 +33
=======================================
+ Hits 8831 8864 +33
Misses 159 159 |
| TEST(execute_call, call_imported_infinite_recursion) | ||
| { | ||
| /* wat2wasm | ||
| (import "mod" "foo" (func (result i32))) | ||
| (func (result i32) | ||
| call 0 |
There was a problem hiding this comment.
Why didn't you add it as a new test, but modified this one? This was testing call instruction and was in execute_call suite, now it goes another code path...
There was a problem hiding this comment.
Actually I think this code can remain, the point of the change was to assert in the host function. Would just need to check for MaxDepth - 1 there.
There was a problem hiding this comment.
Actually this was calling function 0 which is the import. Probably we should duplicate this to have both cases covered.
test/unittests/execute_call_test.cpp
Outdated
| @@ -530,8 +530,8 @@ TEST(execute_call, call_max_depth) | |||
| const auto module = parse(bin); | |||
| auto instance = instantiate(module); | |||
|
|
|||
| EXPECT_THAT(execute(*instance, 0, {}, 2048), Result(42)); | |||
| EXPECT_THAT(execute(*instance, 1, {}, 2048), Traps()); | |||
| EXPECT_THAT(execute(*instance, 0, {}, 512), Result(42)); | |||
There was a problem hiding this comment.
This could you MaxDepth constant from below, too.
| @@ -518,6 +518,9 @@ TEST(execute_call, call_indirect_infinite_recursion) | |||
| EXPECT_TRUE(execute(module, 0, {}).trapped); | |||
| } | |||
|
|
|||
| constexpr int MaxDepth = 2048; | |||
| static_assert(MaxDepth == CallStackLimit); | |||
There was a problem hiding this comment.
I think we can remove this now, given the tests should exactly capture it.
|
@chfast I think you did not see the |
No description provided.