From 77e660614018f07175e8f3569cdc98be03130602 Mon Sep 17 00:00:00 2001 From: Petar Dambovaliev Date: Sat, 23 Nov 2024 00:31:21 +0100 Subject: [PATCH] fix: invoke user recover with implicit panics (#3067) Currently only explicit panic invocations are recovered in the user code. This PR covers the implicit panics that happen because of invalid operations. Associated [issue](https://github.com/gnolang/gno/issues/1148) This maintains the distinction between VM panics and user panics. Here is a list of possible runtime panics that we will cover. - [x] Out-of-Bounds Slice or Array Access - [x] Invalid Slice Indexing - [x] Division by Zero and MOD zero - [x] Type Assertion Failure - [x] Invalid Memory Allocation (bad call to make()) - [x] Out-of-Bounds String Indexing - [x] nil pointer dereference - [x] Write to a nil map Also, fixed a small bug with the builtint function `make`. It wasn't panicking when cap > len --- gnovm/pkg/gnolang/alloc.go | 7 ++ gnovm/pkg/gnolang/debugger_test.go | 2 +- gnovm/pkg/gnolang/machine.go | 18 ++++- gnovm/pkg/gnolang/op_assign.go | 12 +++- gnovm/pkg/gnolang/op_binary.go | 108 ++++++++++++++++++++++++++-- gnovm/pkg/gnolang/op_expressions.go | 4 ++ gnovm/pkg/gnolang/uverse.go | 5 ++ gnovm/pkg/gnolang/values.go | 60 ++++++++++------ gnovm/tests/files/recover12.gno | 15 ++++ gnovm/tests/files/recover13.gno | 15 ++++ gnovm/tests/files/recover14.gno | 15 ++++ gnovm/tests/files/recover15.gno | 15 ++++ gnovm/tests/files/recover16.gno | 14 ++++ gnovm/tests/files/recover17.gno | 15 ++++ gnovm/tests/files/recover18.gno | 15 ++++ gnovm/tests/files/recover19.gno | 15 ++++ 16 files changed, 307 insertions(+), 28 deletions(-) create mode 100644 gnovm/tests/files/recover12.gno create mode 100644 gnovm/tests/files/recover13.gno create mode 100644 gnovm/tests/files/recover14.gno create mode 100644 gnovm/tests/files/recover15.gno create mode 100644 gnovm/tests/files/recover16.gno create mode 100644 gnovm/tests/files/recover17.gno create mode 100644 gnovm/tests/files/recover18.gno create mode 100644 gnovm/tests/files/recover19.gno diff --git a/gnovm/pkg/gnolang/alloc.go b/gnovm/pkg/gnolang/alloc.go index df042038e43..7e942ab61b9 100644 --- a/gnovm/pkg/gnolang/alloc.go +++ b/gnovm/pkg/gnolang/alloc.go @@ -194,6 +194,9 @@ func (alloc *Allocator) NewString(s string) StringValue { } func (alloc *Allocator) NewListArray(n int) *ArrayValue { + if n < 0 { + panic(&Exception{Value: typedString("len out of range")}) + } alloc.AllocateListArray(int64(n)) return &ArrayValue{ List: make([]TypedValue, n), @@ -201,6 +204,10 @@ func (alloc *Allocator) NewListArray(n int) *ArrayValue { } func (alloc *Allocator) NewDataArray(n int) *ArrayValue { + if n < 0 { + panic(&Exception{Value: typedString("len out of range")}) + } + alloc.AllocateDataArray(int64(n)) return &ArrayValue{ Data: make([]byte, n), diff --git a/gnovm/pkg/gnolang/debugger_test.go b/gnovm/pkg/gnolang/debugger_test.go index 44786257d67..63a3ee74675 100644 --- a/gnovm/pkg/gnolang/debugger_test.go +++ b/gnovm/pkg/gnolang/debugger_test.go @@ -158,7 +158,7 @@ func TestDebug(t *testing.T) { {in: "up xxx", out: `"xxx": invalid syntax`}, {in: "b 37\nc\np b\n", out: "(3 int)"}, {in: "b 27\nc\np b\n", out: `("!zero" string)`}, - {in: "b 22\nc\np t.A[3]\n", out: "Command failed: slice index out of bounds: 3 (len=3)"}, + {in: "b 22\nc\np t.A[3]\n", out: "Command failed: &{(\"slice index out of bounds: 3 (len=3)\" string) }"}, {in: "b 43\nc\nc\nc\np i\ndetach\n", out: "(1 int)"}, }) diff --git a/gnovm/pkg/gnolang/machine.go b/gnovm/pkg/gnolang/machine.go index 33bf32730c5..aac8b4f5802 100644 --- a/gnovm/pkg/gnolang/machine.go +++ b/gnovm/pkg/gnolang/machine.go @@ -829,7 +829,9 @@ func (m *Machine) RunFunc(fn Name) { func (m *Machine) RunMain() { defer func() { - if r := recover(); r != nil { + r := recover() + + if r != nil { switch r := r.(type) { case UnhandledPanicError: fmt.Printf("Machine.RunMain() panic: %s\nStacktrace: %s\n", @@ -1280,6 +1282,20 @@ const ( // main run loop. func (m *Machine) Run() { + defer func() { + r := recover() + + if r != nil { + switch r := r.(type) { + case *Exception: + m.Panic(r.Value) + m.Run() + default: + panic(r) + } + } + }() + for { if m.Debugger.enabled { m.Debug() diff --git a/gnovm/pkg/gnolang/op_assign.go b/gnovm/pkg/gnolang/op_assign.go index 8caacbfd1e6..114c8c589c4 100644 --- a/gnovm/pkg/gnolang/op_assign.go +++ b/gnovm/pkg/gnolang/op_assign.go @@ -131,7 +131,11 @@ func (m *Machine) doOpQuoAssign() { } } // lv /= rv - quoAssign(lv.TV, rv) + err := quoAssign(lv.TV, rv) + if err != nil { + panic(err) + } + if lv.Base != nil { m.Realm.DidUpdate(lv.Base.(Object), nil, nil) } @@ -154,7 +158,11 @@ func (m *Machine) doOpRemAssign() { } } // lv %= rv - remAssign(lv.TV, rv) + err := remAssign(lv.TV, rv) + if err != nil { + panic(err) + } + if lv.Base != nil { m.Realm.DidUpdate(lv.Base.(Object), nil, nil) } diff --git a/gnovm/pkg/gnolang/op_binary.go b/gnovm/pkg/gnolang/op_binary.go index 24123d285ad..a541a7da8b5 100644 --- a/gnovm/pkg/gnolang/op_binary.go +++ b/gnovm/pkg/gnolang/op_binary.go @@ -252,7 +252,10 @@ func (m *Machine) doOpQuo() { } // lv / rv - quoAssign(lv, rv) + err := quoAssign(lv, rv) + if err != nil { + panic(err) + } } func (m *Machine) doOpRem() { @@ -266,7 +269,10 @@ func (m *Machine) doOpRem() { } // lv % rv - remAssign(lv, rv) + err := remAssign(lv, rv) + if err != nil { + panic(err) + } } func (m *Machine) doOpShl() { @@ -845,45 +851,94 @@ func mulAssign(lv, rv *TypedValue) { } // for doOpQuo and doOpQuoAssign. -func quoAssign(lv, rv *TypedValue) { +func quoAssign(lv, rv *TypedValue) *Exception { + expt := &Exception{ + Value: typedString("division by zero"), + } + // set the result in lv. // NOTE this block is replicated in op_assign.go switch baseOf(lv.T) { case IntType: + if rv.GetInt() == 0 { + return expt + } lv.SetInt(lv.GetInt() / rv.GetInt()) case Int8Type: + if rv.GetInt8() == 0 { + return expt + } lv.SetInt8(lv.GetInt8() / rv.GetInt8()) case Int16Type: + if rv.GetInt16() == 0 { + return expt + } lv.SetInt16(lv.GetInt16() / rv.GetInt16()) case Int32Type, UntypedRuneType: + if rv.GetInt32() == 0 { + return expt + } lv.SetInt32(lv.GetInt32() / rv.GetInt32()) case Int64Type: + if rv.GetInt64() == 0 { + return expt + } lv.SetInt64(lv.GetInt64() / rv.GetInt64()) case UintType: + if rv.GetUint() == 0 { + return expt + } lv.SetUint(lv.GetUint() / rv.GetUint()) case Uint8Type: + if rv.GetUint8() == 0 { + return expt + } lv.SetUint8(lv.GetUint8() / rv.GetUint8()) case DataByteType: + if rv.GetUint8() == 0 { + return expt + } lv.SetDataByte(lv.GetDataByte() / rv.GetUint8()) case Uint16Type: + if rv.GetUint16() == 0 { + return expt + } lv.SetUint16(lv.GetUint16() / rv.GetUint16()) case Uint32Type: + if rv.GetUint32() == 0 { + return expt + } lv.SetUint32(lv.GetUint32() / rv.GetUint32()) case Uint64Type: + if rv.GetUint64() == 0 { + return expt + } lv.SetUint64(lv.GetUint64() / rv.GetUint64()) case Float32Type: // NOTE: gno doesn't fuse *+. + if rv.GetFloat32() == 0 { + return expt + } lv.SetFloat32(lv.GetFloat32() / rv.GetFloat32()) // XXX FOR DETERMINISM, PANIC IF NAN. case Float64Type: // NOTE: gno doesn't fuse *+. + if rv.GetFloat64() == 0 { + return expt + } lv.SetFloat64(lv.GetFloat64() / rv.GetFloat64()) // XXX FOR DETERMINISM, PANIC IF NAN. case BigintType, UntypedBigintType: + if rv.GetBigInt().Sign() == 0 { + return expt + } lb := lv.GetBigInt() lb = big.NewInt(0).Quo(lb, rv.GetBigInt()) lv.V = BigintValue{V: lb} case BigdecType, UntypedBigdecType: + if rv.GetBigDec().Cmp(apd.New(0, 0)) == 0 { + return expt + } lb := lv.GetBigDec() rb := rv.GetBigDec() quo := apd.New(0, 0) @@ -898,36 +953,79 @@ func quoAssign(lv, rv *TypedValue) { lv.T, )) } + + return nil } // for doOpRem and doOpRemAssign. -func remAssign(lv, rv *TypedValue) { +func remAssign(lv, rv *TypedValue) *Exception { + expt := &Exception{ + Value: typedString("division by zero"), + } + // set the result in lv. // NOTE this block is replicated in op_assign.go switch baseOf(lv.T) { case IntType: + if rv.GetInt() == 0 { + return expt + } lv.SetInt(lv.GetInt() % rv.GetInt()) case Int8Type: + if rv.GetInt8() == 0 { + return expt + } lv.SetInt8(lv.GetInt8() % rv.GetInt8()) case Int16Type: + if rv.GetInt16() == 0 { + return expt + } lv.SetInt16(lv.GetInt16() % rv.GetInt16()) case Int32Type, UntypedRuneType: + if rv.GetInt32() == 0 { + return expt + } lv.SetInt32(lv.GetInt32() % rv.GetInt32()) case Int64Type: + if rv.GetInt64() == 0 { + return expt + } lv.SetInt64(lv.GetInt64() % rv.GetInt64()) case UintType: + if rv.GetUint() == 0 { + return expt + } lv.SetUint(lv.GetUint() % rv.GetUint()) case Uint8Type: + if rv.GetUint8() == 0 { + return expt + } lv.SetUint8(lv.GetUint8() % rv.GetUint8()) case DataByteType: + if rv.GetUint8() == 0 { + return expt + } lv.SetDataByte(lv.GetDataByte() % rv.GetUint8()) case Uint16Type: + if rv.GetUint16() == 0 { + return expt + } lv.SetUint16(lv.GetUint16() % rv.GetUint16()) case Uint32Type: + if rv.GetUint32() == 0 { + return expt + } lv.SetUint32(lv.GetUint32() % rv.GetUint32()) case Uint64Type: + if rv.GetUint64() == 0 { + return expt + } lv.SetUint64(lv.GetUint64() % rv.GetUint64()) case BigintType, UntypedBigintType: + if rv.GetBigInt().Sign() == 0 { + return expt + } + lb := lv.GetBigInt() lb = big.NewInt(0).Rem(lb, rv.GetBigInt()) lv.V = BigintValue{V: lb} @@ -937,6 +1035,8 @@ func remAssign(lv, rv *TypedValue) { lv.T, )) } + + return nil } // for doOpBand and doOpBandAssign. diff --git a/gnovm/pkg/gnolang/op_expressions.go b/gnovm/pkg/gnolang/op_expressions.go index a1d677ca878..b661d693304 100644 --- a/gnovm/pkg/gnolang/op_expressions.go +++ b/gnovm/pkg/gnolang/op_expressions.go @@ -145,6 +145,10 @@ func (m *Machine) doOpStar() { xv := m.PopValue() switch bt := baseOf(xv.T).(type) { case *PointerType: + if xv.V == nil { + panic(&Exception{Value: typedString("nil pointer dereference")}) + } + pv := xv.V.(PointerValue) if pv.TV.T == DataByteType { tv := TypedValue{T: bt.Elt} diff --git a/gnovm/pkg/gnolang/uverse.go b/gnovm/pkg/gnolang/uverse.go index ba66b27499f..2780e6d8034 100644 --- a/gnovm/pkg/gnolang/uverse.go +++ b/gnovm/pkg/gnolang/uverse.go @@ -838,6 +838,11 @@ func makeUverseNode() { li := lv.ConvertGetInt() cv := vargs.TV.GetPointerAtIndexInt(m.Store, 1).Deref() ci := cv.ConvertGetInt() + + if ci < li { + panic(&Exception{Value: typedString(`makeslice: cap out of range`)}) + } + if et.Kind() == Uint8Kind { arrayValue := m.Alloc.NewDataArray(ci) m.PushValue(TypedValue{ diff --git a/gnovm/pkg/gnolang/values.go b/gnovm/pkg/gnolang/values.go index 68c2967811f..e4141772d98 100644 --- a/gnovm/pkg/gnolang/values.go +++ b/gnovm/pkg/gnolang/values.go @@ -436,12 +436,18 @@ func (sv *SliceValue) GetLength() int { func (sv *SliceValue) GetPointerAtIndexInt2(store Store, ii int, et Type) PointerValue { // Necessary run-time slice bounds check if ii < 0 { - panic(fmt.Sprintf( - "slice index out of bounds: %d", ii)) + excpt := &Exception{ + Value: typedString(fmt.Sprintf( + "slice index out of bounds: %d", ii)), + } + panic(excpt) } else if sv.Length <= ii { - panic(fmt.Sprintf( - "slice index out of bounds: %d (len=%d)", - ii, sv.Length)) + excpt := &Exception{ + Value: typedString(fmt.Sprintf( + "slice index out of bounds: %d (len=%d)", + ii, sv.Length)), + } + panic(excpt) } return sv.GetBase(store).GetPointerAtIndexInt2(store, sv.Offset+ii, et) } @@ -1700,6 +1706,9 @@ func (tv *TypedValue) GetPointerToFromTV(alloc *Allocator, store Store, path Val path.Type = VPField path.Depth = 0 case 2: + if tv.V == nil { + panic(&Exception{Value: typedString("nil pointer dereference")}) + } dtv = tv.V.(PointerValue).TV isPtr = true path.Type = VPField @@ -1975,6 +1984,14 @@ func (tv *TypedValue) GetPointerAtIndex(alloc *Allocator, store Store, iv *Typed bv := &TypedValue{ // heap alloc T: Uint8Type, } + + if ii >= len(sv) { + panic(&Exception{Value: typedString(fmt.Sprintf("index out of range [%d] with length %d", ii, len(sv)))}) + } + if ii < 0 { + panic(&Exception{Value: typedString(fmt.Sprintf("invalid slice index %d (index must be non-negative)", ii))}) + } + bv.SetUint8(sv[ii]) return PointerValue{ TV: bv, @@ -1997,7 +2014,7 @@ func (tv *TypedValue) GetPointerAtIndex(alloc *Allocator, store Store, iv *Typed return sv.GetPointerAtIndexInt2(store, ii, bt.Elt) case *MapType: if tv.V == nil { - panic("uninitialized map index") + panic(&Exception{Value: typedString("uninitialized map index")}) } mv := tv.V.(*MapValue) pv := mv.GetPointerForKey(alloc, store, iv) @@ -2149,26 +2166,26 @@ func (tv *TypedValue) GetCapacity() int { func (tv *TypedValue) GetSlice(alloc *Allocator, low, high int) TypedValue { if low < 0 { - panic(fmt.Sprintf( + panic(&Exception{Value: typedString(fmt.Sprintf( "invalid slice index %d (index must be non-negative)", - low)) + low))}) } if high < 0 { - panic(fmt.Sprintf( + panic(&Exception{Value: typedString(fmt.Sprintf( "invalid slice index %d (index must be non-negative)", - high)) + low))}) } if low > high { - panic(fmt.Sprintf( + panic(&Exception{Value: typedString(fmt.Sprintf( "invalid slice index %d > %d", - low, high)) + low, high))}) } switch t := baseOf(tv.T).(type) { case PrimitiveType: if tv.GetLength() < high { - panic(fmt.Sprintf( + panic(&Exception{Value: typedString(fmt.Sprintf( "slice bounds out of range [%d:%d] with string length %d", - low, high, tv.GetLength())) + low, high, tv.GetLength()))}) } if t == StringType || t == UntypedStringType { return TypedValue{ @@ -2176,12 +2193,14 @@ func (tv *TypedValue) GetSlice(alloc *Allocator, low, high int) TypedValue { V: alloc.NewString(tv.GetString()[low:high]), } } - panic("non-string primitive type cannot be sliced") + panic(&Exception{Value: typedString(fmt.Sprintf( + "non-string primitive type cannot be sliced", + ))}) case *ArrayType: if tv.GetLength() < high { - panic(fmt.Sprintf( + panic(&Exception{Value: typedString(fmt.Sprintf( "slice bounds out of range [%d:%d] with array length %d", - low, high, tv.GetLength())) + low, high, tv.GetLength()))}) } av := tv.V.(*ArrayValue) st := alloc.NewType(&SliceType{ @@ -2199,13 +2218,14 @@ func (tv *TypedValue) GetSlice(alloc *Allocator, low, high int) TypedValue { } case *SliceType: if tv.GetCapacity() < high { - panic(fmt.Sprintf( + panic(&Exception{Value: typedString(fmt.Sprintf( "slice bounds out of range [%d:%d] with capacity %d", - low, high, tv.GetCapacity())) + low, high, tv.GetCapacity()))}) } if tv.V == nil { if low != 0 || high != 0 { - panic("nil slice index out of range") + panic(&Exception{Value: typedString(fmt.Sprintf( + "nil slice index out of range"))}) } return TypedValue{ T: tv.T, diff --git a/gnovm/tests/files/recover12.gno b/gnovm/tests/files/recover12.gno new file mode 100644 index 00000000000..ab16959adfb --- /dev/null +++ b/gnovm/tests/files/recover12.gno @@ -0,0 +1,15 @@ +package main + + +func main() { + defer func() { + r := recover() + println("recover:", r) + }() + + arr := []int{1, 2, 3} + _ = arr[3] // Panics because index 3 is out of bounds +} + +// Output: +// recover: slice index out of bounds: 3 (len=3) diff --git a/gnovm/tests/files/recover13.gno b/gnovm/tests/files/recover13.gno new file mode 100644 index 00000000000..d4b1df10ae5 --- /dev/null +++ b/gnovm/tests/files/recover13.gno @@ -0,0 +1,15 @@ +package main + + +func main() { + defer func() { + r := recover() + println("recover:", r) + }() + + arr := []int{1, 2, 3} + _ = arr[-1:] // Panics because of negative index +} + +// Output: +// recover: invalid slice index -1 (index must be non-negative) diff --git a/gnovm/tests/files/recover14.gno b/gnovm/tests/files/recover14.gno new file mode 100644 index 00000000000..30a34ab291a --- /dev/null +++ b/gnovm/tests/files/recover14.gno @@ -0,0 +1,15 @@ +package main + + +func main() { + defer func() { + r := recover() + println("recover:", r) + }() + + x, y := 10, 0 + _ = x / y // Panics because of division by zero +} + +// Output: +// recover: division by zero diff --git a/gnovm/tests/files/recover15.gno b/gnovm/tests/files/recover15.gno new file mode 100644 index 00000000000..74ba3ea66a2 --- /dev/null +++ b/gnovm/tests/files/recover15.gno @@ -0,0 +1,15 @@ +package main + + +func main() { + defer func() { + r := recover() + println("recover:", r) + }() + + var i interface{} = "hello" + _ = i.(int) // Panics because i holds a string, not an int +} + +// Output: +// recover: string is not of type int diff --git a/gnovm/tests/files/recover16.gno b/gnovm/tests/files/recover16.gno new file mode 100644 index 00000000000..31770f6d469 --- /dev/null +++ b/gnovm/tests/files/recover16.gno @@ -0,0 +1,14 @@ +package main + + +func main() { + defer func() { + r := recover() + println("recover:", r) + }() + + _ = make([]int, -1) // Panics because of negative length +} + +// Output: +// recover: len out of range diff --git a/gnovm/tests/files/recover17.gno b/gnovm/tests/files/recover17.gno new file mode 100644 index 00000000000..575801017b3 --- /dev/null +++ b/gnovm/tests/files/recover17.gno @@ -0,0 +1,15 @@ +package main + + +func main() { + defer func() { + r := recover() + println("recover:", r) + }() + + str := "hello" + _ = str[10] // Panics because index 10 is out of bounds +} + +// Output: +// recover: index out of range [10] with length 5 diff --git a/gnovm/tests/files/recover18.gno b/gnovm/tests/files/recover18.gno new file mode 100644 index 00000000000..f717e560dd2 --- /dev/null +++ b/gnovm/tests/files/recover18.gno @@ -0,0 +1,15 @@ +package main + + +func main() { + defer func() { + r := recover() + println("recover:", r) + }() + + var m map[string]int // nil map + m["key"] = 42 // Panics when trying to assign to a nil map +} + +// Output: +// recover: uninitialized map index diff --git a/gnovm/tests/files/recover19.gno b/gnovm/tests/files/recover19.gno new file mode 100644 index 00000000000..e1f0ff4c3b1 --- /dev/null +++ b/gnovm/tests/files/recover19.gno @@ -0,0 +1,15 @@ +package main + + +func main() { + defer func() { + r := recover() + println("recover:", r) + }() + + var p *int + println(*p) +} + +// Output: +// recover: nil pointer dereference