Repository: joernio/joern (scala)

Issue:
Incorrect CFG for for-stmt
Hi all,
the control flow generated for the for-stmt in the following snippet is incorrect:

```
void foo() {
  for (int x=1,y=1; x; --x) {}
}
```

The for-stmt has 7 children (2 locals, 2 assignments, condition, loop expr, body), which makes sense to me. However, the `CfgCreator` skips the two locals and takes the third child (first assignment) as init expr, the fourth child (second assignment) as condition, and so on.

Alwin

Pull request:
Fixes: https://github.com/joernio/joern/issues/1093

Patch:
diff --git a/joern-cli/frontends/c2cpg/src/main/scala/io/joern/c2cpg/astcreation/AstForStatementsCreator.scala b/joern-cli/frontends/c2cpg/src/main/scala/io/joern/c2cpg/astcreation/AstForStatementsCreator.scala
index e20457bda327..9c57ffcc4fc1 100644
--- a/joern-cli/frontends/c2cpg/src/main/scala/io/joern/c2cpg/astcreation/AstForStatementsCreator.scala
+++ b/joern-cli/frontends/c2cpg/src/main/scala/io/joern/c2cpg/astcreation/AstForStatementsCreator.scala
@@ -186,15 +186,22 @@ trait AstForStatementsCreator {
     val code    = s"for ($codeInit$codeCond;$codeIter)"
     val forNode = newControlStructureNode(forStmt, ControlStructureTypes.FOR, code, order)
 
-    val initAsts = nullSafeAst(forStmt.getInitializerStatement, 1)
+    val initAstBlock = NewBlock()
+      .order(1)
+      .argumentIndex(1)
+      .typeFullName(registerType(Defines.voidTypeName))
+      .lineNumber(line(forStmt))
+      .columnNumber(column(forStmt))
+    scope.pushNewScope(initAstBlock)
+    val initAst = Ast(initAstBlock).withChildren(nullSafeAst(forStmt.getInitializerStatement, 1))
+    scope.popScope()
 
-    val continuedOrder = Math.max(initAsts.size, 1)
-    val compareAst     = nullSafeAst(forStmt.getConditionExpression, continuedOrder + 1)
-    val updateAst      = nullSafeAst(forStmt.getIterationExpression, continuedOrder + 2)
-    val stmtAst        = nullSafeAst(forStmt.getBody, continuedOrder + 3)
+    val compareAst = nullSafeAst(forStmt.getConditionExpression, 2)
+    val updateAst  = nullSafeAst(forStmt.getIterationExpression, 3)
+    val stmtAst    = nullSafeAst(forStmt.getBody, 4)
 
     Ast(forNode)
-      .withChildren(initAsts)
+      .withChild(initAst)
       .withChild(compareAst)
       .withChild(updateAst)
       .withChildren(stmtAst)


Test patch:
diff --git a/joern-cli/frontends/c2cpg/src/test/scala/io/joern/c2cpg/standard/ControlStructureTests.scala b/joern-cli/frontends/c2cpg/src/test/scala/io/joern/c2cpg/standard/ControlStructureTests.scala
index daefbef17a43..628068670b28 100644
--- a/joern-cli/frontends/c2cpg/src/test/scala/io/joern/c2cpg/standard/ControlStructureTests.scala
+++ b/joern-cli/frontends/c2cpg/src/test/scala/io/joern/c2cpg/standard/ControlStructureTests.scala
@@ -2,9 +2,10 @@ package io.joern.c2cpg.standard
 
 import io.joern.c2cpg.parser.FileDefaults
 import io.joern.c2cpg.testfixtures.CCodeToCpgSuite
+import io.shiftleft.codepropertygraph.generated.ControlStructureTypes
 import io.shiftleft.semanticcpg.language._
 
-class ControlStructureTests extends CCodeToCpgSuite(FileDefaults.CPP_EXT) {
+class ControlStructureTest1 extends CCodeToCpgSuite(FileDefaults.CPP_EXT) {
 
   override val code: String =
     """
@@ -77,3 +78,42 @@ class ControlStructureTests extends CCodeToCpgSuite(FileDefaults.CPP_EXT) {
   }
 
 }
+
+class ControlStructureTest2 extends CCodeToCpgSuite(FileDefaults.CPP_EXT) {
+
+  override val code: String =
+    """
+      |void foo() {
+      |  for (int x=1,y=1; x; --x) { bar(); };
+      |}
+      |""".stripMargin
+
+  "should be correct for for-loop with multiple assignments" in {
+    inside(cpg.controlStructure.l) { case List(forLoop) =>
+      forLoop.controlStructureType shouldBe ControlStructureTypes.FOR
+      inside(forLoop.astChildren.order(1).l) { case List(assignmentBlock) =>
+        inside(assignmentBlock.astChildren.l) { case List(localX, localY, assignmentX, assignmentY) =>
+          localX.code shouldBe "int x"
+          localX.order shouldBe 1
+          localY.code shouldBe "int y"
+          localY.order shouldBe 2
+          assignmentX.code shouldBe "x=1"
+          assignmentX.order shouldBe 3
+          assignmentY.code shouldBe "y=1"
+          assignmentY.order shouldBe 4
+        }
+      }
+      inside(forLoop.condition.l) { case List(x) =>
+        x.code shouldBe "x"
+        x.order shouldBe 2
+      }
+      inside(forLoop.astChildren.order(3).l) { case List(updateX) =>
+        updateX.code shouldBe "--x"
+      }
+      inside(forLoop.astChildren.order(4).l) { case List(loopBody) =>
+        loopBody.astChildren.isCall.head.code shouldBe "bar()"
+      }
+    }
+  }
+
+}