Repository: joernio/joern (scala)

Issue:
Inconsistency while parsing C macros
While parsing a C file, joern is doing macro expansion in a condition if the macro is the first argument, but not when it is the second,
```c
#define FLAG_A 1

int func(int x) {
    if (x & FLAG_A) {
        return 0;
    }
    else if (FLAG_A & x) {
        return 1;
    }
}
```
```
joern> cpg.call.name(Operators.and).code.p
res1: List[String] = List("x & FLAG_A", "1 & x")
```
imo, macro should not be expanded

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

Patch:
diff --git a/joern-cli/frontends/c2cpg/src/main/scala/io/joern/c2cpg/astcreation/AstCreatorHelper.scala b/joern-cli/frontends/c2cpg/src/main/scala/io/joern/c2cpg/astcreation/AstCreatorHelper.scala
index be00a3ccb154..69450931998c 100644
--- a/joern-cli/frontends/c2cpg/src/main/scala/io/joern/c2cpg/astcreation/AstCreatorHelper.scala
+++ b/joern-cli/frontends/c2cpg/src/main/scala/io/joern/c2cpg/astcreation/AstCreatorHelper.scala
@@ -413,12 +413,9 @@ trait AstCreatorHelper { this: AstCreator =>
 
   protected def astForNode(node: IASTNode): Ast = {
     node match {
-      case id: IASTIdExpression if id.getName.isInstanceOf[CPPASTQualifiedName] =>
-        astForQualifiedName(id.getName.asInstanceOf[CPPASTQualifiedName])
-      case id: IASTIdExpression             => astForIdentifier(id)
+      case expr: IASTExpression             => astForExpression(expr)
       case name: IASTName                   => astForIdentifier(name)
       case decl: IASTDeclSpecifier          => astForIdentifier(decl)
-      case expr: IASTExpression             => astForExpression(expr)
       case l: IASTInitializerList           => astForInitializerList(l)
       case c: ICPPASTConstructorInitializer => astForCPPASTConstructorInitializer(c)
       case d: ICASTDesignatedInitializer    => astForCASTDesignatedInitializer(d)
diff --git a/joern-cli/frontends/c2cpg/src/main/scala/io/joern/c2cpg/astcreation/MacroHandler.scala b/joern-cli/frontends/c2cpg/src/main/scala/io/joern/c2cpg/astcreation/MacroHandler.scala
index 188c69c80d6d..7cb5180f4fa2 100644
--- a/joern-cli/frontends/c2cpg/src/main/scala/io/joern/c2cpg/astcreation/MacroHandler.scala
+++ b/joern-cli/frontends/c2cpg/src/main/scala/io/joern/c2cpg/astcreation/MacroHandler.scala
@@ -5,6 +5,8 @@ import io.shiftleft.codepropertygraph.generated.nodes.{AstNodeNew, ExpressionNew
 import io.joern.x2cpg.Ast
 import org.apache.commons.lang.StringUtils
 import org.eclipse.cdt.core.dom.ast.{IASTMacroExpansionLocation, IASTNode, IASTPreprocessorMacroDefinition}
+import org.eclipse.cdt.core.dom.ast.IASTBinaryExpression
+import org.eclipse.cdt.core.dom.ast.IASTFileLocation
 import org.eclipse.cdt.internal.core.model.ASTStringUtil
 import org.eclipse.cdt.internal.core.parser.scanner.MacroArgumentExtractor
 
@@ -149,11 +151,17 @@ trait MacroHandler { this: AstCreator =>
   private def isExpandedFromMacro(node: IASTNode): Boolean = expandedFromMacro(node).nonEmpty
 
   private def expandedFromMacro(node: IASTNode): Option[IASTMacroExpansionLocation] = {
-    val locations = node.getNodeLocations
-    if (locations.nonEmpty) {
-      node.getNodeLocations.headOption.collect { case x: IASTMacroExpansionLocation => x }
-    } else {
-      None
+    val locations = node.getNodeLocations.toList
+    val locationsSorted = node match {
+      // For binary expressions the expansion locations may occur in any order.
+      // We manually sort them here to ignore this.
+      // TODO: This may also happen with other expressions that allow for multiple sub elements.
+      case _: IASTBinaryExpression => locations.sortBy(_.isInstanceOf[IASTMacroExpansionLocation])
+      case _                       => locations
+    }
+    locationsSorted match {
+      case (head: IASTMacroExpansionLocation) :: _ => Option(head)
+      case _                                       => None
     }
   }
 


Test patch:
diff --git a/joern-cli/frontends/c2cpg/src/test/scala/io/joern/c2cpg/macros/MacroHandlingTests.scala b/joern-cli/frontends/c2cpg/src/test/scala/io/joern/c2cpg/macros/MacroHandlingTests.scala
index 405d74a96931..ab3d3b79a3b1 100644
--- a/joern-cli/frontends/c2cpg/src/test/scala/io/joern/c2cpg/macros/MacroHandlingTests.scala
+++ b/joern-cli/frontends/c2cpg/src/test/scala/io/joern/c2cpg/macros/MacroHandlingTests.scala
@@ -6,6 +6,8 @@ import io.joern.dataflowengineoss.language._
 import io.shiftleft.codepropertygraph.generated.DispatchTypes
 import io.shiftleft.codepropertygraph.generated.Operators
 import io.shiftleft.codepropertygraph.generated.nodes.Block
+import io.shiftleft.codepropertygraph.generated.nodes.Call
+import io.shiftleft.codepropertygraph.generated.nodes.Identifier
 import io.shiftleft.semanticcpg.language._
 
 class MacroHandlingTests extends CCodeToCpgSuite {
@@ -14,9 +16,9 @@ class MacroHandlingTests extends CCodeToCpgSuite {
     val cpg = code("""
        |#define A_MACRO(x,c) (x = 10 + c)
        |int foo() {
-       |int *y;
-       |A_MACRO(*y, 2);
-       |return 10 * y;
+       |  int *y;
+       |  A_MACRO(*y, 2);
+       |  return 10 * y;
        |}
     """.stripMargin)
 
@@ -224,7 +226,30 @@ class MacroHandlingTests extends CCodeToCpgSuite {
     "should not result in malformed CFGs when expanding a nested macro with block" in {
       cpg.all.collectAll[Block].l.count(b => b.cfgOut.size > 1) shouldBe 0
     }
+  }
 
+  "MacroHandlingTests8" should {
+    val cpg = code("""
+       |#define FLAG_A 1
+       |
+       |int func(int x) {
+       |  if(x & FLAG_A) {
+       |    return 0;
+       |  } else if (FLAG_A & x) {
+       |    return 1;
+       |  }
+       |}""".stripMargin)
+
+    "should expand the macro on both sides of binary operators" in {
+      cpg.call.name(Operators.and).code.l shouldBe List("x & FLAG_A", "FLAG_A & x")
+      val List(andCall1, andCall2)                           = cpg.call.name(Operators.and).l
+      val List(andCall1Arg1: Identifier, andCall1Arg2: Call) = andCall1.argument.l
+      andCall1Arg1.name shouldBe "x"
+      andCall1Arg2.name shouldBe "FLAG_A"
+      val List(andCall2Arg1: Call, andCall2Arg2: Identifier) = andCall2.argument.l
+      andCall2Arg1.name shouldBe "FLAG_A"
+      andCall2Arg2.name shouldBe "x"
+    }
   }
 }
 
diff --git a/joern-cli/frontends/c2cpg/src/test/scala/io/joern/c2cpg/passes/types/StructTypeTests.scala b/joern-cli/frontends/c2cpg/src/test/scala/io/joern/c2cpg/passes/types/StructTypeTests.scala
index fed7e7d38dd5..0a7434b694a1 100644
--- a/joern-cli/frontends/c2cpg/src/test/scala/io/joern/c2cpg/passes/types/StructTypeTests.scala
+++ b/joern-cli/frontends/c2cpg/src/test/scala/io/joern/c2cpg/passes/types/StructTypeTests.scala
@@ -34,7 +34,7 @@ class StructTypeTests extends CCodeToCpgSuite {
       val List(argBInit) = bInitCall.argument.l
       argBInit.code shouldBe "SIZE - 1"
       val List(subtractionCall) = bInitCall.ast.isCall.nameExact(Operators.subtraction).l
-      subtractionCall.code shouldBe "5 - 1"
+      subtractionCall.code shouldBe "SIZE - 1"
 
       cInitCall.code shouldBe "c[10]"
       val List(argCInit) = cInitCall.argument.l