Try new approach to reduce search set in type equivalence, some new join orders.

More pragmas added to encourage the join ordering pipeline to make function
comparisons more efficient.

New approach in type equivalence assumes that all types are trivially equivalent
to themselves. Therefore, only type comparisons between non-identical types need
to be considered as interesting roots. The types that are reachable in the type
graph from these roots are the ones considered by the recursive type equivalence
predicate.

Author: MichaelRFairhurst

c/misra/src/rules/RULE-23-5/DangerousDefaultSelectionForPointerInGeneric.ql

@@ -20,18 +20,14 @@ import codingstandards.cpp.types.LvalueConversion
 import codingstandards.cpp.types.SimpleAssignment
 
 predicate typesCompatible(Type t1, Type t2) {
-  TypeEquivalence<TypesCompatibleConfig, TypeFromGeneric>::equalTypes(t1, t2)
+  TypeEquivalence<TypesCompatibleConfig, relevantTypes/2>::equalTypes(t1, t2)
 }
 
-class TypeFromGeneric extends Type {
-  TypeFromGeneric() {
-    exists(C11GenericExpr g |
-      (
-        this = g.getAssociationType(_) or
-        this = g.getControllingExpr().getFullyConverted().getType()
-      )
-    )
-  }
+predicate relevantTypes(Type a, Type b) {
+  exists(C11GenericExpr g |
+    a = g.getAnAssociationType() and
+    b = getLvalueConverted(g.getControllingExpr().getFullyConverted().getType())
+  )
 }
 
 predicate missesOnPointerConversion(Type provided, Type expected) {
@@ -40,11 +36,11 @@ predicate missesOnPointerConversion(Type provided, Type expected) {
   // But 6.5.16.1 simple assignment constraints would have been satisfied:
   (
     // Check as if the controlling expr is assigned to the expected type:
-    SimpleAssignment<TypeFromGeneric>::satisfiesSimplePointerAssignment(expected, provided)
+    SimpleAssignment<relevantTypes/2>::satisfiesSimplePointerAssignment(expected, provided)
     or
     // Since developers typically rely on the compiler to catch const/non-const assignment
     // errors, don't assume a const-to-non-const generic selection miss was intentional.
-    SimpleAssignment<TypeFromGeneric>::satisfiesSimplePointerAssignment(provided, expected)
+    SimpleAssignment<relevantTypes/2>::satisfiesSimplePointerAssignment(provided, expected)
   )
 }
 

c/misra/src/rules/RULE-8-3/DeclarationsOfAFunctionSameNameAndType.ql

@@ -16,6 +16,12 @@ import cpp
 import codingstandards.c.misra
 import codingstandards.cpp.types.Compatible
 
+predicate interestedInFunctions(FunctionDeclarationEntry f1, FunctionDeclarationEntry f2) {
+  f1.getDeclaration() = f2.getDeclaration() and
+  not f1 = f2 and
+  f1.getDeclaration() = f2.getDeclaration()
+}
+
 from FunctionDeclarationEntry f1, FunctionDeclarationEntry f2, string case, string pluralDo
 where
   not isExcluded(f1, Declarations4Package::declarationsOfAFunctionSameNameAndTypeQuery()) and
@@ -24,12 +30,12 @@ where
   f1.getDeclaration() = f2.getDeclaration() and
   //return type check
   (
-    not FunctionDeclarationTypeEquivalence<TypeNamesMatchConfig>::equalReturnTypes(f1, f2) and
+    not FunctionDeclarationTypeEquivalence<TypeNamesMatchConfig, interestedInFunctions/2>::equalReturnTypes(f1, f2) and
     case = "return type" and
     pluralDo = "does"
     or
     //parameter type check
-    not FunctionDeclarationTypeEquivalence<TypeNamesMatchConfig>::equalParameterTypes(f1, f2) and
+    not FunctionDeclarationTypeEquivalence<TypeNamesMatchConfig, interestedInFunctions/2>::equalParameterTypes(f1, f2) and
     case = "parameter types" and
     pluralDo = "do"
     or

c/misra/src/rules/RULE-8-3/DeclarationsOfAnObjectSameNameAndType.ql

@@ -16,15 +16,6 @@ import cpp
 import codingstandards.c.misra
 import codingstandards.cpp.types.Compatible
 
-class RelevantType extends Type {
-  RelevantType() {
-    exists(VariableDeclarationEntry decl |
-      (relevantPair(decl, _) or relevantPair(_, decl)) and
-      decl.getType() = this
-    )
-  }
-}
-
 predicate relevantPair(VariableDeclarationEntry decl1, VariableDeclarationEntry decl2) {
   not decl1 = decl2 and
   not decl1.getVariable().getDeclaringType().isAnonymous() and
@@ -43,12 +34,20 @@ predicate relevantPair(VariableDeclarationEntry decl1, VariableDeclarationEntry
   )
 }
 
+predicate relevantTypes(Type a, Type b) {
+  exists(VariableDeclarationEntry varA, VariableDeclarationEntry varB |
+    a = varA.getType() and
+    b = varB.getType() and
+    relevantPair(varA, varB)
+  )
+}
+
 from VariableDeclarationEntry decl1, VariableDeclarationEntry decl2
 where
   not isExcluded(decl1, Declarations4Package::declarationsOfAnObjectSameNameAndTypeQuery()) and
   not isExcluded(decl2, Declarations4Package::declarationsOfAnObjectSameNameAndTypeQuery()) and
   relevantPair(decl1, decl2) and
-  not TypeEquivalence<TypeNamesMatchConfig, RelevantType>::equalTypes(decl1.getType(),
+  not TypeEquivalence<TypeNamesMatchConfig, relevantTypes/2>::equalTypes(decl1.getType(),
     decl2.getType())
 select decl1,
   "The object $@ of type " + decl1.getType().toString() +

c/misra/src/rules/RULE-8-4/CompatibleDeclarationFunctionDefined.ql

@@ -19,6 +19,16 @@ import codingstandards.c.misra
 import codingstandards.cpp.Identifiers
 import codingstandards.cpp.types.Compatible
 
+predicate interestedInFunctions(FunctionDeclarationEntry f1, FunctionDeclarationEntry f2) {
+  f1.getDeclaration() instanceof ExternalIdentifiers and
+  f1.isDefinition() and
+  f1.getName() = f2.getName() and
+  f1.getDeclaration() = f2.getDeclaration() and
+  not f2.isDefinition() and
+  not f1.isFromTemplateInstantiation(_) and
+  not f2.isFromTemplateInstantiation(_)
+}
+
 from FunctionDeclarationEntry f1
 where
   not isExcluded(f1, Declarations4Package::compatibleDeclarationFunctionDefinedQuery()) and
@@ -38,10 +48,12 @@ where
       f2.getDeclaration() = f1.getDeclaration() and
       (
         //return types differ
-        not FunctionDeclarationTypeEquivalence<TypesCompatibleConfig>::equalReturnTypes(f1, f2)
+        not FunctionDeclarationTypeEquivalence<TypesCompatibleConfig, interestedInFunctions/2>::equalReturnTypes(f1,
+          f2)
         or
         //parameter types differ
-        not FunctionDeclarationTypeEquivalence<TypesCompatibleConfig>::equalParameterTypes(f1, f2)
+        not FunctionDeclarationTypeEquivalence<TypesCompatibleConfig, interestedInFunctions/2>::equalParameterTypes(f1,
+          f2)
         or
         //parameter names differ
         parameterNamesUnmatched(f1, f2)

c/misra/src/rules/RULE-8-4/CompatibleDeclarationObjectDefined.ql

@@ -19,13 +19,13 @@ import codingstandards.c.misra
 import codingstandards.cpp.Identifiers
 import codingstandards.cpp.types.Compatible
 
-class RelevantType extends Type {
-  RelevantType() {
-    exists(VariableDeclarationEntry decl |
-      count(VariableDeclarationEntry others | others.getDeclaration() = decl.getDeclaration()) > 1 and
-      decl.getType() = this
-    )
-  }
+predicate relevantTypes(Type a, Type b) {
+  exists(VariableDeclarationEntry varA, VariableDeclarationEntry varB |
+    not varA = varB and
+    varA.getDeclaration() = varB.getDeclaration() and
+    a = varA.getType() and
+    b = varB.getType()
+  )
 }
 
 from VariableDeclarationEntry decl1
@@ -37,7 +37,7 @@ where
   not exists(VariableDeclarationEntry decl2 |
     not decl2.isDefinition() and
     decl1.getDeclaration() = decl2.getDeclaration() and
-    TypeEquivalence<TypesCompatibleConfig, RelevantType>::equalTypes(decl1.getType(),
+    TypeEquivalence<TypesCompatibleConfig, relevantTypes/2>::equalTypes(decl1.getType(),
       decl2.getType())
   )
 select decl1, "No separate compatible declaration found for this definition."

change_notes/2025-04-25-improve-type-comparison-performance.md

@@ -0,0 +1,6 @@
+ - `RULE-8-3`, `RULE-8-4`, `DCL40-C`, `RULE-23-5`: `DeclarationsOfAFunctionSameNameAndType.ql`, `DeclarationsOfAnObjectSameNameAndType.ql`, `CompatibleDeclarationOfFunctionDefined.ql`, `CompatibleDeclarationObjectDefined.ql`, `IncompatibleFunctionDeclarations.ql`, `DangerousDefaultSelectionForPointerInGeneric.ql`:
+   - Added pragmas to alter join order on function parameter equivalence (names and types).
+   - Refactored expression which the optimizer was confused by, and compiled into a cartesian product. 
+   - Altered the module `Compatible.qll` to only perform expensive equality checks on types that are compared to a non identical other type, and those reachable from those types in the type graph. Types that are identical will trivially be considered equivalent.
+ - `RULE-23-5`: `DangerousDefaultSelectionForPointerInGeneric.ql`:
+   - Altered the module `SimpleAssignment.qll` in accordance with the changes to `Compatible.qll`.

cpp/common/src/codingstandards/cpp/types/Compatible.qll

@@ -25,7 +25,7 @@ class VariableType extends Type {
 }
 
 predicate parameterNamesUnmatched(FunctionDeclarationEntry f1, FunctionDeclarationEntry f2) {
-  f1.getDeclaration() = f2.getDeclaration() and
+  pragma[only_bind_into](f1).getDeclaration() = pragma[only_bind_into](f2).getDeclaration() and
   exists(string p1Name, string p2Name, int i |
     p1Name = f1.getParameterDeclarationEntry(i).getName() and
     p2Name = f2.getParameterDeclarationEntry(i).getName()
@@ -208,34 +208,64 @@ signature module TypeEquivalenceSig {
 module DefaultEquivalence implements TypeEquivalenceSig { }
 
 /**
- * A signature class used to restrict the set of types considered by `TypeEquivalence`, for
+ * A signature predicate used to restrict the set of types considered by `TypeEquivalence`, for
  * performance reasons.
  */
-signature class TypeSubset extends Type;
+signature predicate interestedInEquality(Type a, Type b);
 
 /**
  * A module to check the equivalence of two types, as defined by the provided `TypeEquivalenceSig`.
  *
- * For performance reasons, this module is designed to be used with a `TypeSubset` that restricts
- * the set of considered types. All types reachable (in the type graph) from a type in the subset
- * will be considered. (See `RelevantType`.)
+ * For performance reasons, this module is designed to be used with a predicate
+ * `interestedInEquality` that restricts the set of considered types.
  *
  * To use this module, define a `TypeEquivalenceSig` module and implement a subset of `Type` that
  * selects the relevant root types to be considered. Then use the predicate `equalTypes(a, b)`.
+ * Note that `equalTypes(a, b)` only holds if `interestedIn(a, b)` holds. A type is always
+ * considered to be equal to itself, and this module does not support configurations that declare
+ * otherwise.
+ * 
+ * Further, `interestedInEquality(a, a)` is treated differently from `interestedInEquality(a, b)`,
+ * assuming that `a` and `b` are not identical. This is so that we can construct a set of types
+ * that are not identical, but still may be equivalent by the specified configuration. We also must
+ * consider all types that are reachable from these types, as the equivalence relation is
+ * recursive. Therefore, this module is more performant when most comparisons are identical, and
+ * only a few are not.
  */
-module TypeEquivalence<TypeEquivalenceSig Config, TypeSubset T> {
+module TypeEquivalence<TypeEquivalenceSig Config, interestedInEquality/2 interestedIn> {
   /**
    * Check whether two types are equivalent, as defined by the `TypeEquivalenceSig` module.
+   * 
+   * This only holds if the specified predicate `interestedIn` holds for the types, and always
+   * holds if `t1` and `t2` are identical.
    */
-  predicate equalTypes(RelevantType t1, RelevantType t2) {
+  predicate equalTypes(Type t1, Type t2) {
+    interestedInUnordered(t1, t2) and
+    (
+      // If the types are identical, they are trivially equal.
+      t1 = t2
+      or
+      not t1 = t2 and
+      equalTypesImpl(t1, t2)
+    )
+  }
+
+  /**
+   * This implementation handles only the slow and complex cases of type equivalence, where the
+   * types are not identical.
+   *
+   * Assuming that types a, b must be compared where `a` and `b` are not identical, we wish to
+   * search only the smallest set of possible relevant types. See `RelevantType` for more.
+   */
+  private predicate equalTypesImpl(RelevantType t1, RelevantType t2) {
     if Config::overrideTypeComparison(t1, t2, _)
     then Config::overrideTypeComparison(t1, t2, true)
     else
       if t1 instanceof TypedefType and Config::resolveTypedefs()
-      then equalTypes(t1.(TypedefType).getBaseType(), t2)
+      then equalTypesImpl(t1.(TypedefType).getBaseType(), t2)
       else
         if t2 instanceof TypedefType and Config::resolveTypedefs()
-        then equalTypes(t1, t2.(TypedefType).getBaseType())
+        then equalTypesImpl(t1, t2.(TypedefType).getBaseType())
         else (
           not t1 instanceof DerivedType and
           not t2 instanceof DerivedType and
@@ -251,13 +281,36 @@ module TypeEquivalence<TypeEquivalenceSig Config, TypeSubset T> {
         )
   }
 
+  /** Whether two types will be compared, regardless of order (a, b) or (b, a). */
+  private predicate interestedInUnordered(Type t1, Type t2) {
+    interestedIn(t1, t2) or
+    interestedIn(t2, t1) }
+
+  final private class FinalType = Type;
+
   /**
-   * A type that is either part of the type subset, or that is reachable from a type in the subset.
+   * A type that is compared to another type that is not identical. This is the set of types that
+   * form the roots of our more expensive type equivalence analysis.
    */
-  private class RelevantType instanceof Type {
-    RelevantType() { exists(T t | typeGraph*(t, this)) }
+  private class InterestingType extends FinalType {
+    InterestingType() {
+      exists(Type inexactCompare |
+        interestedInUnordered(this, _) and
+        not inexactCompare = this
+      )
+    }
+  }
 
-    string toString() { result = this.(Type).toString() }
+  /**
+   * A type that is reachable from an `InterestingType` (a type that is compared to a non-identical
+   * type).
+   * 
+   * Since type equivalence is recursive, CodeQL will consider the equality of these types in a
+   * bottom-up evaluation, with leaf nodes first. Therefore, this set must be as small as possible
+   * in order to be efficient.
+   */
+  private class RelevantType extends FinalType {
+    RelevantType() { exists(InterestingType t | typeGraph*(t, this)) }
   }
 
   private class RelevantDerivedType extends RelevantType instanceof DerivedType {
@@ -296,7 +349,7 @@ module TypeEquivalence<TypeEquivalenceSig Config, TypeSubset T> {
   bindingset[t1, t2]
   private predicate equalDerivedTypes(RelevantDerivedType t1, RelevantDerivedType t2) {
     exists(Boolean baseTypesEqual |
-      (baseTypesEqual = true implies equalTypes(t1.getBaseType(), t2.getBaseType())) and
+      (baseTypesEqual = true implies equalTypesImpl(t1.getBaseType(), t2.getBaseType())) and
       (
         Config::equalPointerTypes(t1, t2, baseTypesEqual)
         or
@@ -310,20 +363,20 @@ module TypeEquivalence<TypeEquivalenceSig Config, TypeSubset T> {
       // Note that this case is different from the above, in that we don't merely get the base
       // type (as that could be a TypedefType that points to another SpecifiedType). We need to
       // unspecify the type to see if the base types are equal.
-      (unspecifiedTypesEqual = true implies equalTypes(unspecify(t1), unspecify(t2))) and
+      (unspecifiedTypesEqual = true implies equalTypesImpl(unspecify(t1), unspecify(t2))) and
       Config::equalSpecifiedTypes(t1, t2, unspecifiedTypesEqual)
     )
   }
 
   bindingset[t1, t2]
   private predicate equalFunctionTypes(RelevantFunctionType t1, RelevantFunctionType t2) {
     exists(Boolean returnTypeEqual, Boolean parameterTypesEqual |
-      (returnTypeEqual = true implies equalTypes(t1.getReturnType(), t2.getReturnType())) and
+      (returnTypeEqual = true implies equalTypesImpl(t1.getReturnType(), t2.getReturnType())) and
       (
         parameterTypesEqual = true
         implies
         forall(int i | exists([t1, t2].getParameterType(i)) |
-          equalTypes(t1.getParameterType(i), t2.getParameterType(i))
+          equalTypesImpl(t1.getParameterType(i), t2.getParameterType(i))
         )
       ) and
       (
@@ -337,30 +390,52 @@ module TypeEquivalence<TypeEquivalenceSig Config, TypeSubset T> {
   bindingset[t1, t2]
   private predicate equalTypedefTypes(RelevantTypedefType t1, RelevantTypedefType t2) {
     exists(Boolean baseTypesEqual |
-      (baseTypesEqual = true implies equalTypes(t1.getBaseType(), t2.getBaseType())) and
+      (baseTypesEqual = true implies equalTypesImpl(t1.getBaseType(), t2.getBaseType())) and
       Config::equalTypedefTypes(t1, t2, baseTypesEqual)
     )
   }
 }
 
-module FunctionDeclarationTypeEquivalence<TypeEquivalenceSig Config> {
+signature predicate interestedInFunctionDeclarations(
+  FunctionDeclarationEntry f1, FunctionDeclarationEntry f2
+);
+
+module FunctionDeclarationTypeEquivalence<
+  TypeEquivalenceSig Config, interestedInFunctionDeclarations/2 interestedInFunctions>
+{
+  private predicate interestedInReturnTypes(Type a, Type b) {
+    exists(FunctionDeclarationEntry aFun, FunctionDeclarationEntry bFun |
+      interestedInFunctions(aFun, bFun) and
+      a = aFun.getType() and
+      b = bFun.getType()
+    )
+  }
+
+  private predicate interestedInParameterTypes(Type a, Type b) {
+    exists(FunctionDeclarationEntry aFun, FunctionDeclarationEntry bFun, int i |
+      interestedInFunctions(pragma[only_bind_into](aFun), pragma[only_bind_into](bFun)) and
+      a = aFun.getParameterDeclarationEntry(i).getType() and
+      b = bFun.getParameterDeclarationEntry(i).getType()
+    )
+  }
+
   predicate equalReturnTypes(FunctionDeclarationEntry f1, FunctionDeclarationEntry f2) {
-    TypeEquivalence<Config, FunctionSignatureType>::equalTypes(f1.getType(), f2.getType())
+    TypeEquivalence<Config, interestedInReturnTypes/2>::equalTypes(f1.getType(), f2.getType())
   }
 
   predicate equalParameterTypes(FunctionDeclarationEntry f1, FunctionDeclarationEntry f2) {
+    interestedInFunctions(f1, f2) and
     f1.getDeclaration() = f2.getDeclaration() and
     forall(int i | exists([f1, f2].getParameterDeclarationEntry(i)) |
       equalParameterTypesAt(f1, f2, pragma[only_bind_into](i))
     )
   }
 
   predicate equalParameterTypesAt(FunctionDeclarationEntry f1, FunctionDeclarationEntry f2, int i) {
-    pragma[only_bind_out](f1.getDeclaration()) = pragma[only_bind_out](f2.getDeclaration()) and
-    TypeEquivalence<Config, FunctionSignatureType>::equalTypes(
-      f1.getParameterDeclarationEntry(i).getType(),
-      f2.getParameterDeclarationEntry(i).getType()
-    )
+    interestedInFunctions(f1, f2) and
+    f1.getDeclaration() = f2.getDeclaration() and
+    TypeEquivalence<Config, interestedInParameterTypes/2>::equalTypes(f1.getParameterDeclarationEntry(pragma[only_bind_into](i))
+          .getType(), f2.getParameterDeclarationEntry(pragma[only_bind_into](i)).getType())
   }
 }