From a080d24cf5df755728ab74d447213eae87e62a14 Mon Sep 17 00:00:00 2001 From: Evgenii Stratonikov Date: Tue, 18 Aug 2020 11:13:09 +0300 Subject: [PATCH] vm: fix debugger and add tests 1. `Run()` must be able to continue execution after a breakpoint. 2. VM must stop right before the breakpoint, not after. 3. Initial vm state is NONE, not HALT. --- pkg/vm/context.go | 2 +- pkg/vm/debug_test.go | 42 ++++++++++++++++++++++++++++++++++++++++++ pkg/vm/vm.go | 19 +++++++++---------- 3 files changed, 52 insertions(+), 11 deletions(-) create mode 100644 pkg/vm/debug_test.go diff --git a/pkg/vm/context.go b/pkg/vm/context.go index aff90f0a5..f2cf01ab1 100644 --- a/pkg/vm/context.go +++ b/pkg/vm/context.go @@ -221,7 +221,7 @@ func (c *Context) Equals(s stackitem.Item) bool { func (c *Context) atBreakPoint() bool { for _, n := range c.breakPoints { - if n == c.ip { + if n == c.nextip { return true } } diff --git a/pkg/vm/debug_test.go b/pkg/vm/debug_test.go new file mode 100644 index 000000000..7e6a8877e --- /dev/null +++ b/pkg/vm/debug_test.go @@ -0,0 +1,42 @@ +package vm + +import ( + "math/big" + "testing" + + "github.com/nspcc-dev/neo-go/pkg/vm/opcode" + "github.com/stretchr/testify/require" +) + +func TestVM_Debug(t *testing.T) { + prog := makeProgram(opcode.CALL, 3, opcode.RET, + opcode.PUSH2, opcode.PUSH3, opcode.ADD, opcode.RET) + t.Run("BreakPoint", func(t *testing.T) { + v := load(prog) + v.AddBreakPoint(3) + v.AddBreakPoint(5) + require.NoError(t, v.Run()) + require.Equal(t, 3, v.Context().NextIP()) + require.NoError(t, v.Run()) + require.Equal(t, 5, v.Context().NextIP()) + require.NoError(t, v.Run()) + require.Equal(t, 1, v.estack.len) + require.Equal(t, big.NewInt(5), v.estack.Top().Value()) + }) + t.Run("StepInto", func(t *testing.T) { + v := load(prog) + require.NoError(t, v.StepInto()) + require.Equal(t, 3, v.Context().NextIP()) + require.NoError(t, v.StepOut()) + require.Equal(t, 2, v.Context().NextIP()) + require.Equal(t, 1, v.estack.len) + require.Equal(t, big.NewInt(5), v.estack.Top().Value()) + }) + t.Run("StepOver", func(t *testing.T) { + v := load(prog) + require.NoError(t, v.StepOver()) + require.Equal(t, 2, v.Context().NextIP()) + require.Equal(t, 1, v.estack.len) + require.Equal(t, big.NewInt(5), v.estack.Top().Value()) + }) +} diff --git a/pkg/vm/vm.go b/pkg/vm/vm.go index ac19b20e4..af9cfff6e 100644 --- a/pkg/vm/vm.go +++ b/pkg/vm/vm.go @@ -89,7 +89,7 @@ func New() *VM { // NewWithTrigger returns a new VM for executions triggered by t. func NewWithTrigger(t trigger.Type) *VM { vm := &VM{ - state: HaltState, + state: NoneState, istack: NewStack("invocation"), refs: newRefCounter(), keys: make(map[string]*keys.PublicKey), @@ -358,11 +358,6 @@ func (v *VM) Run() error { // HaltState (the default) or BreakState are safe to continue. v.state = NoneState for { - // check for breakpoint before executing the next instruction - ctx := v.Context() - if ctx != nil && ctx.atBreakPoint() { - v.state = BreakState - } switch { case v.state.HasFlag(FaultState): // Should be caught and reported already by the v.Step(), @@ -379,6 +374,11 @@ func (v *VM) Run() error { v.state = FaultState return errors.New("unknown state") } + // check for breakpoint before executing the next instruction + ctx := v.Context() + if ctx != nil && ctx.atBreakPoint() { + v.state = BreakState + } } } @@ -430,14 +430,15 @@ func (v *VM) StepOut() error { var err error if v.state == BreakState { v.state = NoneState - } else { - v.state = BreakState } expSize := v.istack.len for v.state == NoneState && v.istack.len >= expSize { err = v.StepInto() } + if v.state == NoneState { + v.state = BreakState + } return err } @@ -451,8 +452,6 @@ func (v *VM) StepOver() error { if v.state == BreakState { v.state = NoneState - } else { - v.state = BreakState } expSize := v.istack.len