From 952345c9093b99ddb44ff2749a719e2e5e51273f Mon Sep 17 00:00:00 2001 From: Christian Banse Date: Thu, 2 Mar 2023 18:51:26 +0100 Subject: [PATCH] More Go improvements --- .../aisec/cpg/graph/ArgumentHolder.kt | 4 +- .../aisec/cpg/graph/types/FunctionType.kt | 6 +- .../cpg/graph/statements/ReturnStatement.kt | 11 ---- .../expressions/AssignExpression.kt | 1 - .../cpg/passes/EvaluationOrderGraphPass.kt | 11 ++++ .../src/main/golang/frontend/frontend.go | 6 +- .../src/main/golang/frontend/handler.go | 64 ++++++++++--------- cpg-language-go/src/test/resources/log4j2.xml | 2 +- 8 files changed, 58 insertions(+), 47 deletions(-) diff --git a/cpg-core/src/main/java/de/fraunhofer/aisec/cpg/graph/ArgumentHolder.kt b/cpg-core/src/main/java/de/fraunhofer/aisec/cpg/graph/ArgumentHolder.kt index 0d6ac89e2ec..b2d512e70e4 100644 --- a/cpg-core/src/main/java/de/fraunhofer/aisec/cpg/graph/ArgumentHolder.kt +++ b/cpg-core/src/main/java/de/fraunhofer/aisec/cpg/graph/ArgumentHolder.kt @@ -44,9 +44,7 @@ interface ArgumentHolder : Holder { /** Adds the [expression] to the list of arguments. */ fun addArgument(expression: Expression) - /** - * Removes the [expression] from the list of arguments. - */ + /** Removes the [expression] from the list of arguments. */ fun removeArgument(expression: Expression) {} /** diff --git a/cpg-core/src/main/java/de/fraunhofer/aisec/cpg/graph/types/FunctionType.kt b/cpg-core/src/main/java/de/fraunhofer/aisec/cpg/graph/types/FunctionType.kt index 7cc18683fa9..67525667e75 100644 --- a/cpg-core/src/main/java/de/fraunhofer/aisec/cpg/graph/types/FunctionType.kt +++ b/cpg-core/src/main/java/de/fraunhofer/aisec/cpg/graph/types/FunctionType.kt @@ -57,7 +57,11 @@ class FunctionType : Type { override fun reference(pointer: PointerType.PointerOrigin?): Type { // TODO(oxisto): In the future, we actually could just remove the FunctionPointerType // and just have a regular PointerType here - return FunctionPointerType(parameters.toList(), returnTypes.first(), language) + return FunctionPointerType( + parameters.toList(), + returnTypes.firstOrNull() ?: UnknownType.getUnknownType(), + language + ) } override fun dereference(): Type { diff --git a/cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/graph/statements/ReturnStatement.kt b/cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/graph/statements/ReturnStatement.kt index 2407298ef97..c74a845c10c 100644 --- a/cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/graph/statements/ReturnStatement.kt +++ b/cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/graph/statements/ReturnStatement.kt @@ -60,19 +60,8 @@ class ReturnStatement : Statement(), ArgumentHolder { this.returnValues += expression } -<<<<<<< HEAD - override fun replaceArgument(old: Expression, new: Expression): Boolean { - this.returnValue = new - return true - } - - override fun replaceArgument(old: Expression, new: Expression): Boolean { - this.returnValue = new - return true -======= override fun removeArgument(expression: Expression) { this.returnValues -= expression ->>>>>>> ceb0837bc (DFGPass handling multiple returns) } override fun replaceArgument(old: Expression, new: Expression): Boolean { diff --git a/cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/graph/statements/expressions/AssignExpression.kt b/cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/graph/statements/expressions/AssignExpression.kt index ef9ae9544f8..dd6cd6876d0 100644 --- a/cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/graph/statements/expressions/AssignExpression.kt +++ b/cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/graph/statements/expressions/AssignExpression.kt @@ -174,5 +174,4 @@ class AssignExpression : Expression(), AssignmentHolder, HasType.TypeListener { return findTargets(rhs).map { Assignment(rhs, it) } } } - } diff --git a/cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/passes/EvaluationOrderGraphPass.kt b/cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/passes/EvaluationOrderGraphPass.kt index dca4248eec0..947588e3af4 100644 --- a/cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/passes/EvaluationOrderGraphPass.kt +++ b/cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/passes/EvaluationOrderGraphPass.kt @@ -113,6 +113,7 @@ open class EvaluationOrderGraphPass : Pass() { } map[ReturnStatement::class.java] = { handleReturnStatement(it as ReturnStatement) } map[BinaryOperator::class.java] = { handleBinaryOperator(it as BinaryOperator) } + map[AssignExpression::class.java] = { handleAssignExpression(it as AssignExpression) } map[UnaryOperator::class.java] = { handleUnaryOperator(it as UnaryOperator) } map[CompoundStatement::class.java] = { handleCompoundStatement(it as CompoundStatement) } map[CompoundStatementExpression::class.java] = { @@ -474,6 +475,16 @@ open class EvaluationOrderGraphPass : Pass() { pushToEOG(node) } + protected fun handleAssignExpression(node: AssignExpression) { + // Handle left hand side(s) first + node.lhs.forEach { createEOG(it) } + + // Then the right side(s) + node.rhs.forEach { createEOG(it) } + + pushToEOG(node) + } + protected fun handleCompoundStatement(node: CompoundStatement) { // not all language handle compound statements as scoping blocks, so we need to avoid // creating new scopes here diff --git a/cpg-language-go/src/main/golang/frontend/frontend.go b/cpg-language-go/src/main/golang/frontend/frontend.go index d0e7d0c0dc6..dde4aa231ec 100644 --- a/cpg-language-go/src/main/golang/frontend/frontend.go +++ b/cpg-language-go/src/main/golang/frontend/frontend.go @@ -133,10 +133,14 @@ func (g *GoLanguageFrontend) GetLanguage() (l *cpg.Language, err error) { } func updateCode(fset *token.FileSet, node *cpg.Node, astNode ast.Node) { + node.SetCode(code(fset, astNode)) +} + +func code(fset *token.FileSet, astNode ast.Node) string { var codeBuf bytes.Buffer _ = printer.Fprint(&codeBuf, fset, astNode) - node.SetCode(codeBuf.String()) + return codeBuf.String() } func updateLocation(fset *token.FileSet, node *cpg.Node, astNode ast.Node) { diff --git a/cpg-language-go/src/main/golang/frontend/handler.go b/cpg-language-go/src/main/golang/frontend/handler.go index 9310e05f6e5..a10aca78b6d 100644 --- a/cpg-language-go/src/main/golang/frontend/handler.go +++ b/cpg-language-go/src/main/golang/frontend/handler.go @@ -193,7 +193,7 @@ func (this *GoLanguageFrontend) handleFuncDecl(fset *token.FileSet, funcDecl *as // TODO: why is this a list? var recv = funcDecl.Recv.List[0] - var recordType = this.handleType(recv.Type) + var recordType = this.handleType(fset, recv.Type) // The name of the Go receiver is optional. In fact, if the name is not // specified we probably do not need any receiver variable at all, @@ -261,18 +261,18 @@ func (this *GoLanguageFrontend) handleFuncDecl(fset *token.FileSet, funcDecl *as this.GetScopeManager().AddDeclaration((*cpg.Declaration)(receiver)) } - var t *cpg.Type = this.handleType(funcDecl.Type) + var t *cpg.Type = this.handleType(fset, funcDecl.Type) var returnTypes []*cpg.Type = []*cpg.Type{} if funcDecl.Type.Results != nil { for _, returnVariable := range funcDecl.Type.Results.List { - returnTypes = append(returnTypes, this.handleType(returnVariable.Type)) + returnTypes = append(returnTypes, this.handleType(fset, returnVariable.Type)) // if the function has named return variables, be sure to declare them as well if returnVariable.Names != nil { p := this.NewVariableDeclaration(fset, returnVariable, returnVariable.Names[0].Name) - p.SetType(this.handleType(returnVariable.Type)) + p.SetType(this.handleType(fset, returnVariable.Type)) // add parameter to scope this.GetScopeManager().AddDeclaration((*cpg.Declaration)(p)) @@ -311,7 +311,7 @@ func (this *GoLanguageFrontend) handleFuncDecl(fset *token.FileSet, funcDecl *as p := this.NewParamVariableDeclaration(fset, param, name) - p.SetType(this.handleType(param.Type)) + p.SetType(this.handleType(fset, param.Type)) // add parameter to scope this.GetScopeManager().AddDeclaration((*cpg.Declaration)(p)) @@ -367,7 +367,7 @@ func (this *GoLanguageFrontend) handleValueSpec(fset *token.FileSet, valueDecl * d := this.NewVariableDeclaration(fset, valueDecl, ident.Name) if valueDecl.Type != nil { - t := this.handleType(valueDecl.Type) + t := this.handleType(fset, valueDecl.Type) d.SetType(t) } @@ -438,7 +438,7 @@ func (this *GoLanguageFrontend) handleStructTypeSpec(fset *token.FileSet, typeDe // by its type, it could make sense to name the field according to the type var name string - t := this.handleType(field.Type) + t := this.handleType(fset, field.Type) if field.Names == nil { // retrieve the root type name @@ -476,7 +476,7 @@ func (this *GoLanguageFrontend) handleInterfaceTypeSpec(fset *token.FileSet, typ if !interfaceType.Incomplete { for _, method := range interfaceType.Methods.List { - t := this.handleType(method.Type) + t := this.handleType(fset, method.Type) // Even though this list is called "Methods", it contains all kinds // of things, so we need to proceed with caution. Only if the @@ -536,15 +536,18 @@ func (this *GoLanguageFrontend) handleForStmt(fset *token.FileSet, forStmt *ast. scope.EnterScope((*cpg.Node)(f)) - if initStatement := this.handleStmt(fset, forStmt.Init, forStmt); initStatement != nil { + if forStmt.Init != nil { + initStatement := this.handleStmt(fset, forStmt.Init, forStmt) f.SetInitializerStatement(initStatement) } - if condition := this.handleExpr(fset, forStmt.Cond); condition != nil && !(*jnigi.ObjectRef)(condition).IsNil() { + if forStmt.Cond != nil { + condition := this.handleExpr(fset, forStmt.Cond) f.SetCondition(condition) } - if iter := this.handleStmt(fset, forStmt.Post, forStmt); iter != nil { + if forStmt.Post != nil { + iter := this.handleStmt(fset, forStmt.Post, forStmt) f.SetIterationStatement(iter) } @@ -625,7 +628,7 @@ func (this *GoLanguageFrontend) handleStmt(fset *token.FileSet, stmt ast.Stmt, p case *ast.IncDecStmt: s = (*cpg.Statement)(this.handleIncDecStmt(fset, v)) default: - msg := fmt.Sprintf("Not parsing statement of type %T yet: %+v. Parent == %#v", v, v, parent) + msg := fmt.Sprintf("Not parsing statement of type %T yet: %s", v, code(fset, v)) this.LogError(msg) s = (*cpg.Statement)(this.NewProblemExpression(fset, v, msg)) } @@ -668,7 +671,7 @@ func (this *GoLanguageFrontend) handleExpr(fset *token.FileSet, expr ast.Expr) ( case *ast.ParenExpr: e = this.handleExpr(fset, v.X) default: - msg := fmt.Sprintf("Could not parse expression of type %T: %+v", v, v) + msg := fmt.Sprintf("Not parsing expression of type %T yet: %s", v, code(fset, v)) this.LogError(msg) e = (*cpg.Expression)(this.NewProblemExpression(fset, v, msg)) } @@ -757,7 +760,10 @@ func (this *GoLanguageFrontend) handleIfStmt(fset *token.FileSet, ifStmt *ast.If stmt.SetCondition(cond) then := this.handleBlockStmt(fset, ifStmt.Body) - stmt.SetThenStatement((*cpg.Statement)(then)) + // Somehow this can be nil-ish? + if !then.IsNil() { + stmt.SetThenStatement((*cpg.Statement)(then)) + } if ifStmt.Else != nil { els := this.handleStmt(fset, ifStmt.Else, ifStmt) @@ -838,7 +844,7 @@ func (this *GoLanguageFrontend) handleCallExpr(fset *token.FileSet, callExpr *as this.LogDebug("Handling cast expression: %#v", callExpr) cast := this.NewCastExpression(fset, callExpr) - cast.SetCastType(this.handleType(v)) + cast.SetCastType(this.handleType(fset, v)) if len(callExpr.Args) > 1 { cast.SetExpression(this.handleExpr(fset, callExpr.Args[0])) @@ -904,7 +910,7 @@ func (this *GoLanguageFrontend) handleNewExpr(fset *token.FileSet, callExpr *ast n := this.NewNewExpression(fset, callExpr) // first argument is type - t := this.handleType(callExpr.Args[0]) + t := this.handleType(fset, callExpr.Args[0]) // new is a pointer, so need to reference the type with a pointer var pointer = jnigi.NewObjectRef(cpg.PointerOriginClass) @@ -932,7 +938,7 @@ func (this *GoLanguageFrontend) handleMakeExpr(fset *token.FileSet, callExpr *as } // first argument is always the type, handle it - t := this.handleType(callExpr.Args[0]) + t := this.handleType(fset, callExpr.Args[0]) // actually make() can make more than just arrays, i.e. channels and maps if _, isArray := callExpr.Args[0].(*ast.ArrayType); isArray { @@ -1114,7 +1120,7 @@ func (this *GoLanguageFrontend) handleCompositeLit(fset *token.FileSet, lit *ast c := this.NewConstructExpression(fset, lit) // parse the type field, to see which kind of expression it is - var typ = this.handleType(lit.Type) + var typ = this.handleType(fset, lit.Type) if typ != nil { (*cpg.Node)(c).SetName(typ.GetName()) @@ -1198,7 +1204,7 @@ func (this *GoLanguageFrontend) handleTypeAssertExpr(fset *token.FileSet, assert expr := this.handleExpr(fset, assert.X) // Parse the type - typ := this.handleType(assert.Type) + typ := this.handleType(fset, assert.Type) cast.SetExpression(expr) @@ -1209,10 +1215,10 @@ func (this *GoLanguageFrontend) handleTypeAssertExpr(fset *token.FileSet, assert return cast } -func (this *GoLanguageFrontend) handleType(typeExpr ast.Expr) *cpg.Type { +func (this *GoLanguageFrontend) handleType(fset *token.FileSet, typeExpr ast.Expr) *cpg.Type { var err error - this.LogTrace("Parsing type %T: %+v", typeExpr, typeExpr) + this.LogTrace("Parsing type %T: %s", typeExpr, code(fset, typeExpr)) lang, err := this.GetLanguage() if err != nil { @@ -1237,7 +1243,7 @@ func (this *GoLanguageFrontend) handleType(typeExpr ast.Expr) *cpg.Type { this.LogTrace("FQN type: %s", fqn) return cpg.TypeParser_createFrom(fqn, lang) case *ast.StarExpr: - t := this.handleType(v.X) + t := this.handleType(fset, v.X) var i = jnigi.NewObjectRef(cpg.PointerOriginClass) err = env.GetStaticField(cpg.PointerOriginClass, "POINTER", i) @@ -1249,7 +1255,7 @@ func (this *GoLanguageFrontend) handleType(typeExpr ast.Expr) *cpg.Type { return t.Reference(i) case *ast.ArrayType: - t := this.handleType(v.Elt) + t := this.handleType(fset, v.Elt) var i = jnigi.NewObjectRef(cpg.PointerOriginClass) err = env.GetStaticField(cpg.PointerOriginClass, "ARRAY", i) @@ -1264,8 +1270,8 @@ func (this *GoLanguageFrontend) handleType(typeExpr ast.Expr) *cpg.Type { // we cannot properly represent Golangs built-in map types, yet so we have // to make a shortcut here and represent it as a Java-like map type. t := cpg.TypeParser_createFrom("map", lang) - keyType := this.handleType(v.Key) - valueType := this.handleType(v.Value) + keyType := this.handleType(fset, v.Key) + valueType := this.handleType(fset, v.Value) // TODO(oxisto): Find a better way to represent casts (&(cpg.ObjectType{Type: *t})).AddGeneric(keyType) @@ -1275,7 +1281,7 @@ func (this *GoLanguageFrontend) handleType(typeExpr ast.Expr) *cpg.Type { case *ast.ChanType: // handle them similar to maps t := cpg.TypeParser_createFrom("chan", lang) - chanType := this.handleType(v.Value) + chanType := this.handleType(fset, v.Value) (&(cpg.ObjectType{Type: *t})).AddGeneric(chanType) @@ -1286,7 +1292,7 @@ func (this *GoLanguageFrontend) handleType(typeExpr ast.Expr) *cpg.Type { var returnTypes = []*cpg.Type{} for _, param := range v.Params.List { - parameterTypes = append(parameterTypes, this.handleType(param.Type)) + parameterTypes = append(parameterTypes, this.handleType(fset, param.Type)) } parametersTypesList, err = cpg.ListOf(parameterTypes) @@ -1296,7 +1302,7 @@ func (this *GoLanguageFrontend) handleType(typeExpr ast.Expr) *cpg.Type { if v.Results != nil { for _, ret := range v.Results.List { - returnTypes = append(returnTypes, this.handleType(ret.Type)) + returnTypes = append(returnTypes, this.handleType(fset, ret.Type)) } } @@ -1325,7 +1331,7 @@ func (this *GoLanguageFrontend) handleType(typeExpr ast.Expr) *cpg.Type { // We do not really support dedicated interfaces types, so all we can for now // is parse it as an object type with a pseudo-name for _, method := range v.Methods.List { - name += this.handleType(method.Type).GetName().ToString() + name += this.handleType(fset, method.Type).GetName().ToString() } name += "}" diff --git a/cpg-language-go/src/test/resources/log4j2.xml b/cpg-language-go/src/test/resources/log4j2.xml index 5b73082e2c0..747860628a4 100644 --- a/cpg-language-go/src/test/resources/log4j2.xml +++ b/cpg-language-go/src/test/resources/log4j2.xml @@ -6,7 +6,7 @@ - +