From 19961afb2a1ce7d5675698f70da68404eacb487c Mon Sep 17 00:00:00 2001 From: jaekwon Date: Sat, 6 Jan 2024 22:31:14 -0800 Subject: [PATCH 1/5] initial commit; test fails --- .../gnoland/testdata/unexpected-unreal.txtar | 75 +++++++++ gnovm/pkg/gnolang/op_call.go | 4 +- gnovm/pkg/gnolang/op_exec.go | 16 +- gnovm/pkg/gnolang/uverse.go | 12 +- gnovm/pkg/gnolang/values.go | 152 +++++++++++++++--- 5 files changed, 218 insertions(+), 41 deletions(-) create mode 100644 gno.land/cmd/gnoland/testdata/unexpected-unreal.txtar diff --git a/gno.land/cmd/gnoland/testdata/unexpected-unreal.txtar b/gno.land/cmd/gnoland/testdata/unexpected-unreal.txtar new file mode 100644 index 00000000000..fb67882a6dd --- /dev/null +++ b/gno.land/cmd/gnoland/testdata/unexpected-unreal.txtar @@ -0,0 +1,75 @@ +# 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 -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 -gas-fee 1000000ugnot -gas-wanted 2000000 -broadcast -chainid=tendermint_test test1 +stdout OK!xx + +-- 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 S struct { + Moves []Move +} + +func (s S) clone() S { + mv := s.Moves + return S{Moves: mv} +} + +func (olds S) change() S { + s := olds.clone() + + counter++ + s.Moves = append([]Move{}, s.Moves...) + s.Moves = append(s.Moves, Move{counter, counter, counter}) + return s +} + +var el *S +var counter byte + +func New() { + el = &S{} +} + +func Delta() string { + n := el.change() + *el = n + return Values() +} + +func Values() string { + s := "" + for _, val := range el.Moves { + s += strconv.Itoa(int(val.N1)) + "," + strconv.Itoa(int(val.N2)) + "," + strconv.Itoa(int(val.N3)) + ";" + } + return s +} + diff --git a/gnovm/pkg/gnolang/op_call.go b/gnovm/pkg/gnolang/op_call.go index eebee978919..90bca5a44fc 100644 --- a/gnovm/pkg/gnolang/op_call.go +++ b/gnovm/pkg/gnolang/op_call.go @@ -165,7 +165,7 @@ func (m *Machine) doOpCall() { // Make a copy so that a reference to the arguemnt isn't used // in cases where the non-primitive value type is represented // as a pointer, *StructValue, for example. - b.Values[i] = pv.Copy(m.Alloc) + b.Values[i] = pv.Copy(m.Alloc, m.Store) } } @@ -400,7 +400,7 @@ func (m *Machine) doOpDefer() { func (m *Machine) doOpPanic1() { // Pop exception - var ex TypedValue = m.PopValue().Copy(m.Alloc) + var ex TypedValue = m.PopValue().Copy(m.Alloc, m.Store) // Panic m.Panic(ex) } diff --git a/gnovm/pkg/gnolang/op_exec.go b/gnovm/pkg/gnolang/op_exec.go index 300303135ad..12a9ac16990 100644 --- a/gnovm/pkg/gnolang/op_exec.go +++ b/gnovm/pkg/gnolang/op_exec.go @@ -149,7 +149,7 @@ func (m *Machine) doOpExec(op Op) { *xv = *dv } else { dv = xv - *xv = xv.Copy(m.Alloc) + *xv = xv.Copy(m.Alloc, m.Store) } ll = dv.GetLength() if ll == 0 { // early termination @@ -173,7 +173,7 @@ func (m *Machine) doOpExec(op Op) { case DEFINE: knxp := bs.Key.(*NameExpr).Path ptr := m.LastBlock().GetPointerTo(m.Store, knxp) - ptr.TV.Assign(m.Alloc, iv, false) + ptr.TV.Assign(m.Alloc, m.Store, iv, false) default: panic("should not happen") } @@ -188,7 +188,7 @@ func (m *Machine) doOpExec(op Op) { case DEFINE: vnxp := bs.Value.(*NameExpr).Path ptr := m.LastBlock().GetPointerTo(m.Store, vnxp) - ptr.TV.Assign(m.Alloc, ev, false) + ptr.TV.Assign(m.Alloc, m.Store, ev, false) default: panic("should not happen") } @@ -269,7 +269,7 @@ func (m *Machine) doOpExec(op Op) { case DEFINE: knxp := bs.Key.(*NameExpr).Path ptr := m.LastBlock().GetPointerTo(m.Store, knxp) - ptr.TV.Assign(m.Alloc, iv, false) + ptr.TV.Assign(m.Alloc, m.Store, iv, false) default: panic("should not happen") } @@ -282,7 +282,7 @@ func (m *Machine) doOpExec(op Op) { case DEFINE: vnxp := bs.Value.(*NameExpr).Path ptr := m.LastBlock().GetPointerTo(m.Store, vnxp) - ptr.TV.Assign(m.Alloc, ev, false) + ptr.TV.Assign(m.Alloc, m.Store, ev, false) default: panic("should not happen") } @@ -362,7 +362,7 @@ func (m *Machine) doOpExec(op Op) { case DEFINE: knxp := bs.Key.(*NameExpr).Path ptr := m.LastBlock().GetPointerTo(m.Store, knxp) - ptr.TV.Assign(m.Alloc, kv, false) + ptr.TV.Assign(m.Alloc, m.Store, kv, false) default: panic("should not happen") } @@ -375,7 +375,7 @@ func (m *Machine) doOpExec(op Op) { case DEFINE: vnxp := bs.Value.(*NameExpr).Path ptr := m.LastBlock().GetPointerTo(m.Store, vnxp) - ptr.TV.Assign(m.Alloc, vv, false) + ptr.TV.Assign(m.Alloc, m.Store, vv, false) default: panic("should not happen") } @@ -885,7 +885,7 @@ func (m *Machine) doOpTypeSwitch() { vp := NewValuePath( VPBlock, 1, 0, ss.VarName) ptr := b.GetPointerTo(m.Store, vp) - ptr.TV.Assign(m.Alloc, *xv, false) + ptr.TV.Assign(m.Alloc, m.Store, *xv, false) } // expand block size if nn := cs.GetNumNames(); int(nn) > len(b.Values) { diff --git a/gnovm/pkg/gnolang/uverse.go b/gnovm/pkg/gnolang/uverse.go index 041f1557e61..237f85eb41a 100644 --- a/gnovm/pkg/gnolang/uverse.go +++ b/gnovm/pkg/gnolang/uverse.go @@ -211,7 +211,7 @@ func UverseNode() *PackageNode { list := make([]TypedValue, argsl) if 0 < argsl { for i := 0; i < argsl; i++ { - list[i] = argsb.List[argso+i].unrefCopy(m.Alloc, m.Store) + list[i] = argsb.List[argso+i].Copy(m.Alloc, m.Store) } } m.PushValue(TypedValue{ @@ -296,9 +296,7 @@ func UverseNode() *PackageNode { if argsb.Data == nil { 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) + newElem := argsb.List[argso+i].Copy(m.Alloc, m.Store) list[xvo+xvl+i] = newElem m.Realm.DidUpdate( @@ -376,7 +374,7 @@ func UverseNode() *PackageNode { if 0 < xvl { if xvb.Data == nil { for i := 0; i < xvl; i++ { - list[i] = xvb.List[xvo+i].unrefCopy(m.Alloc, m.Store) + list[i] = xvb.List[xvo+i].Copy(m.Alloc, m.Store) } } else { panic("should not happen") @@ -392,7 +390,7 @@ func UverseNode() *PackageNode { if 0 < argsl { if argsb.Data == nil { for i := 0; i < argsl; i++ { - list[xvl+i] = argsb.List[argso+i].unrefCopy(m.Alloc, m.Store) + list[xvl+i] = argsb.List[argso+i].Copy(m.Alloc, m.Store) } } else { copyDataToList( @@ -473,7 +471,7 @@ func UverseNode() *PackageNode { list := make([]TypedValue, listLen) if 0 < xvl { for i := 0; i < listLen; i++ { - list[i] = xvb.List[xvo+i].unrefCopy(m.Alloc, m.Store) + list[i] = xvb.List[xvo+i].Copy(m.Alloc, m.Store) } } if 0 < argsl { diff --git a/gnovm/pkg/gnolang/values.go b/gnovm/pkg/gnolang/values.go index 53d482613a1..b1c255aa450 100644 --- a/gnovm/pkg/gnolang/values.go +++ b/gnovm/pkg/gnolang/values.go @@ -97,6 +97,7 @@ func (bv *BigintValue) UnmarshalAmino(s string) error { } func (bv BigintValue) Copy(alloc *Allocator) BigintValue { + // XXX touch alloc. return BigintValue{V: big.NewInt(0).Set(bv.V)} } @@ -126,6 +127,7 @@ func (bv *BigdecValue) UnmarshalAmino(s string) error { } func (bv BigdecValue) Copy(alloc *Allocator) BigdecValue { + // XXX touch alloc. cp := apd.New(0, 0) _, err := apd.BaseContext.Add(cp, cp, bv.V) if err != nil { @@ -264,7 +266,8 @@ func (pv PointerValue) Assign2(alloc *Allocator, store Store, rlm *Realm, tv2 Ty tv2.String(), tv.T.String())) } } - *tv = tv2.Copy(alloc) + tv.CopyFrom(alloc, store, tv2) + // *tv = tv2.Copy(alloc) default: panic("should not happen") } @@ -278,11 +281,11 @@ func (pv PointerValue) Assign2(alloc *Allocator, store Store, rlm *Realm, tv2 Ty // General case if rlm != nil && pv.Base != nil { oo1 := pv.TV.GetFirstObject(store) - pv.TV.Assign(alloc, tv2, cu) + pv.TV.Assign(alloc, store, tv2, cu) oo2 := pv.TV.GetFirstObject(store) rlm.DidUpdate(pv.Base.(Object), oo1, oo2) } else { - pv.TV.Assign(alloc, tv2, cu) + pv.TV.Assign(alloc, store, tv2, cu) } } @@ -376,20 +379,60 @@ func (av *ArrayValue) GetPointerAtIndexInt2(store Store, ii int, et Type) Pointe } } -func (av *ArrayValue) Copy(alloc *Allocator) *ArrayValue { +// NOTE: may switch between .Data and .List depending on +// type of av2, but does not count toward allocation since +// it replaces one for the other. +func (av *ArrayValue) CopyFrom(alloc *Allocator, store Store, av2 *ArrayValue) { + if debug { + if av.GetLength() != av2.GetLength() { + panic(fmt.Sprintf( + "expected [%d]array but got [%d]array", + av.GetLength(), + av2.GetLength(), + )) + } + } + if av2.Data == nil { + if av.Data == nil { + // already fine. + } else { + av.Data = nil + av.List = make([]TypedValue, len(av2.List)) + } + avl := len(av2.List) + for i := 0; i < avl; i++ { + av.List[i] = av2.List[i].Copy(alloc, store) + } + } else { // av2.List == nil + if av.List == nil { + // already fine. + } else { + av.Data = make([]byte, len(av2.Data)) + av.List = nil + } + copy(av.Data, av2.Data) + } +} + +func (av *ArrayValue) Copy(alloc *Allocator, store Store) *ArrayValue { /* TODO: consider second ref count field. if av.GetRefCount() == 0 { return av } */ if av.Data == nil { - av2 := alloc.NewListArray(len(av.List)) - copy(av2.List, av.List) + avl := len(av.List) + av2 := alloc.NewListArray(avl) + for i := 0; i < avl; i++ { + av2.List[i] = av.List[i].Copy( + alloc, store) + } + return av2 + } else { // av.List == nil + av2 := alloc.NewDataArray(len(av.Data)) + copy(av2.Data, av.Data) return av2 } - av2 := alloc.NewDataArray(len(av.Data)) - copy(av2.Data, av.Data) - return av2 } // ---------------------------------------- @@ -494,7 +537,29 @@ func (sv *StructValue) GetSubrefPointerTo(store Store, st *StructType, path Valu } } -func (sv *StructValue) Copy(alloc *Allocator) *StructValue { +func (sv *StructValue) CopyFrom(alloc *Allocator, store Store, sv2 *StructValue) { + if debug { + if len(sv.Fields) != len(sv2.Fields) { + panic(fmt.Sprintf( + "expected %d fields but got %d", + len(sv.Fields), + len(sv2.Fields), + )) + } + } + + // Each field needs to be copied individually to ensure that + // value fields are copied as such, even though they may be represented + // as pointers. A good example of this would be a struct that has + // a field that is an array. The value array is represented as a pointer (*ArrayValue). + svf := sv.Fields + sv2f := sv2.Fields + for i := 0; i < len(svf); i++ { + svf[i].CopyFrom(alloc, store, sv2f[i]) + } +} + +func (sv *StructValue) Copy(alloc *Allocator, store Store) *StructValue { /* TODO consider second refcount field if sv.GetRefCount() == 0 { return sv @@ -507,7 +572,7 @@ func (sv *StructValue) Copy(alloc *Allocator) *StructValue { // as pointers. A good example of this would be a struct that has // a field that is an array. The value array is represented as a pointer. for i, field := range sv.Fields { - fields[i] = field.Copy(alloc) + fields[i] = field.Copy(alloc, store) } return alloc.NewStruct(fields) @@ -754,7 +819,7 @@ func (mv *MapValue) GetLength() int { func (mv *MapValue) GetPointerForKey(alloc *Allocator, store Store, key *TypedValue) PointerValue { kmk := key.ComputeMapKey(store, false) if mli, ok := mv.vmap[kmk]; ok { - key2 := key.Copy(alloc) + key2 := key.Copy(alloc, store) return PointerValue{ TV: fillValueTV(store, &mli.Value), Base: mv, @@ -764,7 +829,7 @@ func (mv *MapValue) GetPointerForKey(alloc *Allocator, store Store, key *TypedVa } mli := mv.List.Append(alloc, *key) mv.vmap[kmk] = mli - key2 := key.Copy(alloc) + key2 := key.Copy(alloc, store) return PointerValue{ TV: fillValueTV(store, &mli.Value), Base: mv, @@ -1007,17 +1072,54 @@ func (tv *TypedValue) ClearNum() { *(*uint64)(unsafe.Pointer(&tv.N)) = uint64(0) } -func (tv TypedValue) Copy(alloc *Allocator) (cp TypedValue) { +func (tv *TypedValue) CopyFrom(alloc *Allocator, store Store, tv2 TypedValue) { + switch tv2v := tv.V.(type) { + case BigintValue: + *tv = tv2.Copy(alloc, store) + case BigdecValue: + *tv = tv2.Copy(alloc, store) + case *ArrayValue: + tv.T = tv2.T + tv.V.(*ArrayValue).CopyFrom( + alloc, store, tv2v) + case *StructValue: + tv.T = tv2.T + tv.V.(*StructValue).CopyFrom( + alloc, store, tv2v) + case *NativeValue: + tv.T = tv2.T + tv.V = tv2v.Copy(alloc) // TODO optimize + default: + *tv = tv2 + } + return +} + +func (tv TypedValue) Copy(alloc *Allocator, store Store) (cp TypedValue) { switch cv := tv.V.(type) { case BigintValue: cp.T = tv.T cp.V = cv.Copy(alloc) - case *ArrayValue: + case BigdecValue: cp.T = tv.T cp.V = cv.Copy(alloc) + case RefValue: + cp.T = tv.T + refObject := tv.GetFirstObject(store) + switch refObjectValue := refObject.(type) { + case *ArrayValue: + cp.V = refObjectValue.Copy(alloc, store) + case *StructValue: + cp.V = refObjectValue.Copy(alloc, store) + default: + cp = tv + } + case *ArrayValue: + cp.T = tv.T + cp.V = cv.Copy(alloc, store) case *StructValue: cp.T = tv.T - cp.V = cv.Copy(alloc) + cp.V = cv.Copy(alloc, store) case *NativeValue: cp.T = tv.T cp.V = cv.Copy(alloc) @@ -1027,23 +1129,24 @@ 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. +// XXX DEPRECATED (Copy() now does this). +// unrefCopy performs a copy but first unrefs if ptr. func (tv TypedValue) unrefCopy(alloc *Allocator, store Store) (cp TypedValue) { + panic("XXX") 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) + cp.V = refObjectValue.Copy(alloc, store) case *StructValue: - cp.V = refObjectValue.Copy(alloc) + cp.V = refObjectValue.Copy(alloc, store) default: cp = tv } default: - cp = tv.Copy(alloc) + cp = tv.Copy(alloc, store) } return @@ -1578,7 +1681,7 @@ func (tv *TypedValue) ComputeMapKey(store Store, omitType bool) MapKey { // addressable NativeValue fields/elems. // cu: convert untyped after assignment. pass false // for const definitions, but true for all else. -func (tv *TypedValue) Assign(alloc *Allocator, tv2 TypedValue, cu bool) { +func (tv *TypedValue) Assign(alloc *Allocator, store Store, tv2 TypedValue, cu bool) { if debug { if tv.T == DataByteType { // assignment to data byte types should only @@ -1591,7 +1694,8 @@ func (tv *TypedValue) Assign(alloc *Allocator, tv2 TypedValue, cu bool) { panic("should not happen") } } - *tv = tv2.Copy(alloc) + tv.CopyFrom(alloc, store, tv2) + // *tv = tv2.Copy(alloc) if cu && isUntyped(tv.T) { ConvertUntypedTo(tv, defaultTypeOf(tv.T)) } @@ -1769,7 +1873,7 @@ func (tv *TypedValue) GetPointerTo(alloc *Allocator, store Store, path ValuePath panic("should not happen") } } - dtv2 := dtv.Copy(alloc) + dtv2 := dtv.Copy(alloc, store) alloc.AllocateBoundMethod() bmv := &BoundMethodValue{ Func: mv, From 60948d41d7f2883385d46caa81e19c958ca5ef16 Mon Sep 17 00:00:00 2001 From: jaekwon Date: Mon, 8 Jan 2024 16:50:40 -0800 Subject: [PATCH 2/5] commit working state --- gnovm/pkg/gnolang/op_assign.go | 2 +- gnovm/pkg/gnolang/op_decl.go | 2 +- gnovm/pkg/gnolang/uverse.go | 10 +-- gnovm/pkg/gnolang/values.go | 115 ++++++++++++++++++++++++--------- 4 files changed, 91 insertions(+), 38 deletions(-) diff --git a/gnovm/pkg/gnolang/op_assign.go b/gnovm/pkg/gnolang/op_assign.go index 0cc30861355..ac29ddd5311 100644 --- a/gnovm/pkg/gnolang/op_assign.go +++ b/gnovm/pkg/gnolang/op_assign.go @@ -20,7 +20,7 @@ func (m *Machine) doOpDefine() { } } } - ptr.Assign2(m.Alloc, m.Store, m.Realm, rvs[i], true) + ptr.Assign3(m.Alloc, m.Store, m.Realm, rvs[i], true) } } diff --git a/gnovm/pkg/gnolang/op_decl.go b/gnovm/pkg/gnolang/op_decl.go index 2c20c43ae2f..3563945f24b 100644 --- a/gnovm/pkg/gnolang/op_decl.go +++ b/gnovm/pkg/gnolang/op_decl.go @@ -60,7 +60,7 @@ func (m *Machine) doOpValueDecl() { } nx := s.NameExprs[i] ptr := lb.GetPointerTo(m.Store, nx.Path) - ptr.Assign2(m.Alloc, m.Store, m.Realm, tv, false) + ptr.Assign3(m.Alloc, m.Store, m.Realm, tv, false) } } diff --git a/gnovm/pkg/gnolang/uverse.go b/gnovm/pkg/gnolang/uverse.go index 237f85eb41a..adb011ea2e2 100644 --- a/gnovm/pkg/gnolang/uverse.go +++ b/gnovm/pkg/gnolang/uverse.go @@ -211,7 +211,7 @@ func UverseNode() *PackageNode { list := make([]TypedValue, argsl) if 0 < argsl { for i := 0; i < argsl; i++ { - list[i] = argsb.List[argso+i].Copy(m.Alloc, m.Store) + list[i] = argsb.List[argso+i].unrefCopy(m.Alloc, m.Store) } } m.PushValue(TypedValue{ @@ -296,7 +296,7 @@ func UverseNode() *PackageNode { if argsb.Data == nil { for i := 0; i < argsl; i++ { oldElem := list[xvo+xvl+i] - newElem := argsb.List[argso+i].Copy(m.Alloc, m.Store) + newElem := argsb.List[argso+i].unrefCopy(m.Alloc, m.Store) list[xvo+xvl+i] = newElem m.Realm.DidUpdate( @@ -374,7 +374,7 @@ func UverseNode() *PackageNode { if 0 < xvl { if xvb.Data == nil { for i := 0; i < xvl; i++ { - list[i] = xvb.List[xvo+i].Copy(m.Alloc, m.Store) + list[i] = xvb.List[xvo+i].unrefCopy(m.Alloc, m.Store) } } else { panic("should not happen") @@ -390,7 +390,7 @@ func UverseNode() *PackageNode { if 0 < argsl { if argsb.Data == nil { for i := 0; i < argsl; i++ { - list[xvl+i] = argsb.List[argso+i].Copy(m.Alloc, m.Store) + list[xvl+i] = argsb.List[argso+i].unrefCopy(m.Alloc, m.Store) } } else { copyDataToList( @@ -471,7 +471,7 @@ func UverseNode() *PackageNode { list := make([]TypedValue, listLen) if 0 < xvl { for i := 0; i < listLen; i++ { - list[i] = xvb.List[xvo+i].Copy(m.Alloc, m.Store) + list[i] = xvb.List[xvo+i].unrefCopy(m.Alloc, m.Store) } } if 0 < argsl { diff --git a/gnovm/pkg/gnolang/values.go b/gnovm/pkg/gnolang/values.go index b1c255aa450..d4811409e71 100644 --- a/gnovm/pkg/gnolang/values.go +++ b/gnovm/pkg/gnolang/values.go @@ -266,8 +266,8 @@ func (pv PointerValue) Assign2(alloc *Allocator, store Store, rlm *Realm, tv2 Ty tv2.String(), tv.T.String())) } } - tv.CopyFrom(alloc, store, tv2) - // *tv = tv2.Copy(alloc) + // tv.CopyFrom(alloc, store, tv2) + *tv = tv2.Copy(alloc, store) default: panic("should not happen") } @@ -289,6 +289,39 @@ func (pv PointerValue) Assign2(alloc *Allocator, store Store, rlm *Realm, tv2 Ty } } +// Like Assign2 but allocates first if needed. +// The type is copied from tv2, and a new value is +// constructed of the correct size. +// This is more expensive, and should only be called for := +// define and var decls. +func (pv PointerValue) Assign3(alloc *Allocator, store Store, rlm *Realm, tv2 TypedValue, cu bool) { + if pv.TV.IsUndefined() { + pv.TV.T = tv2.T + switch tv2v := tv2.V.(type) { + case *ArrayValue: + if tv2v.Data == nil { + pv.TV.V = alloc.NewListArray( + tv2v.GetLength()) + } else { + pv.TV.V = alloc.NewDataArray( + tv2v.GetLength()) + } + case *StructValue: + fields := alloc.NewStructFields( + len(tv2v.Fields)) + sv := alloc.NewStruct(fields) + pv.TV.V = sv + default: + pv.TV.V = nil + } + } + + // This is a bit weird because NativeValue + // is not handled above, but Native support + // will be removed anyways so it will resolve. + pv.Assign2(alloc, store, rlm, tv2, cu) +} + func (pv PointerValue) Deref() (tv TypedValue) { if pv.TV.T == DataByteType { dbv := pv.TV.V.(DataByteValue) @@ -382,7 +415,7 @@ func (av *ArrayValue) GetPointerAtIndexInt2(store Store, ii int, et Type) Pointe // NOTE: may switch between .Data and .List depending on // type of av2, but does not count toward allocation since // it replaces one for the other. -func (av *ArrayValue) CopyFrom(alloc *Allocator, store Store, av2 *ArrayValue) { +func (av *ArrayValue) CopyFrom(alloc *Allocator, store Store, av2 *ArrayValue, et Type) { if debug { if av.GetLength() != av2.GetLength() { panic(fmt.Sprintf( @@ -395,19 +428,25 @@ func (av *ArrayValue) CopyFrom(alloc *Allocator, store Store, av2 *ArrayValue) { if av2.Data == nil { if av.Data == nil { // already fine. - } else { + } else { // av.Data != nil + // convert to .List. + av.List = make([]TypedValue, len(av.Data)) + // XXX no need to do this. + // copyDataToList(av.List, av.Data, et) av.Data = nil - av.List = make([]TypedValue, len(av2.List)) } avl := len(av2.List) for i := 0; i < avl; i++ { av.List[i] = av2.List[i].Copy(alloc, store) } - } else { // av2.List == nil + } else { // av2.Data != nil if av.List == nil { // already fine. - } else { - av.Data = make([]byte, len(av2.Data)) + } else { // av.List != nil + // convert to .Data. + av.Data = make([]byte, len(av.List)) + // XXX no need to do this. + // copyListToData(av.Data, av.List) av.List = nil } copy(av.Data, av2.Data) @@ -421,12 +460,17 @@ func (av *ArrayValue) Copy(alloc *Allocator, store Store) *ArrayValue { } */ if av.Data == nil { - avl := len(av.List) - av2 := alloc.NewListArray(avl) - for i := 0; i < avl; i++ { - av2.List[i] = av.List[i].Copy( - alloc, store) - } + /* + avl := len(av.List) + av2 := alloc.NewListArray(avl) + for i := 0; i < avl; i++ { + av2.List[i] = av.List[i].Copy( + alloc, store) + } + return av2 + */ + av2 := alloc.NewListArray(len(av.List)) + copy(av2.List, av.List) return av2 } else { // av.List == nil av2 := alloc.NewDataArray(len(av.Data)) @@ -1073,22 +1117,30 @@ func (tv *TypedValue) ClearNum() { } func (tv *TypedValue) CopyFrom(alloc *Allocator, store Store, tv2 TypedValue) { - switch tv2v := tv.V.(type) { + switch tv.V.(type) { case BigintValue: *tv = tv2.Copy(alloc, store) case BigdecValue: *tv = tv2.Copy(alloc, store) case *ArrayValue: tv.T = tv2.T + tv2v := tv2.V.(*ArrayValue) tv.V.(*ArrayValue).CopyFrom( - alloc, store, tv2v) + alloc, store, tv2v, tv.T.Elem()) case *StructValue: tv.T = tv2.T + tv2v := tv2.V.(*StructValue) tv.V.(*StructValue).CopyFrom( alloc, store, tv2v) case *NativeValue: - tv.T = tv2.T - tv.V = tv2v.Copy(alloc) // TODO optimize + if tv2.IsUndefined() { + tv.T = nil + tv.V = nil + } else { + tv.T = tv2.T + tv2v := tv2.V.(*NativeValue) + tv.V = tv2v.Copy(alloc) // TODO optimize + } default: *tv = tv2 } @@ -1103,17 +1155,19 @@ func (tv TypedValue) Copy(alloc *Allocator, store Store) (cp TypedValue) { case BigdecValue: cp.T = tv.T cp.V = cv.Copy(alloc) - case RefValue: - cp.T = tv.T - refObject := tv.GetFirstObject(store) - switch refObjectValue := refObject.(type) { - case *ArrayValue: - cp.V = refObjectValue.Copy(alloc, store) - case *StructValue: - cp.V = refObjectValue.Copy(alloc, store) - default: - cp = tv - } + /* + case RefValue: + cp.T = tv.T + refObject := tv.GetFirstObject(store) + switch refObjectValue := refObject.(type) { + case *ArrayValue: + cp.V = refObjectValue.Copy(alloc, store) + case *StructValue: + cp.V = refObjectValue.Copy(alloc, store) + default: + cp = tv + } + */ case *ArrayValue: cp.T = tv.T cp.V = cv.Copy(alloc, store) @@ -1132,7 +1186,6 @@ func (tv TypedValue) Copy(alloc *Allocator, store Store) (cp TypedValue) { // XXX DEPRECATED (Copy() now does this). // unrefCopy performs a copy but first unrefs if ptr. func (tv TypedValue) unrefCopy(alloc *Allocator, store Store) (cp TypedValue) { - panic("XXX") switch tv.V.(type) { case RefValue: cp.T = tv.T @@ -1695,7 +1748,7 @@ func (tv *TypedValue) Assign(alloc *Allocator, store Store, tv2 TypedValue, cu b } } tv.CopyFrom(alloc, store, tv2) - // *tv = tv2.Copy(alloc) + // *tv = tv2.Copy(alloc, store) if cu && isUntyped(tv.T) { ConvertUntypedTo(tv, defaultTypeOf(tv.T)) } From 4a90412830517a683187d008ff96d4a2dde3aa75 Mon Sep 17 00:00:00 2001 From: jaekwon Date: Tue, 9 Jan 2024 19:54:55 -0800 Subject: [PATCH 3/5] new approach... lean into the copy --- gnovm/pkg/gnolang/op_assign.go | 2 +- gnovm/pkg/gnolang/op_decl.go | 2 +- gnovm/pkg/gnolang/values.go | 26 +++++++++++++++++++------- 3 files changed, 21 insertions(+), 9 deletions(-) diff --git a/gnovm/pkg/gnolang/op_assign.go b/gnovm/pkg/gnolang/op_assign.go index ac29ddd5311..0cc30861355 100644 --- a/gnovm/pkg/gnolang/op_assign.go +++ b/gnovm/pkg/gnolang/op_assign.go @@ -20,7 +20,7 @@ func (m *Machine) doOpDefine() { } } } - ptr.Assign3(m.Alloc, m.Store, m.Realm, rvs[i], true) + ptr.Assign2(m.Alloc, m.Store, m.Realm, rvs[i], true) } } diff --git a/gnovm/pkg/gnolang/op_decl.go b/gnovm/pkg/gnolang/op_decl.go index 3563945f24b..2c20c43ae2f 100644 --- a/gnovm/pkg/gnolang/op_decl.go +++ b/gnovm/pkg/gnolang/op_decl.go @@ -60,7 +60,7 @@ func (m *Machine) doOpValueDecl() { } nx := s.NameExprs[i] ptr := lb.GetPointerTo(m.Store, nx.Path) - ptr.Assign3(m.Alloc, m.Store, m.Realm, tv, false) + ptr.Assign2(m.Alloc, m.Store, m.Realm, tv, false) } } diff --git a/gnovm/pkg/gnolang/values.go b/gnovm/pkg/gnolang/values.go index d4811409e71..37f7ccfa15b 100644 --- a/gnovm/pkg/gnolang/values.go +++ b/gnovm/pkg/gnolang/values.go @@ -279,11 +279,22 @@ func (pv PointerValue) Assign2(alloc *Allocator, store Store, rlm *Realm, tv2 Ty return } // General case - if rlm != nil && pv.Base != nil { - oo1 := pv.TV.GetFirstObject(store) - pv.TV.Assign(alloc, store, tv2, cu) - oo2 := pv.TV.GetFirstObject(store) - rlm.DidUpdate(pv.Base.(Object), oo1, oo2) + if rlm != nil { + if pv.Base == nil { + oo1 := pv.TV.GetFirstObject(store) + pv.TV.Assign(alloc, store, tv2, cu) + // preserve id if oo1 was real. + if oo1.GetIsReal() { + oo2 := pv.TV.GetFirstObject(store) + oo2.SetObjectID(oo1.GetObjectID()) + rlm.DidUpdate(oo2, nil, nil) // XXX ??? + } + } else { // pv.Base != nil + oo1 := pv.TV.GetFirstObject(store) + pv.TV.Assign(alloc, store, tv2, cu) + oo2 := pv.TV.GetFirstObject(store) + rlm.DidUpdate(pv.Base.(Object), oo1, oo2) + } } else { pv.TV.Assign(alloc, store, tv2, cu) } @@ -416,6 +427,7 @@ func (av *ArrayValue) GetPointerAtIndexInt2(store Store, ii int, et Type) Pointe // type of av2, but does not count toward allocation since // it replaces one for the other. func (av *ArrayValue) CopyFrom(alloc *Allocator, store Store, av2 *ArrayValue, et Type) { + panic("!!") if debug { if av.GetLength() != av2.GetLength() { panic(fmt.Sprintf( @@ -1747,8 +1759,8 @@ func (tv *TypedValue) Assign(alloc *Allocator, store Store, tv2 TypedValue, cu b panic("should not happen") } } - tv.CopyFrom(alloc, store, tv2) - // *tv = tv2.Copy(alloc, store) + //tv.CopyFrom(alloc, store, tv2) + *tv = tv2.Copy(alloc, store) if cu && isUntyped(tv.T) { ConvertUntypedTo(tv, defaultTypeOf(tv.T)) } From 080e82a151dfbeceeda7274c2a0d25ea757f59f3 Mon Sep 17 00:00:00 2001 From: jaekwon Date: Tue, 9 Jan 2024 20:33:54 -0800 Subject: [PATCH 4/5] ... --- gnovm/pkg/gnolang/ownership.go | 5 +++++ gnovm/pkg/gnolang/realm.go | 6 ++++++ gnovm/pkg/gnolang/values.go | 3 +++ 3 files changed, 14 insertions(+) diff --git a/gnovm/pkg/gnolang/ownership.go b/gnovm/pkg/gnolang/ownership.go index f2afc393d05..6ff59684945 100644 --- a/gnovm/pkg/gnolang/ownership.go +++ b/gnovm/pkg/gnolang/ownership.go @@ -104,6 +104,7 @@ type Object interface { IncRefCount() int DecRefCount() int GetRefCount() int + SetRefCount(int) GetIsDirty() bool SetIsDirty(bool, uint64) GetIsEscaped() bool @@ -265,6 +266,10 @@ func (oi *ObjectInfo) GetRefCount() int { return oi.RefCount } +func (oi *ObjectInfo) SetRefCount(rc int) { + oi.RefCount = rc +} + func (oi *ObjectInfo) GetIsDirty() bool { return oi.isDirty } diff --git a/gnovm/pkg/gnolang/realm.go b/gnovm/pkg/gnolang/realm.go index 519b183ad3a..7c2506c180b 100644 --- a/gnovm/pkg/gnolang/realm.go +++ b/gnovm/pkg/gnolang/realm.go @@ -683,6 +683,7 @@ func (rlm *Realm) saveUnsavedObjectRecursively(store Store, oo Object) { } // first, save unsaved children. unsaved := getUnsavedChildObjects(oo) + fmt.Println("!!!!!", oo, ">>", unsaved) for _, uch := range unsaved { if uch.GetIsEscaped() || uch.GetIsNewEscaped() { // no need to save preemptively. @@ -822,6 +823,7 @@ func getChildObjects(val Value, more []Value) []Value { case DataByteValue: panic("should not happen") case PointerValue: + fmt.Println("GETCHILDOBJECTS:POINTER", cv) if cv.Base != nil { more = getSelfOrChildObjects(cv.Base, more) } else { @@ -837,6 +839,7 @@ func getChildObjects(val Value, more []Value) []Value { more = getSelfOrChildObjects(cv.Base, more) return more case *StructValue: + fmt.Println("STRUCT", cv) for _, ctv := range cv.Fields { more = getSelfOrChildObjects(ctv.V, more) } @@ -901,6 +904,7 @@ func getChildObjects2(store Store, val Value) []Object { // Shallow; doesn't recurse into objects. func getUnsavedChildObjects(val Value) []Object { vals := getChildObjects(val, nil) + fmt.Println("getChildObjects", val, ">>", vals) unsaved := make([]Object, 0, len(vals)) for _, val := range vals { // sanity check: @@ -914,6 +918,7 @@ func getUnsavedChildObjects(val Value) []Object { // is already saved. } else if obj, ok := val.(Object); ok { // if object... + fmt.Println("isUnsaved", obj, ">>", isUnsaved(obj)) if isUnsaved(obj) { unsaved = append(unsaved, obj) } @@ -1434,6 +1439,7 @@ func toRefValue(parent Object, val Value) RefValue { PkgPath: pv.PkgPath, } } else if !oo.GetIsReal() { + fmt.Println("!!!", oo.GetObjectInfo()) panic("unexpected unreal object") } else if oo.GetIsDirty() { // This can happen with some circular diff --git a/gnovm/pkg/gnolang/values.go b/gnovm/pkg/gnolang/values.go index 37f7ccfa15b..98f498683de 100644 --- a/gnovm/pkg/gnolang/values.go +++ b/gnovm/pkg/gnolang/values.go @@ -287,7 +287,10 @@ func (pv PointerValue) Assign2(alloc *Allocator, store Store, rlm *Realm, tv2 Ty if oo1.GetIsReal() { oo2 := pv.TV.GetFirstObject(store) oo2.SetObjectID(oo1.GetObjectID()) + oo2.SetRefCount(oo1.GetRefCount()) + fmt.Println(">>>>", oo2.GetObjectID(), oo2.GetIsReal(), oo2.GetIsDirty()) rlm.DidUpdate(oo2, nil, nil) // XXX ??? + fmt.Println(">>>>", oo2.GetObjectID(), oo2.GetIsReal(), oo2.GetIsDirty()) } } else { // pv.Base != nil oo1 := pv.TV.GetFirstObject(store) From 0e6e0dbf4c1371f097cb870302bcf68478fd74ac Mon Sep 17 00:00:00 2001 From: jaekwon Date: Tue, 9 Jan 2024 22:38:50 -0800 Subject: [PATCH 5/5] this almost works but alas... --- .../gnoland/testdata/unexpected-unreal.txtar | 2 +- gnovm/pkg/gnolang/ownership.go | 12 +++++++++ gnovm/pkg/gnolang/realm.go | 23 +++++++++++------ gnovm/pkg/gnolang/values.go | 25 ++++++++++++++----- 4 files changed, 47 insertions(+), 15 deletions(-) diff --git a/gno.land/cmd/gnoland/testdata/unexpected-unreal.txtar b/gno.land/cmd/gnoland/testdata/unexpected-unreal.txtar index fb67882a6dd..d4b74651321 100644 --- a/gno.land/cmd/gnoland/testdata/unexpected-unreal.txtar +++ b/gno.land/cmd/gnoland/testdata/unexpected-unreal.txtar @@ -12,7 +12,7 @@ stdout OK! # execute Delta for the first time gnokey maketx call -pkgpath gno.land/r/demo/xx -func Delta -gas-fee 1000000ugnot -gas-wanted 2000000 -broadcast -chainid=tendermint_test test1 -stdout OK!xx +stdout OK! -- gno.mod -- module gno.land/r/demo/xx diff --git a/gnovm/pkg/gnolang/ownership.go b/gnovm/pkg/gnolang/ownership.go index 6ff59684945..d79e4a37d84 100644 --- a/gnovm/pkg/gnolang/ownership.go +++ b/gnovm/pkg/gnolang/ownership.go @@ -118,6 +118,8 @@ type Object interface { GetIsNewDeleted() bool SetIsNewDeleted(bool) GetIsTransient() bool + GetNextObjectID() ObjectID + SetNextObjectID(ObjectID) // Saves to realm along the way if owned, and also (dirty // or new). @@ -145,6 +147,7 @@ type ObjectInfo struct { isNewReal bool isNewEscaped bool isNewDeleted bool + nextID ObjectID // set if replacing pre-existing object. // XXX huh? owner Object // mem reference to owner. @@ -165,6 +168,7 @@ func (oi *ObjectInfo) Copy() ObjectInfo { isNewReal: oi.isNewReal, isNewEscaped: oi.isNewEscaped, isNewDeleted: oi.isNewDeleted, + nextID: oi.nextID, } } @@ -329,6 +333,14 @@ func (oi *ObjectInfo) GetIsTransient() bool { return false } +func (oi *ObjectInfo) GetNextObjectID() ObjectID { + return oi.nextID +} + +func (oi *ObjectInfo) SetNextObjectID(nid ObjectID) { + oi.nextID = nid +} + func (tv *TypedValue) GetFirstObject(store Store) Object { switch cv := tv.V.(type) { case PointerValue: diff --git a/gnovm/pkg/gnolang/realm.go b/gnovm/pkg/gnolang/realm.go index 7c2506c180b..7de61cd9607 100644 --- a/gnovm/pkg/gnolang/realm.go +++ b/gnovm/pkg/gnolang/realm.go @@ -390,8 +390,21 @@ func (rlm *Realm) incRefCreatedDescendants(store Store, oo Object) { if !oo.GetObjectID().IsZero() { return } - rlm.assignNewObjectID(oo) - rlm.created = append(rlm.created, oo) + noid := oo.GetNextObjectID() + if !noid.IsZero() { + // consume NextObjectID. + oo.SetObjectID(noid) + oo.SetNextObjectID(ObjectID{}) + // rlm.created not touched, not actually. + // instead, is considered updated. + rlm.updated = append(rlm.updated, oo) + // however, we do need the following + // recurse logic to inc refcount + // children of this replacement object. + } else { + rlm.assignNewObjectID(oo) + rlm.created = append(rlm.created, oo) + } // RECURSE GUARD END // recurse for children. @@ -683,7 +696,6 @@ func (rlm *Realm) saveUnsavedObjectRecursively(store Store, oo Object) { } // first, save unsaved children. unsaved := getUnsavedChildObjects(oo) - fmt.Println("!!!!!", oo, ">>", unsaved) for _, uch := range unsaved { if uch.GetIsEscaped() || uch.GetIsNewEscaped() { // no need to save preemptively. @@ -823,7 +835,6 @@ func getChildObjects(val Value, more []Value) []Value { case DataByteValue: panic("should not happen") case PointerValue: - fmt.Println("GETCHILDOBJECTS:POINTER", cv) if cv.Base != nil { more = getSelfOrChildObjects(cv.Base, more) } else { @@ -839,7 +850,6 @@ func getChildObjects(val Value, more []Value) []Value { more = getSelfOrChildObjects(cv.Base, more) return more case *StructValue: - fmt.Println("STRUCT", cv) for _, ctv := range cv.Fields { more = getSelfOrChildObjects(ctv.V, more) } @@ -904,7 +914,6 @@ func getChildObjects2(store Store, val Value) []Object { // Shallow; doesn't recurse into objects. func getUnsavedChildObjects(val Value) []Object { vals := getChildObjects(val, nil) - fmt.Println("getChildObjects", val, ">>", vals) unsaved := make([]Object, 0, len(vals)) for _, val := range vals { // sanity check: @@ -918,7 +927,6 @@ func getUnsavedChildObjects(val Value) []Object { // is already saved. } else if obj, ok := val.(Object); ok { // if object... - fmt.Println("isUnsaved", obj, ">>", isUnsaved(obj)) if isUnsaved(obj) { unsaved = append(unsaved, obj) } @@ -1439,7 +1447,6 @@ func toRefValue(parent Object, val Value) RefValue { PkgPath: pv.PkgPath, } } else if !oo.GetIsReal() { - fmt.Println("!!!", oo.GetObjectInfo()) panic("unexpected unreal object") } else if oo.GetIsDirty() { // This can happen with some circular diff --git a/gnovm/pkg/gnolang/values.go b/gnovm/pkg/gnolang/values.go index 98f498683de..d64c1cca0f5 100644 --- a/gnovm/pkg/gnolang/values.go +++ b/gnovm/pkg/gnolang/values.go @@ -284,13 +284,26 @@ func (pv PointerValue) Assign2(alloc *Allocator, store Store, rlm *Realm, tv2 Ty oo1 := pv.TV.GetFirstObject(store) pv.TV.Assign(alloc, store, tv2, cu) // preserve id if oo1 was real. - if oo1.GetIsReal() { + oo1id := oo1.GetObjectID() + if oo1id.IsZero() { + // passed by intermediary. + oo1id = oo1.GetNextObjectID() + } + // if oo1 is/was real escaped: + if !oo1id.IsZero() { + oo1id := oo1.GetObjectID() + oo1rc := oo1.GetRefCount() oo2 := pv.TV.GetFirstObject(store) - oo2.SetObjectID(oo1.GetObjectID()) - oo2.SetRefCount(oo1.GetRefCount()) - fmt.Println(">>>>", oo2.GetObjectID(), oo2.GetIsReal(), oo2.GetIsDirty()) - rlm.DidUpdate(oo2, nil, nil) // XXX ??? - fmt.Println(">>>>", oo2.GetObjectID(), oo2.GetIsReal(), oo2.GetIsDirty()) + // oo1 is deleted + oo1.SetRefCount(0) + rlm.MarkNewDeleted(oo1) + + // oo2 is updated + oo2.SetRefCount(oo1rc) + // SetNextObjectID() makes + // this register as updated. + oo2.SetNextObjectID(oo1id) + rlm.MarkNewReal(oo2) } } else { // pv.Base != nil oo1 := pv.TV.GetFirstObject(store)