Repository: joernio/joern (scala)

Issue:
oldc vs newc Parsing-Difference for struct types
I noticed that the oldc and newc frontend parse the type of identifiers differently if the type is a struct. In particular, the oldc frontend writes `struct ...` while the newc frontend leaves the `struct` keyword from the type. That makes it difficult, e.g., to query if a struct was freed.

```
joern> val code = """
                 |
                 |struct test {
                 |int a;
                 |};
                 |
                 |void free_struct() {
                 | struct test *ptr;
                 | ptr = kzalloc(sizeof(struct test), GFP_KERNEL);
                 | free(ptr);
                 |}
                 | 
                 |void kfree_struct() {
                 | struct test *ptr;
                 | ptr = kzalloc(sizeof(struct test), GFP_KERNEL);
                 | kfree(ptr);
                 |}          
                 |""".stripMargin 


joern> importCode.oldc.fromString(code) 
...

joern> cpg.call("free").argument(1).evalType.l 
res15: List[String] = List("struct test *")

joern> importCode.c.fromString(code) 
...

joern> cpg.call("free").argument(1).evalType.l 
res17: List[String] = List("test*")
```

Probably related to #585

Pull request:
Fixes: https://github.com/joernio/joern/issues/772 and https://github.com/joernio/joern/issues/774

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 61fa9b0aa620..ce5fb7d5e9d8 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
@@ -141,12 +141,20 @@ trait AstCreatorHelper {
   }
 
   @nowarn
-  protected def typeFor(node: IASTNode): String = node match {
-    case _: IASTIdExpression | _: IASTName | _: IASTArrayDeclarator =>
-      cleanType(ASTTypeUtil.getNodeType(node))
-    case _ =>
-      import org.eclipse.cdt.core.dom.ast.ASTSignatureUtil.getNodeSignature
-      cleanType(getNodeSignature(node))
+  protected def typeFor(node: IASTNode): String = {
+    import org.eclipse.cdt.core.dom.ast.ASTSignatureUtil.getNodeSignature
+    node match {
+      case a: IASTArrayDeclarator if ASTTypeUtil.getNodeType(a).startsWith("? ") =>
+        val tpe = cleanType(getNodeSignature(node)).replace("[]", "")
+        val arr = ASTTypeUtil.getNodeType(node).replace("? ", "")
+        s"$tpe$arr"
+      case _: IASTIdExpression | _: IASTName | _: IASTArrayDeclarator =>
+        cleanType(ASTTypeUtil.getNodeType(node))
+      case d: IASTDeclSpecifier =>
+        cleanType(ASTStringUtil.getReturnTypeString(d, null))
+      case _ =>
+        cleanType(getNodeSignature(node))
+    }
   }
 
   private def notHandledText(node: IASTNode): String =
@@ -279,14 +287,14 @@ trait AstCreatorHelper {
   }
 
   private def pointersAsString(spec: IASTDeclSpecifier, parentDecl: IASTDeclarator): String = {
-    val nodeAsString = ASTStringUtil.getReturnTypeString(spec, null)
+    val tpe = typeFor(spec)
     val pointers = parentDecl.getPointerOperators
     val arr = parentDecl match {
       case p: IASTArrayDeclarator => "[]" * p.getArrayModifiers.length
       case _                      => ""
     }
-    if (pointers.isEmpty) { s"$nodeAsString$arr" } else {
-      s"$nodeAsString$arr${"*" * pointers.size}".strip()
+    if (pointers.isEmpty) { s"$tpe$arr" } else {
+      s"$tpe$arr${"*" * pointers.size}".strip()
     }
   }
 
diff --git a/joern-cli/frontends/c2cpg/src/main/scala/io/joern/c2cpg/astcreation/AstForFunctionsCreator.scala b/joern-cli/frontends/c2cpg/src/main/scala/io/joern/c2cpg/astcreation/AstForFunctionsCreator.scala
index d3bda154801f..8922358ec20d 100644
--- a/joern-cli/frontends/c2cpg/src/main/scala/io/joern/c2cpg/astcreation/AstForFunctionsCreator.scala
+++ b/joern-cli/frontends/c2cpg/src/main/scala/io/joern/c2cpg/astcreation/AstForFunctionsCreator.scala
@@ -23,7 +23,8 @@ trait AstForFunctionsCreator {
     val parentNode: NewTypeDecl = methodAstParentStack.collectFirst { case t: NewTypeDecl => t }.getOrElse {
       val astParentType = methodAstParentStack.head.label
       val astParentFullName = methodAstParentStack.head.properties("FULL_NAME").toString
-      val newTypeDeclNode = newTypeDecl(methodName, methodFullName, method.filename, astParentType, astParentFullName)
+      val newTypeDeclNode =
+        newTypeDecl(methodName, methodFullName, method.filename, methodName, astParentType, astParentFullName)
       Ast.storeInDiffGraph(Ast(newTypeDeclNode), diffGraph)
       newTypeDeclNode
     }
diff --git a/joern-cli/frontends/c2cpg/src/main/scala/io/joern/c2cpg/astcreation/AstForTypesCreator.scala b/joern-cli/frontends/c2cpg/src/main/scala/io/joern/c2cpg/astcreation/AstForTypesCreator.scala
index 485a50031c2e..5a4f5d489061 100644
--- a/joern-cli/frontends/c2cpg/src/main/scala/io/joern/c2cpg/astcreation/AstForTypesCreator.scala
+++ b/joern-cli/frontends/c2cpg/src/main/scala/io/joern/c2cpg/astcreation/AstForTypesCreator.scala
@@ -97,11 +97,12 @@ trait AstForTypesCreator {
 
   protected def astForDeclarator(declaration: IASTSimpleDeclaration, declarator: IASTDeclarator, order: Int): Ast = {
     val declTypeName = registerType(typeForDeclSpecifier(declaration.getDeclSpecifier))
+    val tpe = typeFor(declarator)
     val name = declarator.getName.toString
     declaration match {
       case d if isTypeDef(d) =>
         val filename = fileName(declaration)
-        Ast(newTypeDecl(name, registerType(name), filename, alias = Some(declTypeName), order = order))
+        Ast(newTypeDecl(name, registerType(name), filename, s"$tpe $name", alias = Some(declTypeName), order = order))
       case d if parentIsClassDef(d) =>
         Ast(
           NewMember()
@@ -110,20 +111,18 @@ trait AstForTypesCreator {
             .typeFullName(declTypeName)
             .order(order))
       case _ if declarator.isInstanceOf[IASTArrayDeclarator] =>
-        val tpe = registerType(typeFor(declarator))
         val l = NewLocal()
-          .code(name)
+          .code(s"$tpe $name")
           .name(name)
-          .typeFullName(tpe)
+          .typeFullName(registerType(tpe))
           .order(order)
         scope.addToScope(name, (l, tpe))
         Ast(l)
       case _ =>
-        val tpe = registerType(typeFor(declarator))
         val l = NewLocal()
-          .code(name)
+          .code(s"$declTypeName $name")
           .name(name)
-          .typeFullName(tpe)
+          .typeFullName(registerType(tpe))
           .order(order)
         scope.addToScope(name, (l, tpe))
         Ast(l)
@@ -180,13 +179,14 @@ trait AstForTypesCreator {
     val filename = fileName(aliasDeclaration)
 
     val name = aliasDeclaration.getAlias.toString
-    val mappedName = ASTTypeUtil.getType(aliasDeclaration.getMappingTypeId)
+    val mappedName = registerType(typeFor(aliasDeclaration.getMappingTypeId))
     val typeDeclNode =
       newTypeDecl(
         name,
         registerType(name),
         filename,
-        alias = Some(registerType(mappedName)),
+        mappedName,
+        alias = Some(mappedName),
         line = linenumber,
         column = columnnumber,
         order = order
@@ -230,7 +230,7 @@ trait AstForTypesCreator {
           case spec: IASTNamedTypeSpecifier if declaration.getDeclarators.isEmpty =>
             val filename = fileName(spec)
             val name = nodeSignature(spec.getName)
-            Seq(Ast(newTypeDecl(name, registerType(name), filename, alias = Some(name), order = order)))
+            Seq(Ast(newTypeDecl(name, registerType(name), filename, name, alias = Some(name), order = order)))
           case _ if declaration.getDeclarators.nonEmpty =>
             declaration.getDeclarators.toIndexedSeq.map {
               case d: IASTFunctionDeclarator =>
@@ -281,13 +281,13 @@ trait AstForTypesCreator {
                                    decls: List[IASTDeclarator],
                                    order: Int): Seq[Ast] = {
     val filename = fileName(typeSpecifier)
-
     val declAsts = withOrder(decls) { (d, o) =>
       astForDeclarator(typeSpecifier.getParent.asInstanceOf[IASTSimpleDeclaration], d, order + o)
     }
 
     val name = typeSpecifier.getName.toString
-    val fullname = fullName(typeSpecifier)
+    val fullname = registerType(fullName(typeSpecifier))
+    val code = typeFor(typeSpecifier)
     val nameWithTemplateParams = templateParameters(typeSpecifier).map(fullname + _)
 
     val typeDecl = typeSpecifier match {
@@ -295,13 +295,14 @@ trait AstForTypesCreator {
         val baseClassList = cppClass.getBaseSpecifiers.toSeq.map(s => fixQualifiedName(s.getNameSpecifier.toString))
         baseClassList.foreach(registerType)
         newTypeDecl(name,
-                    registerType(fullname),
+                    fullname,
                     filename,
+                    code,
                     inherits = baseClassList,
                     alias = nameWithTemplateParams,
                     order = order)
       case _ =>
-        newTypeDecl(name, registerType(fullname), filename, alias = nameWithTemplateParams, order = order)
+        newTypeDecl(name, fullname, filename, code, alias = nameWithTemplateParams, order = order)
     }
 
     methodAstParentStack.push(typeDecl)
@@ -326,11 +327,11 @@ trait AstForTypesCreator {
     }
 
     val name = typeSpecifier.getName.toString
-    val fullname = fullName(typeSpecifier)
+    val fullname = registerType(fullName(typeSpecifier))
     val nameWithTemplateParams = templateParameters(typeSpecifier).map(fullname + _)
 
     val typeDecl =
-      newTypeDecl(name, registerType(fullname), filename, alias = nameWithTemplateParams, order = order)
+      newTypeDecl(name, fullname, filename, typeFor(typeSpecifier), alias = nameWithTemplateParams, order = order)
 
     Ast(typeDecl) +: declAsts
   }
@@ -364,22 +365,22 @@ trait AstForTypesCreator {
 
   }
 
-  private def astsForEnum(enumSpecifier: IASTEnumerationSpecifier,
+  private def astsForEnum(typeSpecifier: IASTEnumerationSpecifier,
                           decls: List[IASTDeclarator],
                           order: Int): Seq[Ast] = {
-    val filename = fileName(enumSpecifier)
+    val filename = fileName(typeSpecifier)
     val declAsts = withOrder(decls) { (d, o) =>
-      astForDeclarator(enumSpecifier.getParent.asInstanceOf[IASTSimpleDeclaration], d, order + o)
+      astForDeclarator(typeSpecifier.getParent.asInstanceOf[IASTSimpleDeclaration], d, order + o)
     }
 
-    val (name, fullname) = uniqueName("enum", enumSpecifier.getName.toString, fullName(enumSpecifier))
-    val typeDecl = newTypeDecl(name, registerType(fullname), filename, order = order)
+    val (name, fullname) = uniqueName("enum", typeSpecifier.getName.toString, fullName(typeSpecifier))
+    val typeDecl = newTypeDecl(name, registerType(fullname), filename, typeFor(typeSpecifier), order = order)
 
     methodAstParentStack.push(typeDecl)
     scope.pushNewScope(typeDecl)
 
     var currentOrder = 0
-    val member = enumSpecifier.getEnumerators.toIndexedSeq.flatMap { e =>
+    val member = typeSpecifier.getEnumerators.toIndexedSeq.flatMap { e =>
       val eCpg = astsForEnumerator(e, currentOrder)
       currentOrder = eCpg.size + currentOrder
       eCpg
diff --git a/joern-cli/frontends/c2cpg/src/main/scala/io/joern/c2cpg/astcreation/AstNodeBuilder.scala b/joern-cli/frontends/c2cpg/src/main/scala/io/joern/c2cpg/astcreation/AstNodeBuilder.scala
index 7487ea6e4375..6111fee37766 100644
--- a/joern-cli/frontends/c2cpg/src/main/scala/io/joern/c2cpg/astcreation/AstNodeBuilder.scala
+++ b/joern-cli/frontends/c2cpg/src/main/scala/io/joern/c2cpg/astcreation/AstNodeBuilder.scala
@@ -77,6 +77,7 @@ trait AstNodeBuilder {
   protected def newTypeDecl(name: String,
                             fullname: String,
                             filename: String,
+                            code: String,
                             astParentType: String = "",
                             astParentFullName: String = "",
                             order: Int = -1,
@@ -87,6 +88,7 @@ trait AstNodeBuilder {
     NewTypeDecl()
       .name(name)
       .fullName(fullname)
+      .code(code)
       .isExternal(false)
       .filename(filename)
       .astParentType(astParentType)


Test patch:
diff --git a/joern-cli/frontends/c2cpg/src/test/scala/io/joern/c2cpg/EnumTests.scala b/joern-cli/frontends/c2cpg/src/test/scala/io/joern/c2cpg/EnumTests.scala
index 1c437bf395a3..53ca54635b11 100644
--- a/joern-cli/frontends/c2cpg/src/test/scala/io/joern/c2cpg/EnumTests.scala
+++ b/joern-cli/frontends/c2cpg/src/test/scala/io/joern/c2cpg/EnumTests.scala
@@ -23,6 +23,7 @@ class EnumTests extends AnyWordSpec with Matchers with Inside with CompleteCpgFi
       inside(cpg.typeDecl.internal.l) {
         case List(color) =>
           color.name shouldBe "color"
+          color.code shouldBe "enum color"
           inside(color.member.l) {
             case List(red, yellow, green, blue) =>
               red.name shouldBe "red"
@@ -48,6 +49,7 @@ class EnumTests extends AnyWordSpec with Matchers with Inside with CompleteCpgFi
       inside(cpg.typeDecl.filter(x => !x.isExternal).l) {
         case List(color, c) =>
           color.name shouldBe "color"
+          color.code shouldBe "enum color"
           color.aliasTypeFullName shouldBe None
           c.name shouldBe "C"
           c.aliasTypeFullName shouldBe Some("color")
@@ -74,6 +76,7 @@ class EnumTests extends AnyWordSpec with Matchers with Inside with CompleteCpgFi
       inside(cpg.typeDecl.internal.l) {
         case List(altitude) =>
           altitude.name shouldBe "altitude"
+          altitude.code shouldBe "enum altitude"
           inside(altitude.member.l) {
             case List(high, low) =>
               high.name shouldBe "high"
@@ -99,6 +102,7 @@ class EnumTests extends AnyWordSpec with Matchers with Inside with CompleteCpgFi
       inside(cpg.typeDecl.internal.l) {
         case List(smallenum) =>
           smallenum.name shouldBe "smallenum"
+          smallenum.code shouldBe "enum smallenum"
           inside(smallenum.member.l) {
             case List(a, b, c) =>
               a.name shouldBe "a"
@@ -121,6 +125,7 @@ class EnumTests extends AnyWordSpec with Matchers with Inside with CompleteCpgFi
       inside(cpg.typeDecl.internal.l) {
         case List(anon) =>
           anon.name shouldBe "anonymous_enum_0"
+          anon.code shouldBe "enum"
           inside(anon.member.l) {
             case List(d, e, f) =>
               d.name shouldBe "d"
@@ -141,6 +146,7 @@ class EnumTests extends AnyWordSpec with Matchers with Inside with CompleteCpgFi
       inside(cpg.typeDecl.internal.l) {
         case List(x) =>
           x.name shouldBe "X"
+          x.code shouldBe "enum X"
           inside(x.member.l) {
             case List(a, b) =>
               a.name shouldBe "a"
diff --git a/joern-cli/frontends/c2cpg/src/test/scala/io/joern/c2cpg/TemplateTests.scala b/joern-cli/frontends/c2cpg/src/test/scala/io/joern/c2cpg/TemplateTests.scala
index b5ba6a3b0178..3f3a597f7bfd 100644
--- a/joern-cli/frontends/c2cpg/src/test/scala/io/joern/c2cpg/TemplateTests.scala
+++ b/joern-cli/frontends/c2cpg/src/test/scala/io/joern/c2cpg/TemplateTests.scala
@@ -29,7 +29,7 @@ class TemplateTests extends AnyWordSpec with Matchers with Inside with CompleteC
           a.aliasTypeFullName shouldBe Some("X<int>")
           b.name shouldBe "B"
           b.fullName shouldBe "B"
-          b.aliasTypeFullName shouldBe Some("Y<int,char>")
+          b.aliasTypeFullName shouldBe Some("Y<int, char>")
       }
     }
 
diff --git a/joern-cli/frontends/c2cpg/src/test/scala/io/joern/c2cpg/passes/AstCreationPassTests.scala b/joern-cli/frontends/c2cpg/src/test/scala/io/joern/c2cpg/passes/AstCreationPassTests.scala
index c3bf6cbffdf0..886edc59b651 100644
--- a/joern-cli/frontends/c2cpg/src/test/scala/io/joern/c2cpg/passes/AstCreationPassTests.scala
+++ b/joern-cli/frontends/c2cpg/src/test/scala/io/joern/c2cpg/passes/AstCreationPassTests.scala
@@ -755,15 +755,17 @@ class AstCreationPassTests
         case List(forStmt) =>
           forStmt.controlStructureType shouldBe ControlStructureTypes.FOR
           inside(forStmt.astChildren.order(1).l) {
-            case List(ident) =>
+            case List(ident: Identifier) =>
               ident.code shouldBe "list"
           }
           inside(forStmt.astChildren.order(2).l) {
-            case List(ident) =>
-              ident.code shouldBe "x"
+            case List(x: Local) =>
+              x.name shouldBe "x"
+              x.typeFullName shouldBe "int"
+              x.code shouldBe "int x"
           }
           inside(forStmt.astChildren.order(3).l) {
-            case List(block) =>
+            case List(block: Block) =>
               block.astChildren.isCall.code.l shouldBe List("z = x")
           }
       }
@@ -1469,7 +1471,7 @@ class AstCreationPassTests
         case List(buf: Local) =>
           buf.typeFullName shouldBe "char[256]"
           buf.name shouldBe "buf"
-          buf.code shouldBe "buf"
+          buf.code shouldBe "char[256] buf"
       }
     }
 
diff --git a/joern-cli/frontends/c2cpg/src/test/scala/io/joern/c2cpg/passes/TypeNodePassTests.scala b/joern-cli/frontends/c2cpg/src/test/scala/io/joern/c2cpg/passes/TypeNodePassTests.scala
index abaf824addda..05e896c8f8f7 100644
--- a/joern-cli/frontends/c2cpg/src/test/scala/io/joern/c2cpg/passes/TypeNodePassTests.scala
+++ b/joern-cli/frontends/c2cpg/src/test/scala/io/joern/c2cpg/passes/TypeNodePassTests.scala
@@ -32,6 +32,35 @@ class TypeNodePassTests extends AnyWordSpec with Matchers with Inside with CpgTy
           }
       }
     }
+
+    "create correct types for structs" in CpgTypeNodeFixture("""
+        |struct test {
+        |  int a;
+        |};
+        |
+        |void free_struct() {
+        |  struct test *ptr;
+        |  ptr = kzalloc(sizeof(struct test), GFP_KERNEL);
+        |  free(ptr);
+        |}
+        |""".stripMargin) { cpg =>
+      inside(cpg.call("free").argument(1).l) {
+        case List(arg) =>
+          arg.evalType.l shouldBe List("test*")
+          arg.code shouldBe "ptr"
+      }
+    }
+
+    "create correct types for arrays" in CpgTypeNodeFixture("""
+        |void bad1(size_t a) {
+        |  uint8_t src[1], dst[1];
+        |  memcpy(dst, src, a);
+        |}
+        |""".stripMargin) { cpg =>
+      inside(cpg.call("memcpy").argument(1).evalType.l) {
+        case List(tpe) => tpe shouldBe "uint8_t[1]"
+      }
+    }
   }
 
 }
diff --git a/joern-cli/frontends/c2cpg/src/test/scala/io/joern/c2cpg/querying/LocalsTests.scala b/joern-cli/frontends/c2cpg/src/test/scala/io/joern/c2cpg/querying/LocalsTests.scala
index 2c884b44f52d..a0159ded70d6 100644
--- a/joern-cli/frontends/c2cpg/src/test/scala/io/joern/c2cpg/querying/LocalsTests.scala
+++ b/joern-cli/frontends/c2cpg/src/test/scala/io/joern/c2cpg/querying/LocalsTests.scala
@@ -41,8 +41,15 @@ class LocalsTests extends CCodeToCpgSuite {
   }
 
   "should prove correct (name, type) pairs for locals" in {
-    cpg.method.name("free_list").local.map(l => (l.name, l.typeFullName)).toSet shouldBe
-      Set(("q", "node*"), ("p", "node*"))
+    inside(cpg.method.name("free_list").local.l) {
+      case List(q, p) =>
+        q.name shouldBe "q"
+        q.typeFullName shouldBe "node*"
+        q.code shouldBe "struct node* q"
+        p.name shouldBe "p"
+        p.typeFullName shouldBe "node*"
+        p.code shouldBe "struct node* p"
+    }
   }
 
   "should allow finding filenames by local regex" in {
diff --git a/joern-cli/frontends/c2cpg/src/test/scala/io/joern/c2cpg/standard/TypeDeclTests.scala b/joern-cli/frontends/c2cpg/src/test/scala/io/joern/c2cpg/standard/TypeDeclTests.scala
index fb52c6a7b11f..5b02ba88ce8f 100644
--- a/joern-cli/frontends/c2cpg/src/test/scala/io/joern/c2cpg/standard/TypeDeclTests.scala
+++ b/joern-cli/frontends/c2cpg/src/test/scala/io/joern/c2cpg/standard/TypeDeclTests.scala
@@ -35,6 +35,7 @@ class TypeDeclTests extends CCodeToCpgSuite(FileDefaults.CPP_EXT) {
     x.isExternal shouldBe false
     x.inheritsFromTypeFullName shouldBe List()
     x.aliasTypeFullName shouldBe Some("int")
+    x.code shouldBe "typedef int mytype"
     x.order shouldBe 2
     x.filename.startsWith("/") shouldBe true
     x.filename.endsWith(FileDefaults.CPP_EXT) shouldBe true