From 270f0d2d7ac051b0ffcae39a4d9669d506f9dd9e Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Sat, 16 Nov 2024 18:00:57 +0300 Subject: [PATCH] vm: fix incorrect refcounting in POPITEM We're popping an item (array) off the stack, OK, it triggers refs.Remove() for it. Then we're pushing an inner item to the stack, OK, it triggers refs.Add() for this element. Why are we removing it afterwards? Looks like something went wrong in 324107b31ee113a60d7739fee867e01beb30c298 (and https://github.com/nspcc-dev/neo-go/pull/1670) since a simple test shows zero counter after POPITEM and -1 after popping the only item left on the stack. Signed-off-by: Roman Khimov --- pkg/vm/ref_counter_test.go | 14 ++++++++++++++ pkg/vm/vm.go | 1 - 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/pkg/vm/ref_counter_test.go b/pkg/vm/ref_counter_test.go index 2c2b3752d9..4e57ae0f6a 100644 --- a/pkg/vm/ref_counter_test.go +++ b/pkg/vm/ref_counter_test.go @@ -3,6 +3,7 @@ package vm import ( "testing" + "github.com/nspcc-dev/neo-go/pkg/vm/opcode" "github.com/nspcc-dev/neo-go/pkg/vm/stackitem" "github.com/stretchr/testify/require" ) @@ -46,6 +47,19 @@ func TestRefCounter_Add(t *testing.T) { require.Equal(t, 2, int(*r)) } +func TestRefCounterPopItem(t *testing.T) { + prog := makeProgram(opcode.POPITEM) + v := load(prog) + v.estack.PushVal(stackitem.NewArray([]stackitem.Item{stackitem.Make(42)})) + require.Equal(t, 2, int(v.refs)) + runVM(t, v) + require.Equal(t, 1, v.estack.Len()) + require.Equal(t, 1, int(v.refs)) + _ = v.estack.Pop() + require.Equal(t, 0, v.estack.Len()) + require.Equal(t, 0, int(v.refs)) +} + func BenchmarkRefCounter_Add(b *testing.B) { a := stackitem.NewArray(nil) rc := newRefCounter() diff --git a/pkg/vm/vm.go b/pkg/vm/vm.go index 2569fb9055..8c98192c39 100644 --- a/pkg/vm/vm.go +++ b/pkg/vm/vm.go @@ -1459,7 +1459,6 @@ func (v *VM) execute(ctx *Context, op opcode.Opcode, parameter []byte) (err erro case *stackitem.Struct: item.Remove(index) } - v.refs.Remove(elem) case opcode.SIZE: elem := v.estack.Pop()