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: fix the scope of recover() #1672

Merged
merged 28 commits into from
Mar 30, 2024
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
e19bd08
first working attempt
deelawn Feb 16, 2024
2f13df9
other recovery cases working now
deelawn Feb 16, 2024
5a2f5f2
getting closer. recover tests are passing
deelawn Feb 21, 2024
1942842
comments
deelawn Feb 21, 2024
d8d337c
reset scopes to zero when they themselves go out of scope
deelawn Feb 21, 2024
d6d9b9c
added new recover tests
deelawn Feb 21, 2024
b0b7fc2
removed erroneous pop flag
deelawn Feb 21, 2024
dcdd779
combined and added last call frame safety
deelawn Feb 21, 2024
985e824
added tests
deelawn Feb 22, 2024
2cfe281
use exception struct for panic value and frame management
deelawn Feb 22, 2024
9d8cbb9
fixed testing pkg recover
deelawn Feb 23, 2024
bf4a29c
removed semi-related change to be included in subsequent PR
deelawn Feb 23, 2024
81ac4f8
Merge branch 'master' into bug/recover
deelawn Feb 23, 2024
19852f9
updated machine scope comments
deelawn Feb 23, 2024
20ce354
updated machine scope comments
deelawn Feb 23, 2024
7250f88
Merge branch 'bug/recover' of github.com:deelawn/gno into bug/recover
deelawn Feb 23, 2024
e58363a
Merge branch 'master' into bug/recover
deelawn Feb 23, 2024
7baab0c
account for recover that runs out of frames
deelawn Feb 27, 2024
c2a50f0
Merge branch 'master' into bug/recover
deelawn Feb 27, 2024
cc17f30
removed unnecessary changes
deelawn Feb 27, 2024
5cd9dd4
Rename LastCallFrame that can panic to MustLastCallFrame
deelawn Feb 27, 2024
92e7a33
lint
deelawn Feb 27, 2024
4c0208f
Merge branch 'master' into bug/recover
deelawn Feb 27, 2024
e34a322
don't allow max cycle configuration
deelawn Mar 27, 2024
2866d97
Revert "don't allow max cycle configuration"
deelawn Mar 27, 2024
d1ecd5e
fixed typos
deelawn Mar 29, 2024
7136f30
Merge remote-tracking branch 'upstream/master'
deelawn Mar 29, 2024
0571ff5
Merge branch 'master' into bug/recover
deelawn Mar 29, 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
18 changes: 14 additions & 4 deletions gnovm/cmd/gno/testdata/gno_test/recover.txtar
Original file line number Diff line number Diff line change
Expand Up @@ -32,20 +32,30 @@ package recov

import "testing"

type RecoverySetter struct {
value interface{}
}

func (s *RecoverySetter) Set(v interface{}) {
s.value = v
}

func TestRecover(t *testing.T) {
var setter RecoverySetter
defer func() {
err := testing.Recover()
t.Log("recovered", err)
t.Log("recovered", setter.value)
}()
defer testing.Recover(&setter)

panic("bad panic!")
}

func TestRecoverSkip(t *testing.T) {
var setter RecoverySetter
defer func() {
err := testing.Recover()
t.Log("recovered", err)
t.Log("recovered", setter.value)
}()
defer testing.Recover(&setter)
deelawn marked this conversation as resolved.
Show resolved Hide resolved

t.Skip("skipped")
panic("bad panic!")
Expand Down
7 changes: 7 additions & 0 deletions gnovm/pkg/gnolang/frame.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ type Frame struct {
Defers []Defer // deferred calls
LastPackage *PackageValue // previous package context
LastRealm *Realm // previous realm context

Popped bool // true if frame has been popped
}

func (fr Frame) String() string {
Expand Down Expand Up @@ -84,4 +86,9 @@ type Defer struct {
Args []TypedValue // arguments
Source *DeferStmt // source
Parent *Block

// PanicScope is set to the value of the Machine's PanicScope when the
// defer is created. The PanicScope of the Machine is incremented each time
// a panic occurrs and is decremented each time a panic is recovered.
deelawn marked this conversation as resolved.
Show resolved Hide resolved
PanicScope uint
}
93 changes: 78 additions & 15 deletions gnovm/pkg/gnolang/machine.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,19 @@ import (
"github.com/gnolang/gno/tm2/pkg/std"
)

// Exception represents an panic that originates from a gno program.
deelawn marked this conversation as resolved.
Show resolved Hide resolved
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
}

func (e Exception) Sprint(m *Machine) string {
return e.Value.Sprint(m)
}

//----------------------------------------
// Machine

Expand All @@ -28,13 +41,13 @@ type Machine struct {
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
Exceptions []*TypedValue // if panic'd unless recovered
NumResults int // number of results returned
Cycles int64 // number of "cpu" cycles
Exceptions []Exception
NumResults int // number of results returned
Cycles int64 // number of "cpu" cycles

// Configuration
CheckTypes bool // not yet used
Expand All @@ -44,6 +57,14 @@ type Machine struct {
Output io.Writer
Store Store
Context interface{}

// PanicScope is incremented each time a panic occurs and is reset to
// zero when it is recovered.
PanicScope uint
// DeferPanicScope is set to the value of the defer's panic scope before
// it is executed. It is reset to zero after the defer functions in the current
// scope have finished executing.
DeferPanicScope uint
}

// NewMachine initializes a new gno virtual machine, acting as a shorthand
Expand Down Expand Up @@ -1425,6 +1446,7 @@ func (m *Machine) PushOp(op Op) {
copy(newOps, m.Ops)
m.Ops = newOps
}

m.Ops[m.NumOps] = op
m.NumOps++
}
Expand Down Expand Up @@ -1642,7 +1664,7 @@ func (m *Machine) LastBlock() *Block {
// 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 @@ -1661,7 +1683,7 @@ func (m *Machine) PushFrameBasic(s Stmt) {
// 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 @@ -1698,7 +1720,7 @@ func (m *Machine) PushFrameCall(cx *CallExpr, fv *FuncValue, recv TypedValue) {
}

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 @@ -1724,15 +1746,17 @@ func (m *Machine) PushFrameGoNative(cx *CallExpr, fv *NativeValue) {
func (m *Machine) PopFrame() Frame {
numFrames := len(m.Frames)
f := m.Frames[numFrames-1]
f.Popped = true
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 @@ -1744,6 +1768,7 @@ func (m *Machine) PopFrameAndReset() {
// 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 @@ -1799,18 +1824,28 @@ func (m *Machine) NumFrames() int {
}

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

// 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) *Frame {
return m.lastCallFrame(n, false)
}

// LastCallFrameSafe behaves the same as LastCallFrame, but rather than panicking,
// returns nil if the frame is not found.
func (m *Machine) LastCallFrameSafe(n int) *Frame {
return m.lastCallFrame(n, true)
}

func (m *Machine) lastCallFrame(n int, safe bool) *Frame {
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 {
Expand All @@ -1820,20 +1855,40 @@ func (m *Machine) LastCallFrame(n int) *Frame {
}
}
}
panic("frame not found")

if !safe {
panic("frame not found")
}
deelawn marked this conversation as resolved.
Show resolved Hide resolved

return nil
}

// pops the last non-call (loop) frames
// and returns the last call frame (which is left on stack).
func (m *Machine) PopUntilLastCallFrame() *Frame {
func (m *Machine) PopUntilLastCallFrame(safe bool) *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 {
// TODO: optimize with fr.IsCall
m.Frames = m.Frames[:i+1]
return fr
}

fr.Popped = true
}

// The frame wasn't found. If not in safe mode, panic to avoid any potential
// nil pointer dereferences.
if !safe {
panic("no last call frame found")
}

// In safe mode we return nil and no frames are popped, so update the frames' popped flag.
// This is expected to happen infrequently.
for _, frame := range m.Frames {
frame.Popped = false
}
deelawn marked this conversation as resolved.
Show resolved Hide resolved

return nil
}

Expand Down Expand Up @@ -1926,8 +1981,16 @@ func (m *Machine) CheckEmpty() error {
}

func (m *Machine) Panic(ex TypedValue) {
m.Exceptions = append(m.Exceptions, &ex)
m.PopUntilLastCallFrame()
m.Exceptions = append(
m.Exceptions,
Exception{
Value: ex,
Frame: m.LastCallFrame(1),
},
)

m.PanicScope++
m.PopUntilLastCallFrame(false)
m.PushOp(OpPanic2)
m.PushOp(OpReturnCallDefers)
}
Expand Down
38 changes: 23 additions & 15 deletions gnovm/pkg/gnolang/op_call.go
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ func (m *Machine) doOpCallDeferNativeBody() {

// Assumes that result values are pushed onto the Values stack.
func (m *Machine) doOpReturn() {
cfr := m.PopUntilLastCallFrame()
cfr := m.PopUntilLastCallFrame(true)
// See if we are exiting a realm boundary.
// NOTE: there are other ways to implement realm boundary transitions,
// e.g. with independent Machine instances per realm for example, or
Expand Down Expand Up @@ -212,7 +212,7 @@ func (m *Machine) doOpReturn() {
// i.e. named result vars declared in func signatures.
func (m *Machine) doOpReturnFromBlock() {
// Copy results from block.
cfr := m.PopUntilLastCallFrame()
cfr := m.PopUntilLastCallFrame(true)
ft := cfr.Func.GetType(m.Store)
numParams := len(ft.Params)
numResults := len(ft.Results)
Expand Down Expand Up @@ -264,6 +264,7 @@ func (m *Machine) doOpReturnCallDefers() {
dfr, ok := cfr.PopDefer()
if !ok {
// Done with defers.
m.DeferPanicScope = 0
m.ForcePopOp()
if len(m.Exceptions) > 0 {
// In a state of panic (not return).
Expand All @@ -272,6 +273,9 @@ func (m *Machine) doOpReturnCallDefers() {
}
return
}

m.DeferPanicScope = dfr.PanicScope

// Call last deferred call.
// NOTE: the following logic is largely duplicated in doOpCall().
// Convert if variadic argument.
Expand Down Expand Up @@ -361,10 +365,11 @@ func (m *Machine) doOpDefer() {
case *FuncValue:
// TODO what if value is NativeValue?
cfr.PushDefer(Defer{
Func: cv,
Args: args,
Source: ds,
Parent: lb,
Func: cv,
Args: args,
Source: ds,
Parent: lb,
PanicScope: m.PanicScope,
})
case *BoundMethodValue:
if debug {
Expand All @@ -381,17 +386,19 @@ func (m *Machine) doOpDefer() {
args2[0] = cv.Receiver
copy(args2[1:], args)
cfr.PushDefer(Defer{
Func: cv.Func,
Args: args2,
Source: ds,
Parent: lb,
Func: cv.Func,
Args: args2,
Source: ds,
Parent: lb,
PanicScope: m.PanicScope,
})
case *NativeValue:
cfr.PushDefer(Defer{
GoFunc: cv,
Args: args,
Source: ds,
Parent: lb,
GoFunc: cv,
Args: args,
Source: ds,
Parent: lb,
PanicScope: m.PanicScope,
})
default:
panic("should not happen")
Expand All @@ -410,9 +417,10 @@ func (m *Machine) doOpPanic2() {
// Recovered from panic
m.PushOp(OpReturnFromBlock)
m.PushOp(OpReturnCallDefers)
m.PanicScope = 0
} else {
// Keep panicking
last := m.PopUntilLastCallFrame()
last := m.PopUntilLastCallFrame(true)
if last == nil {
// Build exception string just as go, separated by \n\t.
exs := make([]string, len(m.Exceptions))
Expand Down
26 changes: 23 additions & 3 deletions gnovm/pkg/gnolang/uverse.go
Original file line number Diff line number Diff line change
Expand Up @@ -972,9 +972,29 @@ func UverseNode() *PackageNode {
m.PushValue(TypedValue{})
return
}
// Just like in go, only the last exception is returned to recover.
m.PushValue(*m.Exceptions[len(m.Exceptions)-1])
// The remaining exceptions are removed

// If the exception is out of scope, this recover can't help; return nil.
if m.PanicScope <= m.DeferPanicScope {
m.PushValue(TypedValue{})
return
}

exception := &m.Exceptions[len(m.Exceptions)-1]

// If the frame the exception ocurred in is not popped, it's possible that
// the exception is still in scope and can be recovered.
if !exception.Frame.Popped {
// 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.LastCallFrameSafe(2); frame != nil && frame != exception.Frame {
m.PushValue(TypedValue{})
return
}
}

m.PushValue(exception.Value)
// Recover complete; remove exceptions.
m.Exceptions = nil
},
)
Expand Down
Loading
Loading