From 4f5828a03dd7ca611e6db2e8ec48d429963dd48a Mon Sep 17 00:00:00 2001 From: Christian Banse Date: Thu, 3 Oct 2024 01:08:30 +0200 Subject: [PATCH] Create one `Type` object per type reference Fixes #1768 Fixes #1770 --- .../de/fraunhofer/aisec/cpg/ScopeManager.kt | 25 +++++++ .../de/fraunhofer/aisec/cpg/TypeManager.kt | 50 ++++--------- .../fraunhofer/aisec/cpg/graph/TypeBuilder.kt | 38 +++------- .../ResolveCallExpressionAmbiguityPass.kt | 75 ++++++++----------- .../de/fraunhofer/aisec/cpg/test/TestUtils.kt | 4 +- .../frontends/cxx/CXXLanguageFrontendTest.kt | 5 +- .../aisec/cpg_vis_neo4j/Application.kt | 4 +- 7 files changed, 86 insertions(+), 115 deletions(-) diff --git a/cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/ScopeManager.kt b/cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/ScopeManager.kt index f45809b3a5..125fcf12b2 100644 --- a/cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/ScopeManager.kt +++ b/cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/ScopeManager.kt @@ -31,6 +31,7 @@ import de.fraunhofer.aisec.cpg.graph.declarations.* import de.fraunhofer.aisec.cpg.graph.scopes.* import de.fraunhofer.aisec.cpg.graph.statements.* import de.fraunhofer.aisec.cpg.graph.statements.expressions.* +import de.fraunhofer.aisec.cpg.graph.types.DeclaresType import de.fraunhofer.aisec.cpg.graph.types.FunctionPointerType import de.fraunhofer.aisec.cpg.graph.types.IncompleteType import de.fraunhofer.aisec.cpg.graph.types.Type @@ -1009,6 +1010,30 @@ class ScopeManager : ScopeProvider { return list } + + /** + * This function tries to look up the symbol contained in [name] (using [findSymbols]) and + * returns a [DeclaresType] node, if this name resolved to something which declares a type. + * + * It is important to know that the lookup needs to be unique, so if multiple declarations match + * this symbol, a warning is triggered and null is returned. + */ + fun lookupUniqueTypeSymbolByName(name: Name, startScope: Scope?): DeclaresType? { + var symbols = + lookupSymbolByName(name = name, startScope = startScope) { it is DeclaresType } + .filterIsInstance() + + // We need to have a single match, otherwise we have an ambiguous type, and we cannot + // normalize it. + if (symbols.size > 1) { + LOGGER.warn( + "Lookup of type {} returned more than one symbol which declares a type, this is an ambiguity and the following analysis might not be correct.", + name + ) + } + + return symbols.singleOrNull() + } } /** diff --git a/cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/TypeManager.kt b/cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/TypeManager.kt index 8745e7944d..a7ab6a4bc4 100644 --- a/cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/TypeManager.kt +++ b/cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/TypeManager.kt @@ -28,11 +28,14 @@ package de.fraunhofer.aisec.cpg import de.fraunhofer.aisec.cpg.frontends.CastNotPossible import de.fraunhofer.aisec.cpg.frontends.CastResult import de.fraunhofer.aisec.cpg.frontends.Language +import de.fraunhofer.aisec.cpg.graph.Name import de.fraunhofer.aisec.cpg.graph.declarations.RecordDeclaration import de.fraunhofer.aisec.cpg.graph.declarations.TemplateDeclaration +import de.fraunhofer.aisec.cpg.graph.parseName import de.fraunhofer.aisec.cpg.graph.scopes.Scope import de.fraunhofer.aisec.cpg.graph.scopes.TemplateScope import de.fraunhofer.aisec.cpg.graph.types.* +import de.fraunhofer.aisec.cpg.passes.TypeResolver import java.util.* import java.util.concurrent.ConcurrentHashMap import org.slf4j.Logger @@ -54,8 +57,14 @@ class TypeManager { MutableMap> = ConcurrentHashMap() - val firstOrderTypes: MutableSet = ConcurrentHashMap.newKeySet() - val secondOrderTypes: MutableSet = ConcurrentHashMap.newKeySet() + val firstOrderTypes = mutableListOf() + val secondOrderTypes = mutableListOf() + + /** + * A map of declared types by their name. Useful to check for the existence of a declared type + * by its fully qualified name across all scopes. + */ + @PopulatedByPass(TypeResolver::class) val declaredTypes = mutableMapOf() /** * @param recordDeclaration that is instantiated by a template containing parameterizedtypes @@ -197,26 +206,9 @@ class TypeManager { } if (t.isFirstOrderType) { - // Make sure we only ever return one unique object per type - if (!firstOrderTypes.add(t)) { - return firstOrderTypes.first { it == t && it is T } as T - } else { - log.trace( - "Registering unique first order type {}{}", - t.name, - if ((t as? ObjectType)?.generics?.isNotEmpty() == true) { - " with generics ${t.generics.joinToString(",", "[", "]") { it.name.toString() }}" - } else { - "" - } - ) - } + synchronized(firstOrderTypes) { firstOrderTypes.add(t) } } else if (t is SecondOrderType) { - if (!secondOrderTypes.add(t)) { - return secondOrderTypes.first { it == t && it is T } as T - } else { - log.trace("Registering unique second order type {}", t.name) - } + synchronized(secondOrderTypes) { secondOrderTypes.add(t) } } return t @@ -237,25 +229,13 @@ class TypeManager { * This function returns the first (there should be only one) [Type] with the given [fqn] that * is [Type.Origin.RESOLVED]. */ - fun lookupResolvedType( - fqn: CharSequence, - generics: List? = null, - language: Language<*>? = null - ): Type? { + fun lookupResolvedType(fqn: CharSequence, language: Language<*>? = null): Type? { var primitiveType = language?.getSimpleTypeOf(fqn) if (primitiveType != null) { return primitiveType } - return firstOrderTypes.firstOrNull { - (it.typeOrigin == Type.Origin.RESOLVED || it.typeOrigin == Type.Origin.GUESSED) && - it.root.name == fqn && - if (generics != null) { - (it as? ObjectType)?.generics == generics - } else { - true - } - } + return declaredTypes[language.parseName(fqn)] } } diff --git a/cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/graph/TypeBuilder.kt b/cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/graph/TypeBuilder.kt index a0a0fde85a..10f318283f 100644 --- a/cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/graph/TypeBuilder.kt +++ b/cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/graph/TypeBuilder.kt @@ -113,34 +113,16 @@ fun LanguageProvider.objectType( "Could not create type: translation context not available" ) - val scope = c.scopeManager.currentScope - - synchronized(c.typeManager.firstOrderTypes) { - // We can try to look up the type by its name and return it, if it already exists. - var type = - c.typeManager.firstOrderTypes.firstOrNull { - it is ObjectType && - it.name == name && - it.scope == scope && - it.generics == generics && - it.language == language - } - if (type != null) { - return type - } - - // Otherwise, we either need to create the type because of the generics or because we do not - // know the type yet. - type = ObjectType(name, generics, false, language) - // Apply our usual metadata, such as scope, code, location, if we have any. Make sure only - // to refer by the local name because we will treat types as sort of references when - // creating them and resolve them later. - type.applyMetadata(this, name, rawNode = rawNode, localNameOnly = true) - - // Piping it through register type will ensure that in any case we return the one unique - // type object (per scope) for it. - return c.typeManager.registerType(type) - } + // Otherwise, we either need to create the type because of the generics or because we do not + // know the type yet. + var type = ObjectType(name, generics, false, language) + // Apply our usual metadata, such as scope, code, location, if we have any. Make sure only + // to refer by the local name because we will treat types as sort of references when + // creating them and resolve them later. + type.applyMetadata(this, name, rawNode = rawNode, localNameOnly = true) + + // Piping it through register type will ensure that we know the type and can resolve it later + return c.typeManager.registerType(type) } /** diff --git a/cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/passes/ResolveCallExpressionAmbiguityPass.kt b/cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/passes/ResolveCallExpressionAmbiguityPass.kt index 42f5ec7c8b..de1401854d 100644 --- a/cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/passes/ResolveCallExpressionAmbiguityPass.kt +++ b/cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/passes/ResolveCallExpressionAmbiguityPass.kt @@ -85,27 +85,17 @@ class ResolveCallExpressionAmbiguityPass(ctx: TranslationContext) : TranslationU var callee = call.callee val language = callee.language + // We can skip all callees that are not references + if (callee !is Reference) { + return + } + // Check, if this is cast is really a construct expression (if the language supports // functional-constructs) if (language is HasFunctionStyleConstruction) { - // Make sure, we do not accidentally "construct" primitive types - if (language.builtInTypes.contains(callee.name.toString()) == true) { - return - } - - val fqn = - if (callee.name.parent == null) { - scopeManager.currentNamespace.fqn( - callee.name.localName, - delimiter = callee.name.delimiter - ) - } else { - callee.name - } - // Check for our type. We are only interested in object types - val type = typeManager.lookupResolvedType(fqn) - if (type is ObjectType) { + var type = lookupPotentialTypeFromReference(callee) + if (type is ObjectType && !type.isPrimitive) { walker.replaceCallWithConstruct(type, parent, call) } } @@ -114,36 +104,33 @@ class ResolveCallExpressionAmbiguityPass(ctx: TranslationContext) : TranslationU // cast expression. And this is only really necessary, if the function call has a single // argument. if (language is HasFunctionStyleCasts && call.arguments.size == 1) { - var pointer = false - // If the argument is a UnaryOperator, unwrap them - if (callee is UnaryOperator && callee.operatorCode == "*") { - pointer = true - callee = callee.input + // Check if it is type and replace the call + var type = lookupPotentialTypeFromReference(callee) + if (type != null) { + walker.replaceCallWithCast(type, parent, call, false) } + } + } - // First, check if this is a built-in type - var builtInType = language.getSimpleTypeOf(callee.name) - if (builtInType != null) { - walker.replaceCallWithCast(builtInType, parent, call, false) - } else { - // If not, then this could still refer to an existing type. We need to make sure - // that we take the current namespace into account - val fqn = - if (callee.name.parent == null) { - scopeManager.currentNamespace.fqn( - callee.name.localName, - delimiter = callee.name.delimiter - ) - } else { - callee.name - } - - val type = typeManager.lookupResolvedType(fqn) - if (type != null) { - walker.replaceCallWithCast(type, parent, call, pointer) - } - } + /** This function checks whether our [Reference] refers to a [Type]. */ + private fun lookupPotentialTypeFromReference(ref: Reference): Type? { + var name = ref.name + var scope = ref.scope + + // First, check if it is a simple type + var type = ref.language?.getSimpleTypeOf(name) + if (type != null) { + return type } + + // This could also be a typedef + type = scopeManager.typedefFor(name, scope) + if (type != null) { + return type + } + + // Lastly, check if the reference contains a symbol that points to type (declaration) + return scopeManager.lookupUniqueTypeSymbolByName(name, scope)?.declaredType } override fun cleanup() { diff --git a/cpg-core/src/testFixtures/kotlin/de/fraunhofer/aisec/cpg/test/TestUtils.kt b/cpg-core/src/testFixtures/kotlin/de/fraunhofer/aisec/cpg/test/TestUtils.kt index 8be47cdcd3..fd0c47fa9c 100644 --- a/cpg-core/src/testFixtures/kotlin/de/fraunhofer/aisec/cpg/test/TestUtils.kt +++ b/cpg-core/src/testFixtures/kotlin/de/fraunhofer/aisec/cpg/test/TestUtils.kt @@ -318,7 +318,7 @@ fun assertLiteralValue(expected: T, expr: Expression?, message: Strin assertEquals(expected, assertIs>(expr).value, message) } -fun ContextProvider.assertResolvedType(fqn: String, generics: List? = null): Type { - var type = ctx?.typeManager?.lookupResolvedType(fqn, generics) +fun ContextProvider.assertResolvedType(fqn: String): Type { + var type = ctx?.typeManager?.lookupResolvedType(fqn) return assertNotNull(type) } diff --git a/cpg-language-cxx/src/test/kotlin/de/fraunhofer/aisec/cpg/frontends/cxx/CXXLanguageFrontendTest.kt b/cpg-language-cxx/src/test/kotlin/de/fraunhofer/aisec/cpg/frontends/cxx/CXXLanguageFrontendTest.kt index 3e1b661e95..085febc7a4 100644 --- a/cpg-language-cxx/src/test/kotlin/de/fraunhofer/aisec/cpg/frontends/cxx/CXXLanguageFrontendTest.kt +++ b/cpg-language-cxx/src/test/kotlin/de/fraunhofer/aisec/cpg/frontends/cxx/CXXLanguageFrontendTest.kt @@ -62,10 +62,7 @@ internal class CXXLanguageFrontendTest : BaseTest() { val decl = main val ls = decl.variables["ls"] assertNotNull(ls) - assertEquals( - assertResolvedType("std::vector", listOf(assertResolvedType("int"))), - ls.type - ) + assertEquals(assertResolvedType("std::vector"), ls.type) assertLocalName("ls", ls) val forEachStatement = decl.forEachLoops.firstOrNull() diff --git a/cpg-neo4j/src/main/kotlin/de/fraunhofer/aisec/cpg_vis_neo4j/Application.kt b/cpg-neo4j/src/main/kotlin/de/fraunhofer/aisec/cpg_vis_neo4j/Application.kt index 79c60675a8..d98529132c 100644 --- a/cpg-neo4j/src/main/kotlin/de/fraunhofer/aisec/cpg_vis_neo4j/Application.kt +++ b/cpg-neo4j/src/main/kotlin/de/fraunhofer/aisec/cpg_vis_neo4j/Application.kt @@ -519,8 +519,8 @@ class Application : Callable { if (!noDefaultPasses) { translationConfiguration.defaultPasses() - translationConfiguration.registerPass() - translationConfiguration.registerPass() + // translationConfiguration.registerPass() + // translationConfiguration.registerPass() } if (customPasses != "DEFAULT") { val pieces = customPasses.split(",")