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.

Sign up for GitHub

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

Jump to bottom

WIP fix: handle assignments to dereferenced pointer values #1501

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from
Draft

WIP fix: handle assignments to dereferenced pointer values #1501

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
19961af
initial commit; test fails
jaekwon Jan 7, 2024
60948d4
commit working state
jaekwon Jan 9, 2024
4a90412
new approach... lean into the copy
jaekwon Jan 10, 2024
080e82a
...
jaekwon Jan 10, 2024
0e6e0db
this almost works but alas...
jaekwon Jan 10, 2024
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
75 changes: 75 additions & 0 deletions gno.land/cmd/gnoland/testdata/unexpected-unreal.txtar
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
# Reproducible Test for https://github.com/gnolang/gno/issues/1167
Copy link
Member

Choose a reason for hiding this comment

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

I'm glad you tried txtar-based tests. How was your experience? Do you have any suggestions for improvement? If yes, please refer to #1269.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This wasn't me, I just copied this: https://gist.github.com/thehowl/e36b0f0d652a2a348a2fcd331a310417. Added a comment there. We should avoid using environment variables for production. Other than that I'm liking it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My main thing with tests is this:

I just want "make test" to work as expected, at the root, and at the individual project levels. At the root it should just pass any options and call make test individually.

Anyone who pulls the codebase should be able to get make test passing, and go test ./... should all work as expected too.

Copy link
Member

Choose a reason for hiding this comment

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

My main thing with tests is this:

I just want "make test" to work as expected, at the root, and at the individual project levels. At the root it should just pass any options and call make test individually.

Anyone who pulls the codebase should be able to get make test passing, and go test ./... should all work as expected too.

That is the case!


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
}

4 changes: 2 additions & 2 deletions gnovm/pkg/gnolang/op_call.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

Expand Down Expand Up @@ -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)
}
Expand Down
16 changes: 8 additions & 8 deletions gnovm/pkg/gnolang/op_exec.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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")
}
Expand All @@ -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")
}
Expand Down Expand Up @@ -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")
}
Expand All @@ -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")
}
Expand Down Expand Up @@ -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")
}
Expand All @@ -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")
}
Expand Down Expand Up @@ -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) {
Expand Down
12 changes: 5 additions & 7 deletions gnovm/pkg/gnolang/uverse.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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")
Expand All @@ -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(
Expand Down Expand Up @@ -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 {
Expand Down
Loading
Loading