wagon
wagon copied to clipboard
disasm: fix incorrect stack depth for end/discard
End/Discard ops were being misencoded with incorrect stack depths which caused stack underflows that manifested as vm.ctx.stack
slice index out of bounds runtime panics.
Test case wasm (execute the exported function _ZN4enol4InitEv
)
I'm not 100% familiar with the code in question, so please check if this is the correct fix but my test code runs correctly with this change applied.
@bkeroackdsc thanks for taking a stab at it!
unfortunately, it seems your change brok' a few tests (the non-spec ones for exec/testdata
seem to be ok, but the exec/testdata/spec
do not)
I must admit, I am also a bit lagging with the complete understanding of this piece of code. @vibhavp ? do you have any input?
Sorry for not replying, as I've been a little busy with classes. I'll try my best to address this issue in the coming weeks.
If you take a look at the comment above your change, the reason why we subtract 0 is to get the parent block of the target branch, which is what we want to unwind to.
ping @vibhavp ?
@sbinet and @vibhavp any update on this?
I am trying to run a function in a wasm file that looks like this in wat:
(func $transfer (export "transfer") (type $t4) (param $p0 i32) (param $p1 i32) (param $p2 i32)
(local $l0 i32) (local $l1 i32)
block $B0
get_local $p0
call $load
tee_local $l0
get_local $p2
i32.lt_s
br_if $B0
get_local $p1
call $load
set_local $l1
get_local $p0
get_local $l0
get_local $p2
i32.sub
call $store
get_local $p1
get_local $l1
get_local $p2
i32.add
call $store
end)
The disassemble code tries to do an OpDiscard at the end with a stack of length 0 and gets a "slice bounds out of range", but making the change in this pull request fixed it for me.
Does the PR still break the tests Sebastian was talking about?
Yes it looks like it still fails some of the exec/TestSpec/testdata/spec
tests:
--- FAIL: TestSpec (0.01s)
--- PASS: TestSpec/testdata/spec/unreachable.wasm (0.00s)
--- PASS: TestSpec/testdata/spec/traps_int_rem.wasm (0.00s)
--- PASS: TestSpec/testdata/spec/traps_int_div.wasm (0.00s)
--- PASS: TestSpec/testdata/spec/forward.wasm (0.00s)
--- PASS: TestSpec/testdata/spec/traps_mem.wasm (0.00s)
--- PASS: TestSpec/testdata/spec/address.wasm (0.00s)
--- PASS: TestSpec/testdata/spec/switch.wasm (0.01s)
--- PASS: TestSpec/testdata/spec/globals.wasm (0.00s)
--- PASS: TestSpec/testdata/spec/break-drop.wasm (0.00s)
--- PASS: TestSpec/testdata/spec/tee_local.wasm (0.01s)
--- PASS: TestSpec/testdata/spec/unwind.wasm (0.01s)
--- PASS: TestSpec/testdata/spec/get_local.wasm (0.00s)
--- PASS: TestSpec/testdata/spec/memory_redundancy.wasm (0.00s)
--- PASS: TestSpec/testdata/spec/nop.wasm (0.01s)
--- PASS: TestSpec/testdata/spec/resizing.wasm (0.00s)
--- FAIL: TestSpec/testdata/spec/br_if.wasm (0.01s)
exec_test.go:338: /Users/jeffsimpson/projects/go/src/github.com/go-interpreter/wagon/exec/testdata/spec/br_if.wasm, nested-br_table-value([i32:0]) (16): unexpected return value: got=uint32(8), want=uint32(5) (i32:5)
--- PASS: TestSpec/testdata/spec/select.wasm (0.00s)
--- FAIL: TestSpec/testdata/spec/loop.wasm (0.01s)
exec_test.go:273: /Users/jeffsimpson/projects/go/src/github.com/go-interpreter/wagon/exec/testdata/spec/loop.wasm: disasm: stack underflow
--- PASS: TestSpec/testdata/spec/fac.wasm (0.00s)
--- FAIL: TestSpec/testdata/spec/if.wasm (0.01s)
exec_test.go:273: /Users/jeffsimpson/projects/go/src/github.com/go-interpreter/wagon/exec/testdata/spec/if.wasm: disasm: stack underflow
--- PASS: TestSpec/testdata/spec/return.wasm (0.01s)
--- PASS: TestSpec/testdata/spec/endianness.wasm (0.01s)
--- PASS: TestSpec/testdata/spec/call_indirect.wasm (0.02s)
--- FAIL: TestSpec/testdata/spec/block.wasm (0.01s)
exec_test.go:273: /Users/jeffsimpson/projects/go/src/github.com/go-interpreter/wagon/exec/testdata/spec/block.wasm: disasm: stack underflow
--- PASS: TestSpec/testdata/spec/br.wasm (0.01s)
--- PASS: TestSpec/testdata/spec/br_table.wasm (0.15s)
I haven't has a chance yet to dig into why it fails these tests or what a true solution to this should be.
After further investigation I found that the problem was in my FunctionIndexSpace for 2 native functions that I added. I was using the wrong signature ParamTypes (ValueTypeI64 instead of ValueTypeI32) compared to what the reflected function was expecting.
Once I made the fix everything appears to be working as expected without including this pull request.
Also using the provided wagon-test.wasm I was able to execute the exported function with no problem: _ZN4enol4InitEv() i32 => 0 (uint32)
@bkeroackdsc If this is still giving you an error could you provide some of the test code you are using? Otherwise, do you think this pull request be closed?