From 01f58737457033e37b54c295158e5669501386f3 Mon Sep 17 00:00:00 2001 From: Tobias Specht Date: Fri, 13 Dec 2024 13:28:17 +0100 Subject: [PATCH] Distinguish between scope does not exist and name not found as scope (#1895) * Distinguish between scope does not exist and name not found in scope * Add test case for inference test of parent class * Handle some explicit null cases --------- Co-authored-by: Christian Banse --- .../de/fraunhofer/aisec/cpg/ScopeManager.kt | 68 ++++++++++++------- .../aisec/cpg/passes/SymbolResolver.kt | 25 +++++-- .../aisec/cpg/passes/inference/PassHelper.kt | 14 ++-- .../cpg/frontends/cxx/CXXInferenceTest.kt | 36 ++++++++-- .../cxx/inference/parent_inference.cpp | 18 +++++ 5 files changed, 120 insertions(+), 41 deletions(-) create mode 100644 cpg-language-cxx/src/test/resources/cxx/inference/parent_inference.cpp 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 e20f32ed04..c14dd1b970 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 @@ -509,7 +509,14 @@ class ScopeManager : ScopeProvider { return pair.second } - var (scope, name) = extractScope(ref, startScope) + val extractedScope = extractScope(ref, startScope) + // If the scope extraction fails, we can only return here directly without any result + if (extractedScope == null) { + return null + } + + var scope = extractedScope.scope + val name = extractedScope.adjustedName if (scope == null) { scope = startScope } @@ -555,13 +562,21 @@ class ScopeManager : ScopeProvider { } /** - * This function extracts a scope for the [Name], e.g. if the name is fully qualified. `null` is - * returned, if no scope can be extracted. + * This class represents the result of the [extractScope] operation. It contains a [scope] + * object, if a scope was found and the [adjustedName] that is normalized if any aliases were + * found during scope extraction. + */ + data class ScopeExtraction(val scope: Scope?, val adjustedName: Name) + + /** + * This function extracts a scope for the [Name], e.g. if the name is fully qualified (wrapped + * in a [ScopeExtraction] object. `null` is returned if a scope was specified, but does not + * exist as a [Scope] object. * - * The pair returns the extracted scope and a name that is adjusted by possible import aliases. - * The extracted scope is "responsible" for the name (e.g. declares the parent namespace) and - * the returned name only differs from the provided name if aliasing was involved at the node - * location (e.g. because of imports). + * The returned object contains the extracted scope and a name that is adjusted by possible + * import aliases. The extracted scope is "responsible" for the name (e.g. declares the parent + * namespace) and the returned name only differs from the provided name if aliasing was involved + * at the node location (e.g. because of imports). * * Note: Currently only *fully* qualified names are properly resolved. This function will * probably return imprecise results for partially qualified names, e.g. if a name `A` inside @@ -569,9 +584,9 @@ class ScopeManager : ScopeProvider { * * @param node the nodes name references a namespace constituted by a scope * @param scope the current scope relevant for the name resolution, e.g. parent of node - * @return a pair with the scope of node.name and the alias-adjusted name + * @return a [ScopeExtraction] object with the scope of node.name and the alias-adjusted name */ - fun extractScope(node: HasNameAndLocation, scope: Scope? = currentScope): Pair { + fun extractScope(node: HasNameAndLocation, scope: Scope? = currentScope): ScopeExtraction? { return extractScope(node.name, node.location, scope) } @@ -596,7 +611,7 @@ class ScopeManager : ScopeProvider { name: Name, location: PhysicalLocation? = null, scope: Scope? = currentScope, - ): Pair { + ): ScopeExtraction? { var n = name var s: Scope? = null @@ -611,20 +626,18 @@ class ScopeManager : ScopeProvider { // this is a scoped call. we need to explicitly jump to that particular scope val scopes = filterScopes { (it is NameScope && it.name == scopeName) } - s = - if (scopes.isEmpty()) { - Util.warnWithFileLocation( - location, - LOGGER, - "Could not find the scope $scopeName needed to resolve $n" - ) - null - } else { - scopes[0] - } + if (scopes.isEmpty()) { + Util.warnWithFileLocation( + location, + LOGGER, + "Could not find the scope $scopeName needed to resolve $n" + ) + return null + } + s = scopes[0] } - return Pair(s, n) + return ScopeExtraction(s, n) } /** @@ -892,7 +905,16 @@ class ScopeManager : ScopeProvider { startScope: Scope? = currentScope, predicate: ((Declaration) -> Boolean)? = null, ): List { - val (scope, n) = extractScope(name, location, startScope) + val extractedScope = extractScope(name, location, startScope) + val scope: Scope? + val n: Name + if (extractedScope == null) { + // the scope does not exist at all + return listOf() + } else { + scope = extractedScope.scope + n = extractedScope.adjustedName + } // We need to differentiate between a qualified and unqualified lookup. We have a qualified // lookup, if the scope is not null. In this case we need to stay within the specified scope diff --git a/cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/passes/SymbolResolver.kt b/cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/passes/SymbolResolver.kt index fdc7f0edca..d463d64090 100644 --- a/cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/passes/SymbolResolver.kt +++ b/cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/passes/SymbolResolver.kt @@ -106,7 +106,7 @@ open class SymbolResolver(ctx: TranslationContext) : ComponentPass(ctx) { // Gather all resolution EOG starters; and make sure they really do not have a // predecessor, otherwise we might analyze a node multiple times - val nodes = it.allEOGStarters.filter { it.prevEOGEdges.isEmpty() } + val nodes = it.allEOGStarters.filter { it.prevEOGEdges.isEmpty } for (node in nodes) { walker.iterate(node) @@ -143,7 +143,12 @@ open class SymbolResolver(ctx: TranslationContext) : ComponentPass(ctx) { // If we didn't find anything, we create a new function or method declaration if (target == null) { // Determine the scope where we want to start our inference - var (scope, _) = scopeManager.extractScope(reference) + val extractedScope = scopeManager.extractScope(reference) + if (extractedScope == null) { + return null + } + + var scope = extractedScope.scope if (scope !is NameScope) { scope = null } @@ -528,18 +533,26 @@ open class SymbolResolver(ctx: TranslationContext) : ComponentPass(ctx) { setOf(), mapOf(), setOf(), - CallResolutionResult.SuccessKind.UNRESOLVED, + UNRESOLVED, source.scope, ) val language = source.language if (language == null) { - result.success = CallResolutionResult.SuccessKind.PROBLEMATIC + result.success = PROBLEMATIC return result } // Set the start scope. This can either be the call's scope or a scope specified in an FQN - val (scope, _) = ctx.scopeManager.extractScope(source, source.scope) + val extractedScope = ctx.scopeManager.extractScope(source, source.scope) + + // If we could not extract the scope (even though one was specified), we can only return an + // empty result + if (extractedScope == null) { + return result + } + + val scope = extractedScope.scope result.actualStartScope = scope ?: source.scope // If the function does not allow function overloading, and we have multiple candidate @@ -569,7 +582,7 @@ open class SymbolResolver(ctx: TranslationContext) : ComponentPass(ctx) { // If we have a "problematic" result, we can stop here. In this case we cannot really // determine anything more. - if (result.success == CallResolutionResult.SuccessKind.PROBLEMATIC) { + if (result.success == PROBLEMATIC) { result.bestViable = result.viableFunctions return result } diff --git a/cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/passes/inference/PassHelper.kt b/cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/passes/inference/PassHelper.kt index f88a8fc3dc..37014c17c0 100644 --- a/cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/passes/inference/PassHelper.kt +++ b/cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/passes/inference/PassHelper.kt @@ -64,7 +64,8 @@ import kotlin.collections.forEach */ fun Pass<*>.tryNamespaceInference(name: Name, locationHint: Node?): NamespaceDeclaration? { // Determine the scope where we want to start our inference - var (scope, _) = scopeManager.extractScope(name, location = locationHint?.location) + val extractedScope = scopeManager.extractScope(name, location = locationHint?.location) + var scope = extractedScope?.scope if (scope !is NameScope) { scope = null @@ -99,7 +100,9 @@ internal fun Pass<*>.tryRecordInference( "class" } // Determine the scope where we want to start our inference - var (scope, _) = scopeManager.extractScope(type, scope = type.scope) + val extractedScope = + scopeManager.extractScope(type.name, location = locationHint?.location, scope = type.scope) + var scope = extractedScope?.scope if (scope !is NameScope) { scope = null @@ -173,8 +176,8 @@ internal fun Pass<*>.tryVariableInference( } else if (ref.name.isQualified()) { // For now, we only infer globals at the top-most global level, i.e., no globals in // namespaces - val (scope, _) = scopeManager.extractScope(ref, null) - when (scope) { + val extractedScope = scopeManager.extractScope(ref, null) + when (val scope = extractedScope?.scope) { is NameScope -> { log.warn( "We should infer a namespace variable ${ref.name} at this point, but this is not yet implemented." @@ -218,7 +221,8 @@ internal fun Pass<*>.tryFieldInference( ): VariableDeclaration? { // We only want to infer fields here, this can either happen if we have a reference with an // implicit receiver or if we have a scoped reference and the scope points to a record - val (scope, _) = scopeManager.extractScope(ref) + val extractedScope = scopeManager.extractScope(ref) + val scope = extractedScope?.scope if (scope != null && scope !is RecordScope) { return null } diff --git a/cpg-language-cxx/src/test/kotlin/de/fraunhofer/aisec/cpg/frontends/cxx/CXXInferenceTest.kt b/cpg-language-cxx/src/test/kotlin/de/fraunhofer/aisec/cpg/frontends/cxx/CXXInferenceTest.kt index e5e991e2f7..3a368e86bb 100644 --- a/cpg-language-cxx/src/test/kotlin/de/fraunhofer/aisec/cpg/frontends/cxx/CXXInferenceTest.kt +++ b/cpg-language-cxx/src/test/kotlin/de/fraunhofer/aisec/cpg/frontends/cxx/CXXInferenceTest.kt @@ -32,13 +32,7 @@ import de.fraunhofer.aisec.cpg.graph.scopes.GlobalScope import de.fraunhofer.aisec.cpg.graph.types.BooleanType import de.fraunhofer.aisec.cpg.test.* import java.io.File -import kotlin.test.Test -import kotlin.test.assertContains -import kotlin.test.assertEquals -import kotlin.test.assertIs -import kotlin.test.assertIsNot -import kotlin.test.assertNotNull -import kotlin.test.assertTrue +import kotlin.test.* class CXXInferenceTest { @Test @@ -199,4 +193,32 @@ class CXXInferenceTest { assertEquals(pairType, pair.returnTypes.singleOrNull()) } } + + @Test + fun testInferParentClassInNamespace() { + val file = File("src/test/resources/cxx/inference/parent_inference.cpp") + val tu = + analyzeAndGetFirstTU(listOf(file), file.parentFile.toPath(), true) { + it.registerLanguage() + it.loadIncludes(false) + it.addIncludesToGraph(false) + } + assertNotNull(tu) + + val util = tu.namespaces["ABC"] + assertNotNull(util) + + val recordABCA = util.records["A"] + assertNotNull(recordABCA) + assertTrue(recordABCA.isInferred) + + val recordA = tu.records["A"] + assertNotNull(recordA) + val funcFoo = recordA.functions["foo"] + assertNotNull(funcFoo) + assertTrue(funcFoo.isInferred) + val funcBar = recordA.functions["bar"] + assertNotNull(funcBar) + assertFalse(funcBar.isInferred) + } } diff --git a/cpg-language-cxx/src/test/resources/cxx/inference/parent_inference.cpp b/cpg-language-cxx/src/test/resources/cxx/inference/parent_inference.cpp new file mode 100644 index 0000000000..b4f9913b8d --- /dev/null +++ b/cpg-language-cxx/src/test/resources/cxx/inference/parent_inference.cpp @@ -0,0 +1,18 @@ +/* +namespace ABC { +struct A { + A(); + void foo(); +}; +} +*/ + +struct A : ABC::A { + A() { + foo(); + bar(); + } + void bar() { + + } +};