From ed088be3717f0722f83e37f52e68c69300b5e2a9 Mon Sep 17 00:00:00 2001
From: Mike Fairhurst <michaelrfairhurst@github.com>
Date: Fri, 25 Apr 2025 08:12:40 -0700
Subject: [PATCH 1/4] Fix performance regression from Compatible.qll
---
cpp/common/src/codingstandards/cpp/types/Compatible.qll | 1 +
1 file changed, 1 insertion(+)
diff --git a/cpp/common/src/codingstandards/cpp/types/Compatible.qll b/cpp/common/src/codingstandards/cpp/types/Compatible.qll
index d6f65126e..7ea58d766 100644
--- a/cpp/common/src/codingstandards/cpp/types/Compatible.qll
+++ b/cpp/common/src/codingstandards/cpp/types/Compatible.qll
@@ -69,6 +69,7 @@ module TypesCompatibleConfig implements TypeEquivalenceSig {
/**
* Utilize QlBuiltins::InternSets to efficiently compare the sets of specifiers on two types.
*/
+bindingset[t1, t2]
private predicate specifiersMatchExactly(Type t1, Type t2) {
t1 = t2
or
From 472c93a89b47f0b7c8492c6ea12edd7fe70e616a Mon Sep 17 00:00:00 2001
From: Mike Fairhurst <michaelrfairhurst@github.com>
Date: Fri, 25 Apr 2025 16:29:32 -0700
Subject: [PATCH 2/4] Add changes to join order and prevent cartesian product
through rewrite
---
.../codingstandards/cpp/types/Compatible.qll | 18 ++++++++++++++----
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/cpp/common/src/codingstandards/cpp/types/Compatible.qll b/cpp/common/src/codingstandards/cpp/types/Compatible.qll
index 7ea58d766..33b283082 100644
--- a/cpp/common/src/codingstandards/cpp/types/Compatible.qll
+++ b/cpp/common/src/codingstandards/cpp/types/Compatible.qll
@@ -53,8 +53,11 @@ module TypesCompatibleConfig implements TypeEquivalenceSig {
or
// Enum types are compatible with one of char, int, or signed int, but the implementation
// decides.
- [t1, t2] instanceof Enum and
- ([t1, t2] instanceof CharType or [t1, t2] instanceof IntType)
+ t1 instanceof Enum and
+ (t2 instanceof CharType or t2 instanceof IntType)
+ or
+ t2 instanceof Enum and
+ (t1 instanceof CharType or t1 instanceof IntType)
}
bindingset[t1, t2]
@@ -348,8 +351,15 @@ module FunctionDeclarationTypeEquivalence<TypeEquivalenceSig Config> {
predicate equalParameterTypes(FunctionDeclarationEntry f1, FunctionDeclarationEntry f2) {
f1.getDeclaration() = f2.getDeclaration() and
forall(int i | exists([f1, f2].getParameterDeclarationEntry(i)) |
- TypeEquivalence<Config, FunctionSignatureType>::equalTypes(f1.getParameterDeclarationEntry(i)
- .getType(), f2.getParameterDeclarationEntry(i).getType())
+ 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()
)
}
}
From 7f6b32d4c14035cc9eaf1bbb18fd12a3ff3c816c Mon Sep 17 00:00:00 2001
From: Mike Fairhurst <michaelrfairhurst@github.com>
Date: Sun, 27 Apr 2025 00:44:21 -0700
Subject: [PATCH 3/4] 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.
---
...rousDefaultSelectionForPointerInGeneric.ql | 20 ++-
.../DeclarationsOfAFunctionSameNameAndType.ql | 10 +-
.../DeclarationsOfAnObjectSameNameAndType.ql | 19 ++-
.../CompatibleDeclarationFunctionDefined.ql | 16 ++-
.../CompatibleDeclarationObjectDefined.ql | 16 +--
...-25-improve-type-comparison-performance.md | 6 +
.../codingstandards/cpp/types/Compatible.qll | 127 ++++++++++++++----
.../cpp/types/SimpleAssignment.qll | 65 ++++++---
8 files changed, 199 insertions(+), 80 deletions(-)
create mode 100644 change_notes/2025-04-25-improve-type-comparison-performance.md
diff --git a/c/misra/src/rules/RULE-23-5/DangerousDefaultSelectionForPointerInGeneric.ql b/c/misra/src/rules/RULE-23-5/DangerousDefaultSelectionForPointerInGeneric.ql
index a009ba1b2..f2961e263 100644
--- a/c/misra/src/rules/RULE-23-5/DangerousDefaultSelectionForPointerInGeneric.ql
+++ b/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)
)
}
diff --git a/c/misra/src/rules/RULE-8-3/DeclarationsOfAFunctionSameNameAndType.ql b/c/misra/src/rules/RULE-8-3/DeclarationsOfAFunctionSameNameAndType.ql
index 2de2e4fd0..0be634784 100644
--- a/c/misra/src/rules/RULE-8-3/DeclarationsOfAFunctionSameNameAndType.ql
+++ b/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
diff --git a/c/misra/src/rules/RULE-8-3/DeclarationsOfAnObjectSameNameAndType.ql b/c/misra/src/rules/RULE-8-3/DeclarationsOfAnObjectSameNameAndType.ql
index 12ff583b6..36a84b3b9 100644
--- a/c/misra/src/rules/RULE-8-3/DeclarationsOfAnObjectSameNameAndType.ql
+++ b/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() +
diff --git a/c/misra/src/rules/RULE-8-4/CompatibleDeclarationFunctionDefined.ql b/c/misra/src/rules/RULE-8-4/CompatibleDeclarationFunctionDefined.ql
index 98876ad1b..2f17dd508 100644
--- a/c/misra/src/rules/RULE-8-4/CompatibleDeclarationFunctionDefined.ql
+++ b/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)
diff --git a/c/misra/src/rules/RULE-8-4/CompatibleDeclarationObjectDefined.ql b/c/misra/src/rules/RULE-8-4/CompatibleDeclarationObjectDefined.ql
index 613ce5680..bed30d673 100644
--- a/c/misra/src/rules/RULE-8-4/CompatibleDeclarationObjectDefined.ql
+++ b/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."
diff --git a/change_notes/2025-04-25-improve-type-comparison-performance.md b/change_notes/2025-04-25-improve-type-comparison-performance.md
new file mode 100644
index 000000000..91a019bdf
--- /dev/null
+++ b/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`.
\ No newline at end of file
diff --git a/cpp/common/src/codingstandards/cpp/types/Compatible.qll b/cpp/common/src/codingstandards/cpp/types/Compatible.qll
index 33b283082..db77765b5 100644
--- a/cpp/common/src/codingstandards/cpp/types/Compatible.qll
+++ b/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,7 +363,7 @@ 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)
)
}
@@ -318,12 +371,12 @@ module TypeEquivalence<TypeEquivalenceSig Config, TypeSubset T> {
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,18 +390,41 @@ 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))
@@ -356,11 +432,10 @@ module FunctionDeclarationTypeEquivalence<TypeEquivalenceSig Config> {
}
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())
}
}
diff --git a/cpp/common/src/codingstandards/cpp/types/SimpleAssignment.qll b/cpp/common/src/codingstandards/cpp/types/SimpleAssignment.qll
index 4f7a85c80..a31400a34 100644
--- a/cpp/common/src/codingstandards/cpp/types/SimpleAssignment.qll
+++ b/cpp/common/src/codingstandards/cpp/types/SimpleAssignment.qll
@@ -8,42 +8,67 @@
import codingstandards.cpp.types.LvalueConversion
import codingstandards.cpp.types.Compatible
-module SimpleAssignment<TypeSubset T> {
- final private class FinalType = Type;
-
- private class RelevantType extends FinalType {
- RelevantType() { exists(T t | typeGraph*(t, this) or typeGraph(getLvalueConverted(t), this)) }
-
- string toString() { result = "relevant type" }
- }
-
+module SimpleAssignment<interestedInEquality/2 checksAssignment> {
/**
* Whether a pair of qualified or unqualified pointer types satisfy the simple assignment
* constraints from 6.5.16.1.
*
* There are additional constraints not implemented here involving one or more arithmetic types.
*/
- predicate satisfiesSimplePointerAssignment(RelevantType left, RelevantType right) {
+ predicate satisfiesSimplePointerAssignment(Type left, Type right) {
+ checksAssignment(left, right) and
simplePointerAssignmentImpl(getLvalueConverted(left), right)
}
+ private predicate satisfiedWhenTypesCompatible(Type left, Type right, Type checkA, Type checkB) {
+ interestedInTypes(left, right) and
+ exists(Type leftBase, Type rightBase |
+ // The left operand has atomic, qualified, or unqualified pointer type:
+ leftBase = left.stripTopLevelSpecifiers().(PointerType).getBaseType() and
+ rightBase = right.stripTopLevelSpecifiers().(PointerType).getBaseType() and
+ (
+ // and both operands are pointers to qualified or unqualified versions of compatible types:
+ checkA = leftBase.stripTopLevelSpecifiers() and
+ checkB = rightBase.stripTopLevelSpecifiers()
+ ) and
+ // and the type pointed to by the left has all the qualifiers of the type pointed to by the
+ // right:
+ forall(Specifier s | s = rightBase.getASpecifier() | s = leftBase.getASpecifier())
+ )
+ }
+
+ predicate interestedInTypes(Type left, Type right) {
+ exists(Type unconverted |
+ left = getLvalueConverted(unconverted) and
+ checksAssignment(unconverted, right)
+ )
+ }
+
+ predicate checksCompatibility(Type left, Type right) {
+ // Check if the types are compatible
+ exists(Type assignA, Type assignB |
+ checksAssignment(assignA, assignB) and
+ satisfiedWhenTypesCompatible(assignA, assignB, left, right)
+ )
+ }
+
/**
* Implementation of 6.5.16.1 for a pair of pointer types, that assumes lvalue conversion has been
* performed on the left operand.
*/
- private predicate simplePointerAssignmentImpl(RelevantType left, RelevantType right) {
- exists(RelevantType leftBase, RelevantType rightBase |
+ bindingset[left, right]
+ private predicate simplePointerAssignmentImpl(Type left, Type right) {
+ exists(Type checkA, Type checkB |
+ satisfiedWhenTypesCompatible(left, right, checkA, checkB) and
+ TypeEquivalence<TypesCompatibleConfig, checksCompatibility/2>::equalTypes(checkA, checkB)
+ )
+ or
+ exists(Type leftBase, Type rightBase |
// The left operand has atomic, qualified, or unqualified pointer type:
leftBase = left.stripTopLevelSpecifiers().(PointerType).getBaseType() and
rightBase = right.stripTopLevelSpecifiers().(PointerType).getBaseType() and
- (
- // and both operands are pointers to qualified or unqualified versions of compatible types:
- TypeEquivalence<TypesCompatibleConfig, RelevantType>::equalTypes(leftBase
- .stripTopLevelSpecifiers(), rightBase.stripTopLevelSpecifiers())
- or
- // or one operand is a pointer to a qualified or unqualified version of void
- [leftBase, rightBase].stripTopLevelSpecifiers() instanceof VoidType
- ) and
+ // or one operand is a pointer to a qualified or unqualified version of void
+ [leftBase, rightBase].stripTopLevelSpecifiers() instanceof VoidType and
// and the type pointed to by the left has all the qualifiers of the type pointed to by the
// right:
forall(Specifier s | s = rightBase.getASpecifier() | s = leftBase.getASpecifier())
From 48257745c8e553c125aa876e9755f959888bfc4a Mon Sep 17 00:00:00 2001
From: Mike Fairhurst <michaelrfairhurst@github.com>
Date: Sun, 27 Apr 2025 01:49:42 -0700
Subject: [PATCH 4/4] Add missing file changed
---
.../DCL40-C/IncompatibleFunctionDeclarations.ql | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/c/cert/src/rules/DCL40-C/IncompatibleFunctionDeclarations.ql b/c/cert/src/rules/DCL40-C/IncompatibleFunctionDeclarations.ql
index 95ef0fd68..8cab442e5 100644
--- a/c/cert/src/rules/DCL40-C/IncompatibleFunctionDeclarations.ql
+++ b/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