Skip to content

Commit

Permalink
Restrict symbol resolving to language (#1857)
Browse files Browse the repository at this point in the history
* Restrict symbol resolving to language

Currently, the symbol resolver runs wild and considers all nodes of all languages. When we have more instances where we have multiple languages in the same component, this craates a problem.

This PR now only considers the same language as the ref we are looking for.

* Correcty setting the python language

* Moved language to lookupSymbolByName function

* ++

* ++

* ++

* Update cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/ScopeManager.kt

* Fixed compile error

* Fixed compile error

* Addressed code review
  • Loading branch information
oxisto authored Dec 2, 2024
1 parent eee1553 commit d6371a6
Show file tree
Hide file tree
Showing 13 changed files with 88 additions and 38 deletions.
56 changes: 44 additions & 12 deletions cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/ScopeManager.kt
Original file line number Diff line number Diff line change
Expand Up @@ -813,8 +813,10 @@ class ScopeManager : ScopeProvider {
*
* @return the declaration, or null if it does not exist
*/
fun getRecordForName(name: Name): RecordDeclaration? {
return lookupSymbolByName(name).filterIsInstance<RecordDeclaration>().singleOrNull()
fun getRecordForName(name: Name, language: Language<*>?): RecordDeclaration? {
return lookupSymbolByName(name, language)
.filterIsInstance<RecordDeclaration>()
.singleOrNull()
}

fun typedefFor(alias: Name, scope: Scope? = currentScope): Type? {
Expand Down Expand Up @@ -874,6 +876,18 @@ class ScopeManager : ScopeProvider {
override val scope: Scope?
get() = currentScope

/**
* A convenience function to call [lookupSymbolByName] with the properties of [node]. The
* arguments [scope] and [predicate] are forwarded.
*/
fun lookupSymbolByNameOfNode(
node: Node,
scope: Scope? = node.scope,
predicate: ((Declaration) -> Boolean)? = null,
): List<Declaration> {
return lookupSymbolByName(node.name, node.language, node.location, scope, predicate)
}

/**
* This function tries to convert a [Node.name] into a [Symbol] and then performs a lookup of
* this symbol. This can either be an "unqualified lookup" if [name] is not qualified or a
Expand All @@ -885,12 +899,13 @@ class ScopeManager : ScopeProvider {
* function overloading. But it will only return list of declarations within the same scope; the
* list cannot be spread across different scopes.
*
* This means that as soon one or more declarations for the symbol are found in a "local" scope,
* these shadow all other occurrences of the same / symbol in a "higher" scope and only the ones
* from the lower ones will be returned.
* This means that as soon one or more declarations (of the matching [language]) for the symbol
* are found in a "local" scope, these shadow all other occurrences of the same / symbol in a
* "higher" scope and only the ones from the lower ones will be returned.
*/
fun lookupSymbolByName(
name: Name,
language: Language<*>?,
location: PhysicalLocation? = null,
startScope: Scope? = currentScope,
predicate: ((Declaration) -> Boolean)? = null,
Expand All @@ -900,14 +915,25 @@ class ScopeManager : ScopeProvider {
// 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
val list =
if (scope != null) {
scope.lookupSymbol(n.localName, thisScopeOnly = true, predicate = predicate)
} else {
when {
scope != null -> {
scope
.lookupSymbol(
n.localName,
languageOnly = language,
thisScopeOnly = true,
predicate = predicate
)
.toMutableList()
}
else -> {
// Otherwise, we can look up the symbol alone (without any FQN) starting from
// the startScope
startScope?.lookupSymbol(n.localName, predicate = predicate)
startScope
?.lookupSymbol(n.localName, languageOnly = language, predicate = predicate)
?.toMutableList() ?: mutableListOf()
}
?.toMutableList() ?: return listOf()
}

// If we have both the definition and the declaration of a function declaration in our list,
// we chose only the definition
Expand All @@ -932,9 +958,15 @@ class ScopeManager : ScopeProvider {
* 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? {
fun lookupUniqueTypeSymbolByName(
name: Name,
language: Language<*>?,
startScope: Scope?
): DeclaresType? {
var symbols =
lookupSymbolByName(name = name, startScope = startScope) { it is DeclaresType }
lookupSymbolByName(name = name, language = language, startScope = startScope) {
it is DeclaresType
}
.filterIsInstance<DeclaresType>()

// We need to have a single match, otherwise we have an ambiguous type, and we cannot
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -380,5 +380,5 @@ fun Reference.nameIsType(): Type? {
}

// Lastly, check if the reference contains a symbol that points to type (declaration)
return scopeManager.lookupUniqueTypeSymbolByName(name, scope)?.declaredType
return scopeManager.lookupUniqueTypeSymbolByName(name, language, scope)?.declaredType
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
package de.fraunhofer.aisec.cpg.graph.scopes

import com.fasterxml.jackson.annotation.JsonBackReference
import de.fraunhofer.aisec.cpg.frontends.Language
import de.fraunhofer.aisec.cpg.graph.Name
import de.fraunhofer.aisec.cpg.graph.Node
import de.fraunhofer.aisec.cpg.graph.Node.Companion.TO_STRING_STYLE
Expand Down Expand Up @@ -131,19 +132,15 @@ abstract class Scope(
*/
fun lookupSymbol(
symbol: Symbol,
languageOnly: Language<*>? = null,
thisScopeOnly: Boolean = false,
replaceImports: Boolean = true,
predicate: ((Declaration) -> Boolean)? = null
): List<Declaration> {
// First, try to look for the symbol in the current scope (unless we have a predefined
// search scope). In the latter case we also need to restrict the lookup to the search scope
var modifiedScoped = this.predefinedLookupScopes[symbol]?.targetScope
var scope: Scope? =
if (modifiedScoped != null) {
modifiedScoped
} else {
this
}
var scope: Scope? = modifiedScoped ?: this

var list: MutableList<Declaration>? = null

Expand All @@ -163,9 +160,14 @@ abstract class Scope(
list.replaceImports(symbol)
}

// Filter according to the language
if (languageOnly != null) {
list.removeIf { it.language != languageOnly }
}

// Filter the list according to the predicate, if we have any
if (predicate != null) {
list = list.filter(predicate).toMutableList()
list.removeIf { !predicate.invoke(it) }
}

// If we have a hit, we can break the loop
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ class ImportResolver(ctx: TranslationContext) : ComponentPass(ctx) {
for (part in parts) {
var namespaces =
scopeManager
.lookupSymbolByName(part, import.location, import.scope)
.lookupSymbolByName(part, import.language, import.location, import.scope)
.filterIsInstance<NamespaceDeclaration>()

// We are only interested in "leaf" namespace declarations, meaning that they do not
Expand Down Expand Up @@ -271,7 +271,13 @@ class ImportResolver(ctx: TranslationContext) : ComponentPass(ctx) {

// Let's do some importing. We need to import either a wildcard
if (import.wildcardImport) {
val list = scopeManager.lookupSymbolByName(import.import, import.location, scope)
val list =
scopeManager.lookupSymbolByName(
import.import,
import.language,
import.location,
scope
)
val symbol = list.singleOrNull()
if (symbol != null) {
// In this case, the symbol must point to a name scope
Expand All @@ -284,7 +290,7 @@ class ImportResolver(ctx: TranslationContext) : ComponentPass(ctx) {
// or a symbol directly
val list =
scopeManager
.lookupSymbolByName(import.import, import.location, scope)
.lookupSymbolByName(import.import, import.language, import.location, scope)
.toMutableList()
import.importedSymbols = mutableMapOf(import.symbol to list)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ open class SymbolResolver(ctx: TranslationContext) : ComponentPass(ctx) {

// Find a list of candidate symbols. Currently, this is only used the in the "next-gen" call
// resolution, but in future this will also be used in resolving regular references.
ref.candidates = scopeManager.lookupSymbolByName(ref.name, ref.location).toSet()
ref.candidates = scopeManager.lookupSymbolByNameOfNode(ref).toSet()

// Preparation for a future without legacy call resolving. Taking the first candidate is not
// ideal since we are running into an issue with function pointers here (see workaround
Expand Down Expand Up @@ -590,7 +590,9 @@ open class SymbolResolver(ctx: TranslationContext) : ComponentPass(ctx) {
var candidates = mutableSetOf<Declaration>()
val records = possibleContainingTypes.mapNotNull { it.root.recordDeclaration }.toSet()
for (record in records) {
candidates.addAll(ctx.scopeManager.lookupSymbolByName(record.name.fqn(symbol)))
candidates.addAll(
ctx.scopeManager.lookupSymbolByName(record.name.fqn(symbol), record.language)
)
}

// Find invokes by supertypes
Expand Down Expand Up @@ -700,7 +702,11 @@ open class SymbolResolver(ctx: TranslationContext) : ComponentPass(ctx) {
listOf()
} else {
val firstLevelCandidates =
possibleTypes.map { scopeManager.lookupSymbolByName(it.name.fqn(name)) }.flatten()
possibleTypes
.map { record ->
scopeManager.lookupSymbolByName(record.name.fqn(name), record.language)
}
.flatten()

// C++ does not allow overloading at different hierarchy levels. If we find a
// FunctionDeclaration with the same name as the function in the CallExpression we have
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,8 @@ open class TypeResolver(ctx: TranslationContext) : ComponentPass(ctx) {
// filter for nodes that implement DeclaresType, because otherwise we will get a lot of
// constructor declarations and such with the same name. It seems this is ok since most
// languages will prefer structs/classes over functions when resolving types.
var declares = scopeManager.lookupUniqueTypeSymbolByName(type.name, type.scope)
var declares =
scopeManager.lookupUniqueTypeSymbolByName(type.name, type.language, type.scope)

// If we did not find any declaration, we can try to infer a record declaration for it
if (declares == null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ internal class ScopeManagerTest : BaseTest() {
// resolve symbol
val call =
frontend.newCallExpression(frontend.newReference("A::func1"), "A::func1", false)
val func = final.lookupSymbolByName(call.callee!!.name).firstOrNull()
val func = final.lookupSymbolByNameOfNode(call.callee).firstOrNull()

assertEquals(func1, func)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ class DeclarationHandler(frontend: GoLanguageFrontend) :

// TODO: this will only find methods within the current translation unit.
// this is a limitation that we have for C++ as well
val record = frontend.scopeManager.getRecordForName(recordName)
val record = frontend.scopeManager.getRecordForName(recordName, language)

// now this gets a little bit hacky, we will add it to the record declaration
// this is strictly speaking not 100 % true, since the method property edge is
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -389,7 +389,7 @@ class GoExtraPass(ctx: TranslationContext) : ComponentPass(ctx) {

// Try to see if we already know about this namespace somehow
val namespace =
scopeManager.lookupSymbolByName(import.name, null).filter {
scopeManager.lookupSymbolByNameOfNode(import).filter {
it is NamespaceDeclaration && it.path == import.importURL
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ class DeclarationHandler(lang: LLVMIRLanguageFrontend) :
}

// try to see, if the struct already exists as a record declaration
var record = frontend.scopeManager.getRecordForName(Name(name))
var record = frontend.scopeManager.getRecordForName(Name(name), language)

// if yes, return it
if (record != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -441,7 +441,7 @@ class ExpressionHandler(lang: LLVMIRLanguageFrontend) :
var record = (baseType as? ObjectType)?.recordDeclaration

if (record == null) {
record = frontend.scopeManager.getRecordForName(baseType.name)
record = frontend.scopeManager.getRecordForName(baseType.name, language)
if (record != null) {
(baseType as? ObjectType)?.recordDeclaration = record
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ class LLVMIRLanguageFrontend(language: Language<LLVMIRLanguageFrontend>, ctx: Tr

/** Determines if a struct with [name] exists in the scope. */
fun isKnownStructTypeName(name: String): Boolean {
return this.scopeManager.getRecordForName(Name(name)) != null
return this.scopeManager.getRecordForName(Name(name), language) != null
}

fun getOperandValueAtIndex(instr: LLVMValueRef, idx: Int): Expression {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@
package de.fraunhofer.aisec.cpg.passes

import de.fraunhofer.aisec.cpg.TranslationContext
import de.fraunhofer.aisec.cpg.frontends.Language
import de.fraunhofer.aisec.cpg.frontends.python.PythonLanguage
import de.fraunhofer.aisec.cpg.frontends.python.PythonLanguageFrontend
import de.fraunhofer.aisec.cpg.graph.*
import de.fraunhofer.aisec.cpg.graph.declarations.Declaration
Expand All @@ -44,7 +46,7 @@ import de.fraunhofer.aisec.cpg.passes.configuration.RequiredFrontend
@ExecuteBefore(ImportResolver::class)
@ExecuteBefore(SymbolResolver::class)
@RequiredFrontend(PythonLanguageFrontend::class)
class PythonAddDeclarationsPass(ctx: TranslationContext) : ComponentPass(ctx) {
class PythonAddDeclarationsPass(ctx: TranslationContext) : ComponentPass(ctx), LanguageProvider {
override fun cleanup() {
// nothing to do
}
Expand Down Expand Up @@ -96,11 +98,9 @@ class PythonAddDeclarationsPass(ctx: TranslationContext) : ComponentPass(ctx) {
// - to look for a local symbol, unless
// - a global keyword is present for this symbol and scope
if (targetScope != null) {
scopeManager.lookupSymbolByName(ref.name, ref.location, targetScope)
scopeManager.lookupSymbolByNameOfNode(ref, targetScope)
} else {
scopeManager.lookupSymbolByName(ref.name, ref.location) {
it.scope == scopeManager.currentScope
}
scopeManager.lookupSymbolByNameOfNode(ref) { it.scope == scopeManager.currentScope }
}

// Nothing to create
Expand Down Expand Up @@ -203,4 +203,7 @@ class PythonAddDeclarationsPass(ctx: TranslationContext) : ComponentPass(ctx) {
}
}
}

override val language: Language<*>?
get() = ctx.config.languages.firstOrNull { it is PythonLanguage }
}

0 comments on commit d6371a6

Please sign in to comment.