validation: Detect stack underflow #244
Conversation
Codecov Report
@@ Coverage Diff @@
## master #244 +/- ##
==========================================
- Coverage 97.33% 97.25% -0.09%
==========================================
Files 32 34 +2
Lines 10073 10198 +125
==========================================
+ Hits 9805 9918 +113
- Misses 268 280 +12 |
chfast
force-pushed
the
stack_check
branch
2 times, most recently
from
2ce6d62
to
f990a8c
Compare
|
Please rebase, to get |
It's not merged yet: #241. |
|
Spectests failures are due to false positive stack underflow failures. We may want to fix the issue first. |
lib/fizzy/parser_expr.cpp
Outdated
| @@ -294,6 +326,8 @@ parser_result<Code> parse_expr(const uint8_t* pos, const uint8_t* end, bool have | |||
| { | |||
| if (frame.instruction != Instr::if_) | |||
| throw parser_error{"unexpected else instruction (if instruction missing)"}; | |||
| frame.stack_height = 0; // Reset stack height after if. | |||
There was a problem hiding this comment.
After passing if block, it reuses the same frame (control_stack.top()) for the else block - it has the same instruction and immediates_offset, but stack_height could have been changed in if block.
There was a problem hiding this comment.
Actually I think unreachable also has to be reset here?
There was a problem hiding this comment.
I think it should. In the Validation Algorithm it pops the control frame and then pushes new one with the same types.
There was a problem hiding this comment.
It would be nice to add a test with br inside if
There was a problem hiding this comment.
Fixed.
The if-else is poorly tested as there is big number of possible combinations.
There was a problem hiding this comment.
It would be nice to add a test with
brinsideif
Added one with unreachable, but I will add one with br too. Also br_table is followed by unreachable...
|
@axic Final word here? |
Can have false positives for calls as callee type information is not available during parsing yet (see single DISABLED test).
Based on https://webassembly.github.io/spec/core/appendix/algorithm.html.
Requires #254.