fix: properly mark array elements when an realm slice is updated (gno…

…lang#1305)

Addresses gnolang#1167,  gnolang#960, and gnolang#1170 

Consider the following situation:
- A slice of structs exists with a length of zero and a capacity of one
- A new struct literal is appended to the slice
- The code panics because the newly allocated struct literal was never
marked as "new"

``` go
package append

import (
	"gno.land/p/demo/ufmt"
)

type T struct{ i int }

var a []T

func init() {
        a = make([]T, 0, 1)
}

func Append(i int) {
	a = append(a, T{i: i})
}

```

Invoking the `Append` function will cause a panic.

The solution is to traverse each of the array elements after slice
append assignment to make sure any new or updated elements are marked as
such.

This PR also includes a change to ensure that marking an object as dirty
and managing references to the object are mutually exclusive. I think
this is correct but am not sure.

The changes include txtar test cases that incorporate the issue
described by @tbruyelle in gnolang#1170

---------

Co-authored-by: jaekwon <jae@tendermint.com>

gno.land/cmd/gnoland/testdata/append.txtar

@@ -0,0 +1,129 @@
+# start a new node
+gnoland start
+
+gnokey maketx addpkg -pkgdir $WORK -pkgpath gno.land/r/append -gas-fee 1000000ugnot -gas-wanted 2000000 -broadcast -chainid=tendermint_test test1
+stdout OK!
+
+# Call Append 1
+gnokey maketx call -pkgpath gno.land/r/append -func Append -gas-fee 1000000ugnot -gas-wanted 2000000 -args '1' -broadcast -chainid=tendermint_test test1
+stdout OK!
+
+gnokey maketx call -pkgpath gno.land/r/append -func AppendNil -gas-fee 1000000ugnot -gas-wanted 2000000 -broadcast -chainid=tendermint_test test1
+stdout OK!
+
+# Call Append 2
+gnokey maketx call -pkgpath gno.land/r/append -func Append -gas-fee 1000000ugnot -gas-wanted 2000000 -args '2' -broadcast -chainid=tendermint_test test1
+stdout OK!
+
+# Call Append 3
+gnokey maketx call -pkgpath gno.land/r/append -func Append -gas-fee 1000000ugnot -gas-wanted 2000000 -args '3' -broadcast -chainid=tendermint_test test1
+stdout OK!
+
+# Call render
+gnokey maketx call -pkgpath gno.land/r/append -func Render -gas-fee 1000000ugnot -gas-wanted 2000000 -args '' -broadcast -chainid=tendermint_test test1
+stdout '("1-2-3-" string)'
+stdout OK!
+
+# Call Pop
+gnokey maketx call -pkgpath gno.land/r/append -func Pop -gas-fee 1000000ugnot -gas-wanted 2000000 -broadcast -chainid=tendermint_test test1
+stdout OK!
+
+# Call render
+gnokey maketx call -pkgpath gno.land/r/append -func Render -gas-fee 1000000ugnot -gas-wanted 2000000 -args '' -broadcast -chainid=tendermint_test test1
+stdout '("2-3-" string)'
+stdout OK!
+
+# Call Append 42
+gnokey maketx call -pkgpath gno.land/r/append -func Append -gas-fee 1000000ugnot -gas-wanted 2000000 -args '42' -broadcast -chainid=tendermint_test test1
+stdout OK!
+
+# Call render
+gnokey maketx call -pkgpath gno.land/r/append -func Render -gas-fee 1000000ugnot -gas-wanted 2000000 -args '' -broadcast -chainid=tendermint_test test1
+stdout '("2-3-42-" string)'
+stdout OK!
+
+gnokey maketx call -pkgpath gno.land/r/append -func CopyAppend -gas-fee 1000000ugnot -gas-wanted 2000000 -broadcast -chainid=tendermint_test test1
+stdout OK!
+
+gnokey maketx call -pkgpath gno.land/r/append -func PopB -gas-fee 1000000ugnot -gas-wanted 2000000 -broadcast -chainid=tendermint_test test1
+stdout OK!
+
+# Call render
+gnokey maketx call -pkgpath gno.land/r/append -func Render -gas-fee 1000000ugnot -gas-wanted 2000000 -args '' -broadcast -chainid=tendermint_test test1
+stdout '("2-3-42-" string)'
+stdout OK!
+
+gnokey maketx call -pkgpath gno.land/r/append -func AppendMoreAndC -gas-fee 1000000ugnot -gas-wanted 2000000 -broadcast -chainid=tendermint_test test1
+stdout OK!
+
+gnokey maketx call -pkgpath gno.land/r/append -func ReassignC -gas-fee 1000000ugnot -gas-wanted 2000000 -broadcast -chainid=tendermint_test test1
+stdout OK!
+
+gnokey maketx call -pkgpath gno.land/r/append -func Render -gas-fee 1000000ugnot -gas-wanted 2000000 -args '' -broadcast -chainid=tendermint_test test1
+stdout '("2-3-42-70-100-" string)'
+stdout OK!
+
+gnokey maketx call -pkgpath gno.land/r/append -func Render -gas-fee 1000000ugnot -gas-wanted 2000000 -args 'd' -broadcast -chainid=tendermint_test test1
+stdout '("1-" string)'
+stdout OK!
+
+-- append.gno --
+package append
+
+import (
+ "gno.land/p/demo/ufmt"
+)
+
+type T struct{ i int }
+
+var a, b, d []T
+var c = []T{{i: 100}}
+
+
+func init() {
+ a = make([]T, 0, 1)
+}
+
+func Pop() {
+ a = append(a[:0], a[1:]...)
+}
+
+func Append(i int) {
+ a = append(a, T{i: i})
+}
+
+func CopyAppend() {
+ b = append(a, T{i: 50}, T{i: 60})
+}
+
+func PopB() {
+ b = append(b[:0], b[1:]...)
+}
+
+func AppendMoreAndC() {
+ // Fill to capacity
+ a = append(a, T{i: 70})
+ // Above capacity; make new array
+ a = append(a, c...)
+}
+
+func ReassignC() {
+ c[0] = T{i: 200}
+}
+
+func AppendNil() {
+ d = append(d, a...)
+}
+
+func Render(path string) string {
+ source := a
+ if path == "d" {
+ source = d
+ }
+
+ var s string
+ for i:=0;i<len(source);i++{
+ s+=ufmt.Sprintf("%d-", source[i].i)
+ }
+ return s
+}

gno.land/cmd/gnoland/testdata/issue-1167.txtar

@@ -0,0 +1,114 @@
+# Reproducible Test for https://github.com/gnolang/gno/issues/1167
+
+gnoland start
+
+# add contract
+gnokey maketx addpkg -pkgdir $WORK -pkgpath gno.land/r/demo/xx -gas-fee 1000000ugnot -gas-wanted 2000000 -broadcast -chainid=tendermint_test test1
+stdout OK!
+
+# execute New
+gnokey maketx call -pkgpath gno.land/r/demo/xx -func New -args X -gas-fee 1000000ugnot -gas-wanted 2000000 -broadcast -chainid=tendermint_test test1
+stdout OK!
+
+# execute Delta for the first time
+gnokey maketx call -pkgpath gno.land/r/demo/xx -func Delta -args X -gas-fee 1000000ugnot -gas-wanted 2000000 -broadcast -chainid=tendermint_test test1
+stdout OK!
+stdout '"1,1,1;" string'
+
+# execute Delta for the second time
+gnokey maketx call -pkgpath gno.land/r/demo/xx -func Delta -args X -gas-fee 1000000ugnot -gas-wanted 2000000 -broadcast -chainid=tendermint_test test1
+stdout OK!
+stdout '1,1,1;2,2,2;" string'
+
+# execute Delta for the third time
+gnokey maketx call -pkgpath gno.land/r/demo/xx -func Delta -args X -gas-fee 1000000ugnot -gas-wanted 2000000 -broadcast -chainid=tendermint_test test1
+stdout OK!
+stdout '1,1,1;2,2,2;3,3,3;" string'
+
+# execute Render
+gnokey maketx call -pkgpath gno.land/r/demo/xx -func Render -args X -gas-fee 1000000ugnot -gas-wanted 2000000 -broadcast -chainid=tendermint_test test1
+stdout OK!
+stdout '1,1,1;2,2,2;3,3,3;" string'
+
+-- gno.mod --
+module gno.land/r/demo/xx
+
+require (
+ gno.land/p/demo/avl v0.0.0-latest
+)
+
+-- realm.gno --
+package xx
+
+import (
+ "strconv"
+
+ "gno.land/p/demo/avl"
+)
+
+type Move struct {
+ N1, N2, N3 byte
+}
+
+type Position struct {
+ Moves []Move
+}
+
+func (p Position) clone() Position {
+ mv := p.Moves
+ return Position{Moves: mv}
+}
+
+func (oldp Position) update() Position {
+ p := oldp.clone()
+
+ counter++
+ // This is a workaround for the wrong behaviour (ie. uncomment this line):
+ // p.Moves = append([]Move{}, p.Moves...)
+ p.Moves = append(p.Moves, Move{counter, counter, counter})
+ return p
+}
+
+type Game struct {
+ Position Position
+}
+
+var games avl.Tree // id -> *Game
+
+var counter byte
+
+func New(s string) string {
+ // Bug shows if Moves has a cap > 0 when initialised.
+ el := &Game{Position: Position{Moves: make([]Move, 0, 2)}}
+ games.Set(s, el)
+ return values(el.Position)
+}
+
+func Delta(s string) string {
+ v, _ := games.Get(s)
+ g, ok := v.(*Game)
+ if !ok {
+ panic("invalid game")
+ }
+ n := g.Position.update()
+ g.Position = n
+ ret := values(n)
+ return ret
+}
+
+func Render(s string) string {
+ v, _ := games.Get(s)
+ g, ok := v.(*Game)
+ if !ok {
+ panic("invalid game")
+ }
+ return values(g.Position)
+}
+
+func values(x Position) string {
+ s := ""
+ for _, val := range x.Moves {
+ s += strconv.Itoa(int(val.N1)) + "," + strconv.Itoa(int(val.N2)) + "," + strconv.Itoa(int(val.N3)) + ";"
+ }
+ return s
+}

gnovm/pkg/gnolang/realm.go

@@ -151,6 +151,7 @@ func (rlm *Realm) DidUpdate(po, xo, co Object) {
  // Updates to .newCreated/.newEscaped /.newDeleted made here. (first gen)
  // More appends happen during FinalizeRealmTransactions(). (second+ gen)
  rlm.MarkDirty(po)
+
  if co != nil {
  co.IncRefCount()
  if co.GetRefCount() > 1 {
@@ -166,6 +167,7 @@ func (rlm *Realm) DidUpdate(po, xo, co Object) {
  rlm.MarkNewReal(co)
  }
  }
+
  if xo != nil {
  xo.DecRefCount()
  if xo.GetRefCount() == 0 {

gnovm/pkg/gnolang/uverse.go

@@ -210,9 +210,9 @@ func UverseNode() *PackageNode {
  // append(nil, *SliceValue) new list ---------
  list := make([]TypedValue, argsl)
  if 0 < argsl {
- copy(
- list[:argsl],
-  argsb.List[argso:argso+argsl])
+ for i := 0; i < argsl; i++ {
+ list[i] = argsb.List[argso+i].unrefCopy(m.Alloc, m.Store)
+ }
  }
  m.PushValue(TypedValue{
  T: xt,
@@ -294,14 +294,25 @@ func UverseNode() *PackageNode {
  // append(*SliceValue.List, *SliceValue) ---------
  list := xvb.List
  if argsb.Data == nil {
- copy(
- list[xvo+xvl:xvo+xvl+argsl],
- argsb.List[argso:argso+argsl])
+ for i := 0; i < argsl; i++ {
+ oldElem := list[xvo+xvl+i]
+ // unrefCopy will resolve references and copy their values
+ // to copy by value rather than by reference.
+ newElem := argsb.List[argso+i].unrefCopy(m.Alloc, m.Store)
+ list[xvo+xvl+i] = newElem
+
+ m.Realm.DidUpdate(
+ xvb,
+ oldElem.GetFirstObject(m.Store),
+ newElem.GetFirstObject(m.Store),
+ )
+ }
  } else {
  copyDataToList(
  list[xvo+xvl:xvo+xvl+argsl],
  argsb.Data[argso:argso+argsl],
  xt.Elem())
+ m.Realm.DidUpdate(xvb, nil, nil)
  }
  } else {
  // append(*SliceValue.Data, *SliceValue) ---------
@@ -310,6 +321,7 @@ func UverseNode() *PackageNode {
  copyListToData(
  data[xvo+xvl:xvo+xvl+argsl],
  argsb.List[argso:argso+argsl])
+ m.Realm.DidUpdate(xvb, nil, nil)
  } else {
  copy(
  data[xvo+xvl:xvo+xvl+argsl],
@@ -363,9 +375,9 @@ func UverseNode() *PackageNode {
  list := make([]TypedValue, xvl+argsl)
  if 0 < xvl {
  if xvb.Data == nil {
- copy(
- list[:xvl],
-  xvb.List[xvo:xvo+xvl])
+ for i := 0; i < xvl; i++ {
+ list[i] = xvb.List[xvo+i].unrefCopy(m.Alloc, m.Store)
+ }
  } else {
  panic("should not happen")
  /*
@@ -379,9 +391,9 @@ func UverseNode() *PackageNode {
  }
  if 0 < argsl {
  if argsb.Data == nil {
- copy(
- list[xvl:xvl+argsl],
-  argsb.List[argso:argso+argsl])
+ for i := 0; i < argsl; i++ {
+ list[xvl+i] = argsb.List[argso+i].unrefCopy(m.Alloc, m.Store)
+ }
  } else {
  copyDataToList(
  list[xvl:xvl+argsl],
@@ -457,11 +469,12 @@ func UverseNode() *PackageNode {
  return
  } else {
  // append(*SliceValue, *NativeValue) new list --------
- list := make([]TypedValue, xvl+argsl)
+ listLen := xvl + argsl
+ list := make([]TypedValue, listLen)
  if 0 < xvl {
- copy(
- list[:xvl],
-  xvb.List[xvo:xvo+xvl])
+ for i := 0; i < listLen; i++ {
+ list[i] = xvb.List[xvo+i].unrefCopy(m.Alloc, m.Store)
+ }
  }
  if 0 < argsl {
  copyNativeToList(

gnovm/pkg/gnolang/values.go

@@ -1027,6 +1027,28 @@ func (tv TypedValue) Copy(alloc *Allocator) (cp TypedValue) {
  return
 }
 
+// unrefCopy makes a copy of the underlying value in the case of reference values.
+// It copies other values as expected using the normal Copy method.
+func (tv TypedValue) unrefCopy(alloc *Allocator, store Store) (cp TypedValue) {
+ switch tv.V.(type) {
+ case RefValue:
+ cp.T = tv.T
+ refObject := tv.GetFirstObject(store)
+ switch refObjectValue := refObject.(type) {
+ case *ArrayValue:
+ cp.V = refObjectValue.Copy(alloc)
+ case *StructValue:
+ cp.V = refObjectValue.Copy(alloc)
+ default:
+ cp = tv
+ }
+ default:
+ cp = tv.Copy(alloc)
+ }
+
+ return
+}
+
 // Returns encoded bytes for primitive values.
 // These bytes are used for both value hashes as well
 // as hash key bytes.