Removing legacy resolve functions from SymbolResolver

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

@@ -32,10 +32,9 @@ 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
 import de.fraunhofer.aisec.cpg.helpers.Util
+import de.fraunhofer.aisec.cpg.passes.SymbolResolver
 import de.fraunhofer.aisec.cpg.sarif.PhysicalLocation
 import java.util.*
 import java.util.function.Predicate
@@ -72,13 +71,6 @@ class ScopeManager : ScopeProvider {
     /** Represents an alias with the name [to] for the particular name [from]. */
     data class Alias(var from: Name, var to: Name)
 
-    /**
-     * A cache map of reference tags (computed with [Reference.referenceTag]) and their respective
-     * pair of original [Reference] and resolved [ValueDeclaration]. This is used by
-     * [resolveReference] as a caching mechanism.
-     */
-    private val symbolTable = mutableMapOf<ReferenceTag, Pair<Reference, ValueDeclaration>>()
-
     /** True, if the scope manager is currently in a [FunctionScope]. */
     val isInFunction: Boolean
         get() = this.firstScopeOrNull { it is FunctionScope } != null
@@ -489,81 +481,6 @@ class ScopeManager : ScopeProvider {
         scope?.addTypedef(typedef)
     }
 
-    /**
-     * Resolves only references to Values in the current scope, static references to other visible
-     * records are not resolved over the ScopeManager.
-     *
-     * @param ref
-     * @return
-     */
-    fun resolveReference(ref: Reference): ValueDeclaration? {
-        val startScope = ref.scope
-
-        // Retrieve a unique tag for the particular reference based on the current scope
-        val tag = ref.referenceTag
-
-        // If we find a match in our symbol table, we can immediately return the declaration. We
-        // need to be careful about potential collisions in our tags, since they are based on the
-        // hash-code of the scope. We therefore take the extra precaution to compare the scope in
-        // case we get a hit. This should not take too much performance overhead.
-        val pair = symbolTable[tag]
-        if (pair != null && ref.scope == pair.first.scope) {
-            return pair.second
-        }
-
-        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
-        }
-
-        // Try to resolve value declarations according to our criteria
-        val decl =
-            resolve<ValueDeclaration>(scope) {
-                    if (it.name.lastPartsMatch(name)) {
-                        val helper = ref.resolutionHelper
-                        return@resolve when {
-                            // If the reference seems to point to a function (using a function
-                            // pointer) the entire signature is checked for equality
-                            helper?.type is FunctionPointerType && it is FunctionDeclaration -> {
-                                val fptrType = helper.type as FunctionPointerType
-                                // TODO(oxisto): Support multiple return values
-                                val returnType =
-                                    it.returnTypes.firstOrNull() ?: IncompleteType(ref.language)
-                                returnType == fptrType.returnType &&
-                                    it.matchesSignature(fptrType.parameters) !=
-                                        IncompatibleSignature
-                            }
-                            // If our language has first-class functions, we can safely return them
-                            // as a reference
-                            ref.language is HasFirstClassFunctions -> {
-                                true
-                            }
-                            // Otherwise, we are not looking for functions here
-                            else -> {
-                                it !is FunctionDeclaration
-                            }
-                        }
-                    }
-
-                    return@resolve false
-                }
-                .firstOrNull()
-
-        // Update the symbol cache, if we found a declaration for the tag
-        if (decl != null) {
-            symbolTable[tag] = Pair(ref, decl)
-        }
-
-        return decl
-    }
-
     /**
      * 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
@@ -721,88 +638,6 @@ class ScopeManager : ScopeProvider {
         return ret
     }
 
-    /**
-     * Traverses the scope upwards and looks for declarations of type [T] which matches the
-     * condition [predicate].
-     *
-     * It returns a list of all declarations that match the predicate, ordered by reachability in
-     * the scope stack. This means that "local" declarations will be in the list first, global items
-     * will be last.
-     *
-     * @param searchScope the scope to start the search in
-     * @param predicate predicate the element must match to
-     * @param <T>
-     */
-    internal inline fun <reified T : Declaration> resolve(
-        searchScope: Scope?,
-        stopIfFound: Boolean = false,
-        noinline predicate: (T) -> Boolean,
-    ): List<T> {
-        return resolve(T::class.java, searchScope, stopIfFound, predicate)
-    }
-
-    internal fun <T : Declaration> resolve(
-        klass: Class<T>,
-        searchScope: Scope?,
-        stopIfFound: Boolean = false,
-        predicate: (T) -> Boolean,
-    ): List<T> {
-        var scope = searchScope
-        val declarations = mutableListOf<T>()
-
-        while (scope != null) {
-            if (scope is ValueDeclarationScope) {
-                declarations.addAll(
-                    scope.valueDeclarations.filterIsInstance(klass).filter(predicate)
-                )
-            }
-
-            if (scope is StructureDeclarationScope) {
-                var list = scope.structureDeclarations.filterIsInstance(klass).filter(predicate)
-
-                // this was taken over from the old resolveStructureDeclaration.
-                // TODO(oxisto): why is this only when the list is empty?
-                if (list.isEmpty()) {
-                    for (declaration in scope.structureDeclarations) {
-                        if (declaration is RecordDeclaration) {
-                            list = declaration.templates.filterIsInstance(klass).filter(predicate)
-                        }
-                    }
-                }
-
-                declarations.addAll(list)
-            }
-
-            // some (all?) languages require us to stop immediately if we found something on this
-            // scope. This is the case where function overloading is allowed, but only within the
-            // same scope
-            if (stopIfFound && declarations.isNotEmpty()) {
-                return declarations
-            }
-
-            // go upwards in the scope tree
-            scope = scope.parent
-        }
-
-        return declarations
-    }
-
-    /**
-     * Resolves function templates of the given [CallExpression].
-     *
-     * @param scope where we are searching for the FunctionTemplateDeclarations
-     * @param call CallExpression we want to resolve an invocation target for
-     * @return List of FunctionTemplateDeclaration that match the name provided in the
-     *   CallExpression and therefore are invocation candidates
-     */
-    @JvmOverloads
-    fun resolveFunctionTemplateDeclaration(
-        call: CallExpression,
-        scope: Scope? = currentScope,
-    ): List<FunctionTemplateDeclaration> {
-        return resolve(scope, true) { c -> c.name.lastPartsMatch(call.name) }
-    }
-
     /**
      * Retrieves the [RecordDeclaration] for the given name in the given scope.
      *
@@ -878,14 +713,26 @@ class ScopeManager : ScopeProvider {
      * A convenience function to call [lookupSymbolByName] with the properties of [node]. The
      * arguments [scope] and [predicate] are forwarded.
      */
-    fun lookupSymbolByNameOfNode(
+    fun lookupSymbolByNodeName(
         node: Node,
         scope: Scope? = node.scope,
         predicate: ((Declaration) -> Boolean)? = null,
     ): List<Declaration> {
         return lookupSymbolByName(node.name, node.language, node.location, scope, predicate)
     }
 
+    /**
+     * A convenience function to call [lookupSymbolByName] with the properties of [node].
+     * Additionally, it adds a predicate to the search that the declaration must be of type [T].
+     */
+    inline fun <reified T : Declaration> lookupSymbolByNodeNameOfType(
+        node: Node,
+        scope: Scope? = node.scope,
+    ): List<T> {
+        return lookupSymbolByName(node.name, node.language, node.location, scope) { it is T }
+            .filterIsInstance<T>()
+    }
+
     /**
      * 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

cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/passes/SymbolResolver.kt

@@ -30,7 +30,6 @@ import de.fraunhofer.aisec.cpg.CallResolutionResult.SuccessKind.*
 import de.fraunhofer.aisec.cpg.frontends.*
 import de.fraunhofer.aisec.cpg.graph.*
 import de.fraunhofer.aisec.cpg.graph.declarations.*
-import de.fraunhofer.aisec.cpg.graph.scopes.NameScope
 import de.fraunhofer.aisec.cpg.graph.scopes.Symbol
 import de.fraunhofer.aisec.cpg.graph.statements.expressions.*
 import de.fraunhofer.aisec.cpg.graph.types.*
@@ -41,6 +40,7 @@ import de.fraunhofer.aisec.cpg.passes.configuration.DependsOn
 import de.fraunhofer.aisec.cpg.passes.inference.startInference
 import de.fraunhofer.aisec.cpg.passes.inference.tryFieldInference
 import de.fraunhofer.aisec.cpg.passes.inference.tryFunctionInference
+import de.fraunhofer.aisec.cpg.passes.inference.tryFunctionInferenceFromFunctionPointer
 import de.fraunhofer.aisec.cpg.passes.inference.tryVariableInference
 import de.fraunhofer.aisec.cpg.processing.strategy.Strategy
 import org.slf4j.Logger
@@ -134,41 +134,6 @@ open class SymbolResolver(ctx: TranslationContext) : ComponentPass(ctx) {
         return language is HasSuperClasses && reference.name.endsWith(language.superClassKeyword)
     }
 
-    /** This function seems to resolve function pointers pointing to a [MethodDeclaration]. */
-    protected fun resolveMethodFunctionPointer(
-        reference: Reference,
-        type: FunctionPointerType,
-    ): ValueDeclaration? {
-        var target = scopeManager.resolveReference(reference)
-
-        // 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
-            val extractedScope = scopeManager.extractScope(reference)
-            if (extractedScope == null) {
-                return null
-            }
-
-            var scope = extractedScope.scope
-            if (scope !is NameScope) {
-                scope = null
-            }
-
-            target =
-                (scope?.astNode ?: reference.translationUnit)
-                    ?.startInference(ctx)
-                    ?.inferFunctionDeclaration(
-                        reference.name,
-                        null,
-                        false,
-                        type.parameters,
-                        type.returnType,
-                    )
-        }
-
-        return target
-    }
-
     /**
      * This function handles symbol resolving for a [Reference]. After a successful lookup of the
      * symbol contained in [Reference.name], the property [Reference.refersTo] is set to the best
@@ -193,6 +158,7 @@ open class SymbolResolver(ctx: TranslationContext) : ComponentPass(ctx) {
      */
     protected fun handleReference(currentClass: RecordDeclaration?, ref: Reference) {
         val language = ref.language
+        val helperType = ref.resolutionHelper?.type
 
         // Ignore references to anonymous identifiers, if the language supports it (e.g., the _
         // identifier in Go)
@@ -208,9 +174,27 @@ open class SymbolResolver(ctx: TranslationContext) : ComponentPass(ctx) {
             return
         }
 
+        // If our resolution helper indicates that this reference is the target of a variable with a
+        // function pointer, we need to take the (return) type arguments of the function pointer
+        // into consideration
+        val predicate: ((Declaration) -> Boolean)? =
+            if (helperType is FunctionPointerType) {
+                { declaration ->
+                    if (declaration is FunctionDeclaration) {
+                        declaration.returnTypes == listOf(helperType.returnType) &&
+                            declaration.matchesSignature(helperType.parameters) !=
+                                IncompatibleSignature
+                    } else {
+                        false
+                    }
+                }
+            } else {
+                null
+            }
+
         // 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.lookupSymbolByNameOfNode(ref).toSet()
+        ref.candidates = scopeManager.lookupSymbolByNodeName(ref, predicate = predicate).toSet()
 
         // We need to choose the best viable candidate out of the ones we have for our reference.
         // Hopefully we have only one, but there might be instances where more than one is a valid
@@ -236,25 +220,6 @@ open class SymbolResolver(ctx: TranslationContext) : ComponentPass(ctx) {
             }
         }
 
-        // Some stupid C++ workaround to use the legacy call resolver when we try to resolve targets
-        // for function pointers. At least we are only invoking the legacy resolver for a very small
-        // percentage of references now.
-        if (wouldResolveTo is FunctionDeclaration) {
-            // We need to invoke the legacy resolver, just to be sure
-            var legacy = scopeManager.resolveReference(ref)
-
-            // This is just for us to catch these differences in symbol resolving in the future. The
-            // difference is pretty much only that the legacy system takes parameters of the
-            // function-pointer-type into account and the new system does not (yet), because it just
-            // takes the first match. This will be needed to solve in the future.
-            if (legacy != wouldResolveTo) {
-                log.warn(
-                    "The legacy symbol resolution and the new system produced different results here. This needs to be investigated in the future. For now, we take the legacy result."
-                )
-                wouldResolveTo = legacy
-            }
-        }
-
         // Only consider resolving, if the language frontend did not specify a resolution. If we
         // already have populated the wouldResolveTo variable, we can re-use this instead of
         // resolving again
@@ -265,14 +230,16 @@ open class SymbolResolver(ctx: TranslationContext) : ComponentPass(ctx) {
             recordDeclType = currentClass.toType()
         }
 
-        val helperType = ref.resolutionHelper?.type
-        if (helperType is FunctionPointerType && refersTo == null) {
-            refersTo = resolveMethodFunctionPointer(ref, helperType)
-        }
-
         // If we did not resolve the reference up to this point, we can try to infer the declaration
         if (refersTo == null) {
-            refersTo = tryVariableInference(ref)
+            // If its a function pointer, we can try to infer a function
+            refersTo =
+                if (helperType is FunctionPointerType) {
+                    tryFunctionInferenceFromFunctionPointer(ref, helperType)
+                } else {
+                    // Otherwise, we can try to infer a variable
+                    tryVariableInference(ref)
+                }
         }
 
         if (refersTo != null) {

cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/passes/TemplateCallResolverHelper.kt

@@ -132,7 +132,8 @@ fun applyMissingParams(
                 if (missingParam.refersTo == null) {
                     val currentScope = scopeManager.currentScope
                     scopeManager.jumpTo(missingParam.scope)
-                    missingParam.refersTo = scopeManager.resolveReference(missingParam)
+                    missingParam.refersTo =
+                        scopeManager.lookupSymbolByNodeName(missingParam).singleOrNull()
                     scopeManager.jumpTo(currentScope)
                 }
                 missingParam = missingParam.refersTo