Skip to content

Condition value should be consumed before pushing onto the call stack #356

@tolauwae

Description

@tolauwae

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)

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    Projects

    Status

    Done

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions