From 18d627e7f772c483acd4808173c51a491210e3cd Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Mon, 16 May 2022 16:07:25 +0300 Subject: [PATCH] vm: count map key in the refcounter as well Thanks @ixje for spotting this. --- pkg/vm/ref_counter.go | 12 ++++++++---- pkg/vm/ref_counter_test.go | 14 ++++++++++++++ pkg/vm/vm.go | 12 +++++++++--- pkg/vm/vm_test.go | 20 ++++++++++---------- 4 files changed, 41 insertions(+), 17 deletions(-) diff --git a/pkg/vm/ref_counter.go b/pkg/vm/ref_counter.go index f119e3959..a493a43ce 100644 --- a/pkg/vm/ref_counter.go +++ b/pkg/vm/ref_counter.go @@ -37,8 +37,10 @@ func (r *refCounter) Add(item stackitem.Item) { r.Add(it) } case *stackitem.Map: - for i := range t.Value().([]stackitem.MapElement) { - r.Add(t.Value().([]stackitem.MapElement)[i].Value) + elems := t.Value().([]stackitem.MapElement) + for i := range elems { + r.Add(elems[i].Key) + r.Add(elems[i].Value) } } } @@ -60,8 +62,10 @@ func (r *refCounter) Remove(item stackitem.Item) { r.Remove(it) } case *stackitem.Map: - for i := range t.Value().([]stackitem.MapElement) { - r.Remove(t.Value().([]stackitem.MapElement)[i].Value) + elems := t.Value().([]stackitem.MapElement) + for i := range elems { + r.Remove(elems[i].Key) + r.Remove(elems[i].Value) } } } diff --git a/pkg/vm/ref_counter_test.go b/pkg/vm/ref_counter_test.go index 9e0a82d99..f0e63b318 100644 --- a/pkg/vm/ref_counter_test.go +++ b/pkg/vm/ref_counter_test.go @@ -30,6 +30,20 @@ func TestRefCounter_Add(t *testing.T) { r.Remove(arr) require.Equal(t, 2, int(*r)) + + m := stackitem.NewMap() + m.Add(stackitem.NewByteArray([]byte("some")), stackitem.NewBool(false)) + r.Add(m) + require.Equal(t, 5, int(*r)) // map + key + value + + r.Add(m) + require.Equal(t, 6, int(*r)) // map only + + r.Remove(m) + require.Equal(t, 5, int(*r)) + + r.Remove(m) + require.Equal(t, 2, int(*r)) } func BenchmarkRefCounter_Add(b *testing.B) { diff --git a/pkg/vm/vm.go b/pkg/vm/vm.go index 717d5468a..cc446ee4d 100644 --- a/pkg/vm/vm.go +++ b/pkg/vm/vm.go @@ -1250,6 +1250,8 @@ func (v *VM) execute(ctx *Context, op opcode.Opcode, parameter []byte) (err erro case *stackitem.Map: if i := t.Index(key.value); i >= 0 { v.refs.Remove(t.Value().([]stackitem.MapElement)[i].Value) + } else { + v.refs.Add(key.value) } t.Add(key.value, item) v.refs.Add(item) @@ -1312,7 +1314,9 @@ func (v *VM) execute(ctx *Context, op opcode.Opcode, parameter []byte) (err erro index := t.Index(key.Item()) // NEO 2.0 doesn't error on missing key. if index >= 0 { - v.refs.Remove(t.Value().([]stackitem.MapElement)[index].Value) + elems := t.Value().([]stackitem.MapElement) + v.refs.Remove(elems[index].Key) + v.refs.Remove(elems[index].Value) t.Drop(index) } default: @@ -1333,8 +1337,10 @@ func (v *VM) execute(ctx *Context, op opcode.Opcode, parameter []byte) (err erro } t.Clear() case *stackitem.Map: - for i := range t.Value().([]stackitem.MapElement) { - v.refs.Remove(t.Value().([]stackitem.MapElement)[i].Value) + elems := t.Value().([]stackitem.MapElement) + for i := range elems { + v.refs.Remove(elems[i].Key) + v.refs.Remove(elems[i].Value) } t.Clear() default: diff --git a/pkg/vm/vm_test.go b/pkg/vm/vm_test.go index fcd25ef4a..179fbd8c1 100644 --- a/pkg/vm/vm_test.go +++ b/pkg/vm/vm_test.go @@ -395,13 +395,13 @@ func TestStackLimit(t *testing.T) { {opcode.DUP, 6}, {opcode.PUSH2, 7}, {opcode.LDSFLD0, 8}, - {opcode.SETITEM, 6}, // -3 items and 1 new element in map - {opcode.DUP, 7}, - {opcode.PUSH2, 8}, - {opcode.LDSFLD0, 9}, - {opcode.SETITEM, 6}, // -3 items and no new elements in map - {opcode.DUP, 7}, - {opcode.PUSH2, 8}, + {opcode.SETITEM, 7}, // -3 items and 1 new kv pair in map + {opcode.DUP, 8}, + {opcode.PUSH2, 9}, + {opcode.LDSFLD0, 10}, + {opcode.SETITEM, 7}, // -3 items and no new elements in map + {opcode.DUP, 8}, + {opcode.PUSH2, 9}, {opcode.REMOVE, 5}, // as we have right after NEWMAP {opcode.DROP, 4}, // DROP map with no elements } @@ -1402,7 +1402,7 @@ func TestSETITEMBigMapBad(t *testing.T) { // 2. SETITEM each of them to a map. // 3. Replace each of them with a scalar value. func TestSETITEMMapStackLimit(t *testing.T) { - size := MaxStackSize/2 - 3 + size := MaxStackSize/2 - 4 m := stackitem.NewMap() m.Add(stackitem.NewBigInteger(big.NewInt(1)), stackitem.NewArray(makeArrayOfType(size, stackitem.BooleanT))) m.Add(stackitem.NewBigInteger(big.NewInt(2)), stackitem.NewArray(makeArrayOfType(size, stackitem.BooleanT))) @@ -2036,8 +2036,8 @@ func TestPACKMAP_UNPACK_PACKMAP_MaxSize(t *testing.T) { } vm.estack.PushVal(len(elements)) runVM(t, vm) - // check reference counter = 1+1+1024 - assert.Equal(t, 1+1+len(elements), int(vm.refs)) + // check reference counter = 1+1+1024*2 + assert.Equal(t, 1+1+len(elements)*2, int(vm.refs)) assert.Equal(t, 2, vm.estack.Len()) m := vm.estack.Peek(0).value.(*stackitem.Map).Value().([]stackitem.MapElement) assert.Equal(t, len(elements), len(m))