From f381feb963aefd90effab6ca561ca5a1c175afc8 Mon Sep 17 00:00:00 2001 From: Mike Fairhurst Date: Sun, 22 Feb 2026 11:41:29 -0800 Subject: [PATCH 1/5] Initial draft for non-const function objects. --- .../cpp/exclusions/cpp/RuleMetadata.qll | 3 + .../cpp/exclusions/cpp/SideEffects6.qll | 44 +++++++++++++ .../codingstandards/cpp/types/Predicate.qll | 66 +++++++++++++++++++ .../codingstandards/cpp/types/Templates.qll | 49 ++++++++++++++ .../NonConstPredicateFunctionObject.ql | 33 ++++++++++ .../PredicateWithPersistentSideEffects.ql | 41 ++++++++++++ .../NonConstPredicateFunctionObject.expected | 1 + .../NonConstPredicateFunctionObject.qlref | 1 + ...redicateWithPersistentSideEffects.expected | 1 + .../PredicateWithPersistentSideEffects.qlref | 1 + cpp/misra/test/rules/RULE-28-3-1/test.cpp | 43 ++++++++++++ rule_packages/cpp/SideEffects6.json | 41 ++++++++++++ rules.csv | 2 +- 13 files changed, 325 insertions(+), 1 deletion(-) create mode 100644 cpp/common/src/codingstandards/cpp/exclusions/cpp/SideEffects6.qll create mode 100644 cpp/common/src/codingstandards/cpp/types/Predicate.qll create mode 100644 cpp/common/src/codingstandards/cpp/types/Templates.qll create mode 100644 cpp/misra/src/rules/RULE-28-3-1/NonConstPredicateFunctionObject.ql create mode 100644 cpp/misra/src/rules/RULE-28-3-1/PredicateWithPersistentSideEffects.ql create mode 100644 cpp/misra/test/rules/RULE-28-3-1/NonConstPredicateFunctionObject.expected create mode 100644 cpp/misra/test/rules/RULE-28-3-1/NonConstPredicateFunctionObject.qlref create mode 100644 cpp/misra/test/rules/RULE-28-3-1/PredicateWithPersistentSideEffects.expected create mode 100644 cpp/misra/test/rules/RULE-28-3-1/PredicateWithPersistentSideEffects.qlref create mode 100644 cpp/misra/test/rules/RULE-28-3-1/test.cpp create mode 100644 rule_packages/cpp/SideEffects6.json diff --git a/cpp/common/src/codingstandards/cpp/exclusions/cpp/RuleMetadata.qll b/cpp/common/src/codingstandards/cpp/exclusions/cpp/RuleMetadata.qll index 10f4029904..507e730307 100644 --- a/cpp/common/src/codingstandards/cpp/exclusions/cpp/RuleMetadata.qll +++ b/cpp/common/src/codingstandards/cpp/exclusions/cpp/RuleMetadata.qll @@ -61,6 +61,7 @@ import Representation import Scope import SideEffects1 import SideEffects2 +import SideEffects6 import SmartPointers1 import SmartPointers2 import Statements @@ -134,6 +135,7 @@ newtype TCPPQuery = TScopePackageQuery(ScopeQuery q) or TSideEffects1PackageQuery(SideEffects1Query q) or TSideEffects2PackageQuery(SideEffects2Query q) or + TSideEffects6PackageQuery(SideEffects6Query q) or TSmartPointers1PackageQuery(SmartPointers1Query q) or TSmartPointers2PackageQuery(SmartPointers2Query q) or TStatementsPackageQuery(StatementsQuery q) or @@ -207,6 +209,7 @@ predicate isQueryMetadata(Query query, string queryId, string ruleId, string cat isScopeQueryMetadata(query, queryId, ruleId, category) or isSideEffects1QueryMetadata(query, queryId, ruleId, category) or isSideEffects2QueryMetadata(query, queryId, ruleId, category) or + isSideEffects6QueryMetadata(query, queryId, ruleId, category) or isSmartPointers1QueryMetadata(query, queryId, ruleId, category) or isSmartPointers2QueryMetadata(query, queryId, ruleId, category) or isStatementsQueryMetadata(query, queryId, ruleId, category) or diff --git a/cpp/common/src/codingstandards/cpp/exclusions/cpp/SideEffects6.qll b/cpp/common/src/codingstandards/cpp/exclusions/cpp/SideEffects6.qll new file mode 100644 index 0000000000..05564b71ca --- /dev/null +++ b/cpp/common/src/codingstandards/cpp/exclusions/cpp/SideEffects6.qll @@ -0,0 +1,44 @@ +//** THIS FILE IS AUTOGENERATED, DO NOT MODIFY DIRECTLY. **/ +import cpp +import RuleMetadata +import codingstandards.cpp.exclusions.RuleMetadata + +newtype SideEffects6Query = + TPredicateWithPersistentSideEffectsQuery() or + TNonConstPredicateFunctionObjectQuery() + +predicate isSideEffects6QueryMetadata(Query query, string queryId, string ruleId, string category) { + query = + // `Query` instance for the `predicateWithPersistentSideEffects` query + SideEffects6Package::predicateWithPersistentSideEffectsQuery() and + queryId = + // `@id` for the `predicateWithPersistentSideEffects` query + "cpp/misra/predicate-with-persistent-side-effects" and + ruleId = "RULE-28-3-1" and + category = "required" + or + query = + // `Query` instance for the `nonConstPredicateFunctionObject` query + SideEffects6Package::nonConstPredicateFunctionObjectQuery() and + queryId = + // `@id` for the `nonConstPredicateFunctionObject` query + "cpp/misra/non-const-predicate-function-object" and + ruleId = "RULE-28-3-1" and + category = "required" +} + +module SideEffects6Package { + Query predicateWithPersistentSideEffectsQuery() { + //autogenerate `Query` type + result = + // `Query` type for `predicateWithPersistentSideEffects` query + TQueryCPP(TSideEffects6PackageQuery(TPredicateWithPersistentSideEffectsQuery())) + } + + Query nonConstPredicateFunctionObjectQuery() { + //autogenerate `Query` type + result = + // `Query` type for `nonConstPredicateFunctionObject` query + TQueryCPP(TSideEffects6PackageQuery(TNonConstPredicateFunctionObjectQuery())) + } +} diff --git a/cpp/common/src/codingstandards/cpp/types/Predicate.qll b/cpp/common/src/codingstandards/cpp/types/Predicate.qll new file mode 100644 index 0000000000..865bf2ac3c --- /dev/null +++ b/cpp/common/src/codingstandards/cpp/types/Predicate.qll @@ -0,0 +1,66 @@ +import cpp +private import codingstandards.cpp.StdNamespace +private import codingstandards.cpp.types.Templates +private import semmle.code.cpp.dataflow.new.DataFlow + +Namespace getTemplateParameterNamespace(TypeTemplateParameter param) { + exists(Declaration decl | + param = decl.(TemplateClass).getATemplateArgument() or + param = decl.(TemplateVariable).getATemplateArgument() or + param = decl.(TemplateFunction).getATemplateArgument() + | + result = decl.getNamespace() + ) +} + +class PredicateType extends TypeTemplateParameter { + PredicateType() { + this.getName().matches(["%Compare%", "%Predicate%"]) and + getTemplateParameterNamespace(this) instanceof StdNS + } + + Type getASubstitutedType(Substitution sub) { result = sub.getSubstitutedTypeForParameter(this) } +} + +class PredicateFunctionObject extends Class { + PredicateType pred; + Function operator; + Substitution sub; + + PredicateFunctionObject() { + this = pred.getASubstitutedType(sub) and + operator.getDeclaringType() = this and + operator.getName() = "operator()" + } + + PredicateType getPredicateType() { result = pred } + + Function getCallOperator() { result = operator } + + Substitution getSubstitution() { result = sub } +} + +class PredicateFunctionPointerUse extends FunctionAccess { + Expr functionPointerArgument; + FunctionCall templateFunctionCall; + FunctionTemplateInstantiation instantiation; + Substitution sub; + PredicateType pred; + Parameter parameter; + int index; + + PredicateFunctionPointerUse() { + functionPointerArgument = templateFunctionCall.getArgument(index) and + templateFunctionCall.getTarget() = instantiation and + parameter = instantiation.getParameter(index) and + sub.asFunctionSubstitution() = instantiation and + parameter.getType() = sub.getSubstitutedTypeForParameter(pred) and + exists(DataFlow::Node func, DataFlow::Node arg | + func.asExpr() = this and + arg.asExpr() = functionPointerArgument and + DataFlow::localFlow(func, arg) + ) + } + + PredicateType getPredicateType() { result = pred } +} diff --git a/cpp/common/src/codingstandards/cpp/types/Templates.qll b/cpp/common/src/codingstandards/cpp/types/Templates.qll new file mode 100644 index 0000000000..acd6f199df --- /dev/null +++ b/cpp/common/src/codingstandards/cpp/types/Templates.qll @@ -0,0 +1,49 @@ +import cpp + +private newtype TSubstitution = + TClassSubstitution(ClassTemplateInstantiation cti) or + TFunctionSubstitution(FunctionTemplateInstantiation fti) or + TVariableSubstitution(VariableTemplateInstantiation vti) + +class Substitution extends TSubstitution { + ClassTemplateInstantiation asClassSubstitution() { this = TClassSubstitution(result) } + + FunctionTemplateInstantiation asFunctionSubstitution() { this = TFunctionSubstitution(result) } + + VariableTemplateInstantiation asVariableSubstitution() { this = TVariableSubstitution(result) } + + TypeTemplateParameter getTemplateParameter(int index) { + result = this.asClassSubstitution().getTemplate().getTemplateArgument(index) or + result = this.asFunctionSubstitution().getTemplate().getTemplateArgument(index) or + result = this.asVariableSubstitution().getTemplate().getTemplateArgument(index) + } + + Type getSubstitutedType(int index) { + result = this.asClassSubstitution().getTemplateArgument(index) or + result = this.asFunctionSubstitution().getTemplateArgument(index) or + result = this.asVariableSubstitution().getTemplateArgument(index) + } + + Type getSubstitutedTypeForParameter(TypeTemplateParameter param) { + exists(int idx | + this.getTemplateParameter(idx) = param and + result = this.getSubstitutedType(idx) + ) + } + + string toString() { + result = this.asClassSubstitution().toString() or + result = this.asFunctionSubstitution().toString() or + result = this.asVariableSubstitution().toString() + } + + Locatable getASubstitutionSite() { + result.(TypeMention).getMentionedType() = this.asClassSubstitution() + or + result.(Call).getTarget() = this.asFunctionSubstitution() + or + result.(FunctionAccess).getTarget() = this.asFunctionSubstitution() + or + result.(VariableAccess).getTarget() = this.asVariableSubstitution() + } +} diff --git a/cpp/misra/src/rules/RULE-28-3-1/NonConstPredicateFunctionObject.ql b/cpp/misra/src/rules/RULE-28-3-1/NonConstPredicateFunctionObject.ql new file mode 100644 index 0000000000..030bf24f39 --- /dev/null +++ b/cpp/misra/src/rules/RULE-28-3-1/NonConstPredicateFunctionObject.ql @@ -0,0 +1,33 @@ +/** + * @id cpp/misra/non-const-predicate-function-object + * @name RULE-28-3-1: Predicates shall not have persistent side effects + * @description Much of the behavior of predicates is implementation defined, such as how and when + * it is invoked with which argument values, and if it is copied or moved. Therefore, + * predicate function objects should be declared const. + * @kind problem + * @precision very-high + * @problem.severity error + * @tags external/misra/id/rule-28-3-1 + * scope/system + * correctness + * maintainability + * portability + * external/misra/enforcement/undecidable + * external/misra/obligation/required + */ + +import cpp +import codingstandards.cpp.misra +import codingstandards.cpp.SideEffect +import codingstandards.cpp.types.Predicate + +from MemberFunction callOperator, PredicateFunctionObject obj, Locatable usageSite +where + not isExcluded([callOperator, usageSite], + SideEffects6Package::nonConstPredicateFunctionObjectQuery()) and + obj.getSubstitution().getASubstitutionSite() = usageSite and + callOperator = obj.getCallOperator() and + not callOperator instanceof ConstMemberFunction +select usageSite, "Predicate usage of $@ has $@", callOperator.getDeclaringType(), + "callable object " + callOperator.getDeclaringType().getName(), callOperator, + "non const operator()." diff --git a/cpp/misra/src/rules/RULE-28-3-1/PredicateWithPersistentSideEffects.ql b/cpp/misra/src/rules/RULE-28-3-1/PredicateWithPersistentSideEffects.ql new file mode 100644 index 0000000000..304a4a2488 --- /dev/null +++ b/cpp/misra/src/rules/RULE-28-3-1/PredicateWithPersistentSideEffects.ql @@ -0,0 +1,41 @@ +/** + * @id cpp/misra/predicate-with-persistent-side-effects + * @name RULE-28-3-1: Predicates shall not have persistent side effects + * @description Much of the behavior of predicates is implementation defined, such as how and when + * it is invoked with which argument values, and if it is copied or moved. Therefore, + * persistent side effects in a predicate cannot be relied upon and should not occur. + * @kind problem + * @precision very-high + * @problem.severity error + * @tags external/misra/id/rule-28-3-1 + * scope/system + * correctness + * maintainability + * portability + * external/misra/enforcement/undecidable + * external/misra/obligation/required + */ + +import cpp +import codingstandards.cpp.misra +import codingstandards.cpp.SideEffect +import codingstandards.cpp.types.Predicate + +predicate functionHasSideEffect(Function f) { + hasExternalOrGlobalSideEffectInFunction(f) +} + +predicate isPredicateObject(PredicateFunctionObject obj, PredicateType pred, Locatable usageSite, Function callOperator) { + obj.getSubstitution().getASubstitutionSite() = usageSite and + pred = obj.getPredicateType() and + callOperator = obj.getCallOperator() +} + +predicate isPredicateFunctionPointerUse(PredicateFunctionPointerUse predPtrUse, PredicateType pred, Function func) { + pred = predPtrUse.getPredicateType() and + func = predPtrUse.getTarget() +} + +where + not isExcluded(x, SideEffects6Package::predicateWithPersistentSideEffectsQuery()) and +select diff --git a/cpp/misra/test/rules/RULE-28-3-1/NonConstPredicateFunctionObject.expected b/cpp/misra/test/rules/RULE-28-3-1/NonConstPredicateFunctionObject.expected new file mode 100644 index 0000000000..c60f9bdfec --- /dev/null +++ b/cpp/misra/test/rules/RULE-28-3-1/NonConstPredicateFunctionObject.expected @@ -0,0 +1 @@ +| test.cpp:21:3:21:11 | call to sort | Predicate usage of $@ has $@ | test.cpp:10:8:10:9 | F1 | callable object F1 | test.cpp:11:8:11:17 | operator() | non const operator(). | diff --git a/cpp/misra/test/rules/RULE-28-3-1/NonConstPredicateFunctionObject.qlref b/cpp/misra/test/rules/RULE-28-3-1/NonConstPredicateFunctionObject.qlref new file mode 100644 index 0000000000..a540a2284b --- /dev/null +++ b/cpp/misra/test/rules/RULE-28-3-1/NonConstPredicateFunctionObject.qlref @@ -0,0 +1 @@ +rules/RULE-28-3-1/NonConstPredicateFunctionObject.ql \ No newline at end of file diff --git a/cpp/misra/test/rules/RULE-28-3-1/PredicateWithPersistentSideEffects.expected b/cpp/misra/test/rules/RULE-28-3-1/PredicateWithPersistentSideEffects.expected new file mode 100644 index 0000000000..2ec1a0ac6c --- /dev/null +++ b/cpp/misra/test/rules/RULE-28-3-1/PredicateWithPersistentSideEffects.expected @@ -0,0 +1 @@ +No expected results have yet been specified \ No newline at end of file diff --git a/cpp/misra/test/rules/RULE-28-3-1/PredicateWithPersistentSideEffects.qlref b/cpp/misra/test/rules/RULE-28-3-1/PredicateWithPersistentSideEffects.qlref new file mode 100644 index 0000000000..cbfe07b956 --- /dev/null +++ b/cpp/misra/test/rules/RULE-28-3-1/PredicateWithPersistentSideEffects.qlref @@ -0,0 +1 @@ +rules/RULE-28-3-1/PredicateWithPersistentSideEffects.ql \ No newline at end of file diff --git a/cpp/misra/test/rules/RULE-28-3-1/test.cpp b/cpp/misra/test/rules/RULE-28-3-1/test.cpp new file mode 100644 index 0000000000..4b60713c07 --- /dev/null +++ b/cpp/misra/test/rules/RULE-28-3-1/test.cpp @@ -0,0 +1,43 @@ +#include +#include +#include +// #include +#include + +// Test cases for Rule 28.3.1#NonConstPredicateFunctionObject +// This query checks that predicate function objects have const operator() + +struct F1 { + bool operator()(std::int32_t l1, std::int32_t l2) { return l1 > l2; } +}; + +void test_function_object_non_const_in_set() { + // TODO: implement stubs for set comparator. + // std::set l1; // NON_COMPLIANT +} + +void test_function_object_non_const_in_sort() { + std::vector l1 = {5, 2, 8, 1, 9}; + std::sort(l1.begin(), l1.end(), F1{}); // NON_COMPLIANT +} + +struct F2 { + bool operator()(std::int32_t l1, std::int32_t l2) const { return l1 > l2; } +}; + +void test_function_object_const_in_set() { + // TODO: implement stubs for set comparator. + // std::set l1; // COMPLIANT +} + +void test_function_object_const_in_sort() { + std::vector l1 = {5, 2, 8, 1, 9}; + std::sort(l1.begin(), l1.end(), F2{}); // COMPLIANT +} + +// Compliant: Using standard library predicates (always have const operator()) +void test_standard_library_predicates() { + std::vector l1 = {1, 2, 3, 4, 5}; + // TODO: implement stubs for greater comparator. + // std::sort(l1.begin(), l1.end(), std::greater()); // COMPLIANT +} \ No newline at end of file diff --git a/rule_packages/cpp/SideEffects6.json b/rule_packages/cpp/SideEffects6.json new file mode 100644 index 0000000000..e82475b1ba --- /dev/null +++ b/rule_packages/cpp/SideEffects6.json @@ -0,0 +1,41 @@ +{ + "MISRA-C++-2023": { + "RULE-28-3-1": { + "properties": { + "enforcement": "undecidable", + "obligation": "required" + }, + "queries": [ + { + "description": "Much of the behavior of predicates is implementation defined, such as how and when it is invoked with which argument values, and if it is copied or moved. Therefore, persistent side effects in a predicate cannot be relied upon and should not occur.", + "kind": "problem", + "name": "Predicates shall not have persistent side effects", + "precision": "very-high", + "severity": "error", + "short_name": "PredicateWithPersistentSideEffects", + "tags": [ + "scope/system", + "correctness", + "maintainability", + "portability" + ] + }, + { + "description": "Much of the behavior of predicates is implementation defined, such as how and when it is invoked with which argument values, and if it is copied or moved. Therefore, predicate function objects should be declared const.", + "kind": "problem", + "name": "Predicates shall not have persistent side effects", + "precision": "very-high", + "severity": "error", + "short_name": "NonConstPredicateFunctionObject", + "tags": [ + "scope/system", + "correctness", + "maintainability", + "portability" + ] + } + ], + "title": "Predicates shall not have persistent side effects" + } + } +} \ No newline at end of file diff --git a/rules.csv b/rules.csv index 08fa09a573..0f32d8e8f4 100644 --- a/rules.csv +++ b/rules.csv @@ -995,7 +995,7 @@ cpp,MISRA-C++-2023,RULE-25-5-1,Yes,Required,Decidable,Single Translation Unit,Th cpp,MISRA-C++-2023,RULE-25-5-2,Yes,Mandatory,Decidable,Single Translation Unit,"The pointers returned by the C++ Standard Library functions localeconv, getenv, setlocale or strerror must only be used as if they have pointer to const-qualified type",RULE-21-19,ImportMisra23,Import, cpp,MISRA-C++-2023,RULE-25-5-3,Yes,Mandatory,Undecidable,System,"The pointer returned by the C++ Standard Library functions asctime, ctime, gmtime, localtime, localeconv, getenv, setlocale or strerror must not be used following a subsequent call to the same function",RULE-21-20,ImportMisra23,Import, cpp,MISRA-C++-2023,RULE-26-3-1,Yes,Advisory,Decidable,Single Translation Unit,std::vector should not be specialized with bool,A18-1-2,ImportMisra23,Import, -cpp,MISRA-C++-2023,RULE-28-3-1,Yes,Required,Undecidable,System,Predicates shall not have persistent side effects,A25-1-1,SideEffects3,Easy, +cpp,MISRA-C++-2023,RULE-28-3-1,Yes,Required,Undecidable,System,Predicates shall not have persistent side effects,A25-1-1,SideEffects6,Easy, cpp,MISRA-C++-2023,RULE-28-6-1,Yes,Required,Decidable,Single Translation Unit,The argument to std::move shall be a non-const lvalue,A18-9-3,Preconditions,Easy, cpp,MISRA-C++-2023,RULE-28-6-2,Yes,Required,Decidable,Single Translation Unit,Forwarding references and std::forward shall be used together,A18-9-2,ImportMisra23,Import, cpp,MISRA-C++-2023,RULE-28-6-3,Yes,Required,Decidable,Single Translation Unit,An object shall not be used while in a potentially moved-from state,A12-8-3,ImportMisra23,Import, From 8dba8132e44547582ac906c328051d410c947fd2 Mon Sep 17 00:00:00 2001 From: Mike Fairhurst Date: Sun, 22 Feb 2026 12:20:05 -0800 Subject: [PATCH 2/5] Next query first pass working --- .../PredicateWithPersistentSideEffects.ql | 36 +++++----- ...redicateWithPersistentSideEffects.expected | 4 +- cpp/misra/test/rules/RULE-28-3-1/test.cpp | 66 +++++++++++++++++++ 3 files changed, 88 insertions(+), 18 deletions(-) diff --git a/cpp/misra/src/rules/RULE-28-3-1/PredicateWithPersistentSideEffects.ql b/cpp/misra/src/rules/RULE-28-3-1/PredicateWithPersistentSideEffects.ql index 304a4a2488..aac2e36c77 100644 --- a/cpp/misra/src/rules/RULE-28-3-1/PredicateWithPersistentSideEffects.ql +++ b/cpp/misra/src/rules/RULE-28-3-1/PredicateWithPersistentSideEffects.ql @@ -18,24 +18,26 @@ import cpp import codingstandards.cpp.misra +import codingstandards.cpp.sideeffect.DefaultEffects import codingstandards.cpp.SideEffect import codingstandards.cpp.types.Predicate -predicate functionHasSideEffect(Function f) { - hasExternalOrGlobalSideEffectInFunction(f) -} - -predicate isPredicateObject(PredicateFunctionObject obj, PredicateType pred, Locatable usageSite, Function callOperator) { - obj.getSubstitution().getASubstitutionSite() = usageSite and - pred = obj.getPredicateType() and - callOperator = obj.getCallOperator() -} - -predicate isPredicateFunctionPointerUse(PredicateFunctionPointerUse predPtrUse, PredicateType pred, Function func) { - pred = predPtrUse.getPredicateType() and - func = predPtrUse.getTarget() -} - +from Locatable usageSite, Function f, SideEffect effect where - not isExcluded(x, SideEffects6Package::predicateWithPersistentSideEffectsQuery()) and -select + not isExcluded([usageSite, effect], SideEffects6Package::predicateWithPersistentSideEffectsQuery()) and + effect = getAnExternalOrGlobalSideEffectInFunction(f) and + not effect instanceof ConstructorFieldInit and + ( + // Case 1: a function pointer used directly as a predicate argument + exists(PredicateFunctionPointerUse use | + use = usageSite and + f = use.getTarget() + ) + or + // Case 2: a function object whose call operator has side effects + exists(PredicateFunctionObject obj | + usageSite = obj.getSubstitution().getASubstitutionSite() and + f = obj.getCallOperator() + ) + ) +select usageSite, "Predicate $@ has a $@.", f, f.getName(), effect, "persistent side effect" diff --git a/cpp/misra/test/rules/RULE-28-3-1/PredicateWithPersistentSideEffects.expected b/cpp/misra/test/rules/RULE-28-3-1/PredicateWithPersistentSideEffects.expected index 2ec1a0ac6c..a9b8910044 100644 --- a/cpp/misra/test/rules/RULE-28-3-1/PredicateWithPersistentSideEffects.expected +++ b/cpp/misra/test/rules/RULE-28-3-1/PredicateWithPersistentSideEffects.expected @@ -1 +1,3 @@ -No expected results have yet been specified \ No newline at end of file +| test.cpp:60:35:60:49 | cmp_with_static | Predicate $@ has a $@. | test.cpp:52:6:52:20 | cmp_with_static | cmp_with_static | test.cpp:54:3:54:11 | ++ ... | persistent side effect | +| test.cpp:73:35:73:49 | cmp_with_global | Predicate $@ has a $@. | test.cpp:66:6:66:20 | cmp_with_global | cmp_with_global | test.cpp:67:3:67:26 | ++ ... | persistent side effect | +| test.cpp:86:3:86:11 | call to sort | Predicate $@ has a $@. | test.cpp:78:8:78:17 | operator() | operator() | test.cpp:79:5:79:28 | ++ ... | persistent side effect | diff --git a/cpp/misra/test/rules/RULE-28-3-1/test.cpp b/cpp/misra/test/rules/RULE-28-3-1/test.cpp index 4b60713c07..bc4309f16d 100644 --- a/cpp/misra/test/rules/RULE-28-3-1/test.cpp +++ b/cpp/misra/test/rules/RULE-28-3-1/test.cpp @@ -40,4 +40,70 @@ void test_standard_library_predicates() { std::vector l1 = {1, 2, 3, 4, 5}; // TODO: implement stubs for greater comparator. // std::sort(l1.begin(), l1.end(), std::greater()); // COMPLIANT +} + +// ============================================================================= +// Test cases for Rule 28.3.1#PredicateWithPersistentSideEffects +// This query checks that predicates do not have persistent (global/static) +// side effects. +// ============================================================================= + +// Non-compliant: free function predicate modifying a static local variable +bool cmp_with_static(std::int32_t l1, std::int32_t l2) { + static std::int32_t g_count = 0; + ++g_count; // NON_COMPLIANT + return l1 < l2; +} + +void test_predicate_fn_static_local() { + std::vector l1 = {5, 2, 8, 1, 9}; + std::sort(l1.begin(), l1.end(), cmp_with_static); // NON_COMPLIANT +} + +// Non-compliant: free function predicate modifying a global variable +std::int32_t g_predicate_call_count = 0; + +bool cmp_with_global(std::int32_t l1, std::int32_t l2) { + ++g_predicate_call_count; // NON_COMPLIANT + return l1 < l2; +} + +void test_predicate_fn_global() { + std::vector l1 = {5, 2, 8, 1, 9}; + std::sort(l1.begin(), l1.end(), cmp_with_global); // NON_COMPLIANT +} + +// Non-compliant: function object whose operator() modifies a global variable +struct F3_SideEffect { + bool operator()(std::int32_t l1, std::int32_t l2) const { + ++g_predicate_call_count; // NON_COMPLIANT + return l1 < l2; + } +}; + +void test_function_object_with_global_side_effect() { + std::vector l1 = {5, 2, 8, 1, 9}; + std::sort(l1.begin(), l1.end(), F3_SideEffect{}); // NON_COMPLIANT +} + +// Compliant: free function predicate with no side effects +bool cmp_pure(std::int32_t l1, std::int32_t l2) { // COMPLIANT + return l1 < l2; +} + +void test_predicate_fn_pure() { + std::vector l1 = {5, 2, 8, 1, 9}; + std::sort(l1.begin(), l1.end(), cmp_pure); // COMPLIANT +} + +// Compliant: function object with const operator() and no side effects +struct F4_Pure { + bool operator()(std::int32_t l1, std::int32_t l2) const { // COMPLIANT + return l1 < l2; + } +}; + +void test_function_object_pure() { + std::vector l1 = {5, 2, 8, 1, 9}; + std::sort(l1.begin(), l1.end(), F4_Pure{}); // COMPLIANT } \ No newline at end of file From 1b83422c5a3a50571a09f336eff343dbce57ba43 Mon Sep 17 00:00:00 2001 From: Mike Fairhurst Date: Sun, 22 Feb 2026 16:06:13 -0800 Subject: [PATCH 3/5] Add portability as an allowed tag --- schemas/rule-package.schema.json | 1 + 1 file changed, 1 insertion(+) diff --git a/schemas/rule-package.schema.json b/schemas/rule-package.schema.json index fff79fede0..78714e08f7 100644 --- a/schemas/rule-package.schema.json +++ b/schemas/rule-package.schema.json @@ -336,6 +336,7 @@ "security", "concurrency", "performance", + "portability", "external/cert/audit", "external/autosar/audit", "external/autosar/default-disabled", From dd8135c5368cdd23454b9b0b5f1318f7ba043771 Mon Sep 17 00:00:00 2001 From: Mike Fairhurst Date: Sun, 22 Feb 2026 16:53:23 -0800 Subject: [PATCH 4/5] Add comments to new shared APIs. --- .../src/codingstandards/cpp/ast/Templates.qll | 16 +++++ .../codingstandards/cpp/types/Predicate.qll | 72 ++++++++++++++++--- .../codingstandards/cpp/types/Templates.qll | 31 ++++++++ 3 files changed, 109 insertions(+), 10 deletions(-) create mode 100644 cpp/common/src/codingstandards/cpp/ast/Templates.qll diff --git a/cpp/common/src/codingstandards/cpp/ast/Templates.qll b/cpp/common/src/codingstandards/cpp/ast/Templates.qll new file mode 100644 index 0000000000..c668739f81 --- /dev/null +++ b/cpp/common/src/codingstandards/cpp/ast/Templates.qll @@ -0,0 +1,16 @@ +import cpp + +/** + * A predicate to simplify getting a namespace for a template parameter, since + * `TemplateParameterType`'s `getNamespace()` has no result, and `enclosingElement()` has no result, + * and there are multiple cases to navigate to work around this. + */ +Namespace getTemplateParameterNamespace(TypeTemplateParameter param) { + exists(Declaration decl | + param = decl.(TemplateClass).getATemplateArgument() or + param = decl.(TemplateVariable).getATemplateArgument() or + param = decl.(TemplateFunction).getATemplateArgument() + | + result = decl.getNamespace() + ) +} diff --git a/cpp/common/src/codingstandards/cpp/types/Predicate.qll b/cpp/common/src/codingstandards/cpp/types/Predicate.qll index 865bf2ac3c..1dc69c997d 100644 --- a/cpp/common/src/codingstandards/cpp/types/Predicate.qll +++ b/cpp/common/src/codingstandards/cpp/types/Predicate.qll @@ -1,27 +1,51 @@ +/** + * A library for handling "predicate" types, which are function parameters in the standard library. + * + * For example, `std::sort` takes a predicate as its third argument, and `std::set` takes a + * predicate as its second template parameter. Predicates are always template parameters, and we + * can identify them by their name -- this is what MISRA expects us to do. + */ + import cpp private import codingstandards.cpp.StdNamespace +private import codingstandards.cpp.ast.Templates private import codingstandards.cpp.types.Templates private import semmle.code.cpp.dataflow.new.DataFlow -Namespace getTemplateParameterNamespace(TypeTemplateParameter param) { - exists(Declaration decl | - param = decl.(TemplateClass).getATemplateArgument() or - param = decl.(TemplateVariable).getATemplateArgument() or - param = decl.(TemplateFunction).getATemplateArgument() - | - result = decl.getNamespace() - ) -} - +/** + * A "predicate" type parameter as defined by MISRA, which is a standard library function named + * "Compare" or "Predicate" or "BinaryPredicate" in the standard library. + * + * To be more widely useful, we match more flexibly on the name, as `_Compare` is also common, etc. + * + * To get a particular `Type` that is _used_ as a predicate type, see `getASubstitutedType()`, + * rather than the type parameter itself, see `getASubstitutedType()`. + */ class PredicateType extends TypeTemplateParameter { PredicateType() { this.getName().matches(["%Compare%", "%Predicate%"]) and getTemplateParameterNamespace(this) instanceof StdNS } + /** + * Get a type that is used (anywhere, via some substitution) as this predicate type parameter. + * + * For example, `std::sort(..., ..., [](int a, int b) { return a < b; })` creates a `Closure` + * type, and substitutes that closure type for the predicate type parameter of `std::sort`. + */ Type getASubstitutedType(Substitution sub) { result = sub.getSubstitutedTypeForParameter(this) } } +/** + * A class type that has been substituted for a predicate type parameter, and has an `operator()` + * member function. + * + * For example, the closure type in `std::sort(..., ..., [](int a, int b) { return a < b; })` is a + * `PredicateFunctionObject`, and so is any `std::less` that is used as a predicate type + * parameter, etc. + * + * This does not cover function pointer types, as these are not class types. + */ class PredicateFunctionObject extends Class { PredicateType pred; Function operator; @@ -33,13 +57,38 @@ class PredicateFunctionObject extends Class { operator.getName() = "operator()" } + /** + * Get the predicate type parameter that this function object is being substituted for. + */ PredicateType getPredicateType() { result = pred } + /** + * Get the `operator()` function that this function object defines. This is the function that will + * be invoked and essentially defines the predicate behavior. + */ Function getCallOperator() { result = operator } + /** + * Get the `Substitution` object that makes this type a `PredicateFunctionObject`. + * + * This is a particular instantiation of some template that contains a predicate type parameter, + * which is substituted by this type in that instantiation. The `Substitution` object may refer + * to a `TemplateClass`, `TemplateVariable`, or `TemplateFunction`. + */ Substitution getSubstitution() { result = sub } } +/** + * Gets a function access where the purpose of that access is to pass the accessed function as a + * predicate argument to a standard library template. + * + * For example, in `std::sort(..., ..., myCompare)`, where `myCompare` is a function, then + * `myCompare` will be converted into a function pointer and that function pointer will be used as + * a predicate in that `std::sort` call. + * + * This is more complex to identify than `PredicateFunctionObject` because the addressee of the + * function pointer is not necessarily statically known. + */ class PredicateFunctionPointerUse extends FunctionAccess { Expr functionPointerArgument; FunctionCall templateFunctionCall; @@ -62,5 +111,8 @@ class PredicateFunctionPointerUse extends FunctionAccess { ) } + /** + * Get the predicate type parameter that this function pointer is being substituted for. + */ PredicateType getPredicateType() { result = pred } } diff --git a/cpp/common/src/codingstandards/cpp/types/Templates.qll b/cpp/common/src/codingstandards/cpp/types/Templates.qll index acd6f199df..ac821cfb40 100644 --- a/cpp/common/src/codingstandards/cpp/types/Templates.qll +++ b/cpp/common/src/codingstandards/cpp/types/Templates.qll @@ -5,6 +5,13 @@ private newtype TSubstitution = TFunctionSubstitution(FunctionTemplateInstantiation fti) or TVariableSubstitution(VariableTemplateInstantiation vti) +/** + * A class to facilitate working with template substitutions, especially since templates may be a + * `TemplateClass`, `TemplateFunction`, or `TemplateVariable`, which complicates their usage. + * + * A `Substitution` in particular refers to an instantiation of that template of some kind, and + * allows analysis of which parameters were substituted with which types in that instatiation. + */ class Substitution extends TSubstitution { ClassTemplateInstantiation asClassSubstitution() { this = TClassSubstitution(result) } @@ -12,18 +19,34 @@ class Substitution extends TSubstitution { VariableTemplateInstantiation asVariableSubstitution() { this = TVariableSubstitution(result) } + /** + * Get the nth template parameter type of the template that is being substituted. + * + * For example, in `std::vector`, the 0th template parameter is `typename T`. + */ TypeTemplateParameter getTemplateParameter(int index) { result = this.asClassSubstitution().getTemplate().getTemplateArgument(index) or result = this.asFunctionSubstitution().getTemplate().getTemplateArgument(index) or result = this.asVariableSubstitution().getTemplate().getTemplateArgument(index) } + /** + * Get the type that is substituting the nth template parameter in this substitution. + * + * For example, in `std::vector`, the 0th substituted type is `int`. + */ Type getSubstitutedType(int index) { result = this.asClassSubstitution().getTemplateArgument(index) or result = this.asFunctionSubstitution().getTemplateArgument(index) or result = this.asVariableSubstitution().getTemplateArgument(index) } + /** + * Get the type that is substituting the given template parameter in this substitution. + * + * For example, in `std::vector`, this predicate matches the given type `int` with the type + * parameter `typename T`. + */ Type getSubstitutedTypeForParameter(TypeTemplateParameter param) { exists(int idx | this.getTemplateParameter(idx) = param and @@ -37,6 +60,14 @@ class Substitution extends TSubstitution { result = this.asVariableSubstitution().toString() } + /** + * Get a `Locatable` that represents a where this substitution was declared in the source code. + * + * The result may be a `TypeMention`, `Call`, etc. depending on the kind of template and how it is + * being used, but it handles the various template cases for you. + * + * Note that this instantiation may have been declared in multiple places. + */ Locatable getASubstitutionSite() { result.(TypeMention).getMentionedType() = this.asClassSubstitution() or From 372fd8f6d83122d6757e0a106732a7fb19a6c246 Mon Sep 17 00:00:00 2001 From: Mike Fairhurst Date: Sun, 22 Feb 2026 17:19:09 -0800 Subject: [PATCH 5/5] Fix stubs to add std::set back, and std::less --- .../test/includes/standard-library/deque.h | 1 + .../test/includes/standard-library/functional.h | 16 ++++++++++++++++ .../test/includes/standard-library/iosfwd.h | 1 + cpp/common/test/includes/standard-library/map | 3 ++- .../test/includes/standard-library/memory.h | 6 ++++++ cpp/common/test/includes/standard-library/set | 3 ++- .../test/includes/standard-library/string | 7 +------ .../test/includes/standard-library/vector.h | 1 + .../NonConstPredicateFunctionObject.expected | 3 ++- .../PredicateWithPersistentSideEffects.expected | 6 +++--- cpp/misra/test/rules/RULE-28-3-1/test.cpp | 17 ++++------------- 11 files changed, 39 insertions(+), 25 deletions(-) diff --git a/cpp/common/test/includes/standard-library/deque.h b/cpp/common/test/includes/standard-library/deque.h index 00b44b704a..93db52ef93 100644 --- a/cpp/common/test/includes/standard-library/deque.h +++ b/cpp/common/test/includes/standard-library/deque.h @@ -2,6 +2,7 @@ #define _GHLIBCPP_DEQUE #include #include +#include "memory.h" namespace std { template > class deque { diff --git a/cpp/common/test/includes/standard-library/functional.h b/cpp/common/test/includes/standard-library/functional.h index 78d9c47745..acdb5d020d 100644 --- a/cpp/common/test/includes/standard-library/functional.h +++ b/cpp/common/test/includes/standard-library/functional.h @@ -90,5 +90,21 @@ template class function { template function(F &&); template function &operator=(F &&); }; + +template +struct less { + bool operator()(const T &x, const T &y) const; + typedef T first_argument_type; + typedef T second_argument_type; + typedef bool result_type; +}; + +template +struct greater { + bool operator()(const T &x, const T &y) const; + typedef T first_argument_type; + typedef T second_argument_type; + typedef bool result_type; +}; } // namespace std #endif \ No newline at end of file diff --git a/cpp/common/test/includes/standard-library/iosfwd.h b/cpp/common/test/includes/standard-library/iosfwd.h index a8fb1d305d..f292938c60 100644 --- a/cpp/common/test/includes/standard-library/iosfwd.h +++ b/cpp/common/test/includes/standard-library/iosfwd.h @@ -1,5 +1,6 @@ #ifndef _GHLIBCPP_IOSFWD #define _GHLIBCPP_IOSFWD +#include "memory.h" namespace std { template class char_traits; template <> class char_traits; diff --git a/cpp/common/test/includes/standard-library/map b/cpp/common/test/includes/standard-library/map index 74d952f554..9b092f625f 100644 --- a/cpp/common/test/includes/standard-library/map +++ b/cpp/common/test/includes/standard-library/map @@ -1,5 +1,6 @@ #include "iterator.h" -#include +#include "memory.h" +#include "functional.h" namespace std { diff --git a/cpp/common/test/includes/standard-library/memory.h b/cpp/common/test/includes/standard-library/memory.h index 494f428422..df41cac4fd 100644 --- a/cpp/common/test/includes/standard-library/memory.h +++ b/cpp/common/test/includes/standard-library/memory.h @@ -128,6 +128,12 @@ class bad_alloc : public exception { bad_alloc &operator=(const bad_alloc &) noexcept; virtual const char *what() const noexcept; }; + +template class allocator { +public: + allocator() throw(); + typedef size_t size_type; +}; } // namespace std #endif // _GHLIBCPP_MEMORY \ No newline at end of file diff --git a/cpp/common/test/includes/standard-library/set b/cpp/common/test/includes/standard-library/set index 46a0ab1c3f..e23eb8461a 100644 --- a/cpp/common/test/includes/standard-library/set +++ b/cpp/common/test/includes/standard-library/set @@ -1,5 +1,6 @@ #include "iterator.h" -#include +#include "memory.h" +#include "functional.h" namespace std { template , diff --git a/cpp/common/test/includes/standard-library/string b/cpp/common/test/includes/standard-library/string index 3f60c1838c..13dccf15d0 100644 --- a/cpp/common/test/includes/standard-library/string +++ b/cpp/common/test/includes/standard-library/string @@ -1,6 +1,7 @@ #ifndef _GHLIBCPP_STRING #define _GHLIBCPP_STRING #include "cwchar" +#include "memory.h" #include "initializer_list" #include "ios.h" #include "iosfwd.h" @@ -88,12 +89,6 @@ template <> struct char_traits { static int_type eof(); }; -template class allocator { -public: - allocator() throw(); - typedef size_t size_type; -}; - template , class Allocator = allocator> class basic_string { diff --git a/cpp/common/test/includes/standard-library/vector.h b/cpp/common/test/includes/standard-library/vector.h index 6d0293f8f5..37272e164d 100644 --- a/cpp/common/test/includes/standard-library/vector.h +++ b/cpp/common/test/includes/standard-library/vector.h @@ -2,6 +2,7 @@ #define _GHLIBCPP_VECTOR #include #include +#include "memory.h" namespace std { diff --git a/cpp/misra/test/rules/RULE-28-3-1/NonConstPredicateFunctionObject.expected b/cpp/misra/test/rules/RULE-28-3-1/NonConstPredicateFunctionObject.expected index c60f9bdfec..241dedd279 100644 --- a/cpp/misra/test/rules/RULE-28-3-1/NonConstPredicateFunctionObject.expected +++ b/cpp/misra/test/rules/RULE-28-3-1/NonConstPredicateFunctionObject.expected @@ -1 +1,2 @@ -| test.cpp:21:3:21:11 | call to sort | Predicate usage of $@ has $@ | test.cpp:10:8:10:9 | F1 | callable object F1 | test.cpp:11:8:11:17 | operator() | non const operator(). | +| test.cpp:15:8:15:10 | type mention | Predicate usage of $@ has $@ | test.cpp:10:8:10:9 | F1 | callable object F1 | test.cpp:11:8:11:17 | operator() | non const operator(). | +| test.cpp:20:3:20:11 | call to sort | Predicate usage of $@ has $@ | test.cpp:10:8:10:9 | F1 | callable object F1 | test.cpp:11:8:11:17 | operator() | non const operator(). | diff --git a/cpp/misra/test/rules/RULE-28-3-1/PredicateWithPersistentSideEffects.expected b/cpp/misra/test/rules/RULE-28-3-1/PredicateWithPersistentSideEffects.expected index a9b8910044..0296b0a611 100644 --- a/cpp/misra/test/rules/RULE-28-3-1/PredicateWithPersistentSideEffects.expected +++ b/cpp/misra/test/rules/RULE-28-3-1/PredicateWithPersistentSideEffects.expected @@ -1,3 +1,3 @@ -| test.cpp:60:35:60:49 | cmp_with_static | Predicate $@ has a $@. | test.cpp:52:6:52:20 | cmp_with_static | cmp_with_static | test.cpp:54:3:54:11 | ++ ... | persistent side effect | -| test.cpp:73:35:73:49 | cmp_with_global | Predicate $@ has a $@. | test.cpp:66:6:66:20 | cmp_with_global | cmp_with_global | test.cpp:67:3:67:26 | ++ ... | persistent side effect | -| test.cpp:86:3:86:11 | call to sort | Predicate $@ has a $@. | test.cpp:78:8:78:17 | operator() | operator() | test.cpp:79:5:79:28 | ++ ... | persistent side effect | +| test.cpp:51:35:51:49 | cmp_with_static | Predicate $@ has a $@. | test.cpp:43:6:43:20 | cmp_with_static | cmp_with_static | test.cpp:45:3:45:11 | ++ ... | persistent side effect | +| test.cpp:64:35:64:49 | cmp_with_global | Predicate $@ has a $@. | test.cpp:57:6:57:20 | cmp_with_global | cmp_with_global | test.cpp:58:3:58:26 | ++ ... | persistent side effect | +| test.cpp:77:3:77:11 | call to sort | Predicate $@ has a $@. | test.cpp:69:8:69:17 | operator() | operator() | test.cpp:70:5:70:28 | ++ ... | persistent side effect | diff --git a/cpp/misra/test/rules/RULE-28-3-1/test.cpp b/cpp/misra/test/rules/RULE-28-3-1/test.cpp index bc4309f16d..281c34ba25 100644 --- a/cpp/misra/test/rules/RULE-28-3-1/test.cpp +++ b/cpp/misra/test/rules/RULE-28-3-1/test.cpp @@ -1,7 +1,7 @@ #include #include #include -// #include +#include #include // Test cases for Rule 28.3.1#NonConstPredicateFunctionObject @@ -12,8 +12,7 @@ struct F1 { }; void test_function_object_non_const_in_set() { - // TODO: implement stubs for set comparator. - // std::set l1; // NON_COMPLIANT + std::set l1; // NON_COMPLIANT } void test_function_object_non_const_in_sort() { @@ -26,8 +25,7 @@ struct F2 { }; void test_function_object_const_in_set() { - // TODO: implement stubs for set comparator. - // std::set l1; // COMPLIANT + std::set l1; // COMPLIANT } void test_function_object_const_in_sort() { @@ -38,16 +36,9 @@ void test_function_object_const_in_sort() { // Compliant: Using standard library predicates (always have const operator()) void test_standard_library_predicates() { std::vector l1 = {1, 2, 3, 4, 5}; - // TODO: implement stubs for greater comparator. - // std::sort(l1.begin(), l1.end(), std::greater()); // COMPLIANT + std::sort(l1.begin(), l1.end(), std::greater()); // COMPLIANT } -// ============================================================================= -// Test cases for Rule 28.3.1#PredicateWithPersistentSideEffects -// This query checks that predicates do not have persistent (global/static) -// side effects. -// ============================================================================= - // Non-compliant: free function predicate modifying a static local variable bool cmp_with_static(std::int32_t l1, std::int32_t l2) { static std::int32_t g_count = 0;