Merge pull request #891 from github/michaelrfairhurst/compatible-types-performance-fix-alt

Michaelrfairhurst/compatible types performance fix alt

Author: knewbury01

c/cert/src/rules/DCL40-C/IncompatibleFunctionDeclarations.ql

@@ -19,6 +19,12 @@ import codingstandards.c.cert
 import codingstandards.cpp.types.Compatible
 import ExternalIdentifiers
 
+predicate interestedInFunctions(FunctionDeclarationEntry f1, FunctionDeclarationEntry f2) {
+  not f1 = f2 and
+  f1.getDeclaration() = f2.getDeclaration() and
+  f1.getName() = f2.getName()
+}
+
 from ExternalIdentifiers d, FunctionDeclarationEntry f1, FunctionDeclarationEntry f2
 where
   not isExcluded(f1, Declarations2Package::incompatibleFunctionDeclarationsQuery()) and
@@ -29,10 +35,12 @@ where
   f1.getName() = f2.getName() and
   (
     //return type check
-    not FunctionDeclarationTypeEquivalence<TypesCompatibleConfig>::equalReturnTypes(f1, f2)
+    not FunctionDeclarationTypeEquivalence<TypesCompatibleConfig, interestedInFunctions/2>::equalReturnTypes(f1,
+      f2)
     or
     //parameter type check
-    not FunctionDeclarationTypeEquivalence<TypesCompatibleConfig>::equalParameterTypes(f1, f2)
+    not FunctionDeclarationTypeEquivalence<TypesCompatibleConfig, interestedInFunctions/2>::equalParameterTypes(f1,
+      f2)
   ) and
   // Apply ordering on start line, trying to avoid the optimiser applying this join too early
   // in the pipeline

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,11 @@ import cpp
 import codingstandards.c.misra
 import codingstandards.cpp.types.Compatible
 
+predicate interestedInFunctions(FunctionDeclarationEntry f1, FunctionDeclarationEntry f2) {
+  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 +29,14 @@ 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,17 @@ 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.getDeclaration() = f2.getDeclaration() and
+  // This condition should always hold, but removing it affects join order performance.
+  f1.getName() = f2.getName() and
+  not f2.isDefinition() and
+  not f1.isFromTemplateInstantiation(_) and
+  not f2.isFromTemplateInstantiation(_)
+}
+
 from FunctionDeclarationEntry f1
 where
   not isExcluded(f1, Declarations4Package::compatibleDeclarationFunctionDefinedQuery()) and
@@ -38,10 +49,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 compute equality in two stages. Firstly, all pairs of possible type comparisons (including recursive comparisons) are found, then those pairwise comparisons are evaluated in a second stage. This greatly reduces the number of comparisons and greatly improves performance.
+ - `RULE-23-5`: `DangerousDefaultSelectionForPointerInGeneric.ql`:
+   - Altered the module `SimpleAssignment.qll` in accordance with the changes to `Compatible.qll`.