Fixed bug in defer handling

Fraunhofer-AISEC · Sep 19, 2023 · 1633639 · 1633639
1 parent 9b51f0a
commit 1633639
Show file tree

Hide file tree

Showing 4 changed files with 30 additions and 8 deletions.
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
@@ -245,7 +245,7 @@ open class EvaluationOrderGraphPass(ctx: TranslationContext) : TranslationUnitPa
         pushToEOG(node)
     }
 
-    protected fun handleRecordDeclaration(node: RecordDeclaration) {
+    protected open fun handleRecordDeclaration(node: RecordDeclaration) {
         scopeManager.enterScope(node)
         handleStatementHolder(node)
         currentPredecessors.clear()

diff --git a/cpg-language-go/src/main/kotlin/de/fraunhofer/aisec/cpg/passes/GoEvaluationOrderGraphPass.kt b/cpg-language-go/src/main/kotlin/de/fraunhofer/aisec/cpg/passes/GoEvaluationOrderGraphPass.kt
@@ -28,6 +28,10 @@ package de.fraunhofer.aisec.cpg.passes
 import de.fraunhofer.aisec.cpg.TranslationContext
 import de.fraunhofer.aisec.cpg.frontends.golang.GoLanguage
 import de.fraunhofer.aisec.cpg.graph.declarations.FunctionDeclaration
+import de.fraunhofer.aisec.cpg.graph.declarations.NamespaceDeclaration
+import de.fraunhofer.aisec.cpg.graph.declarations.RecordDeclaration
+import de.fraunhofer.aisec.cpg.graph.followNextEOGEdgesUntilHit
+import de.fraunhofer.aisec.cpg.graph.statements.ReturnStatement
 import de.fraunhofer.aisec.cpg.graph.statements.expressions.CallExpression
 import de.fraunhofer.aisec.cpg.graph.statements.expressions.UnaryOperator
 
@@ -78,6 +82,23 @@ class GoEvaluationOrderGraphPass(ctx: TranslationContext) : EvaluationOrderGraph
         }
     }
 
+    /**
+     * We need to intentionally override [handleRecordDeclaration] to NOT create the EOG for its
+     * children, e.g., [RecordDeclaration.methods]. The reason for this is that Go only has external
+     * methods declarations, and we do a little cheat by adding the methods both to the namespace of
+     * the current file and to the [RecordDeclaration.methods].
+     *
+     * But, due to this, the original [EvaluationOrderGraphPass] would create the EOG for methods
+     * twice, once for the object in the [RecordDeclaration] and once for the declaration inside the
+     * [NamespaceDeclaration] of the current file.
+     */
+    override fun handleRecordDeclaration(node: RecordDeclaration) {
+        scopeManager.enterScope(node)
+        handleStatementHolder(node)
+        currentPredecessors.clear()
+        scopeManager.leaveScope(node)
+    }
+
     override fun handleFunctionDeclaration(node: FunctionDeclaration) {
         // First, call the regular EOG handler
         super.handleFunctionDeclaration(node)
@@ -88,14 +109,13 @@ class GoEvaluationOrderGraphPass(ctx: TranslationContext) : EvaluationOrderGraph
         // We need to follow the path from the defer statement to all return statements that are
         // reachable from this point.
         for (defer in defers ?: listOf()) {
-            // TODO(oxisto): This is broken! this returns 8192 paths instead of 4
-            /*val paths = defer.followNextEOGEdgesUntilHit { it is ReturnStatement }
+            val paths = defer.followNextEOGEdgesUntilHit { it is ReturnStatement }
             for (path in paths.fulfilled) {
                 // It is a bit philosophical whether the deferred call happens before or after the
                 // return statement in the EOG. For now, it is easier to have it as the last node
                 // AFTER the return statement
                 addEOGEdge(path.last(), defer.input)
-            }*/
+            }
         }
     }
 }
diff --git a/cpg-language-go/src/test/kotlin/de/fraunhofer/aisec/cpg/frontends/golang/StatementTest.kt b/cpg-language-go/src/test/kotlin/de/fraunhofer/aisec/cpg/frontends/golang/StatementTest.kt
@@ -92,10 +92,10 @@ class StatementTest {
         val p = tu.namespaces["p"]
         assertNotNull(p)
 
-        val main = p.functions["main"]
-        assertNotNull(main)
+        val `do` = p.methods["Do"]
+        assertNotNull(`do`)
 
-        val op = main.allChildren<UnaryOperator> { it.name.localName == "defer" }.firstOrNull()
+        val op = `do`.allChildren<UnaryOperator> { it.name.localName == "defer" }.firstOrNull()
         assertNotNull(op)
 
         // The EOG for the defer statement itself should be in the regular EOG path

diff --git a/cpg-language-go/src/test/resources/golang/defer.go b/cpg-language-go/src/test/resources/golang/defer.go
@@ -5,7 +5,9 @@ import (
 	"os"
 )
 
-func main() {
+type MyStruct struct{}
+
+func (s MyStruct) Do() {
 	i := 1
 	do()
 	defer that(i)