diff --git a/change_notes/2026-02-21-share-implementation-of-m5-14-1.md b/change_notes/2026-02-21-share-implementation-of-m5-14-1.md new file mode 100644 index 0000000000..5d12697dbc --- /dev/null +++ b/change_notes/2026-02-21-share-implementation-of-m5-14-1.md @@ -0,0 +1,2 @@ + - `M5-14-1` - `RightHandOperandOfALogicalAndOperatorsContainSideEffects.ql`: + - Implementation has been refactor to share logic with Rule 8.14.1. No observable changes to results expected. \ No newline at end of file diff --git a/cpp/autosar/src/rules/M5-14-1/RightHandOperandOfALogicalAndOperatorsContainSideEffects.ql b/cpp/autosar/src/rules/M5-14-1/RightHandOperandOfALogicalAndOperatorsContainSideEffects.ql index 871e9828c8..2d8822543d 100644 --- a/cpp/autosar/src/rules/M5-14-1/RightHandOperandOfALogicalAndOperatorsContainSideEffects.ql +++ b/cpp/autosar/src/rules/M5-14-1/RightHandOperandOfALogicalAndOperatorsContainSideEffects.ql @@ -16,15 +16,14 @@ import cpp import codingstandards.cpp.autosar -import codingstandards.cpp.SideEffect -import codingstandards.cpp.sideeffect.DefaultEffects -import codingstandards.cpp.Expr +import codingstandards.cpp.rules.shortcircuitedpersistentsideeffectshared.ShortCircuitedPersistentSideEffectShared -from BinaryLogicalOperation op, Expr rhs -where - not isExcluded(op, - SideEffects1Package::rightHandOperandOfALogicalAndOperatorsContainSideEffectsQuery()) and - rhs = op.getRightOperand() and - hasSideEffect(rhs) and - not rhs instanceof UnevaluatedExprExtension -select op, "The $@ may have a side effect that is not always evaluated.", rhs, "right-hand operand" +module RightHandOperandOfALogicalAndOperatorsContainSideEffectsConfig implements + ShortCircuitedPersistentSideEffectSharedConfigSig +{ + Query getQuery() { + result = SideEffects1Package::rightHandOperandOfALogicalAndOperatorsContainSideEffectsQuery() + } +} + +import ShortCircuitedPersistentSideEffectShared diff --git a/cpp/autosar/test/rules/M5-14-1/RightHandOperandOfALogicalAndOperators.testref b/cpp/autosar/test/rules/M5-14-1/RightHandOperandOfALogicalAndOperators.testref new file mode 100644 index 0000000000..6b4788d133 --- /dev/null +++ b/cpp/autosar/test/rules/M5-14-1/RightHandOperandOfALogicalAndOperators.testref @@ -0,0 +1 @@ +cpp/common/test/rules/shortcircuitedpersistentsideeffectshared/ShortCircuitedPersistentSideEffectShared.ql \ No newline at end of file diff --git a/cpp/autosar/test/rules/M5-14-1/RightHandOperandOfALogicalAndOperatorsContainSideEffects.expected b/cpp/autosar/test/rules/M5-14-1/RightHandOperandOfALogicalAndOperatorsContainSideEffects.expected deleted file mode 100644 index 75b3a45304..0000000000 --- a/cpp/autosar/test/rules/M5-14-1/RightHandOperandOfALogicalAndOperatorsContainSideEffects.expected +++ /dev/null @@ -1,4 +0,0 @@ -| test.cpp:15:7:15:14 | ... \|\| ... | The $@ may have a side effect that is not always evaluated. | test.cpp:15:12:15:14 | ... ++ | right-hand operand | -| test.cpp:18:7:18:21 | ... \|\| ... | The $@ may have a side effect that is not always evaluated. | test.cpp:18:13:18:20 | ... == ... | right-hand operand | -| test.cpp:21:7:21:15 | ... \|\| ... | The $@ may have a side effect that is not always evaluated. | test.cpp:21:12:21:13 | call to f1 | right-hand operand | -| test.cpp:40:7:40:41 | ... \|\| ... | The $@ may have a side effect that is not always evaluated. | test.cpp:40:26:40:26 | call to operator== | right-hand operand | diff --git a/cpp/autosar/test/rules/M5-14-1/RightHandOperandOfALogicalAndOperatorsContainSideEffects.qlref b/cpp/autosar/test/rules/M5-14-1/RightHandOperandOfALogicalAndOperatorsContainSideEffects.qlref deleted file mode 100644 index c3f427556a..0000000000 --- a/cpp/autosar/test/rules/M5-14-1/RightHandOperandOfALogicalAndOperatorsContainSideEffects.qlref +++ /dev/null @@ -1 +0,0 @@ -rules/M5-14-1/RightHandOperandOfALogicalAndOperatorsContainSideEffects.ql \ No newline at end of file diff --git a/cpp/autosar/test/rules/M5-14-1/test.cpp b/cpp/autosar/test/rules/M5-14-1/test.cpp deleted file mode 100644 index 76fd08c2ca..0000000000 --- a/cpp/autosar/test/rules/M5-14-1/test.cpp +++ /dev/null @@ -1,42 +0,0 @@ -int i = 0; - -bool f1() { - i++; - return i == 1; -} - -bool f2() { - int i = 0; - return i++ == 1; -} - -void f3(bool b) { - int j = 0; - if (b || i++) { // NON_COMPLIANT - } - - if (b || (j == i++)) { // NON_COMPLIANT - } - - if (b || f1()) { // NON_COMPLIANT, f1 has global side-effects - } - - if (b || f2()) { // COMPLIANT, f2 has local side-effects - } -} - -int g1 = 0; -int f4() { return g1++; } -int f5() { return 1; } - -#include - -void f6() { - if (1 && sizeof(f4())) { - } // COMPLIANT - sizeof operands not evaluated - if (1 &&noexcept(f4()) &&noexcept(f4())) { - } // COMPLIANT - noexcept operands not evaluated - - if (1 || (typeid(f5()) == typeid(f4()))) { - } // NON_COMPLIANT - typeid operands not evaluated, but the ==operator is -} \ No newline at end of file diff --git a/cpp/common/src/codingstandards/cpp/exclusions/cpp/RuleMetadata.qll b/cpp/common/src/codingstandards/cpp/exclusions/cpp/RuleMetadata.qll index 10f4029904..8aab3e27c4 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 SideEffects5 import SmartPointers1 import SmartPointers2 import Statements @@ -134,6 +135,7 @@ newtype TCPPQuery = TScopePackageQuery(ScopeQuery q) or TSideEffects1PackageQuery(SideEffects1Query q) or TSideEffects2PackageQuery(SideEffects2Query q) or + TSideEffects5PackageQuery(SideEffects5Query 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 + isSideEffects5QueryMetadata(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/SideEffects5.qll b/cpp/common/src/codingstandards/cpp/exclusions/cpp/SideEffects5.qll new file mode 100644 index 0000000000..7f6e5caab3 --- /dev/null +++ b/cpp/common/src/codingstandards/cpp/exclusions/cpp/SideEffects5.qll @@ -0,0 +1,26 @@ +//** THIS FILE IS AUTOGENERATED, DO NOT MODIFY DIRECTLY. **/ +import cpp +import RuleMetadata +import codingstandards.cpp.exclusions.RuleMetadata + +newtype SideEffects5Query = TShortCircuitedPersistentSideEffectQuery() + +predicate isSideEffects5QueryMetadata(Query query, string queryId, string ruleId, string category) { + query = + // `Query` instance for the `shortCircuitedPersistentSideEffect` query + SideEffects5Package::shortCircuitedPersistentSideEffectQuery() and + queryId = + // `@id` for the `shortCircuitedPersistentSideEffect` query + "cpp/misra/short-circuited-persistent-side-effect" and + ruleId = "RULE-8-14-1" and + category = "advisory" +} + +module SideEffects5Package { + Query shortCircuitedPersistentSideEffectQuery() { + //autogenerate `Query` type + result = + // `Query` type for `shortCircuitedPersistentSideEffect` query + TQueryCPP(TSideEffects5PackageQuery(TShortCircuitedPersistentSideEffectQuery())) + } +} diff --git a/cpp/common/src/codingstandards/cpp/rules/shortcircuitedpersistentsideeffectshared/ShortCircuitedPersistentSideEffectShared.qll b/cpp/common/src/codingstandards/cpp/rules/shortcircuitedpersistentsideeffectshared/ShortCircuitedPersistentSideEffectShared.qll new file mode 100644 index 0000000000..b98518ebbb --- /dev/null +++ b/cpp/common/src/codingstandards/cpp/rules/shortcircuitedpersistentsideeffectshared/ShortCircuitedPersistentSideEffectShared.qll @@ -0,0 +1,32 @@ +/** + * Provides a configurable module ShortCircuitedPersistentSideEffectShared with a `problems` predicate + * for the following issue: + * The right-hand operand of a logical && or || operator should not contain persistent + * side effects, as this may violate developer intent and expectations. + */ + +import cpp +import codingstandards.cpp.Customizations +import codingstandards.cpp.Exclusions +import codingstandards.cpp.SideEffect +import codingstandards.cpp.sideeffect.DefaultEffects +import codingstandards.cpp.Expr + +signature module ShortCircuitedPersistentSideEffectSharedConfigSig { + Query getQuery(); +} + +module ShortCircuitedPersistentSideEffectShared< + ShortCircuitedPersistentSideEffectSharedConfigSig Config> +{ + query predicate problems( + BinaryLogicalOperation op, string message, Expr rhs, string rhsDescription + ) { + not isExcluded(op, Config::getQuery()) and + rhs = op.getRightOperand() and + hasSideEffect(rhs) and + not rhs instanceof UnevaluatedExprExtension and + message = "The $@ may have a side effect that is not always evaluated." and + rhsDescription = "right-hand operand" + } +} diff --git a/cpp/common/test/rules/shortcircuitedpersistentsideeffectshared/ShortCircuitedPersistentSideEffectShared.expected b/cpp/common/test/rules/shortcircuitedpersistentsideeffectshared/ShortCircuitedPersistentSideEffectShared.expected new file mode 100644 index 0000000000..46041335d1 --- /dev/null +++ b/cpp/common/test/rules/shortcircuitedpersistentsideeffectshared/ShortCircuitedPersistentSideEffectShared.expected @@ -0,0 +1,7 @@ +| test.cpp:20:7:20:14 | ... \|\| ... | The $@ may have a side effect that is not always evaluated. | test.cpp:20:12:20:14 | ... ++ | right-hand operand | +| test.cpp:23:7:23:21 | ... \|\| ... | The $@ may have a side effect that is not always evaluated. | test.cpp:23:13:23:20 | ... == ... | right-hand operand | +| test.cpp:26:7:26:15 | ... \|\| ... | The $@ may have a side effect that is not always evaluated. | test.cpp:26:12:26:13 | call to f1 | right-hand operand | +| test.cpp:29:7:29:16 | ... \|\| ... | The $@ may have a side effect that is not always evaluated. | test.cpp:29:12:29:13 | call to f3 | right-hand operand | +| test.cpp:45:7:45:41 | ... \|\| ... | The $@ may have a side effect that is not always evaluated. | test.cpp:45:26:45:26 | call to operator== | right-hand operand | +| test.cpp:56:7:56:15 | ... \|\| ... | The $@ may have a side effect that is not always evaluated. | test.cpp:56:12:56:13 | call to f8 | right-hand operand | +| test.cpp:66:7:66:16 | ... \|\| ... | The $@ may have a side effect that is not always evaluated. | test.cpp:66:12:66:14 | call to f10 | right-hand operand | diff --git a/cpp/common/test/rules/shortcircuitedpersistentsideeffectshared/ShortCircuitedPersistentSideEffectShared.ql b/cpp/common/test/rules/shortcircuitedpersistentsideeffectshared/ShortCircuitedPersistentSideEffectShared.ql new file mode 100644 index 0000000000..b68d3ddeaf --- /dev/null +++ b/cpp/common/test/rules/shortcircuitedpersistentsideeffectshared/ShortCircuitedPersistentSideEffectShared.ql @@ -0,0 +1,8 @@ +// GENERATED FILE - DO NOT MODIFY +import codingstandards.cpp.rules.shortcircuitedpersistentsideeffectshared.ShortCircuitedPersistentSideEffectShared + +module TestFileConfig implements ShortCircuitedPersistentSideEffectSharedConfigSig { + Query getQuery() { result instanceof TestQuery } +} + +import ShortCircuitedPersistentSideEffectShared diff --git a/cpp/common/test/rules/shortcircuitedpersistentsideeffectshared/test.cpp b/cpp/common/test/rules/shortcircuitedpersistentsideeffectshared/test.cpp new file mode 100644 index 0000000000..ab82688058 --- /dev/null +++ b/cpp/common/test/rules/shortcircuitedpersistentsideeffectshared/test.cpp @@ -0,0 +1,68 @@ +int i = 0; + +bool f1() { + i++; + return i == 1; +} + +bool f2() { + int i = 0; + return i++ == 1; +} + +bool f3(int &i) { + i++; + return i == 1; +} + +void f4(bool b) { + int j = 0; + if (b || i++) { // NON_COMPLIANT + } + + if (b || (j == i++)) { // NON_COMPLIANT + } + + if (b || f1()) { // NON_COMPLIANT, f1 has global side-effects + } + + if (b || f3(j)) { // NON_COMPLIANT, f3 has local side-effects + } +} + +int g1 = 0; +int f5() { return g1++; } +int f6() { return 1; } + +#include + +void f7() { + if (1 && sizeof(f5())) { + } // COMPLIANT - sizeof operands not evaluated + if (1 &&noexcept(f5()) &&noexcept(f5())) { + } // COMPLIANT - noexcept operands not evaluated + + if (1 || (typeid(f6()) == typeid(f5()))) { + } // NON_COMPLIANT - typeid operands not evaluated, but the ==operator is +} + +bool f8() { + static int k = 0; + k++; + return k == 1; +} + +void f9(bool b) { + if (b || f8()) { // NON_COMPLIANT, f8 has static side-effects + } +} + +bool f10() { + volatile bool m = 0; + return m; +} + +void f11(bool b) { + if (b || f10()) { // NON_COMPLIANT, f10 has volatile side-effects + } +} \ No newline at end of file diff --git a/cpp/misra/src/rules/RULE-8-14-1/ShortCircuitedPersistentSideEffect.ql b/cpp/misra/src/rules/RULE-8-14-1/ShortCircuitedPersistentSideEffect.ql new file mode 100644 index 0000000000..4dfd77a42c --- /dev/null +++ b/cpp/misra/src/rules/RULE-8-14-1/ShortCircuitedPersistentSideEffect.ql @@ -0,0 +1,28 @@ +/** + * @id cpp/misra/short-circuited-persistent-side-effect + * @name RULE-8-14-1: The right-hand operand of a logical && or || operator should not contain persistent side effects + * @description The right-hand operand of a logical && or || operator should not contain persistent + * side effects, as such side effects will only conditionally occur, which may violate + * developer intent and/or expectations. + * @kind problem + * @precision high + * @problem.severity error + * @tags external/misra/id/rule-8-14-1 + * scope/system + * correctness + * readability + * external/misra/enforcement/undecidable + * external/misra/obligation/advisory + */ + +import cpp +import codingstandards.cpp.misra +import codingstandards.cpp.rules.shortcircuitedpersistentsideeffectshared.ShortCircuitedPersistentSideEffectShared + +module ShortCircuitedPersistentSideEffectConfig implements + ShortCircuitedPersistentSideEffectSharedConfigSig +{ + Query getQuery() { result = SideEffects5Package::shortCircuitedPersistentSideEffectQuery() } +} + +import ShortCircuitedPersistentSideEffectShared diff --git a/cpp/misra/test/rules/RULE-8-14-1/ShortCircuitedPersistentSideEffect.testref b/cpp/misra/test/rules/RULE-8-14-1/ShortCircuitedPersistentSideEffect.testref new file mode 100644 index 0000000000..6b4788d133 --- /dev/null +++ b/cpp/misra/test/rules/RULE-8-14-1/ShortCircuitedPersistentSideEffect.testref @@ -0,0 +1 @@ +cpp/common/test/rules/shortcircuitedpersistentsideeffectshared/ShortCircuitedPersistentSideEffectShared.ql \ No newline at end of file diff --git a/rule_packages/cpp/SideEffects1.json b/rule_packages/cpp/SideEffects1.json index 587a6ceb66..f4f5ed2be8 100644 --- a/rule_packages/cpp/SideEffects1.json +++ b/rule_packages/cpp/SideEffects1.json @@ -39,6 +39,7 @@ "precision": "high", "severity": "warning", "short_name": "RightHandOperandOfALogicalAndOperatorsContainSideEffects", + "shared_implementation_short_name": "ShortCircuitedPersistentSideEffectShared", "tags": [ "correctness" ] diff --git a/rule_packages/cpp/SideEffects5.json b/rule_packages/cpp/SideEffects5.json new file mode 100644 index 0000000000..53496ba0be --- /dev/null +++ b/rule_packages/cpp/SideEffects5.json @@ -0,0 +1,27 @@ +{ + "MISRA-C++-2023": { + "RULE-8-14-1": { + "properties": { + "enforcement": "undecidable", + "obligation": "advisory" + }, + "queries": [ + { + "description": "The right-hand operand of a logical && or || operator should not contain persistent side effects, as such side effects will only conditionally occur, which may violate developer intent and/or expectations.", + "kind": "problem", + "name": "The right-hand operand of a logical && or || operator should not contain persistent side effects", + "precision": "high", + "severity": "error", + "short_name": "ShortCircuitedPersistentSideEffect", + "shared_implementation_short_name": "ShortCircuitedPersistentSideEffectShared", + "tags": [ + "scope/system", + "correctness", + "readability" + ] + } + ], + "title": "The right-hand operand of a logical && or || operator should not contain persistent side effects" + } + } +} \ No newline at end of file diff --git a/rules.csv b/rules.csv index 08fa09a573..0cd38c6482 100644 --- a/rules.csv +++ b/rules.csv @@ -898,7 +898,7 @@ cpp,MISRA-C++-2023,RULE-8-3-2,Yes,Advisory,Decidable,Single Translation Unit,The cpp,MISRA-C++-2023,RULE-8-7-1,Yes,Required,Undecidable,System,Pointer arithmetic shall not form an invalid pointer,ARR30-C,Memory1,Easy, cpp,MISRA-C++-2023,RULE-8-7-2,Yes,Required,Undecidable,System,Subtraction between pointers shall only be applied to pointers that address elements of the same array,ARR36-C,Memory2,Easy, cpp,MISRA-C++-2023,RULE-8-9-1,Yes,Required,Undecidable,System,"The built-in relational operators >, >=, < and <= shall not be applied to objects of pointer type, except where they point to elements of the same array",ARR36-C,Memory3,Easy, -cpp,MISRA-C++-2023,RULE-8-14-1,Yes,Advisory,Undecidable,System,The right-hand operand of a logical && or operator should not contain persistent side effects,"M5-14-1, RULE-13-5",SideEffects3,Medium, +cpp,MISRA-C++-2023,RULE-8-14-1,Yes,Advisory,Undecidable,System,The right-hand operand of a logical && or operator should not contain persistent side effects,"M5-14-1, RULE-13-5",SideEffects5,Medium, cpp,MISRA-C++-2023,RULE-8-18-1,Yes,Mandatory,Undecidable,System,An object or subobject must not be copied to an overlapping object,"M0-2-1, RULE-19-1",Memory4,Hard, cpp,MISRA-C++-2023,RULE-8-18-2,Yes,Advisory,Decidable,Single Translation Unit,The result of an assignment operator should not be used,RULE-13-4,ImportMisra23,Import, cpp,MISRA-C++-2023,RULE-8-19-1,Yes,Advisory,Decidable,Single Translation Unit,The comma operator should not be used,M5-18-1,ImportMisra23,Import,