diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 70030ee32ac..785475de5cc 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -68,11 +68,19 @@ jobs: id: build env: VERSION: ${{ env.version }} + - name: Prepare report.xml for Codecov + run: | + # this is needed because codecov incorrectly reports lines that have no coverage information (good or bad) as a miss + # See https://github.com/codecov/feedback/issues/564 and https://github.com/Kotlin/kotlinx-kover/issues/699. + # Actually these lines should just not exist in the coverage XML file, since they are only structural elements, such + # as brackets. + cat cpg-all/build/reports/kover/report.xml | grep -v 'mi="0" ci="0" mb="0" cb="0"' > cpg-all/build/reports/kover/report-codecov.xml + rm cpg-all/build/reports/kover/report.xml - name: Upload Code Coverage uses: codecov/codecov-action@v5 with: fail_ci_if_error: true - files: ./cpg-all/build/reports/kover/report.xml + files: ./cpg-all/build/reports/kover/report-codecov.xml token: ${{ secrets.CODECOV_TOKEN }} verbose: true - name: Prepare test and coverage reports 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 8dfbd68a64c..61b0d0f0be7 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 @@ -57,8 +57,8 @@ class TypeManager { MutableMap> = ConcurrentHashMap() - val firstOrderTypes: MutableSet = ConcurrentHashMap.newKeySet() - val secondOrderTypes: MutableSet = ConcurrentHashMap.newKeySet() + val firstOrderTypes = mutableListOf() + val secondOrderTypes = mutableListOf() /** * @param recordDeclaration that is instantiated by a template containing parameterizedtypes @@ -200,26 +200,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 diff --git a/cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/graph/NodeBuilder.kt b/cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/graph/NodeBuilder.kt index ac060fa20ab..5a2e01a2638 100644 --- a/cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/graph/NodeBuilder.kt +++ b/cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/graph/NodeBuilder.kt @@ -299,8 +299,8 @@ fun T.codeAndLocationFromOtherRawNode(rawNode: AstNode?): T * are between the child nodes. * * @param parentNode Used to extract the code for this node. - * @param newLineType The char(s) used to describe a new line, usually either "\n" or "\r\n". This - * is needed because the location block spanning the children usually comprises more than one + * @param lineBreakSequence The char(s) used to describe a new line, usually either "\n" or "\r\n". + * This is needed because the location block spanning the children usually comprises more than one * line. */ context(CodeAndLocationProvider) 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 a0a0fde85a6..10f318283f3 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/inference/DFGFunctionSummaries.kt b/cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/passes/inference/DFGFunctionSummaries.kt index 61e58c87d82..873aa1d873b 100644 --- a/cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/passes/inference/DFGFunctionSummaries.kt +++ b/cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/passes/inference/DFGFunctionSummaries.kt @@ -31,13 +31,10 @@ import com.fasterxml.jackson.dataformat.yaml.YAMLFactory import com.fasterxml.jackson.module.kotlin.readValue import com.fasterxml.jackson.module.kotlin.registerKotlinModule import de.fraunhofer.aisec.cpg.IncompatibleSignature -import de.fraunhofer.aisec.cpg.ancestors +import de.fraunhofer.aisec.cpg.SignatureMatches import de.fraunhofer.aisec.cpg.frontends.CastNotPossible -import de.fraunhofer.aisec.cpg.graph.ContextProvider -import de.fraunhofer.aisec.cpg.graph.LanguageProvider import de.fraunhofer.aisec.cpg.graph.Node import de.fraunhofer.aisec.cpg.graph.declarations.* -import de.fraunhofer.aisec.cpg.graph.objectType import de.fraunhofer.aisec.cpg.graph.parseName import de.fraunhofer.aisec.cpg.graph.types.Type import de.fraunhofer.aisec.cpg.graph.unknownType @@ -117,10 +114,10 @@ class DFGFunctionSummaries { private fun findFunctionDeclarationEntry(functionDecl: FunctionDeclaration): List? { if (functionToDFGEntryMap.isEmpty()) return null - val provider = functionDecl val language = functionDecl.language val languageName = language?.javaClass?.name val methodName = functionDecl.name + val typeManager = functionDecl.ctx?.typeManager ?: return null // The language and the method name have to match. If a signature is specified, it also has // to match to the one of the FunctionDeclaration, null indicates that we accept everything. @@ -131,7 +128,12 @@ class DFGFunctionSummaries { // Split the name if we have a FQN val entryMethodName = language.parseName(it.methodName) val entryRecord = - entryMethodName.parent?.let { provider.lookupType(entryMethodName.parent) } + entryMethodName.parent?.let { + typeManager.lookupResolvedType( + entryMethodName.parent, + language = language + ) + } methodName.lastPartsMatch( entryMethodName.localName ) && // The local name has to match @@ -148,7 +150,10 @@ class DFGFunctionSummaries { (it.signature == null || functionDecl.matchesSignature( it.signature.map { signatureType -> - provider.lookupType(signatureType) + typeManager.lookupResolvedType( + signatureType, + language = language + ) ?: functionDecl.unknownType() } ) != IncompatibleSignature) } else { @@ -184,57 +189,50 @@ class DFGFunctionSummaries { .map { Pair( language.parseName(it.methodName).parent?.let { it1 -> - functionDecl.objectType(it1) - }, + typeManager.lookupResolvedType(it1, language = language) + } ?: language.unknownType(), it ) } - var mostPreciseClassEntries = mutableListOf() - var mostPreciseType = typeEntryList.first().first - var superTypes = mostPreciseType?.ancestors?.map { it.type } ?: setOf() - for (typeEntry in typeEntryList) { - if (typeEntry.first == mostPreciseType) { - mostPreciseClassEntries.add(typeEntry.second) - } else if (typeEntry.first in superTypes) { - mostPreciseClassEntries.clear() - mostPreciseClassEntries.add(typeEntry.second) - mostPreciseType = typeEntry.first - superTypes = mostPreciseType?.ancestors?.map { it.type } ?: setOf() - } - } - val maxSignature = mostPreciseClassEntries.mapNotNull { it.signature?.size }.max() - if (mostPreciseClassEntries.size > 1) { - mostPreciseClassEntries = - mostPreciseClassEntries - .filter { it.signature?.size == maxSignature } - .toMutableList() - } - // Filter parameter types. We start with parameter 0 and continue. Let's hope we remove - // some entries here. - var argIndex = 0 - while (mostPreciseClassEntries.size > 1 && argIndex < maxSignature) { - mostPreciseType = - mostPreciseClassEntries.first().signature?.get(argIndex)?.let { - functionDecl.objectType(it) - } - superTypes = mostPreciseType?.ancestors?.map { it.type } ?: setOf() - val newMostPrecise = mutableListOf() - for (entry in mostPreciseClassEntries) { - val currentType = - entry.signature?.get(argIndex)?.let { functionDecl.objectType(it) } - if (currentType == mostPreciseType) { - newMostPrecise.add(entry) - } else if (currentType in superTypes) { - newMostPrecise.clear() - newMostPrecise.add(entry) - mostPreciseType = currentType - superTypes = mostPreciseType?.ancestors?.map { it.type } ?: setOf() + val uniqueTypes = typeEntryList.map { it.first }.distinct() + val targetType = + language.parseName(functionDecl.name).parent?.let { it1 -> + typeManager.lookupResolvedType(it1, language = language) + } ?: language.unknownType() + + var mostPreciseType = + uniqueTypes + .map { Pair(it, language?.tryCast(targetType, it)) } + .sortedBy { it.second?.depthDistance } + .firstOrNull() + ?.first + + var mostPreciseClassEntries = + typeEntryList.filter { it.first == mostPreciseType }.map { it.second } + + var signatureResults = + mostPreciseClassEntries + .map { + Pair( + it, + functionDecl.matchesSignature( + it.signature?.map { + typeManager.lookupResolvedType(it, language = language) + ?: language.unknownType() + } ?: listOf() + ) + ) } - } - argIndex++ - mostPreciseClassEntries = newMostPrecise - } - functionToDFGEntryMap[mostPreciseClassEntries.first()] + .filter { it.second is SignatureMatches } + .associate { it } + + val rankings = signatureResults.entries.map { Pair(it.value.ranking, it.key) } + + // Find the best (lowest) rank and find functions with the specific rank + val bestRanking = rankings.minBy { it.first }.first + val list = rankings.filter { it.first == bestRanking }.map { it.second } + + functionToDFGEntryMap[list.first()] } else { null } @@ -356,19 +354,4 @@ class DFGFunctionSummaries { val log: Logger = LoggerFactory.getLogger(DFGFunctionSummaries::class.java) } - - fun ContextProvider.lookupType(fqn: CharSequence): Type { - // Try to look up the type from the specified FQN string - var type = - ctx?.typeManager?.lookupResolvedType( - fqn.toString(), - language = (this as? LanguageProvider)?.language - ) - return if (type == null) { - log.warn("Could not find specified type $fqn. Using UnknownType") - this.unknownType() - } else { - type - } - } } diff --git a/cpg-core/src/test/kotlin/de/fraunhofer/aisec/cpg/enhancements/DFGFunctionSummariesTest.kt b/cpg-core/src/test/kotlin/de/fraunhofer/aisec/cpg/enhancements/DFGFunctionSummariesTest.kt index da08592c2f9..68158c61373 100644 --- a/cpg-core/src/test/kotlin/de/fraunhofer/aisec/cpg/enhancements/DFGFunctionSummariesTest.kt +++ b/cpg-core/src/test/kotlin/de/fraunhofer/aisec/cpg/enhancements/DFGFunctionSummariesTest.kt @@ -84,46 +84,48 @@ class DFGFunctionSummariesTest { .build { translationResult { translationUnit("DfgInferredCall.c") { - function("main", t("int")) { - body { - // We need three types with a type hierarchy. - val objectType = t("test.Object") - val listType = t("test.List") - ctx?.let { - val recordDecl = - this@translationUnit.startInference(it) - ?.inferRecordDeclaration( - listType, - ) - listType.recordDeclaration = recordDecl - recordDecl?.addSuperClass(objectType) - listType.superTypes.add(objectType) - } + namespace("test") { + // We need three types with a type hierarchy. + val objectType = t("test.Object") + val listType = t("test.List") + ctx?.let { + val recordDecl = + startInference(it) + ?.inferRecordDeclaration( + listType, + ) + listType.recordDeclaration = recordDecl + recordDecl?.addSuperClass(objectType) + listType.superTypes.add(objectType) + } - val specialListType = t("test.SpecialList") - ctx?.let { - val recordDecl = - this@translationUnit.startInference(it) - ?.inferRecordDeclaration( - specialListType, - ) - specialListType.recordDeclaration = recordDecl - recordDecl?.addSuperClass(listType) - specialListType.superTypes.add(listType) - } + val specialListType = t("test.SpecialList") + ctx?.let { + val recordDecl = + startInference(it) + ?.inferRecordDeclaration( + specialListType, + ) + specialListType.recordDeclaration = recordDecl + recordDecl?.addSuperClass(listType) + specialListType.superTypes.add(listType) + } - val verySpecialListType = t("test.VerySpecialList") - ctx?.let { - val recordDecl = - this@translationUnit.startInference(it) - ?.inferRecordDeclaration( - specialListType, - ) - specialListType.recordDeclaration = recordDecl - recordDecl?.addSuperClass(listType) - specialListType.superTypes.add(listType) - } + val verySpecialListType = t("test.VerySpecialList") + ctx?.let { + val recordDecl = + startInference(it) + ?.inferRecordDeclaration( + verySpecialListType, + ) + verySpecialListType.recordDeclaration = recordDecl + recordDecl?.addSuperClass(specialListType) + verySpecialListType.superTypes.add(listType) + } + } + function("main", t("int")) { + body { memberCall("addAll", construct("test.VerySpecialList")) { literal(1, t("int")) construct("test.Object") 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 04410153f9c..21435dddbf6 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 @@ -300,7 +300,7 @@ fun assertUsageOfMemberAndBase(usingNode: Node?, usedBase: Node?, usedMember: De } fun assertFullName(fqn: String, node: Node?, message: String? = null) { - assertNotNull(node) + assertNotNull(node, message) assertEquals(fqn, node.name.toString(), message) } @@ -317,8 +317,8 @@ fun assertLiteralValue(expected: T, expr: Expression?, message: Strin assertEquals(expected, assertIs>(expr).value, message) } -fun ContextProvider.assertResolvedType(fqn: String, generics: List? = null): Type { +fun ContextProvider.assertResolvedType(fqn: String): Type { var type = - ctx?.typeManager?.lookupResolvedType(fqn, generics, (this as? LanguageProvider)?.language) + ctx?.typeManager?.lookupResolvedType(fqn, language = (this as? LanguageProvider)?.language) 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 4e6243fd573..8592f8d0f7b 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 @@ -61,10 +61,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-language-java/src/main/kotlin/de/fraunhofer/aisec/cpg/passes/JavaExternalTypeHierarchyResolver.kt b/cpg-language-java/src/main/kotlin/de/fraunhofer/aisec/cpg/passes/JavaExternalTypeHierarchyResolver.kt index 84a75a95b9a..1bd43e1d1c5 100644 --- a/cpg-language-java/src/main/kotlin/de/fraunhofer/aisec/cpg/passes/JavaExternalTypeHierarchyResolver.kt +++ b/cpg-language-java/src/main/kotlin/de/fraunhofer/aisec/cpg/passes/JavaExternalTypeHierarchyResolver.kt @@ -25,7 +25,6 @@ */ package de.fraunhofer.aisec.cpg.passes -import com.github.javaparser.resolution.UnsolvedSymbolException import com.github.javaparser.symbolsolver.resolution.typesolvers.CombinedTypeSolver import com.github.javaparser.symbolsolver.resolution.typesolvers.JavaParserTypeSolver import com.github.javaparser.symbolsolver.resolution.typesolvers.ReflectionTypeSolver @@ -71,30 +70,24 @@ class JavaExternalTypeHierarchyResolver(ctx: TranslationContext) : ComponentPass } // Iterate over all known types and add their (direct) supertypes. - for (t in typeManager.firstOrderTypes) { + var types = typeManager.firstOrderTypes.toList() + for (t in types) { val symbol = resolver.tryToSolveType(t.typeName) if (symbol.isSolved) { - try { - val resolvedSuperTypes = symbol.correspondingDeclaration.getAncestors(true) - for (anc in resolvedSuperTypes) { - // We need to try to resolve the type first in order to create weirdly - // scoped types - var superType = typeManager.lookupResolvedType(anc.qualifiedName) + val resolvedSuperTypes = symbol.correspondingDeclaration.getAncestors(true) + for (anc in resolvedSuperTypes) { + // We need to try to resolve the type first in order to create weirdly + // scoped types + var superType = typeManager.lookupResolvedType(anc.qualifiedName) - // Otherwise, we can create this in the global scope - if (superType == null) { - superType = provider.objectType(anc.qualifiedName) - superType.typeOrigin = Type.Origin.RESOLVED - } - - // Add all resolved supertypes to the type. - t.superTypes.add(superType) + // Otherwise, we can create this in the global scope + if (superType == null) { + superType = provider.objectType(anc.qualifiedName) + superType.typeOrigin = Type.Origin.RESOLVED } - } catch (e: UnsolvedSymbolException) { - // Even if the symbol itself is resolved, "getAncestors()" may throw exception. - LOGGER.warn( - "Could not resolve supertypes of ${symbol.correspondingDeclaration}" - ) + + // Add all resolved supertypes to the type. + t.superTypes.add(superType) } } } diff --git a/cpg-language-java/src/test/kotlin/de/fraunhofer/aisec/cpg/graph/types/TypeTests.kt b/cpg-language-java/src/test/kotlin/de/fraunhofer/aisec/cpg/graph/types/TypeTests.kt index 5b608b175b6..07b0c561303 100644 --- a/cpg-language-java/src/test/kotlin/de/fraunhofer/aisec/cpg/graph/types/TypeTests.kt +++ b/cpg-language-java/src/test/kotlin/de/fraunhofer/aisec/cpg/graph/types/TypeTests.kt @@ -111,18 +111,20 @@ internal class TypeTests : BaseTest() { fun testCommonTypeTestJava() { val topLevel = Path.of("src", "test", "resources", "compiling", "hierarchy") val result = analyze("java", topLevel, true) { it.registerLanguage(JavaLanguage()) } - val root = assertNotNull(result.records["multistep.Root"]).toType() - val level0 = assertNotNull(result.records["multistep.Level0"]).toType() - val level1 = assertNotNull(result.records["multistep.Level1"]).toType() - val level1b = assertNotNull(result.records["multistep.Level1B"]).toType() - val level2 = assertNotNull(result.records["multistep.Level2"]).toType() - val unrelated = assertNotNull(result.records["multistep.Unrelated"]).toType() - println( - result.finalCtx.typeManager.firstOrderTypes - .filter { it.typeName == "multistep.Root" } - .map { it.superTypes } - ) - getCommonTypeTestGeneral(root, level0, level1, level1b, level2, unrelated) + with(result) { + val root = assertResolvedType("multistep.Root") + val level0 = assertResolvedType("multistep.Level0") + val level1 = assertResolvedType("multistep.Level1") + val level1b = assertResolvedType("multistep.Level1B") + val level2 = assertResolvedType("multistep.Level2") + val unrelated = assertResolvedType("multistep.Unrelated") + println( + result.finalCtx.typeManager.firstOrderTypes + .filter { it.typeName == "multistep.Root" } + .map { it.superTypes } + ) + getCommonTypeTestGeneral(root, level0, level1, level1b, level2, unrelated) + } } private fun getCommonTypeTestGeneral( @@ -169,7 +171,11 @@ internal class TypeTests : BaseTest() { // Check unrelated type behavior: No common root class for (t in listOf(root, level0, level1, level1b, level2)) { - assertFullName("java.lang.Object", setOf(unrelated, t).commonType) + assertFullName( + "java.lang.Object", + setOf(unrelated, t).commonType, + "${t.typeName} and ${unrelated.typeName} do not have a common type (java.lang.Object) which they should" + ) } } }