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): Make GnoVM slice allocations before go runtime allocations #2781

Merged
merged 4 commits into from
Sep 12, 2024
Merged
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
16 changes: 16 additions & 0 deletions gno.land/cmd/gnoland/testdata/alloc_array.txtar
thehowl marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
loadpkg gno.land/r/alloc $WORK

gnoland start

! gnokey maketx call -pkgpath gno.land/r/alloc -func DoAlloc -gas-fee 1000000ugnot -gas-wanted 2000000 -broadcast -chainid=tendermint_test test1
stderr 'Data: allocation limit exceeded'

-- alloc.gno --
package alloc

var buffer interface{}

func DoAlloc() {
var arr [1_000_000_000_000_000]byte
buffer = arr
}
16 changes: 16 additions & 0 deletions gno.land/cmd/gnoland/testdata/alloc_byte_slice.txtar
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
loadpkg gno.land/r/alloc $WORK

gnoland start

! gnokey maketx call -pkgpath gno.land/r/alloc -func DoAlloc -gas-fee 1000000ugnot -gas-wanted 2000000 -broadcast -chainid=tendermint_test test1
stderr 'Data: allocation limit exceeded'

-- alloc.gno --
package alloc

var buffer []byte

func DoAlloc() {
buffer := make([]byte, 1_000_000_000_000)
buffer[1] = 'a'
}
16 changes: 16 additions & 0 deletions gno.land/cmd/gnoland/testdata/alloc_slice.txtar
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
loadpkg gno.land/r/alloc $WORK

gnoland start

! gnokey maketx call -pkgpath gno.land/r/alloc -func DoAlloc -gas-fee 1000000ugnot -gas-wanted 2000000 -broadcast -chainid=tendermint_test test1
stderr 'Data: allocation limit exceeded'

-- alloc.gno --
package alloc

var buffer []int

func DoAlloc() {
buffer := make([]int, 1_000_000_000_000)
buffer[1] = 1
}
11 changes: 9 additions & 2 deletions gnovm/pkg/gnolang/alloc.go
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,12 @@ func (alloc *Allocator) NewSlice(base Value, offset, length, maxcap int) *SliceV
}
}

// NOTE: also allocates the underlying array from list.
// NewSliceFromList allocates a new slice with the underlying array value
// populated from `list`. This should not be called from areas in the codebase
// that are doing allocations with potentially large user provided values, e.g.
// `make()` and `append()`. Using `Alloc.NewListArray` can be used is most cases
// to allocate the space for the `TypedValue` list before doing the allocation
// in the go runtime -- see the `make()` code in uverse.go.
func (alloc *Allocator) NewSliceFromList(list []TypedValue) *SliceValue {
alloc.AllocateSlice()
alloc.AllocateListArray(int64(cap(list)))
Expand All @@ -238,7 +243,9 @@ func (alloc *Allocator) NewSliceFromList(list []TypedValue) *SliceValue {
}
}

// NOTE: also allocates the underlying array from data.
// NewSliceFromData allocates a new slice with the underlying data array
// value populated from `data`. See the doc for `NewSliceFromList` for
// correct usage notes.
func (alloc *Allocator) NewSliceFromData(data []byte) *SliceValue {
alloc.AllocateSlice()
alloc.AllocateDataArray(int64(cap(data)))
Expand Down
116 changes: 63 additions & 53 deletions gnovm/pkg/gnolang/uverse.go
Original file line number Diff line number Diff line change
Expand Up @@ -193,32 +193,32 @@
return
} else if arg0Type.Elem().Kind() == Uint8Kind {
// append(nil, *SliceValue) new data bytes ---
data := make([]byte, arg1Length)
arrayValue := m.Alloc.NewDataArray(arg1Length)
if arg1Base.Data == nil {
copyListToData(
data[:arg1Length],
arrayValue.Data[:arg1Length],
arg1Base.List[arg1Offset:arg1EndIndex])
} else {
copy(
data[:arg1Length],
arrayValue.Data[:arg1Length],
arg1Base.Data[arg1Offset:arg1EndIndex])
}
m.PushValue(TypedValue{
T: arg0Type,
V: m.Alloc.NewSliceFromData(data),
V: m.Alloc.NewSlice(arrayValue, 0, arg1Length, arg1Length),
})
return
} else {
// append(nil, *SliceValue) new list ---------
list := make([]TypedValue, arg1Length)
if 0 < arg1Length {
arrayValue := m.Alloc.NewListArray(arg1Length)
if arg1Length > 0 {
for i := 0; i < arg1Length; i++ {
list[i] = arg1Base.List[arg1Offset+i].unrefCopy(m.Alloc, m.Store)
arrayValue.List[i] = arg1Base.List[arg1Offset+i].unrefCopy(m.Alloc, m.Store)
}
}
m.PushValue(TypedValue{
T: arg0Type,
V: m.Alloc.NewSliceFromList(list),
V: m.Alloc.NewSlice(arrayValue, 0, arg1Length, arg1Length),
})
return
}
Expand All @@ -236,27 +236,27 @@
return
} else if arg0Type.Elem().Kind() == Uint8Kind {
// append(nil, *NativeValue) new data bytes --
data := make([]byte, arg1NativeValueLength)
arrayValue := m.Alloc.NewDataArray(arg1NativeValueLength)

Check warning on line 239 in gnovm/pkg/gnolang/uverse.go

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/gnolang/uverse.go#L239

Added line #L239 was not covered by tests
copyNativeToData(
data[:arg1NativeValueLength],
arrayValue.Data[:arg1NativeValueLength],

Check warning on line 241 in gnovm/pkg/gnolang/uverse.go

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/gnolang/uverse.go#L241

Added line #L241 was not covered by tests
arg1NativeValue, arg1NativeValueLength)
m.PushValue(TypedValue{
T: arg0Type,
V: m.Alloc.NewSliceFromData(data),
V: m.Alloc.NewSlice(arrayValue, 0, arg1NativeValueLength, arg1NativeValueLength),

Check warning on line 245 in gnovm/pkg/gnolang/uverse.go

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/gnolang/uverse.go#L245

Added line #L245 was not covered by tests
})
return
} else {
// append(nil, *NativeValue) new list --------
list := make([]TypedValue, arg1NativeValueLength)
if 0 < arg1NativeValueLength {
arrayValue := m.Alloc.NewListArray(arg1NativeValueLength)
if arg1NativeValueLength > 0 {

Check warning on line 251 in gnovm/pkg/gnolang/uverse.go

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/gnolang/uverse.go#L250-L251

Added lines #L250 - L251 were not covered by tests
copyNativeToList(
m.Alloc,
list[:arg1NativeValueLength],
arrayValue.List[:arg1NativeValueLength],

Check warning on line 254 in gnovm/pkg/gnolang/uverse.go

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/gnolang/uverse.go#L254

Added line #L254 was not covered by tests
arg1NativeValue, arg1NativeValueLength)
}
m.PushValue(TypedValue{
T: arg0Type,
V: m.Alloc.NewSliceFromList(list),
V: m.Alloc.NewSlice(arrayValue, 0, arg1NativeValueLength, arg1NativeValueLength),

Check warning on line 259 in gnovm/pkg/gnolang/uverse.go

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/gnolang/uverse.go#L259

Added line #L259 was not covered by tests
})
return
}
Expand Down Expand Up @@ -344,63 +344,65 @@
}
} else if arg0Type.Elem().Kind() == Uint8Kind {
// append(*SliceValue, *SliceValue) new data bytes ---
data := make([]byte, arg0Length+arg1Length)
newLength := arg0Length + arg1Length
arrayValue := m.Alloc.NewDataArray(newLength)
if 0 < arg0Length {
if arg0Base.Data == nil {
copyListToData(
data[:arg0Length],
arrayValue.Data[:arg0Length],

Check warning on line 352 in gnovm/pkg/gnolang/uverse.go

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/gnolang/uverse.go#L352

Added line #L352 was not covered by tests
arg0Base.List[arg0Offset:arg0Offset+arg0Length])
} else {
copy(
data[:arg0Length],
arrayValue.Data[:arg0Length],
arg0Base.Data[arg0Offset:arg0Offset+arg0Length])
}
}
if 0 < arg1Length {
if arg1Base.Data == nil {
copyListToData(
data[arg0Length:arg0Length+arg1Length],
arrayValue.Data[arg0Length:newLength],
arg1Base.List[arg1Offset:arg1Offset+arg1Length])
} else {
copy(
data[arg0Length:arg0Length+arg1Length],
arrayValue.Data[arg0Length:newLength],
arg1Base.Data[arg1Offset:arg1Offset+arg1Length])
}
}
m.PushValue(TypedValue{
T: arg0Type,
V: m.Alloc.NewSliceFromData(data),
V: m.Alloc.NewSlice(arrayValue, 0, newLength, newLength),
})
return
} else {
// append(*SliceValue, *SliceValue) new list ---------
list := make([]TypedValue, arg0Length+arg1Length)
if 0 < arg0Length {
arrayLen := arg0Length + arg1Length
arrayValue := m.Alloc.NewListArray(arrayLen)
if arg0Length > 0 {
if arg0Base.Data == nil {
for i := 0; i < arg0Length; i++ {
list[i] = arg0Base.List[arg0Offset+i].unrefCopy(m.Alloc, m.Store)
arrayValue.List[i] = arg0Base.List[arg0Offset+i].unrefCopy(m.Alloc, m.Store)
}
} else {
panic("should not happen")
}
}

if 0 < arg1Length {
if arg1Length > 0 {
if arg1Base.Data == nil {
for i := 0; i < arg1Length; i++ {
list[arg0Length+i] = arg1Base.List[arg1Offset+i].unrefCopy(m.Alloc, m.Store)
arrayValue.List[arg0Length+i] = arg1Base.List[arg1Offset+i].unrefCopy(m.Alloc, m.Store)
}
} else {
copyDataToList(
list[arg0Length:arg0Length+arg1Length],
arrayValue.List[arg0Length:arg0Length+arg1Length],

Check warning on line 397 in gnovm/pkg/gnolang/uverse.go

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/gnolang/uverse.go#L397

Added line #L397 was not covered by tests
arg1Base.Data[arg1Offset:arg1Offset+arg1Length],
arg1Type.Elem(),
)
}
}
m.PushValue(TypedValue{
T: arg0Type,
V: m.Alloc.NewSliceFromList(list),
V: m.Alloc.NewSlice(arrayValue, 0, arrayLen, arrayLen),
})
return
}
Expand Down Expand Up @@ -441,46 +443,47 @@
}
} else if arg0Type.Elem().Kind() == Uint8Kind {
// append(*SliceValue, *NativeValue) new data bytes --
data := make([]byte, arg0Length+arg1NativeValueLength)
newLength := arg0Length + arg1NativeValueLength
arrayValue := m.Alloc.NewDataArray(newLength)

Check warning on line 447 in gnovm/pkg/gnolang/uverse.go

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/gnolang/uverse.go#L446-L447

Added lines #L446 - L447 were not covered by tests
if 0 < arg0Length {
if arg0Base.Data == nil {
copyListToData(
data[:arg0Length],
arrayValue.Data[:arg0Length],

Check warning on line 451 in gnovm/pkg/gnolang/uverse.go

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/gnolang/uverse.go#L451

Added line #L451 was not covered by tests
arg0Base.List[arg0Offset:arg0Offset+arg0Length])
} else {
copy(
data[:arg0Length],
arrayValue.Data[:arg0Length],

Check warning on line 455 in gnovm/pkg/gnolang/uverse.go

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/gnolang/uverse.go#L455

Added line #L455 was not covered by tests
arg0Base.Data[arg0Offset:arg0Offset+arg0Length])
}
}
if 0 < arg1NativeValueLength {
copyNativeToData(
data[arg0Length:arg0Length+arg1NativeValueLength],
arrayValue.Data[arg0Length:newLength],

Check warning on line 461 in gnovm/pkg/gnolang/uverse.go

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/gnolang/uverse.go#L461

Added line #L461 was not covered by tests
arg1NativeValue, arg1NativeValueLength)
}
m.PushValue(TypedValue{
T: arg0Type,
V: m.Alloc.NewSliceFromData(data),
V: m.Alloc.NewSlice(arrayValue, 0, newLength, newLength),

Check warning on line 466 in gnovm/pkg/gnolang/uverse.go

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/gnolang/uverse.go#L466

Added line #L466 was not covered by tests
})
return
} else {
// append(*SliceValue, *NativeValue) new list --------
listLen := arg0Length + arg1NativeValueLength
list := make([]TypedValue, listLen)
arrayValue := m.Alloc.NewListArray(listLen)

Check warning on line 472 in gnovm/pkg/gnolang/uverse.go

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/gnolang/uverse.go#L472

Added line #L472 was not covered by tests
if 0 < arg0Length {
for i := 0; i < listLen; i++ {
list[i] = arg0Base.List[arg0Offset+i].unrefCopy(m.Alloc, m.Store)
arrayValue.List[i] = arg0Base.List[arg0Offset+i].unrefCopy(m.Alloc, m.Store)

Check warning on line 475 in gnovm/pkg/gnolang/uverse.go

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/gnolang/uverse.go#L475

Added line #L475 was not covered by tests
}
}
if 0 < arg1NativeValueLength {
copyNativeToList(
m.Alloc,
list[arg0Length:listLen],
arrayValue.List[arg0Length:listLen],

Check warning on line 481 in gnovm/pkg/gnolang/uverse.go

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/gnolang/uverse.go#L481

Added line #L481 was not covered by tests
arg1NativeValue, arg1NativeValueLength)
}
m.PushValue(TypedValue{
T: arg0Type,
V: m.Alloc.NewSliceFromList(list),
V: m.Alloc.NewSlice(arrayValue, 0, listLen, listLen),

Check warning on line 486 in gnovm/pkg/gnolang/uverse.go

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/gnolang/uverse.go#L486

Added line #L486 was not covered by tests
})
return
}
Expand Down Expand Up @@ -779,25 +782,25 @@
lv := vargs.TV.GetPointerAtIndexInt(m.Store, 0).Deref()
li := lv.ConvertGetInt()
if et.Kind() == Uint8Kind {
data := make([]byte, li)
arrayValue := m.Alloc.NewDataArray(li)
m.PushValue(TypedValue{
T: tt,
V: m.Alloc.NewSliceFromData(data),
V: m.Alloc.NewSlice(arrayValue, 0, li, li),
})
return
} else {
list := make([]TypedValue, li)
arrayValue := m.Alloc.NewListArray(li)
if et.Kind() == InterfaceKind {
// leave as is
} else {
// init zero elements with concrete type.
for i := 0; i < li; i++ {
list[i] = defaultTypedValue(m.Alloc, et)
arrayValue.List[i] = defaultTypedValue(m.Alloc, et)
}
}
m.PushValue(TypedValue{
T: tt,
V: m.Alloc.NewSliceFromList(list),
V: m.Alloc.NewSlice(arrayValue, 0, li, li),
})
return
}
Expand All @@ -807,30 +810,37 @@
cv := vargs.TV.GetPointerAtIndexInt(m.Store, 1).Deref()
ci := cv.ConvertGetInt()
if et.Kind() == Uint8Kind {
data := make([]byte, li, ci)
arrayValue := m.Alloc.NewDataArray(ci)
m.PushValue(TypedValue{
T: tt,
V: m.Alloc.NewSliceFromData(data),
V: m.Alloc.NewSlice(arrayValue, 0, li, ci),
})
return
} else {
list := make([]TypedValue, li, ci)
arrayValue := m.Alloc.NewListArray(ci)
if et := bt.Elem(); et.Kind() == InterfaceKind {
// leave as is
} else {
// init zero elements with concrete type.
// the elements beyond len l within cap c
// must also be initialized, for a future
// slice operation may refer to them.
// XXX can this be removed?
list2 := list[:ci]
// Initialize all elements within capacity with default
// type values. These need to be initialized because future
// slice operations could get messy otherwise. Simple capacity
// expansions like `a = a[:cap(a)]` would make it trivial to
// initialize zero values at the time of the slice operation.
// But sequences of operations like:
// a := make([]int, 1, 10)
// a = a[7:cap(a)]
// a = a[3:5]
//
// require a bit more work to handle correctly, requiring that
// all new TypedValue slice elements be checked to ensure they have
// a value for every slice operation, which is not desirable.
for i := 0; i < ci; i++ {
list2[i] = defaultTypedValue(m.Alloc, et)
arrayValue.List[i] = defaultTypedValue(m.Alloc, et)
}
}
m.PushValue(TypedValue{
T: tt,
V: m.Alloc.NewSliceFromList(list),
V: m.Alloc.NewSlice(arrayValue, 0, li, ci),
})
return
}
Expand Down
Loading