Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(gnovm): assign to deref pointers #1919

Closed
wants to merge 18 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 68 additions & 0 deletions gno.land/cmd/gnoland/testdata/issue-1326.txtar
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
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 '("1,1,1;" string)'
stdout OK!

gnokey maketx call -pkgpath gno.land/r/demo/xx -func Delta -gas-fee 1000000ugnot -gas-wanted 2000000 -broadcast -chainid=tendermint_test test1
stdout '("1,1,1;2,2,2;" string)'
stdout OK!

-- realm.gno --
package xx

import (
"strconv"
)

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 = new(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
}
61 changes: 61 additions & 0 deletions gno.land/cmd/gnoland/testdata/star-assign.txtar
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
loadpkg gno.land/p/demo/ufmt

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!

gnokey maketx call -pkgpath gno.land/r/demo/xx -func Delta -gas-fee 1000000ugnot -gas-wanted 2000000 -broadcast -chainid=tendermint_test test1
stdout OK!

gnokey maketx call -pkgpath gno.land/r/demo/xx -func Values -gas-fee 1000000ugnot -gas-wanted 2000000 -broadcast -chainid=tendermint_test test1
stdout '("1, hello, 100, 100, 8, 5, 8" string)'
stdout OK!

-- realm.gno --
package xx

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

type A struct {
nums []int
}

var (
intPtr *int
strPtr *string
aPtr *A
concretePtr *int
concreteValue int
)

func New() {
intPtr = new(int)
strPtr = new(string)
aPtr = &A{}
concretePtr = &concreteValue
}

func Delta() {
*intPtr++
deelawn marked this conversation as resolved.
Show resolved Hide resolved
*strPtr += "hello"
*aPtr = A{nums: []int{8, 5, 8}}
*concretePtr = 100
}

func Values() string {
var results string
results += ufmt.Sprintf("%d, %s, %d, %d", *intPtr, *strPtr, *concretePtr, concreteValue)
for _, n := range aPtr.nums {
results += ufmt.Sprintf(", %d", n)
}

return results
}
23 changes: 22 additions & 1 deletion gnovm/pkg/gnolang/machine.go
Original file line number Diff line number Diff line change
Expand Up @@ -1904,7 +1904,14 @@
m.PushOp(OpEval)
case *StarExpr:
// evaluate X (a reference)
m.PushExpr(lx.X)

// The star expression is on the LHS, so evaluate the expression as
// a reference. This ensures the value that is pushed is a pointer to
// the pointer value represented by the lx.X expression. This will be
// helpful if the underlying pointer value dos not have a base;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does not have a base

// the base of the reference to the pointer value can be used instead,
// to properly mark the owning object as dirty.
m.PushExpr(&RefExpr{X: lx.X})
m.PushOp(OpEval)
case *CompositeLitExpr: // for *RefExpr e.g. &mystruct{}
// evaluate lx.
Expand All @@ -1931,6 +1938,20 @@
return xv.GetPointerTo(m.Alloc, m.Store, lx.Path)
case *StarExpr:
ptr := m.PopValue().V.(PointerValue)

// A star expression on the lefthand side of an assign statement is a bit of a
// special case and needs to be handled. The value pushed for it is always a
// reference to the actual value, so first dereference it by assigning it to
// valuePtr. If the pointer value has no base, use the base of its parent.

Check warning on line 1945 in gnovm/pkg/gnolang/machine.go

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/gnolang/machine.go#L1942-L1945

Added lines #L1942 - L1945 were not covered by tests
// Assigning a non-nil base ensures the parent gets marked as dirty and the
// updated value is persisted during realm finalization.
valuePtr := ptr.TV.V.(PointerValue)
if valuePtr.Base == nil {
valuePtr.Base = ptr.Base
}

ptr = valuePtr

return ptr
case *CompositeLitExpr: // for *RefExpr
tv := *m.PopValue()
Expand Down
Loading