From 6ea826c363e744dcab3aa49c907528648c61cb41 Mon Sep 17 00:00:00 2001 From: Alessandro Arzilli Date: Tue, 22 Feb 2022 18:55:59 +0100 Subject: [PATCH] proc: better error messages for ambiguous function calls/type casts (#2903) Try to produce better error messages when we can't distinguish between a function call and a type cast. Fixes #2902 --- pkg/proc/eval.go | 74 ++++++++++++++++++++++++++++++---- pkg/proc/eval_go117.go | 10 +++++ pkg/proc/eval_go118.go | 8 ++++ pkg/proc/fncall.go | 3 ++ service/test/variables_test.go | 12 +++++- 5 files changed, 99 insertions(+), 8 deletions(-) create mode 100644 pkg/proc/eval_go117.go create mode 100644 pkg/proc/eval_go118.go diff --git a/pkg/proc/eval.go b/pkg/proc/eval.go index 29218c8e..921ea795 100644 --- a/pkg/proc/eval.go +++ b/pkg/proc/eval.go @@ -745,13 +745,7 @@ func (scope *EvalScope) evalToplevelTypeCast(t ast.Expr, cfg LoadConfig) (*Varia func (scope *EvalScope) evalAST(t ast.Expr) (*Variable, error) { switch node := t.(type) { case *ast.CallExpr: - if len(node.Args) == 1 { - v, err := scope.evalTypeCast(node) - if err == nil || err != reader.ErrTypeNotFound { - return v, err - } - } - return evalFunctionCall(scope, node) + return scope.evalTypeCastOrFuncCall(node) case *ast.Ident: return scope.evalIdent(node) @@ -850,6 +844,70 @@ func removeParen(n ast.Expr) ast.Expr { return n } +// evalTypeCastOrFuncCall evaluates a type cast or a function call +func (scope *EvalScope) evalTypeCastOrFuncCall(node *ast.CallExpr) (*Variable, error) { + if len(node.Args) != 1 { + // Things that have more or less than one argument are always function calls. + return evalFunctionCall(scope, node) + } + + ambiguous := func() (*Variable, error) { + // Ambiguous, could be a function call or a type cast, if node.Fun can be + // evaluated then try to treat it as a function call, otherwise try the + // type cast. + _, err0 := scope.evalAST(node.Fun) + if err0 == nil { + return evalFunctionCall(scope, node) + } + v, err := scope.evalTypeCast(node) + if err == reader.ErrTypeNotFound { + return nil, fmt.Errorf("could not evaluate function or type %s: %v", exprToString(node.Fun), err0) + } + return v, err + } + + fnnode := removeParen(node.Fun) + if n, _ := fnnode.(*ast.StarExpr); n != nil { + fnnode = removeParen(n.X) + } + + switch n := fnnode.(type) { + case *ast.BasicLit: + // It can only be a ("type string")(x) type cast + return scope.evalTypeCast(node) + case *ast.ArrayType, *ast.StructType, *ast.FuncType, *ast.InterfaceType, *ast.MapType, *ast.ChanType: + return scope.evalTypeCast(node) + case *ast.SelectorExpr: + if _, isident := n.X.(*ast.Ident); isident { + return ambiguous() + } + return evalFunctionCall(scope, node) + case *ast.Ident: + if supportedBuiltins[n.Name] { + return evalFunctionCall(scope, node) + } + return ambiguous() + case *ast.IndexExpr: + // Ambiguous, could be a parametric type + switch n.X.(type) { + case *ast.Ident, *ast.SelectorExpr: + // Do the type-cast first since evaluating node.Fun could be expensive. + v, err := scope.evalTypeCast(node) + if err == nil || err != reader.ErrTypeNotFound { + return v, err + } + return evalFunctionCall(scope, node) + default: + return evalFunctionCall(scope, node) + } + case *astIndexListExpr: + return scope.evalTypeCast(node) + default: + // All other expressions must be function calls + return evalFunctionCall(scope, node) + } +} + // Eval type cast expressions func (scope *EvalScope) evalTypeCast(node *ast.CallExpr) (*Variable, error) { argv, err := scope.evalAST(node.Args[0]) @@ -970,6 +1028,8 @@ func convertInt(n uint64, signed bool, size int64) uint64 { return r } +var supportedBuiltins = map[string]bool{"cap": true, "len": true, "complex": true, "imag": true, "real": true} + func (scope *EvalScope) evalBuiltinCall(node *ast.CallExpr) (*Variable, error) { fnnode, ok := node.Fun.(*ast.Ident) if !ok { diff --git a/pkg/proc/eval_go117.go b/pkg/proc/eval_go117.go new file mode 100644 index 00000000..b1add2a1 --- /dev/null +++ b/pkg/proc/eval_go117.go @@ -0,0 +1,10 @@ +//go:build !go1.18 +// +build !go1.18 + +package proc + +import "go/ast" + +type astIndexListExpr struct { + ast.Expr +} diff --git a/pkg/proc/eval_go118.go b/pkg/proc/eval_go118.go new file mode 100644 index 00000000..0d5c8504 --- /dev/null +++ b/pkg/proc/eval_go118.go @@ -0,0 +1,8 @@ +//go:build go1.18 +// +build go1.18 + +package proc + +import "go/ast" + +type astIndexListExpr = ast.IndexListExpr diff --git a/pkg/proc/fncall.go b/pkg/proc/fncall.go index a5124625..9a9ddc50 100644 --- a/pkg/proc/fncall.go +++ b/pkg/proc/fncall.go @@ -547,6 +547,9 @@ func funcCallEvalArgs(scope *EvalScope, fncall *functionCallState, formalScope * actualArg, err := scope.evalAST(fncall.expr.Args[i]) if err != nil { + if _, ispanic := err.(fncallPanicErr); ispanic { + return err + } return fmt.Errorf("error evaluating %q as argument %s in function %s: %v", exprToString(fncall.expr.Args[i]), formalArg.name, fncall.fn.Name, err) } actualArg.Name = exprToString(fncall.expr.Args[i]) diff --git a/service/test/variables_test.go b/service/test/variables_test.go index caffbf6e..1c5bf9aa 100644 --- a/service/test/variables_test.go +++ b/service/test/variables_test.go @@ -840,6 +840,16 @@ func TestEvalExpression(t *testing.T) { {"ni8 << 8", false, "0", "0", "int8", nil}, {"ni8 >> 1", false, "-3", "-3", "int8", nil}, {"bytearray[0] * bytearray[0]", false, "144", "144", "uint8", nil}, + + // function call / typecast errors + {"unknownthing(1, 2)", false, "", "", "", errors.New("function calls not allowed without using 'call'")}, + {"(unknownthing)(1, 2)", false, "", "", "", errors.New("function calls not allowed without using 'call'")}, + {"afunc(2)", false, "", "", "", errors.New("function calls not allowed without using 'call'")}, + {"(afunc)(2)", false, "", "", "", errors.New("function calls not allowed without using 'call'")}, + {"(*afunc)(2)", false, "", "", "", errors.New("could not evaluate function or type (*afunc): expression \"afunc\" (func()) can not be dereferenced")}, + {"unknownthing(2)", false, "", "", "", errors.New("could not evaluate function or type unknownthing: could not find symbol value for unknownthing")}, + {"(*unknownthing)(2)", false, "", "", "", errors.New("could not evaluate function or type (*unknownthing): could not find symbol value for unknownthing")}, + {"(*strings.Split)(2)", false, "", "", "", errors.New("could not evaluate function or type (*strings.Split): could not find symbol value for strings")}, } ver, _ := goversion.Parse(runtime.Version()) @@ -1230,7 +1240,7 @@ func TestCallFunction(t *testing.T) { {`onetwothree(intcallpanic(2))`, []string{`:[]int:[]int len: 3, cap: 3, [3,4,5]`}, nil}, {`onetwothree(intcallpanic(0))`, []string{`~panic:interface {}:interface {}(string) "panic requested"`}, nil}, {`onetwothree(intcallpanic(2)+1)`, []string{`:[]int:[]int len: 3, cap: 3, [4,5,6]`}, nil}, - {`onetwothree(intcallpanic("not a number"))`, nil, errors.New("can not convert \"not a number\" constant to int")}, + {`onetwothree(intcallpanic("not a number"))`, nil, errors.New("error evaluating \"intcallpanic(\\\"not a number\\\")\" as argument n in function main.onetwothree: can not convert \"not a number\" constant to int")}, // Variable setting tests {`pa2 = getAStructPtr(8); pa2`, []string{`pa2:*main.astruct:*main.astruct {X: 8}`}, nil},