This repository was archived by the owner on May 11, 2020. It is now read-only.
disasm: Fix referencing the incorrect function type in call_indirect #51
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
In Disassemble, the function uses the integer immediate for call_indirect as an index into the global index space, while it should be an index into the Type Section of the module. This causes it to reference an incorrect function, which would cause stack underflows later on. This CL fixes that. Also, fix the select operator decreasing the stack height by 3 instead of 2. (Fixes go-interpreter#49)
|
Review status: 0 of 1 files reviewed at latest revision, all discussions resolved, some commit checks failed. Comments from Reviewable |
|
it would seem travis fails in the same way than what was reported in #49: |
|
with a bit of print-foo: with: diff --git a/exec/vm.go b/exec/vm.go
index cb1ba36..5d7ca42 100644
--- a/exec/vm.go
+++ b/exec/vm.go
@@ -9,6 +9,7 @@ import (
"encoding/binary"
"errors"
"fmt"
+ "log"
"math"
"github.com/go-interpreter/wagon/disasm"
@@ -342,6 +343,14 @@ outer:
case compile.OpDiscardPreserveTop:
top := vm.ctx.stack[len(vm.ctx.stack)-1]
place := vm.fetchInt64()
+ end := len(vm.ctx.stack) - int(place)
+ defer func() {
+ if e := recover(); e != nil {
+ log.Printf("==> place=%d len=%d, end=%d", place, len(vm.ctx.stack), end)
+ log.Printf("error: %v", e)
+ panic(e)
+ }
+ }()
vm.ctx.stack = vm.ctx.stack[:len(vm.ctx.stack)-int(place)]
vm.pushUint64(top)
default: |
|
That error looks the same as the one fixed by #52 |
Replace with BlockInfo.PairIndex four indices: * IfElseIndex - For an if instruction, this is the index to the corresponding else instruction. * ElseIfIndex - For an else instruction, this is the index to the corresponding if instruction. * EndIndex - For a block/loop/if/else instruction, this is the index to the corresponding end instruction. * BlockStartIndex - For an end instruction, this is the index to the instruction that started the block itself.
This ensures the stack length always starts with 0.
|
first pass looks ok, but, could you add a test? |
SetDebugMode allows logging functionality to be enabled or disabled at will.
This CL adds the new WebAssembly module from Bug go-interpreter#49.
|
Reviewed 3 of 4 files at r2, 4 of 5 files at r3. disasm/disasm.go, line 462 at r3 (raw file):
perhaps guard this possibly expansive loop with a test on whether we are in verbose/debug mode or not ? Comments from Reviewable |
|
Review status: 6 of 8 files reviewed at latest revision, 1 unresolved discussion. disasm/disasm.go, line 462 at r3 (raw file): Previously, sbinet (Sebastien Binet) wrote…
Done. Comments from Reviewable |
|
Reviewed 2 of 2 files at r4. Comments from Reviewable |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In Disassemble, the function uses the integer immediate for
call_indirect as an index into the global index space, while it should
be an index into the Type Section of the module. This causes it to
reference an incorrect function, which would cause stack underflows
later on. This CL fixes that.
Also, fix the select operator decreasing the stack height by 3 instead
of 2.
(Fixes #49)
This change is