Repository: joernio/joern (scala)

Issue:
[CFG][c2cpg?] CFG in try-catch statements is broken for multiple returns
```
> importCode.cpp.fromString("""int main() {
     |   try {
     |     if (1+1) {
     |       return foo();
     |     }
     |     return bar();
     |   } catch (...) {
     |   }
     | }""")
> cpg.call.name("foo").cfgNext.l
val res1: List[io.shiftleft.codepropertygraph.generated.nodes.CfgNode] = List(
  Return(
    argumentIndex = -1,
    argumentName = None,
    code = "return foo();",
    columnNumber = Some(value = 7),
    depthFirstOrder = 14,
    internalFlags = 4,
    lineNumber = Some(value = 4),
    order = 1
  ),
  Call(
    argumentIndex = -1,
    argumentName = None,
    code = "bar()",
    columnNumber = Some(value = 12),
    depthFirstOrder = 12,
    dispatchName = "",
    dispatchType = "STATIC_DISPATCH",
    dynamicTypeHintFullName = IndexedSeq(),
    internalFlags = 4,
    lineNumber = Some(value = 6),
    methodFullName = "<unresolvedNamespace>.bar:<unresolvedSignature>(0)",
    methodInstFullName = None,
    name = "bar",
    order = 1,
    possibleTypes = IndexedSeq(),
    resolved = None,
    signature = "<unresolvedSignature>",
    typeFullName = "ANY"
  )
)
```

There is a CFG edge from the `foo()` call to the `bar()` call, even though it’s impossible for the execution to take that path. A CFG-successor in the catch block would make some sense, but a CFG successor within the same try block doesn’t.

@DavidBakerEffendi 
You did some work w.r.t. returns in try blocks: https://github.com/joernio/joern/blob/master/joern-cli/frontends/x2cpg/src/main/scala/io/joern/x2cpg/passes/controlflow/cfgcreation/CfgCreator.scala#L88
I just want to double check: is is change indented / validated for multiple returns there?
I am unable to find any issue with c2cpg' representation of the try-catch there (its the same as e.g., in javasrc2cpg). 
Do you have any idea whats wrong?

Pull request:
@DavidBakerEffendi You may want to have a look. The tests you added for this case for javasrc2cpg are still working as expected.

Fixes: https://github.com/joernio/joern/issues/5338

Patch:
diff --git a/joern-cli/frontends/x2cpg/src/main/scala/io/joern/x2cpg/passes/controlflow/cfgcreation/CfgCreator.scala b/joern-cli/frontends/x2cpg/src/main/scala/io/joern/x2cpg/passes/controlflow/cfgcreation/CfgCreator.scala
index f59b5776ffd8..f3330da1bc97 100644
--- a/joern-cli/frontends/x2cpg/src/main/scala/io/joern/x2cpg/passes/controlflow/cfgcreation/CfgCreator.scala
+++ b/joern-cli/frontends/x2cpg/src/main/scala/io/joern/x2cpg/passes/controlflow/cfgcreation/CfgCreator.scala
@@ -85,8 +85,12 @@ class CfgCreator(entryNode: Method, diffGraph: DiffGraphBuilder) {
 
   /** Returns true if this node is a child to some `try` control structure, false if otherwise.
     */
-  private def withinATryBlock(x: AstNode): Boolean =
-    x.inAst.isControlStructure.exists(_.controlStructureType == ControlStructureTypes.TRY)
+  private def withinATryBlock(x: AstNode): Boolean = {
+    if (x._astIn.hasNext) {
+      val parentNode = x.parentBlock.astParent
+      parentNode.isControlStructure.isTry.nonEmpty
+    } else false
+  }
 
   /** This method dispatches AST nodes by type and calls corresponding conversion methods.
     */


Test patch:
diff --git a/joern-cli/frontends/c2cpg/src/test/scala/io/joern/c2cpg/passes/cfg/CfgCreationPassTests.scala b/joern-cli/frontends/c2cpg/src/test/scala/io/joern/c2cpg/passes/cfg/CfgCreationPassTests.scala
index 82638bb2415a..6fa20cc5ed1a 100644
--- a/joern-cli/frontends/c2cpg/src/test/scala/io/joern/c2cpg/passes/cfg/CfgCreationPassTests.scala
+++ b/joern-cli/frontends/c2cpg/src/test/scala/io/joern/c2cpg/passes/cfg/CfgCreationPassTests.scala
@@ -597,11 +597,25 @@ class CppCfgCreationPassTests extends CfgTestFixture(() => new CCfgTestCpg(FileD
       succOf("d") should contain theSameElementsAs expected(("RET", AlwaysEdge))
     }
 
+    "be correct for try with multiple returns" in {
+      implicit val cpg: Cpg = code("""
+          |try {
+          |  if (1+1) {
+          |    return foo();
+          |  }
+          |  return bar();
+          |} catch (int x) {}
+          |""".stripMargin)
+      succOf("foo()") should contain theSameElementsAs expected(("return foo();", AlwaysEdge))
+      succOf("return foo();") should contain theSameElementsAs expected(("RET", AlwaysEdge))
+      succOf("bar()") should contain theSameElementsAs expected(("return bar();", AlwaysEdge), ("RET", AlwaysEdge))
+    }
+
     "be correct for throw statement" in {
       implicit val cpg: Cpg = code("""
-                                     |throw foo();
-                                     |bar();
-                                     |""".stripMargin)
+       |throw foo();
+       |bar();
+       |""".stripMargin)
       succOf("func") should contain theSameElementsAs expected(("foo()", AlwaysEdge))
       succOf("foo()") should contain theSameElementsAs expected(("throw foo()", AlwaysEdge))
       succOf("throw foo()") should contain theSameElementsAs expected(("RET", AlwaysEdge))
@@ -610,9 +624,9 @@ class CppCfgCreationPassTests extends CfgTestFixture(() => new CCfgTestCpg(FileD
 
     "be correct for throw statement in if-else" in {
       implicit val cpg: Cpg = code("""
-                                     |if (true) throw foo();
-                                     |else bar();
-                                     |""".stripMargin)
+       |if (true) throw foo();
+       |else bar();
+       |""".stripMargin)
       succOf("func") should contain theSameElementsAs expected(("true", AlwaysEdge))
       succOf("true") should contain theSameElementsAs expected(("foo()", TrueEdge), ("bar()", FalseEdge))
       succOf("foo()") should contain theSameElementsAs expected(("throw foo()", AlwaysEdge))