Skip to content

Commit

Permalink
fix: don't pass value types by reference (#1263)
Browse files Browse the repository at this point in the history
Addresses #1096

These changes ensure that passing non-primitive value types to a
function do not result in the modification of the original value, even
if the value type is represented as a pointer inside the VM.
  • Loading branch information
deelawn authored Nov 7, 2023
1 parent a88e3e3 commit 1732214
Show file tree
Hide file tree
Showing 3 changed files with 101 additions and 2 deletions.
6 changes: 5 additions & 1 deletion gnovm/pkg/gnolang/op_call.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,11 @@ func (m *Machine) doOpCall() {
}
// TODO: some more pt <> pv.Type
// reconciliations/conversions necessary.
b.Values[i] = pv

// 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)
}
}

Expand Down
10 changes: 9 additions & 1 deletion gnovm/pkg/gnolang/values.go
Original file line number Diff line number Diff line change
Expand Up @@ -500,7 +500,15 @@ func (sv *StructValue) Copy(alloc *Allocator) *StructValue {
}
*/
fields := alloc.NewStructFields(len(sv.Fields))
copy(fields, sv.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.
for i, field := range sv.Fields {
fields[i] = field.Copy(alloc)
}

return alloc.NewStruct(fields)
}

Expand Down
87 changes: 87 additions & 0 deletions gnovm/tests/files/issue-1096.gno
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
package main

import "fmt"

type X struct {
Array [8]int
Test bool
}

type Y [8]int

func main() {
x := X{}
x.Array[1] = 888
println(x.Array[1])
println(x.Array[2])
println(x.Test)

x.manip()
println(x.Array[1])
println(x.Array[2])
println(x.Test)

println("-----")

y := Y{}
y[1] = 888
println(y[1])
println(y[2])

y.manip()
println(y[1])
println(y[2])
println("-----")

x = X{}
println(x.Array[1])
println(x.Array[2])
println(x.Test)

x.Array[1] = 888
println(x.Array[1])
println(x.Array[2])
println(x.Test)

manip(x)
println(x.Array[1])
println(x.Array[2])
println(x.Test)
}

func (x X) manip() {
x.Array[2] = 999
x.Test = true
}

func manip(x X) {
x.Array[2] = 999
x.Test = true
}

func (y Y) manip() {
y[2] = 111
}

// Output:
// 888
// 0
// false
// 888
// 0
// false
// -----
// 888
// 0
// 888
// 0
// -----
// 0
// 0
// false
// 888
// 0
// false
// 888
// 0
// false

0 comments on commit 1732214

Please sign in to comment.