From 2835a0655a9cfed1403f2712203c297581dc1d27 Mon Sep 17 00:00:00 2001 From: Omar Sy Date: Thu, 3 Oct 2024 18:11:31 +0200 Subject: [PATCH 1/5] chore(gnovm): prevent assignment to non-assignable expressions --- gnovm/pkg/gnolang/preprocess.go | 6 ++++++ gnovm/pkg/gnolang/type_check.go | 23 +++++++++++++++++++++++ gnovm/tests/files/assign29.gno | 8 ++++++++ gnovm/tests/files/var31.gno | 8 ++++++++ 4 files changed, 45 insertions(+) create mode 100644 gnovm/tests/files/assign29.gno create mode 100644 gnovm/tests/files/var31.gno diff --git a/gnovm/pkg/gnolang/preprocess.go b/gnovm/pkg/gnolang/preprocess.go index 9168fc6f7c1..100edcb2601 100644 --- a/gnovm/pkg/gnolang/preprocess.go +++ b/gnovm/pkg/gnolang/preprocess.go @@ -457,6 +457,9 @@ func Preprocess(store Store, ctx BlockNode, n Node) Node { case *AssignStmt: checkValDefineMismatch(n) + for _, rx := range n.Rhs { + checkExprIsAssignable(rx) + } if n.Op == DEFINE { for _, lx := range n.Lhs { ln := lx.(*NameExpr).Name @@ -489,6 +492,9 @@ func Preprocess(store Store, ctx BlockNode, n Node) Node { d := n.(Decl) if cd, ok := d.(*ValueDecl); ok { checkValDefineMismatch(cd) + for _, rx := range cd.Values { + checkExprIsAssignable(rx) + } } // recursively predefine dependencies. d2, ppd := predefineNow(store, last, d) diff --git a/gnovm/pkg/gnolang/type_check.go b/gnovm/pkg/gnolang/type_check.go index 31025fef152..cb3059c2098 100644 --- a/gnovm/pkg/gnolang/type_check.go +++ b/gnovm/pkg/gnolang/type_check.go @@ -279,6 +279,29 @@ func checkValDefineMismatch(n Node) { panic(fmt.Sprintf("assignment mismatch: %d variable(s) but %d value(s)", numNames, numValues)) } +func checkExprIsAssignable(exp Expr) { + switch exp.(type) { + case *NameExpr, + *BasicLitExpr, + *BinaryExpr, + *CallExpr, + *IndexExpr, + *SelectorExpr, + *SliceExpr, + *StarExpr, + *RefExpr, + *TypeAssertExpr, + *UnaryExpr, + *CompositeLitExpr, + *KeyValueExpr, + *FuncLitExpr, + *ConstExpr: + return + } + + panic(fmt.Sprintf("%s (type) is not an expression", exp.String())) +} + // Assert that xt can be assigned as dt (dest type). // If autoNative is true, a broad range of xt can match against // a target native dt type, if and only if dt is a native type. diff --git a/gnovm/tests/files/assign29.gno b/gnovm/tests/files/assign29.gno new file mode 100644 index 00000000000..747b7051ea6 --- /dev/null +++ b/gnovm/tests/files/assign29.gno @@ -0,0 +1,8 @@ +package main + +func main() { + t := struct{} +} + +// Error: +// main/files/assign29.gno:4:2: struct { } (type) is not an expression diff --git a/gnovm/tests/files/var31.gno b/gnovm/tests/files/var31.gno new file mode 100644 index 00000000000..036bc19f828 --- /dev/null +++ b/gnovm/tests/files/var31.gno @@ -0,0 +1,8 @@ +package main + +func main() { + var t = struct{} +} + +// Error: +// main/files/var31.gno:4:6: struct { } (type) is not an expression From 0646ab109fe22f6ed742fd791ff348bad18942ef Mon Sep 17 00:00:00 2001 From: Omar Sy Date: Fri, 25 Oct 2024 00:34:19 +0200 Subject: [PATCH 2/5] feat: use eval type to check if it is typedeclaration --- gnovm/pkg/gnolang/preprocess.go | 13 +++++++------ gnovm/pkg/gnolang/type_check.go | 29 +++++++++-------------------- gnovm/tests/files/assign29.gno | 2 +- gnovm/tests/files/assign30.gno | 8 ++++++++ gnovm/tests/files/var31.gno | 2 +- 5 files changed, 26 insertions(+), 28 deletions(-) create mode 100644 gnovm/tests/files/assign30.gno diff --git a/gnovm/pkg/gnolang/preprocess.go b/gnovm/pkg/gnolang/preprocess.go index 100edcb2601..96343cecc4b 100644 --- a/gnovm/pkg/gnolang/preprocess.go +++ b/gnovm/pkg/gnolang/preprocess.go @@ -457,9 +457,6 @@ func Preprocess(store Store, ctx BlockNode, n Node) Node { case *AssignStmt: checkValDefineMismatch(n) - for _, rx := range n.Rhs { - checkExprIsAssignable(rx) - } if n.Op == DEFINE { for _, lx := range n.Lhs { ln := lx.(*NameExpr).Name @@ -492,9 +489,6 @@ func Preprocess(store Store, ctx BlockNode, n Node) Node { d := n.(Decl) if cd, ok := d.(*ValueDecl); ok { checkValDefineMismatch(cd) - for _, rx := range cd.Values { - checkExprIsAssignable(rx) - } } // recursively predefine dependencies. d2, ppd := predefineNow(store, last, d) @@ -1879,6 +1873,10 @@ func Preprocess(store Store, ctx BlockNode, n Node) Node { // TRANS_LEAVE ----------------------- case *AssignStmt: n.AssertCompatible(store, last) + + for _, n := range n.Rhs { + checkExprIsNotTypeDecl(store, last, n) + } // NOTE: keep DEFINE and ASSIGN in sync. if n.Op == DEFINE { // Rhs consts become default *ConstExprs. @@ -2189,6 +2187,9 @@ func Preprocess(store Store, ctx BlockNode, n Node) Node { // TRANS_LEAVE ----------------------- case *ValueDecl: + for _, v := range n.Values { + checkExprIsNotTypeDecl(store, last, v) + } // evaluate value if const expr. if n.Const { // NOTE: may or may not be a *ConstExpr, diff --git a/gnovm/pkg/gnolang/type_check.go b/gnovm/pkg/gnolang/type_check.go index cb3059c2098..d437054335f 100644 --- a/gnovm/pkg/gnolang/type_check.go +++ b/gnovm/pkg/gnolang/type_check.go @@ -279,27 +279,16 @@ func checkValDefineMismatch(n Node) { panic(fmt.Sprintf("assignment mismatch: %d variable(s) but %d value(s)", numNames, numValues)) } -func checkExprIsAssignable(exp Expr) { - switch exp.(type) { - case *NameExpr, - *BasicLitExpr, - *BinaryExpr, - *CallExpr, - *IndexExpr, - *SelectorExpr, - *SliceExpr, - *StarExpr, - *RefExpr, - *TypeAssertExpr, - *UnaryExpr, - *CompositeLitExpr, - *KeyValueExpr, - *FuncLitExpr, - *ConstExpr: - return +func checkExprIsNotTypeDecl(store Store, last BlockNode, exp Expr) { + switch n := exp.(type) { + case *CallExpr, *TypeAssertExpr, *IndexExpr: + default: + tt := evalStaticTypeOf(store, last, n) + if _, ok := tt.(*TypeType); ok { + tt = evalStaticType(store, last, n) + panic(fmt.Sprintf("%s (type) is not an expression", tt.String())) + } } - - panic(fmt.Sprintf("%s (type) is not an expression", exp.String())) } // Assert that xt can be assigned as dt (dest type). diff --git a/gnovm/tests/files/assign29.gno b/gnovm/tests/files/assign29.gno index 747b7051ea6..ca605f5ecbe 100644 --- a/gnovm/tests/files/assign29.gno +++ b/gnovm/tests/files/assign29.gno @@ -5,4 +5,4 @@ func main() { } // Error: -// main/files/assign29.gno:4:2: struct { } (type) is not an expression +// main/files/assign29.gno:4:2: struct{} (type) is not an expression diff --git a/gnovm/tests/files/assign30.gno b/gnovm/tests/files/assign30.gno new file mode 100644 index 00000000000..ad72f880f27 --- /dev/null +++ b/gnovm/tests/files/assign30.gno @@ -0,0 +1,8 @@ +package main + +func main() { + t := *struct{} +} + +// Error: +// main/files/assign30.gno:4:2: *struct{} (type) is not an expression diff --git a/gnovm/tests/files/var31.gno b/gnovm/tests/files/var31.gno index 036bc19f828..813e6ff9e22 100644 --- a/gnovm/tests/files/var31.gno +++ b/gnovm/tests/files/var31.gno @@ -5,4 +5,4 @@ func main() { } // Error: -// main/files/var31.gno:4:6: struct { } (type) is not an expression +// main/files/var31.gno:4:6: struct{} (type) is not an expression From 89572dfb371e17d62aec23d67426094c83cb0f4a Mon Sep 17 00:00:00 2001 From: Omar Sy Date: Fri, 25 Oct 2024 21:20:39 +0200 Subject: [PATCH 3/5] refactor: type checking and assignment validation in preprocess.go and type_check.go --- gnovm/pkg/gnolang/preprocess.go | 8 ++------ gnovm/pkg/gnolang/type_check.go | 19 +++++++++++-------- 2 files changed, 13 insertions(+), 14 deletions(-) diff --git a/gnovm/pkg/gnolang/preprocess.go b/gnovm/pkg/gnolang/preprocess.go index 96343cecc4b..bbdfce2b6b3 100644 --- a/gnovm/pkg/gnolang/preprocess.go +++ b/gnovm/pkg/gnolang/preprocess.go @@ -1874,9 +1874,6 @@ func Preprocess(store Store, ctx BlockNode, n Node) Node { case *AssignStmt: n.AssertCompatible(store, last) - for _, n := range n.Rhs { - checkExprIsNotTypeDecl(store, last, n) - } // NOTE: keep DEFINE and ASSIGN in sync. if n.Op == DEFINE { // Rhs consts become default *ConstExprs. @@ -2187,9 +2184,8 @@ func Preprocess(store Store, ctx BlockNode, n Node) Node { // TRANS_LEAVE ----------------------- case *ValueDecl: - for _, v := range n.Values { - checkExprIsNotTypeDecl(store, last, v) - } + assertValidAssignRhs(store, last, n.Values) + // evaluate value if const expr. if n.Const { // NOTE: may or may not be a *ConstExpr, diff --git a/gnovm/pkg/gnolang/type_check.go b/gnovm/pkg/gnolang/type_check.go index d437054335f..44fe521c311 100644 --- a/gnovm/pkg/gnolang/type_check.go +++ b/gnovm/pkg/gnolang/type_check.go @@ -279,14 +279,16 @@ func checkValDefineMismatch(n Node) { panic(fmt.Sprintf("assignment mismatch: %d variable(s) but %d value(s)", numNames, numValues)) } -func checkExprIsNotTypeDecl(store Store, last BlockNode, exp Expr) { - switch n := exp.(type) { - case *CallExpr, *TypeAssertExpr, *IndexExpr: - default: - tt := evalStaticTypeOf(store, last, n) - if _, ok := tt.(*TypeType); ok { - tt = evalStaticType(store, last, n) - panic(fmt.Sprintf("%s (type) is not an expression", tt.String())) +func assertValidAssignRhs(store Store, last BlockNode, exps Exprs) { + for _, exp := range exps { + switch n := exp.(type) { + case *CallExpr, *TypeAssertExpr, *IndexExpr: + default: + tt := evalStaticTypeOf(store, last, n) + if _, ok := tt.(*TypeType); ok { + tt = evalStaticType(store, last, n) + panic(fmt.Sprintf("%s (type) is not an expression", tt.String())) + } } } } @@ -786,6 +788,7 @@ func (x *RangeStmt) AssertCompatible(store Store, last BlockNode) { func (x *AssignStmt) AssertCompatible(store Store, last BlockNode) { if x.Op == ASSIGN || x.Op == DEFINE { + assertValidAssignRhs(store, last, x.Rhs) if len(x.Lhs) > len(x.Rhs) { if len(x.Rhs) != 1 { panic(fmt.Sprintf("assignment mismatch: %d variables but %d values", len(x.Lhs), len(x.Rhs))) From 7d935f616323f9707d899c449ed4661546aeb392 Mon Sep 17 00:00:00 2001 From: Omar Sy Date: Fri, 25 Oct 2024 22:39:15 +0200 Subject: [PATCH 4/5] refactor: Move assertValidAssignRhs function in type_check.go --- gnovm/pkg/gnolang/type_check.go | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/gnovm/pkg/gnolang/type_check.go b/gnovm/pkg/gnolang/type_check.go index 44fe521c311..abad7d9cb26 100644 --- a/gnovm/pkg/gnolang/type_check.go +++ b/gnovm/pkg/gnolang/type_check.go @@ -279,20 +279,6 @@ func checkValDefineMismatch(n Node) { panic(fmt.Sprintf("assignment mismatch: %d variable(s) but %d value(s)", numNames, numValues)) } -func assertValidAssignRhs(store Store, last BlockNode, exps Exprs) { - for _, exp := range exps { - switch n := exp.(type) { - case *CallExpr, *TypeAssertExpr, *IndexExpr: - default: - tt := evalStaticTypeOf(store, last, n) - if _, ok := tt.(*TypeType); ok { - tt = evalStaticType(store, last, n) - panic(fmt.Sprintf("%s (type) is not an expression", tt.String())) - } - } - } -} - // Assert that xt can be assigned as dt (dest type). // If autoNative is true, a broad range of xt can match against // a target native dt type, if and only if dt is a native type. @@ -929,6 +915,20 @@ func assertValidAssignLhs(store Store, last BlockNode, lx Expr) { } } +func assertValidAssignRhs(store Store, last BlockNode, exps Exprs) { + for _, exp := range exps { + switch n := exp.(type) { + case *CallExpr, *TypeAssertExpr, *IndexExpr: + default: + tt := evalStaticTypeOf(store, last, n) + if _, ok := tt.(*TypeType); ok { + tt = evalStaticType(store, last, n) + panic(fmt.Sprintf("%s (type) is not an expression", tt.String())) + } + } + } +} + func kindString(xt Type) string { if xt != nil { return xt.Kind().String() From 7f89510e007fac10548688ca091598c4ced6e8e2 Mon Sep 17 00:00:00 2001 From: Omar Sy Date: Sat, 2 Nov 2024 17:44:16 +0100 Subject: [PATCH 5/5] feat: handle nil case --- gnovm/pkg/gnolang/preprocess.go | 2 +- gnovm/pkg/gnolang/type_check.go | 38 +++++++++++++++++++++++++-------- gnovm/tests/files/assign31.gno | 9 ++++++++ gnovm/tests/files/var32.gno | 8 +++++++ gnovm/tests/files/var33.gno | 9 ++++++++ 5 files changed, 56 insertions(+), 10 deletions(-) create mode 100644 gnovm/tests/files/assign31.gno create mode 100644 gnovm/tests/files/var32.gno create mode 100644 gnovm/tests/files/var33.gno diff --git a/gnovm/pkg/gnolang/preprocess.go b/gnovm/pkg/gnolang/preprocess.go index bbdfce2b6b3..fb17260a438 100644 --- a/gnovm/pkg/gnolang/preprocess.go +++ b/gnovm/pkg/gnolang/preprocess.go @@ -2184,7 +2184,7 @@ func Preprocess(store Store, ctx BlockNode, n Node) Node { // TRANS_LEAVE ----------------------- case *ValueDecl: - assertValidAssignRhs(store, last, n.Values) + assertValidAssignRhs(store, last, n) // evaluate value if const expr. if n.Const { diff --git a/gnovm/pkg/gnolang/type_check.go b/gnovm/pkg/gnolang/type_check.go index abad7d9cb26..c3c638d1363 100644 --- a/gnovm/pkg/gnolang/type_check.go +++ b/gnovm/pkg/gnolang/type_check.go @@ -774,7 +774,7 @@ func (x *RangeStmt) AssertCompatible(store Store, last BlockNode) { func (x *AssignStmt) AssertCompatible(store Store, last BlockNode) { if x.Op == ASSIGN || x.Op == DEFINE { - assertValidAssignRhs(store, last, x.Rhs) + assertValidAssignRhs(store, last, x) if len(x.Lhs) > len(x.Rhs) { if len(x.Rhs) != 1 { panic(fmt.Sprintf("assignment mismatch: %d variables but %d values", len(x.Lhs), len(x.Rhs))) @@ -915,17 +915,37 @@ func assertValidAssignLhs(store Store, last BlockNode, lx Expr) { } } -func assertValidAssignRhs(store Store, last BlockNode, exps Exprs) { +func assertValidAssignRhs(store Store, last BlockNode, n Node) { + var exps []Expr + switch x := n.(type) { + case *ValueDecl: + exps = x.Values + case *AssignStmt: + exps = x.Rhs + default: + panic(fmt.Sprintf("unexpected node type %T", n)) + } + for _, exp := range exps { - switch n := exp.(type) { - case *CallExpr, *TypeAssertExpr, *IndexExpr: - default: - tt := evalStaticTypeOf(store, last, n) - if _, ok := tt.(*TypeType); ok { - tt = evalStaticType(store, last, n) - panic(fmt.Sprintf("%s (type) is not an expression", tt.String())) + tt := evalStaticTypeOfRaw(store, last, exp) + if tt == nil { + switch x := n.(type) { + case *ValueDecl: + if x.Type != nil { + continue + } + panic("use of untyped nil in variable declaration") + case *AssignStmt: + if x.Op != DEFINE { + continue + } + panic("use of untyped nil in assignment") } } + if _, ok := tt.(*TypeType); ok { + tt = evalStaticType(store, last, exp) + panic(fmt.Sprintf("%s (type) is not an expression", tt.String())) + } } } diff --git a/gnovm/tests/files/assign31.gno b/gnovm/tests/files/assign31.gno new file mode 100644 index 00000000000..8c96c04501e --- /dev/null +++ b/gnovm/tests/files/assign31.gno @@ -0,0 +1,9 @@ +package main + +func main() { + a := nil + println(a) +} + +// Error: +// main/files/assign31.gno:4:2: use of untyped nil in assignment diff --git a/gnovm/tests/files/var32.gno b/gnovm/tests/files/var32.gno new file mode 100644 index 00000000000..827c3951f94 --- /dev/null +++ b/gnovm/tests/files/var32.gno @@ -0,0 +1,8 @@ +package main + +func main() { + var t = nil +} + +// Error: +// main/files/var32.gno:4:6: use of untyped nil in variable declaration diff --git a/gnovm/tests/files/var33.gno b/gnovm/tests/files/var33.gno new file mode 100644 index 00000000000..ce883dce47c --- /dev/null +++ b/gnovm/tests/files/var33.gno @@ -0,0 +1,9 @@ +package main + +func main() { + var t *int = nil + println("pass") +} + +// Output: +// pass