[CPG] Update CFG creator to follow the new JumpArgument Edge

build.sbt

@@ -2,7 +2,7 @@ name                     := "joern"
 ThisBuild / organization := "io.joern"
 ThisBuild / scalaVersion := "3.7.4"
 
-val cpgVersion = "1.7.64"
+val cpgVersion = "1.7.65"
 
 lazy val joerncli          = Projects.joerncli
 lazy val querydb           = Projects.querydb

joern-cli/frontends/x2cpg/src/main/scala/io/joern/x2cpg/passes/controlflow/cfgcreation/CfgCreator.scala

@@ -5,6 +5,8 @@ import io.shiftleft.codepropertygraph.generated.nodes.*
 import io.shiftleft.codepropertygraph.generated.{ControlStructureTypes, DispatchTypes, EdgeTypes, Operators}
 import io.shiftleft.semanticcpg.language.*
 import io.shiftleft.codepropertygraph.generated.DiffGraphBuilder
+import org.slf4j.LoggerFactory
+import java.util.concurrent.ConcurrentHashMap
 
 /** Translation of abstract syntax trees into control flow graphs
   *
@@ -186,7 +188,13 @@ class CfgCreator(entryNode: Method, diffGraph: DiffGraphBuilder) {
 
   protected def cfgForThrowStatement(node: ControlStructure): Cfg = {
     val throwExprCfg = Iterator(node)
-      .coalesce(_._argumentOut.cast[AstNode], _.astChildren.order(1))
+      .coalesce(
+        _._argumentOut.cast[AstNode],
+        { node =>
+          CfgCreator.warnOnce("Using order fallback for throw statement argument")
+          node.astChildren.order(1)
+        }
+      )
       .headOption
       .map(cfgFor)
       .getOrElse(Cfg.empty)
@@ -200,7 +208,17 @@ class CfgCreator(entryNode: Method, diffGraph: DiffGraphBuilder) {
     * to "jumpsToLabel".
     */
   protected def cfgForBreakStatement(node: ControlStructure): Cfg = {
-    node.astChildren.find(_.order == 1) match {
+    val jumpArgument = Iterator(node)
+      .coalesce(
+        _._jumpArgumentOut.cast[AstNode],
+        { node =>
+          CfgCreator.warnOnce("Using order fallback for break statement jump argument")
+          node.astChildren.order(1)
+        }
+      )
+      .headOption
+
+    jumpArgument match {
       case Some(jumpLabel: JumpLabel) =>
         val labelName = jumpLabel.name
         Cfg(entryNode = Option(node), jumpsToLabel = List((node, labelName)))
@@ -219,7 +237,17 @@ class CfgCreator(entryNode: Method, diffGraph: DiffGraphBuilder) {
   }
 
   protected def cfgForContinueStatement(node: ControlStructure): Cfg = {
-    node.astChildren.find(_.order == 1) match {
+    val jumpArgument = Iterator(node)
+      .coalesce(
+        _._jumpArgumentOut.cast[AstNode],
+        { node =>
+          CfgCreator.warnOnce("Using order fallback for continue statement jump argument")
+          node.astChildren.order(1)
+        }
+      )
+      .headOption
+
+    jumpArgument match {
       case Some(jumpLabel: JumpLabel) =>
         val labelName = jumpLabel.name
         Cfg(entryNode = Option(node), jumpsToLabel = List((node, labelName)))
@@ -380,18 +408,46 @@ class CfgCreator(entryNode: Method, diffGraph: DiffGraphBuilder) {
     val children = node.astChildren.l
     val nLocals  = children.count(_.isLocal)
     val initExprCfg = Iterator(node)
-      .coalesce(_._forInitOut.cast[AstNode], _.astChildren.order(nLocals + 1))
+      .coalesce(
+        _._forInitOut.cast[AstNode],
+        { node =>
+          CfgCreator.warnOnce("Using order fallback for for statement init")
+          node.astChildren.order(nLocals + 1)
+        }
+      )
+      .headOption
+      .map(cfgFor)
+      .getOrElse(Cfg.empty)
+    val conditionCfg = Iterator(node)
+      .coalesce(
+        _._conditionOut.cast[AstNode],
+        { node =>
+          CfgCreator.warnOnce("Using order fallback for for statement condition")
+          node.astChildren.order(nLocals + 2)
+        }
+      )
       .headOption
       .map(cfgFor)
       .getOrElse(Cfg.empty)
-    val conditionCfg = children.find(_.order == nLocals + 2).map(cfgFor).getOrElse(Cfg.empty)
     val loopExprCfg = Iterator(node)
-      .coalesce(_._forUpdateOut.cast[AstNode], _.astChildren.order(nLocals + 3))
+      .coalesce(
+        _._forUpdateOut.cast[AstNode],
+        { node =>
+          CfgCreator.warnOnce("Using order fallback for for statement update")
+          node.astChildren.order(nLocals + 3)
+        }
+      )
       .headOption
       .map(cfgFor)
       .getOrElse(Cfg.empty)
     val bodyCfg = Iterator(node)
-      .coalesce(_._forBodyOut.cast[AstNode], _.astChildren.order(nLocals + 4))
+      .coalesce(
+        _._forBodyOut.cast[AstNode],
+        { node =>
+          CfgCreator.warnOnce("Using order fallback for for statement body")
+          node.astChildren.order(nLocals + 4)
+        }
+      )
       .headOption
       .map(cfgFor)
       .getOrElse(Cfg.empty)
@@ -426,7 +482,13 @@ class CfgCreator(entryNode: Method, diffGraph: DiffGraphBuilder) {
     */
   protected def cfgForDoStatement(node: ControlStructure): Cfg = {
     val bodyCfg = Iterator(node)
-      .coalesce(_._doBodyOut.cast[AstNode], _.astChildren.order(1))
+      .coalesce(
+        _._doBodyOut.cast[AstNode],
+        { node =>
+          CfgCreator.warnOnce("Using order fallback for do-while statement body")
+          node.astChildren.order(1)
+        }
+      )
       .headOption
       .map(cfgFor)
       .getOrElse(Cfg.empty)
@@ -539,7 +601,10 @@ class CfgCreator(entryNode: Method, diffGraph: DiffGraphBuilder) {
     val maybeTryBlock = Iterator(node)
       .coalesce(
         _._tryBodyOut.cast[AstNode].filter(_.astChildren.nonEmpty),
-        _.astChildren.order(1).where(_.astChildren) // Filter out empty `try` bodies
+        { node =>
+          CfgCreator.warnOnce("Using order fallback for try statement body")
+          node.astChildren.order(1).where(_.astChildren)
+        }
       )
       .headOption
 
@@ -552,7 +617,13 @@ class CfgCreator(entryNode: Method, diffGraph: DiffGraphBuilder) {
       else catchControlStructures.iterator
 
     val catchBodyCfgs = Iterator(node)
-      .coalesce(_._catchBodyOut.cast[AstNode], _ => catchBodyFallback)
+      .coalesce(
+        _._catchBodyOut.cast[AstNode],
+        { _ =>
+          CfgCreator.warnOnce("Using order fallback for try statement catch body")
+          catchBodyFallback
+        }
+      )
       .map(cfgFor)
       .toList match {
       case Nil  => List(Cfg.empty)
@@ -568,7 +639,13 @@ class CfgCreator(entryNode: Method, diffGraph: DiffGraphBuilder) {
       }
 
     val maybeFinallyBodyCfg = Iterator(node)
-      .coalesce(_._finallyBodyOut.cast[AstNode], _ => finallyBodyFallback)
+      .coalesce(
+        _._finallyBodyOut.cast[AstNode],
+        { _ =>
+          CfgCreator.warnOnce("Using order fallback for try statement finally body")
+          finallyBodyFallback
+        }
+      )
       .map(cfgFor)
       .headOption // Assume there can only be one
       .toList
@@ -665,6 +742,16 @@ class CfgCreator(entryNode: Method, diffGraph: DiffGraphBuilder) {
 
 object CfgCreator {
 
+  private val logger         = LoggerFactory.getLogger(getClass)
+  private val loggedWarnings = ConcurrentHashMap.newKeySet[String]()
+
+  def warnOnce(msg: String): Unit = {
+    // Adds a point of contention, but log spam would be worse. This should be triggered less and less anyways as frontends get updated.
+    if (loggedWarnings.add(msg)) {
+      logger.warn(msg)
+    }
+  }
+
   implicit class FringeWrapper(fringe: List[(CfgNode, CfgEdgeType)]) {
     def withEdgeType(edgeType: CfgEdgeType): List[(CfgNode, CfgEdgeType)] = {
       fringe.map { case (x, _) => (x, edgeType) }

joern-cli/frontends/x2cpg/src/test/scala/io/joern/x2cpg/passes/CfgCreationControlStructureEdgeTests.scala

@@ -305,5 +305,130 @@ class CfgCreationControlStructureEdgeTests extends AnyWordSpec with Matchers {
 
       method.out(EdgeTypes.CFG).cast[CfgNode].code.toList.shouldBe(List("realThrowExpr"))
     }
+
+    "prefer CONDITION edge over legacy AST order in FOR loops" in {
+      val cpg   = Cpg.empty
+      val graph = cpg.graph
+
+      val method       = graph.addNode(NewMethod().name("testFor").fullName("testFor").signature("void()"))
+      val methodReturn = graph.addNode(NewMethodReturn().typeFullName("void").order(2))
+      val methodBlock  = graph.addNode(NewBlock().code("methodBlock").order(1))
+
+      val forStructure =
+        graph.addNode(NewControlStructure().controlStructureType(ControlStructureTypes.FOR).code("for"))
+      val local = graph.addNode(NewControlStructure().code("local").order(1)) // Local node pushes indices by +1
+
+      // Legacy order would expect condition at `nLocals + 2` -> order(3).
+      // We put condition at order(4) and body call at order(3) so that legacy fallback fails.
+      val condition = graph.addNode(NewLiteral().code("cond").order(4))
+      val bodyCall  = graph.addNode(NewCall().name("bodyCall").code("bodyCall").order(3))
+
+      graph.applyDiff { diffGraphBuilder =>
+        diffGraphBuilder.addEdge(method, methodReturn, EdgeTypes.AST)
+        diffGraphBuilder.addEdge(method, methodBlock, EdgeTypes.AST)
+        diffGraphBuilder.addEdge(methodBlock, forStructure, EdgeTypes.AST)
+
+        diffGraphBuilder.addEdge(forStructure, local, EdgeTypes.AST)
+        diffGraphBuilder.addEdge(forStructure, condition, EdgeTypes.AST)
+        diffGraphBuilder.addEdge(forStructure, bodyCall, EdgeTypes.AST)
+
+        // Use explicit new edges, missing update and init intentionally
+        diffGraphBuilder.addEdge(forStructure, condition, EdgeTypes.CONDITION)
+        diffGraphBuilder.addEdge(forStructure, bodyCall, EdgeTypes.FOR_BODY)
+      }
+
+      new CfgCreationPass(cpg).createAndApply()
+
+      // The condition gets evaluated first, passing condition check goes to body
+      val condCfgNode = method.out(EdgeTypes.CFG).cast[CfgNode].head
+      condCfgNode.code shouldBe "cond"
+      condCfgNode.out(EdgeTypes.CFG).cast[CfgNode].code.filterNot(_ == "<empty>").toList shouldBe List("bodyCall")
+    }
+
+    "prefer JUMP_ARGUMENT edge over legacy AST order in BREAK statements" in {
+      val cpg   = Cpg.empty
+      val graph = cpg.graph
+
+      val method       = graph.addNode(NewMethod().name("testBreak").fullName("testBreak").signature("void()"))
+      val methodReturn = graph.addNode(NewMethodReturn().typeFullName("void").order(2))
+      val methodBlock  = graph.addNode(NewBlock().code("methodBlock").order(1))
+
+      val doStructure = graph.addNode(NewControlStructure().controlStructureType(ControlStructureTypes.DO).code("do"))
+      val doBodyBlock = graph.addNode(NewBlock().code("doBody").order(1))
+      val condition   = graph.addNode(NewLiteral().code("cond").order(2))
+
+      val breakStatement =
+        graph.addNode(NewControlStructure().controlStructureType(ControlStructureTypes.BREAK).code("break").order(1))
+      // Break with 2 levels, but at order(2) instead of the assumed legacy order(1)
+      val breakArg = graph.addNode(NewLiteral().code("2").order(2))
+      val dummyArg = graph.addNode(NewLiteral().code("1").order(1)) // This is the old order fallback
+
+      graph.applyDiff { diffGraphBuilder =>
+        diffGraphBuilder.addEdge(method, methodReturn, EdgeTypes.AST)
+        diffGraphBuilder.addEdge(method, methodBlock, EdgeTypes.AST)
+        diffGraphBuilder.addEdge(methodBlock, doStructure, EdgeTypes.AST)
+
+        diffGraphBuilder.addEdge(doStructure, doBodyBlock, EdgeTypes.AST)
+        diffGraphBuilder.addEdge(doStructure, condition, EdgeTypes.AST)
+        diffGraphBuilder.addEdge(doStructure, doBodyBlock, EdgeTypes.DO_BODY)
+        diffGraphBuilder.addEdge(doStructure, condition, EdgeTypes.CONDITION)
+
+        diffGraphBuilder.addEdge(doBodyBlock, breakStatement, EdgeTypes.AST)
+
+        diffGraphBuilder.addEdge(breakStatement, dummyArg, EdgeTypes.AST)
+        diffGraphBuilder.addEdge(breakStatement, breakArg, EdgeTypes.AST)
+        // Set JUMP_ARGUMENT specifically to breakArg
+        diffGraphBuilder.addEdge(breakStatement, breakArg, EdgeTypes.JUMP_ARGUMENT)
+      }
+
+      new CfgCreationPass(cpg).createAndApply()
+
+      val breakCfgNode = method.out(EdgeTypes.CFG).cast[CfgNode].head
+      breakCfgNode.code shouldBe "break"
+    }
+
+    "prefer JUMP_ARGUMENT edge over legacy AST order in CONTINUE statements" in {
+      val cpg   = Cpg.empty
+      val graph = cpg.graph
+
+      val method       = graph.addNode(NewMethod().name("testContinue").fullName("testContinue").signature("void()"))
+      val methodReturn = graph.addNode(NewMethodReturn().typeFullName("void").order(2))
+      val methodBlock  = graph.addNode(NewBlock().code("methodBlock").order(1))
+
+      val doStructure = graph.addNode(NewControlStructure().controlStructureType(ControlStructureTypes.DO).code("do"))
+      val doBodyBlock = graph.addNode(NewBlock().code("doBody").order(1))
+      val condition   = graph.addNode(NewLiteral().code("cond").order(2))
+
+      val continueStatement = graph.addNode(
+        NewControlStructure().controlStructureType(ControlStructureTypes.CONTINUE).code("continue").order(1)
+      )
+      // Continue with 2 levels, but at order(2) instead of the assumed legacy order(1)
+      val continueArg = graph.addNode(NewLiteral().code("2").order(2))
+      val dummyArg    = graph.addNode(NewLiteral().code("1").order(1)) // This is the old order fallback
+
+      graph.applyDiff { diffGraphBuilder =>
+        diffGraphBuilder.addEdge(method, methodReturn, EdgeTypes.AST)
+        diffGraphBuilder.addEdge(method, methodBlock, EdgeTypes.AST)
+        diffGraphBuilder.addEdge(methodBlock, doStructure, EdgeTypes.AST)
+
+        diffGraphBuilder.addEdge(doStructure, doBodyBlock, EdgeTypes.AST)
+        diffGraphBuilder.addEdge(doStructure, condition, EdgeTypes.AST)
+        diffGraphBuilder.addEdge(doStructure, doBodyBlock, EdgeTypes.DO_BODY)
+        diffGraphBuilder.addEdge(doStructure, condition, EdgeTypes.CONDITION)
+
+        diffGraphBuilder.addEdge(doBodyBlock, continueStatement, EdgeTypes.AST)
+
+        diffGraphBuilder.addEdge(continueStatement, dummyArg, EdgeTypes.AST)
+        diffGraphBuilder.addEdge(continueStatement, continueArg, EdgeTypes.AST)
+        // Set JUMP_ARGUMENT specifically to continueArg
+        diffGraphBuilder.addEdge(continueStatement, continueArg, EdgeTypes.JUMP_ARGUMENT)
+      }
+
+      new CfgCreationPass(cpg).createAndApply()
+
+      val continueCfgNode = method.out(EdgeTypes.CFG).cast[CfgNode].head
+      continueCfgNode.code shouldBe "continue"
+    }
+
   }
 }