Yes, and sorry I should have maybe clarified this or make another PR for it:
While testing the multi value support with control instructions, some programs showed unexpected behaviour. The problem is that the condition value should be consumed before pushing onto the call stack. This was not the case, and therefore the stored sp in the frame was too high by one (it still included the condition). Because this frame->sp is later used in pop_block to restore the stack and to place result values, the mismatch causes
- restoring the stack to an incorrect (too high) position
- writing result values at the wrong (too high) indices
- unintentionally leaving stale values on the stack
This can be demonstrated using the following example:
(module
(import "env" "print_int" (func $print (param i32)))
(func (export "main")
i32.const 42
i32.const 0
(if (result i32)
(then (i32.const 3))
(else (i32.const 7))
)
(call $print) ;; expected: 7
(call $print) ;; expected: 42
)
)
The old implementation:
- when pushing the
if block, the stack was [42, 0], sp=1 and frame->sp=1
- after that, the condition value was popped, resulting in the stack being
[42] and sp=0
- execution of the
if block results in the stack [42, 7] and sp=0
pop_block then uses the (incorrect) frame->sp=1 to write the results (incorrectly too high) on the stack: [42, 7, 7], leaving a stale 7 (from the block execution) on there
Therefore, this implementation gives the following output:
❯ ./wdcli example.wasm
7
7
The expected behaviour is that the stack contains [42, 7] after the if block. Therefore I made the interpreter consume the condition value before the if block gets pushed. As such, the frame->sp contains the correct stack pointer (not including the condition value), and the block popping works as expected. The example then outputs:
❯ ./wdcli example.wasm
7
42
Originally posted by @abelstuker in #330 (comment)
Yes, and sorry I should have maybe clarified this or make another PR for it:
While testing the multi value support with control instructions, some programs showed unexpected behaviour. The problem is that the condition value should be consumed before pushing onto the call stack. This was not the case, and therefore the stored
spin the frame was too high by one (it still included the condition). Because thisframe->spis later used inpop_blockto restore the stack and to place result values, the mismatch causesThis can be demonstrated using the following example:
The old implementation:
ifblock, the stack was[42, 0],sp=1andframe->sp=1[42]andsp=0ifblock results in the stack[42, 7]andsp=0pop_blockthen uses the (incorrect)frame->sp=1to write the results (incorrectly too high) on the stack:[42, 7, 7], leaving a stale 7 (from the block execution) on thereTherefore, this implementation gives the following output:
The expected behaviour is that the stack contains
[42, 7]after theifblock. Therefore I made the interpreter consume the condition value before theifblock gets pushed. As such, theframe->spcontains the correct stack pointer (not including the condition value), and the block popping works as expected. The example then outputs:Originally posted by @abelstuker in #330 (comment)