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): revert to storing frames in the machine as values instead of pointers #2596

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
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
2 changes: 1 addition & 1 deletion gnovm/pkg/gnolang/gno_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ func setupMachine(b *testing.B, numValues, numStmts, numExprs, numBlocks, numFra
Exprs: make([]Expr, numExprs),
Stmts: make([]Stmt, numStmts),
Blocks: make([]*Block, numBlocks),
Frames: make([]*Frame, numFrames),
Frames: make([]Frame, numFrames),
Exceptions: make([]Exception, numExceptions),
}
return m
Expand Down
93 changes: 64 additions & 29 deletions gnovm/pkg/gnolang/machine.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,11 @@
type Exception struct {
// Value is the value passed to panic.
Value TypedValue
// Frame is used to reference the frame a panic occurred in so that recover() knows if the
// currently executing deferred function is able to recover from the panic.
Frame *Frame
// FrameIndex is used to compare frame equality. This equality check is only ever used if
// the frames being compare have both not been popped yet.
FrameIndex int
// FramePopped is set to true when the frame where the exception occurred has been popped.
FramePopped bool
}

func (e Exception) Sprint(m *Machine) string {
Expand All @@ -43,7 +45,7 @@
Exprs []Expr // pending expressions
Stmts []Stmt // pending statements
Blocks []*Block // block (scope) stack
Frames []*Frame // func call stack
Frames []Frame // func call stack
Package *PackageValue // active package
Realm *Realm // active realm
Alloc *Allocator // memory allocations
Expand Down Expand Up @@ -1753,7 +1755,7 @@
// Pushes a frame with one less statement.
func (m *Machine) PushFrameBasic(s Stmt) {
label := s.GetLabel()
fr := &Frame{
fr := Frame{
Label: label,
Source: s,
NumOps: m.NumOps,
Expand All @@ -1772,7 +1774,7 @@
// ensure the counts are consistent, otherwise we mask
// bugs with frame pops.
func (m *Machine) PushFrameCall(cx *CallExpr, fv *FuncValue, recv TypedValue) {
fr := &Frame{
fr := Frame{
Source: cx,
NumOps: m.NumOps,
NumValues: m.NumValues - cx.NumArgs - 1,
Expand Down Expand Up @@ -1824,7 +1826,7 @@
}

func (m *Machine) PushFrameGoNative(cx *CallExpr, fv *NativeValue) {
fr := &Frame{
fr := Frame{
Source: cx,
NumOps: m.NumOps,
NumValues: m.NumValues - cx.NumArgs - 1,
Expand All @@ -1850,17 +1852,27 @@
func (m *Machine) PopFrame() Frame {
numFrames := len(m.Frames)
f := m.Frames[numFrames-1]
f.Popped = true

if len(m.Exceptions) != 0 {
// If there are exceptions, check if any exceptions reference this
// frame index. If one does, mark it as popped.
for i := len(m.Exceptions) - 1; i >= 0; i-- {
if m.Exceptions[i].FrameIndex == numFrames-1 {
m.Exceptions[i].FramePopped = true
break
}
}
}

if debug {
m.Printf("-F %#v\n", f)
}
m.Frames = m.Frames[:numFrames-1]
return *f
return f
}

func (m *Machine) PopFrameAndReset() {
fr := m.PopFrame()
fr.Popped = true
m.NumOps = fr.NumOps
m.NumValues = fr.NumValues
m.Exprs = m.Exprs[:fr.NumExprs]
Expand All @@ -1872,7 +1884,6 @@
// TODO: optimize by passing in last frame.
func (m *Machine) PopFrameAndReturn() {
fr := m.PopFrame()
fr.Popped = true
if debug {
// TODO: optimize with fr.IsCall
if fr.Func == nil && fr.GoFunc == nil {
Expand Down Expand Up @@ -1928,33 +1939,49 @@
}

func (m *Machine) LastFrame() *Frame {
return m.Frames[len(m.Frames)-1]
return &m.Frames[len(m.Frames)-1]
}

// MustLastCallFrame returns the last call frame with an offset of n. It panics if the frame is not found.
func (m *Machine) MustLastCallFrame(n int) *Frame {
return m.lastCallFrame(n, true)
frame, _ := m.lastCallFrame(n, true)
return frame
}

// LastCallFrame behaves the same as MustLastCallFrame, but rather than panicking,
// returns nil if the frame is not found.
func (m *Machine) LastCallFrame(n int) *Frame {
return m.lastCallFrame(n, false)
frame, _ := m.lastCallFrame(n, false)
return frame

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

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/gnolang/machine.go#L1954-L1955

Added lines #L1954 - L1955 were not covered by tests
}

// MustLastCallFrameIndex returns the index of the last call frame. It
// panics if no call frame is found.
func (m *Machine) MustLastCallFrameIdx(n int) int {
_, idx := m.lastCallFrame(n, true)
return idx
}

// LastCallFrameIdx returns the index of the last call frame. If no
// call frame is found, it returns -1.
func (m *Machine) LastCallFrameIdx(n int) int {
_, idx := m.lastCallFrame(n, false)
return idx
}

// TODO: this function and PopUntilLastCallFrame() is used in conjunction
// spanning two disjoint operations upon return. Optimize.
// If n is 1, returns the immediately last call frame.
func (m *Machine) lastCallFrame(n int, mustBeFound bool) *Frame {
func (m *Machine) lastCallFrame(n int, mustBeFound bool) (*Frame, int) {
if n == 0 {
panic("n must be positive")
}
for i := len(m.Frames) - 1; i >= 0; i-- {
fr := m.Frames[i]
fr := &m.Frames[i]
if fr.Func != nil || fr.GoFunc != nil {
// TODO: optimize with fr.IsCall
if n == 1 {
return fr
return fr, i
} else {
n-- // continue
}
Expand All @@ -1965,27 +1992,35 @@
panic("frame not found")
}

return nil
return nil, -1
}

// pops the last non-call (loop) frames
// and returns the last call frame (which is left on stack).
func (m *Machine) PopUntilLastCallFrame() *Frame {
for i := len(m.Frames) - 1; i >= 0; i-- {
fr := m.Frames[i]
fr := &m.Frames[i]
if fr.Func != nil || fr.GoFunc != nil {
if len(m.Exceptions) != 0 {
for j := len(m.Exceptions) - 1; j >= 0; j-- {
if m.Exceptions[j].FrameIndex <= i {
// Exception frame indexes that aren't popped can't be greater
// that the current maximum frame index.
break
}

if m.Exceptions[j].FrameIndex > i {
// This exception has an index in the range of what is
// about to be popped so mark it as such.
m.Exceptions[j].FramePopped = true
}
}
}

// TODO: optimize with fr.IsCall
m.Frames = m.Frames[:i+1]
return fr
}

fr.Popped = true
}

// No frames are popped, so revert all the frames' popped flag.
// This is expected to happen infrequently.
for _, frame := range m.Frames {
frame.Popped = false
}

return nil
Expand Down Expand Up @@ -2085,8 +2120,8 @@
m.Exceptions = append(
m.Exceptions,
Exception{
Value: ex,
Frame: m.MustLastCallFrame(1),
Value: ex,
FrameIndex: m.MustLastCallFrameIdx(1),
},
)

Expand Down
4 changes: 2 additions & 2 deletions gnovm/pkg/gnolang/uverse.go
Original file line number Diff line number Diff line change
Expand Up @@ -1002,11 +1002,11 @@ func UverseNode() *PackageNode {

// If the frame the exception occurred in is not popped, it's possible that
// the exception is still in scope and can be recovered.
if !exception.Frame.Popped {
if !exception.FramePopped {
// If the frame is not the current frame, the exception is not in scope; return nil.
// This retrieves the second most recent call frame because the first most recent
// is the call to recover itself.
if frame := m.LastCallFrame(2); frame == nil || (frame != nil && frame != exception.Frame) {
if frameIdx := m.LastCallFrameIdx(2); frameIdx == -1 || frameIdx != exception.FrameIndex {
m.PushValue(TypedValue{})
return
}
Expand Down
28 changes: 14 additions & 14 deletions gnovm/stdlibs/std/native_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ func TestPrevRealmIsOrigin(t *testing.T) {
ctx = ExecContext{
OrigCaller: user,
}
msgCallFrame = &gno.Frame{LastPackage: &gno.PackageValue{PkgPath: "main"}}
msgRunFrame = &gno.Frame{LastPackage: &gno.PackageValue{PkgPath: "gno.land/r/g1337/run"}}
msgCallFrame = gno.Frame{LastPackage: &gno.PackageValue{PkgPath: "main"}}
msgRunFrame = gno.Frame{LastPackage: &gno.PackageValue{PkgPath: "gno.land/r/g1337/run"}}
)
tests := []struct {
name string
Expand All @@ -29,7 +29,7 @@ func TestPrevRealmIsOrigin(t *testing.T) {
name: "no frames",
machine: &gno.Machine{
Context: ctx,
Frames: []*gno.Frame{},
Frames: []gno.Frame{},
},
expectedAddr: user,
expectedPkgPath: "",
Expand All @@ -39,7 +39,7 @@ func TestPrevRealmIsOrigin(t *testing.T) {
name: "one frame w/o LastPackage",
machine: &gno.Machine{
Context: ctx,
Frames: []*gno.Frame{
Frames: []gno.Frame{
{LastPackage: nil},
},
},
Expand All @@ -51,7 +51,7 @@ func TestPrevRealmIsOrigin(t *testing.T) {
name: "one package frame",
machine: &gno.Machine{
Context: ctx,
Frames: []*gno.Frame{
Frames: []gno.Frame{
{LastPackage: &gno.PackageValue{PkgPath: "gno.land/p/xxx"}},
},
},
Expand All @@ -63,7 +63,7 @@ func TestPrevRealmIsOrigin(t *testing.T) {
name: "one realm frame",
machine: &gno.Machine{
Context: ctx,
Frames: []*gno.Frame{
Frames: []gno.Frame{
{LastPackage: &gno.PackageValue{PkgPath: "gno.land/r/xxx"}},
},
},
Expand All @@ -75,7 +75,7 @@ func TestPrevRealmIsOrigin(t *testing.T) {
name: "one msgCall frame",
machine: &gno.Machine{
Context: ctx,
Frames: []*gno.Frame{
Frames: []gno.Frame{
msgCallFrame,
},
},
Expand All @@ -87,7 +87,7 @@ func TestPrevRealmIsOrigin(t *testing.T) {
name: "one msgRun frame",
machine: &gno.Machine{
Context: ctx,
Frames: []*gno.Frame{
Frames: []gno.Frame{
msgRunFrame,
},
},
Expand All @@ -99,7 +99,7 @@ func TestPrevRealmIsOrigin(t *testing.T) {
name: "one package frame and one msgCall frame",
machine: &gno.Machine{
Context: ctx,
Frames: []*gno.Frame{
Frames: []gno.Frame{
msgCallFrame,
{LastPackage: &gno.PackageValue{PkgPath: "gno.land/p/xxx"}},
},
Expand All @@ -112,7 +112,7 @@ func TestPrevRealmIsOrigin(t *testing.T) {
name: "one realm frame and one msgCall frame",
machine: &gno.Machine{
Context: ctx,
Frames: []*gno.Frame{
Frames: []gno.Frame{
msgCallFrame,
{LastPackage: &gno.PackageValue{PkgPath: "gno.land/r/xxx"}},
},
Expand All @@ -125,7 +125,7 @@ func TestPrevRealmIsOrigin(t *testing.T) {
name: "one package frame and one msgRun frame",
machine: &gno.Machine{
Context: ctx,
Frames: []*gno.Frame{
Frames: []gno.Frame{
msgRunFrame,
{LastPackage: &gno.PackageValue{PkgPath: "gno.land/p/xxx"}},
},
Expand All @@ -138,7 +138,7 @@ func TestPrevRealmIsOrigin(t *testing.T) {
name: "one realm frame and one msgRun frame",
machine: &gno.Machine{
Context: ctx,
Frames: []*gno.Frame{
Frames: []gno.Frame{
msgRunFrame,
{LastPackage: &gno.PackageValue{PkgPath: "gno.land/r/xxx"}},
},
Expand All @@ -151,7 +151,7 @@ func TestPrevRealmIsOrigin(t *testing.T) {
name: "multiple frames with one realm",
machine: &gno.Machine{
Context: ctx,
Frames: []*gno.Frame{
Frames: []gno.Frame{
{LastPackage: &gno.PackageValue{PkgPath: "gno.land/p/xxx"}},
{LastPackage: &gno.PackageValue{PkgPath: "gno.land/p/xxx"}},
{LastPackage: &gno.PackageValue{PkgPath: "gno.land/r/xxx"}},
Expand All @@ -165,7 +165,7 @@ func TestPrevRealmIsOrigin(t *testing.T) {
name: "multiple frames with multiple realms",
machine: &gno.Machine{
Context: ctx,
Frames: []*gno.Frame{
Frames: []gno.Frame{
{LastPackage: &gno.PackageValue{PkgPath: "gno.land/r/zzz"}},
{LastPackage: &gno.PackageValue{PkgPath: "gno.land/r/zzz"}},
{LastPackage: &gno.PackageValue{PkgPath: "gno.land/r/yyy"}},
Expand Down
2 changes: 1 addition & 1 deletion gnovm/tests/file.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ func TestContext(pkgPath string, send std.Coins) *teststd.TestExecContext {
}
return &teststd.TestExecContext{
ExecContext: ctx,
RealmFrames: make(map[*gno.Frame]teststd.RealmOverride),
RealmFrames: make(map[int]teststd.RealmOverride),
}
}

Expand Down
10 changes: 5 additions & 5 deletions gnovm/tests/stdlibs/std/std.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ type TestExecContext struct {
std.ExecContext

// These are used to set up the result of CurrentRealm() and PrevRealm().
RealmFrames map[*gno.Frame]RealmOverride
RealmFrames map[int]RealmOverride
}

var _ std.ExecContexter = &TestExecContext{}
Expand Down Expand Up @@ -114,21 +114,21 @@ func X_testSetOrigPkgAddr(m *gno.Machine, addr string) {

func X_testSetRealm(m *gno.Machine, addr, pkgPath string) {
// Associate the given Realm with the caller's frame.
var frame *gno.Frame
frameIdx := -1
// When calling this function from Gno, the two top frames are the following:
// #6 [FRAME FUNC:testSetRealm RECV:(undefined) (2 args) 17/6/0/10/8 LASTPKG:std ...]
// #5 [FRAME FUNC:TestSetRealm RECV:(undefined) (1 args) 14/5/0/8/7 LASTPKG:gno.land/r/tyZ1Vcsta ...]
// We want to set the Realm of the frame where TestSetRealm is being called, hence -3.
for i := m.NumFrames() - 3; i >= 0; i-- {
// Must be a frame from calling a function.
if fr := m.Frames[i]; fr.Func != nil {
frame = fr
frameIdx = i
break
}
}

ctx := m.Context.(*TestExecContext)
ctx.RealmFrames[frame] = RealmOverride{
ctx.RealmFrames[frameIdx] = RealmOverride{
Addr: crypto.Bech32Address(addr),
PkgPath: pkgPath,
}
Expand All @@ -147,7 +147,7 @@ func X_getRealm(m *gno.Machine, height int) (address string, pkgPath string) {

for i := m.NumFrames() - 1; i >= 0; i-- {
fr := m.Frames[i]
override, overridden := ctx.RealmFrames[m.Frames[max(i-1, 0)]]
override, overridden := ctx.RealmFrames[max(i-1, 0)]
if !overridden &&
(fr.LastPackage == nil || !fr.LastPackage.IsRealm()) {
continue
Expand Down
Loading