diff --git a/cpp/common/src/codingstandards/cpp/Loops.qll b/cpp/common/src/codingstandards/cpp/Loops.qll index aa3dc64ea..a18c8e34a 100644 --- a/cpp/common/src/codingstandards/cpp/Loops.qll +++ b/cpp/common/src/codingstandards/cpp/Loops.qll @@ -374,3 +374,43 @@ predicate isInvalidLoop(ForStmt forLoop, string reason, Locatable reasonLocation reason = "its $@ is not a boolean" and reasonLabel = "loop control variable" } + +/** + * A comparison expression that has the minimum qualification as being a valid termination + * condition of a legacy for-loop. It is characterized by a value read from a variable being + * compared to a value, which is supposed to be the loop bound. + */ +class LegacyForLoopCondition extends RelationalOperation { + /** + * The legacy for-loop this relational operation is a condition of. + */ + ForStmt forLoop; + VariableAccess loopCounter; + Expr loopBound; + + LegacyForLoopCondition() { + this = forLoop.getCondition() and + exists(Expr loopCounterExpr | + loopCounterExpr = this.getAnOperand() and + loopBound = this.getAnOperand() and + loopCounter = loopCounterExpr.getAChild*() and + loopCounter.getTarget() = getAnIterationVariable(forLoop) and + loopBound != loopCounterExpr + ) + } + + /** + * Gets the for-loop this expression is a termination condition of. + */ + ForStmt getForLoop() { result = forLoop } + + /** + * Gets the variable access to the loop counter variable, appearing in this loop condition. + */ + VariableAccess getLoopCounter() { result = loopCounter } + + /** + * Gets the variable access to the loop bound variable, appearing in this loop condition. + */ + Expr getLoopBound() { result = loopBound } +} diff --git a/cpp/common/src/codingstandards/cpp/ast/Increment.qll b/cpp/common/src/codingstandards/cpp/ast/Increment.qll new file mode 100644 index 000000000..00c893baa --- /dev/null +++ b/cpp/common/src/codingstandards/cpp/ast/Increment.qll @@ -0,0 +1,76 @@ +/** + * Provides a library for working with expressions that update the value + * of a numeric variable by incrementing or decrementing it by a certain + * amount. + */ + +import cpp + +private class AssignAddOrSubExpr extends AssignArithmeticOperation { + AssignAddOrSubExpr() { + this instanceof AssignAddExpr or + this instanceof AssignSubExpr + } +} + +private class AddOrSubExpr extends BinaryArithmeticOperation { + AddOrSubExpr() { + this instanceof AddExpr or + this instanceof SubExpr + } +} + +/** + * An expression that updates a numeric variable by adding to or subtracting + * from it a certain amount. + */ +abstract class StepCrementUpdateExpr extends Expr { + /** + * The expression in the abstract syntax tree that represents the amount of + * value by which the variable is updated. + */ + abstract Expr getAmountExpr(); +} + +/** + * An increment or decrement operator application, either postfix or prefix. + */ +class PostfixOrPrefixCrementExpr extends CrementOperation, StepCrementUpdateExpr { + override Expr getAmountExpr() { none() } +} + +/** + * An add-then-assign or subtract-then-assign expression in a shortened form, + * i.e. `+=` or `-=`. + */ +class AssignAddOrSubUpdateExpr extends AssignAddOrSubExpr, StepCrementUpdateExpr { + override Expr getAmountExpr() { result = this.getRValue() } +} + +/** + * An add-then-assign expression or a subtract-then-assign expression, i.e. + * `x = x + E` or `x = x - E`, where `x` is some variable and `E` an + * arbitrary expression. + */ +class AddOrSubThenAssignExpr extends AssignExpr, StepCrementUpdateExpr { + /** The `x` as in the left-hand side of `x = x + E`. */ + VariableAccess lvalueVariable; + /** The `x + E` as in `x = x + E`. */ + AddOrSubExpr addOrSubExpr; + /** The `E` as in `x = x + E`. */ + Expr amountExpr; + + AddOrSubThenAssignExpr() { + this.getLValue() = lvalueVariable and + this.getRValue() = addOrSubExpr and + exists(VariableAccess lvalueVariableAsRvalue | + lvalueVariableAsRvalue = addOrSubExpr.getAnOperand() and + amountExpr = addOrSubExpr.getAnOperand() and + lvalueVariableAsRvalue != amountExpr + | + lvalueVariable.getTarget() = lvalueVariableAsRvalue.(VariableAccess).getTarget() + ) + } + + override Expr getAmountExpr() { result = amountExpr } +} diff --git a/cpp/common/src/codingstandards/cpp/exclusions/cpp/RuleMetadata.qll b/cpp/common/src/codingstandards/cpp/exclusions/cpp/RuleMetadata.qll index e7b834bd8..2e2403165 100644 --- a/cpp/common/src/codingstandards/cpp/exclusions/cpp/RuleMetadata.qll +++ b/cpp/common/src/codingstandards/cpp/exclusions/cpp/RuleMetadata.qll @@ -50,6 +50,7 @@ import SideEffects1 import SideEffects2 import SmartPointers1 import SmartPointers2 +import Statements import Strings import Templates import Toolchain @@ -108,6 +109,7 @@ newtype TCPPQuery = TSideEffects2PackageQuery(SideEffects2Query q) or TSmartPointers1PackageQuery(SmartPointers1Query q) or TSmartPointers2PackageQuery(SmartPointers2Query q) or + TStatementsPackageQuery(StatementsQuery q) or TStringsPackageQuery(StringsQuery q) or TTemplatesPackageQuery(TemplatesQuery q) or TToolchainPackageQuery(ToolchainQuery q) or @@ -166,6 +168,7 @@ predicate isQueryMetadata(Query query, string queryId, string ruleId, string cat isSideEffects2QueryMetadata(query, queryId, ruleId, category) or isSmartPointers1QueryMetadata(query, queryId, ruleId, category) or isSmartPointers2QueryMetadata(query, queryId, ruleId, category) or + isStatementsQueryMetadata(query, queryId, ruleId, category) or isStringsQueryMetadata(query, queryId, ruleId, category) or isTemplatesQueryMetadata(query, queryId, ruleId, category) or isToolchainQueryMetadata(query, queryId, ruleId, category) or diff --git a/cpp/common/src/codingstandards/cpp/exclusions/cpp/Statements.qll b/cpp/common/src/codingstandards/cpp/exclusions/cpp/Statements.qll new file mode 100644 index 000000000..fe202ce31 --- /dev/null +++ b/cpp/common/src/codingstandards/cpp/exclusions/cpp/Statements.qll @@ -0,0 +1,61 @@ +//** THIS FILE IS AUTOGENERATED, DO NOT MODIFY DIRECTLY. **/ +import cpp +import RuleMetadata +import codingstandards.cpp.exclusions.RuleMetadata + +newtype StatementsQuery = + TAppropriateStructureOfSwitchStatementQuery() or + TLegacyForStatementsShouldBeSimpleQuery() or + TForRangeInitializerAtMostOneFunctionCallQuery() + +predicate isStatementsQueryMetadata(Query query, string queryId, string ruleId, string category) { + query = + // `Query` instance for the `appropriateStructureOfSwitchStatement` query + StatementsPackage::appropriateStructureOfSwitchStatementQuery() and + queryId = + // `@id` for the `appropriateStructureOfSwitchStatement` query + "cpp/misra/appropriate-structure-of-switch-statement" and + ruleId = "RULE-9-4-2" and + category = "required" + or + query = + // `Query` instance for the `legacyForStatementsShouldBeSimple` query + StatementsPackage::legacyForStatementsShouldBeSimpleQuery() and + queryId = + // `@id` for the `legacyForStatementsShouldBeSimple` query + "cpp/misra/legacy-for-statements-should-be-simple" and + ruleId = "RULE-9-5-1" and + category = "advisory" + or + query = + // `Query` instance for the `forRangeInitializerAtMostOneFunctionCall` query + StatementsPackage::forRangeInitializerAtMostOneFunctionCallQuery() and + queryId = + // `@id` for the `forRangeInitializerAtMostOneFunctionCall` query + "cpp/misra/for-range-initializer-at-most-one-function-call" and + ruleId = "RULE-9-5-2" and + category = "required" +} + +module StatementsPackage { + Query appropriateStructureOfSwitchStatementQuery() { + //autogenerate `Query` type + result = + // `Query` type for `appropriateStructureOfSwitchStatement` query + TQueryCPP(TStatementsPackageQuery(TAppropriateStructureOfSwitchStatementQuery())) + } + + Query legacyForStatementsShouldBeSimpleQuery() { + //autogenerate `Query` type + result = + // `Query` type for `legacyForStatementsShouldBeSimple` query + TQueryCPP(TStatementsPackageQuery(TLegacyForStatementsShouldBeSimpleQuery())) + } + + Query forRangeInitializerAtMostOneFunctionCallQuery() { + //autogenerate `Query` type + result = + // `Query` type for `forRangeInitializerAtMostOneFunctionCall` query + TQueryCPP(TStatementsPackageQuery(TForRangeInitializerAtMostOneFunctionCallQuery())) + } +} diff --git a/cpp/misra/src/rules/RULE-9-4-2/AppropriateStructureOfSwitchStatement.ql b/cpp/misra/src/rules/RULE-9-4-2/AppropriateStructureOfSwitchStatement.ql new file mode 100644 index 000000000..893ebfe73 --- /dev/null +++ b/cpp/misra/src/rules/RULE-9-4-2/AppropriateStructureOfSwitchStatement.ql @@ -0,0 +1,92 @@ +/** + * @id cpp/misra/appropriate-structure-of-switch-statement + * @name RULE-9-4-2: The structure of a switch statement shall be appropriate + * @description A switch statement should have an appropriate structure with proper cases, default + * labels, and break statements to ensure clear control flow and prevent unintended + * fall-through behavior. + * @kind problem + * @precision very-high + * @problem.severity error + * @tags external/misra/id/rule-9-4-2 + * correctness + * maintainability + * readability + * external/misra/allocated-target/single-translation-unit + * external/misra/enforcement/decidable + * external/misra/obligation/required + */ + +import cpp +import codingstandards.cpp.misra +import codingstandards.cpp.SwitchStatement +import codingstandards.cpp.Noreturn + +from SwitchStmt switch, string message +where + not isExcluded(switch, StatementsPackage::appropriateStructureOfSwitchStatementQuery()) and + /* 1. There is a statement that appears as an initializer and is not a declaration statement. */ + exists(Stmt initializer | initializer = switch.getInitialization() | + not initializer instanceof DeclStmt + ) and + message = "contains a statement that is not a simple declaration" + or + /* 2. There is a switch case label that does not lead a branch (i.e. a switch case label is nested). */ + exists(SwitchCase case | case = switch.getASwitchCase() | case instanceof NestedSwitchCase) and + message = "contains a switch label that is not directly within the switch body" + or + /* 3. There is a non-case label in a label group. */ + exists(SwitchCase case | case = switch.getASwitchCase() | + case.getAStmt().getChildStmt*() instanceof LabelStmt + ) and + message = "contains a statement label that is not a case label" + or + /* 4. There is a statement before the first case label. */ + exists(Stmt switchBody | switchBody = switch.getStmt() | + not switchBody.getChild(0) instanceof SwitchCase + ) and + message = "has a statement that is not a case label as its first element" + or + /* 5. There is a switch case whose terminator is not one of the allowed kinds. */ + exists(SwitchCase case, Stmt lastStmt | + case = switch.getASwitchCase() and lastStmt = case.getLastStmt() + | + not ( + lastStmt instanceof BreakStmt or + lastStmt instanceof ReturnStmt or + lastStmt instanceof GotoStmt or + lastStmt instanceof ContinueStmt or + lastStmt.(ExprStmt).getExpr() instanceof ThrowExpr or + lastStmt.(ExprStmt).getExpr().(Call).getTarget() instanceof NoreturnFunction or + lastStmt.getAnAttribute().getName().matches("%fallthrough") // We'd like to consider compiler variants such as `clang::fallthrough`. + ) + ) and + message = "is missing a terminator that moves the control out of its body" + or + /* 6. The switch statement does not have more than two unique branches. */ + count(SwitchCase case | + case = switch.getASwitchCase() and + /* + * If the next switch case is the following statement of this switch case, then the two + * switch cases are consecutive and should be considered as constituting one branch + * together. + */ + + not case.getNextSwitchCase() = case.getFollowingStmt() + | + case + ) < 2 and + message = "contains less than two branches" + or + /* 7-1. The switch statement is not an enum switch statement and is missing a default case. */ + not switch instanceof EnumSwitch and + not switch.hasDefaultCase() and + message = "lacks a default case" + or + /* + * 7-2. The switch statement is an enum switch statement and is missing a branch for a + * variant. + */ + + exists(switch.(EnumSwitch).getAMissingCase()) and + message = "lacks a case for one of its variants" +select switch, "Switch statement " + message + "." diff --git a/cpp/misra/src/rules/RULE-9-5-1/LegacyForStatementsShouldBeSimple.ql b/cpp/misra/src/rules/RULE-9-5-1/LegacyForStatementsShouldBeSimple.ql new file mode 100644 index 000000000..8d44c50bd --- /dev/null +++ b/cpp/misra/src/rules/RULE-9-5-1/LegacyForStatementsShouldBeSimple.ql @@ -0,0 +1,505 @@ +/** + * @id cpp/misra/legacy-for-statements-should-be-simple + * @name RULE-9-5-1: Legacy for statements should be simple + * @description Legacy for statements with complex initialization, condition, and increment + * expressions can be difficult to understand and maintain. Simple for loops are more + * readable. + * @kind problem + * @precision high + * @problem.severity recommendation + * @tags external/misra/id/rule-9-5-1 + * maintainability + * readability + * external/misra/allocated-target/single-translation-unit + * external/misra/enforcement/decidable + * external/misra/obligation/advisory + */ + +import cpp +import semmle.code.cpp.dataflow.internal.AddressFlow +import codingstandards.cpp.misra +import codingstandards.cpp.Call +import codingstandards.cpp.Loops +import codingstandards.cpp.ast.Increment +import codingstandards.cpp.misra.BuiltInTypeRules::MisraCpp23BuiltInTypes + +Variable getDeclaredVariableInForLoop(ForStmt forLoop) { + result = forLoop.getADeclaration().getADeclarationEntry().(VariableDeclarationEntry).getVariable() +} + +/** + * Holds if the given expression may mutate the variable. + */ +predicate variableModifiedInExpression(Expr expr, VariableAccess va) { + /* + * 1. Direct modification (assignment, increment, etc.) or a function call. + */ + + expr.getAChild+() = va and + va.isModified() + or + /* + * 2. Address taken for non-const access that can potentially lead to modification. + * This overlaps with the former example on cases where `expr` is a function call. + */ + + valueToUpdate(va, _, expr) +} + +/** + * Gets the loop step of a legacy for loop. + * + * This predicate assumes the update expression of the given for loop is an add-and-assign + * (`+=`) or sub-and-assign (`-=`) expression, so the update expression that is an increment + * (`++`) or a decrement (`--`) operation should be handled using different means than this + * predicate. + */ +Expr getLoopStepOfForStmt(ForStmt forLoop) { + /* + * NOTE: We compute the transitive closure of `getAChild` on the update expression, + * since the update expression may be a compound one that embeds the four aforementioned + * expression types, such as a comma expression (e.g. `i += 1, E` where `E` is an + * arbitrary expression). + * + * This may be detrimental to performance, but we keep it for soundness. A possible + * alternative is an IR-based solution. + */ + + /* 1. Get the expression `E` when the update expression is `i += E` or `i -= E`. */ + result = forLoop.getUpdate().getAChild*().(StepCrementUpdateExpr).getAmountExpr() +} + +/** + * Holds if either of the following holds for the given variable access: + * 1. Another variable access of the same variable as the given variable access is taken an + * address and is assigned to a non-const pointer variable, i.e. initialization, assignment, + * and pass-by-value. + * 2. Another variable access of the same variable as the given variable access is assigned + * to a non-const reference variable (thus constituting a `T` -> `&T` conversion.), i.e. + * initialization and assignment. + * + * Note that pass-by-reference is dealt with in a different predicate named + * `loopVariablePassedAsArgumentToNonConstReferenceParameter`, due to implementation + * limitations. + */ +predicate loopVariableAssignedToNonConstPointerOrReferenceType( + ForStmt forLoop, VariableAccess loopVariableAccessInCondition +) { + exists(Expr assignmentRhs, Type targetType, DerivedType strippedType | + isAssignment(assignmentRhs, targetType, _) and + strippedType = targetType.stripTopLevelSpecifiers() and + not strippedType.getBaseType().isConst() and + ( + strippedType instanceof PointerType or + strippedType instanceof ReferenceType + ) + | + assignmentRhs.getEnclosingStmt().getParent*() = forLoop.getStmt() and + ( + /* 1. The address is taken: A loop variable access */ + assignmentRhs.(AddressOfExpr).getOperand() = + loopVariableAccessInCondition.getTarget().getAnAccess() + or + /* 2. A reference is taken: A loop variable access */ + assignmentRhs = loopVariableAccessInCondition.getTarget().getAnAccess() + ) + ) +} + +/** + * Holds if the given variable access has another variable access with the same target + * variable that is passed as reference to a non-const reference parameter of a function, + * constituting a `T` -> `&T` conversion. + * + * This is an adapted part of + * `BuiltinTypeRules::MisraCpp23BuiltInTypes::isPreConversionAssignment` that is only + * relevant to an argument passed to a parameter, seen as an assignment. + * + * This predicate adds two constraints to the target type, as compared to the original + * portion of the predicate: + * + * 1. This predicate adds a type constraint that the target type is a `ReferenceType`. + * 2. This predicate adds the constraint that the target type is not `const`. + * + * Also, this predicate requires that the call is the body of the given for-loop. + */ +predicate loopVariablePassedAsArgumentToNonConstReferenceParameter( + ForStmt forLoop, VariableAccess loopVariableAccessInCondition +) { + exists(Type targetType, ReferenceType strippedReferenceType | + exists(Call call, int i | + call.getArgument(i) = loopVariableAccessInCondition.getTarget().getAnAccess() and + call.getEnclosingStmt().getParent*() = forLoop.getStmt() and + strippedReferenceType = targetType.stripTopLevelSpecifiers() and + not strippedReferenceType.getBaseType().isConst() + | + /* A regular function call */ + targetType = call.getTarget().getParameter(i).getType() + or + /* A function call where the argument is passed as varargs */ + call.getTarget().getNumberOfParameters() <= i and + /* The rule states that the type should match the "adjusted" type of the argument */ + targetType = loopVariableAccessInCondition.getFullyConverted().getType() + or + /* An expression call - get the function type, then the parameter type */ + targetType = getExprCallFunctionType(call).getParameterType(i) + ) + ) +} + +private newtype TAlertType = + /* 1. There is a counter variable that is not of an integer type. */ + TNonIntegerTypeCounterVariable(ForStmt forLoop, Variable loopCounter) { + loopCounter = getDeclaredVariableInForLoop(forLoop) and + exists(Type type | type = loopCounter.getType() | + not ( + type instanceof IntegralType or + type instanceof FixedWidthIntegralType + ) + ) + } or + /* + * 2. The loop condition checks termination without comparing the counter variable to the + * loop bound using a relational operator. + */ + + TNoRelationalOperatorInLoopCondition(ForStmt forLoop, Expr condition) { + condition = forLoop.getCondition() and + not condition instanceof LegacyForLoopCondition + } or + /* 3-1. The loop counter is mutated somewhere other than its update expression. */ + TLoopCounterMutatedInLoopBody(ForStmt forLoop, Variable loopCounterVariable) { + loopCounterVariable = getDeclaredVariableInForLoop(forLoop) and + exists(Expr mutatingExpr | not mutatingExpr = forLoop.getUpdate().getAChild*() | + variableModifiedInExpression(mutatingExpr, loopCounterVariable.getAnAccess()) + ) + } or + /* 3-2. The loop counter is not updated using either of `++`, `--`, `+=`, or `-=`. */ + TLoopCounterUpdatedNotByCrementOrAddSubAssignmentExpr( + ForStmt forLoop, Variable loopCounterVariable, Expr updateExpr + ) { + loopCounterVariable = getDeclaredVariableInForLoop(forLoop) and + updateExpr = forLoop.getUpdate() and + variableModifiedInExpression(updateExpr, loopCounterVariable.getAnAccess()) and + not updateExpr instanceof StepCrementUpdateExpr + } or + /* 4. The type size of the loop counter is smaller than that of the loop bound. */ + TLoopCounterSmallerThanLoopBound(ForStmt forLoop, LegacyForLoopCondition forLoopCondition) { + forLoopCondition = forLoop.getCondition() and + exists(Expr loopCounter, Expr loopBound | + loopCounter = forLoopCondition.getLoopCounter() and + loopBound = forLoopCondition.getLoopBound() + | + typeUpperBound(loopCounter.getType()) < upperBound(loopBound.getFullyConverted()) + ) + } or + /* 5-1-1. The loop bound is a variable that is mutated in the for loop. */ + TLoopBoundIsMutatedVariableAccess( + ForStmt forLoop, VariableAccess variableAccess, VariableAccess mutatedVariableAccess + ) { + exists(Expr loopBoundExpr, Expr mutatingExpr | + loopBoundExpr = forLoop.getCondition().(LegacyForLoopCondition).getLoopBound() and + ( + /* 1. The mutating expression may be in the loop body. */ + mutatingExpr = forLoop.getStmt().getChildStmt().getAChild*() + or + /* 2. The mutating expression may be in the loop updating expression. */ + mutatingExpr = forLoop.getUpdate().getAChild*() + or + /* 3. The mutating expression may be in the loop condition */ + mutatingExpr = forLoop.getCondition().getAChild*() + or + /* 4. The mutating expression may be in the loop initializer */ + mutatingExpr = forLoop.getInitialization().getAChild*() + ) and + variableAccess = loopBoundExpr.getAChild*() and + mutatedVariableAccess = variableAccess.getTarget().getAnAccess() and + variableModifiedInExpression(mutatingExpr, mutatedVariableAccess) + ) + } or + /* 5-1-2. The loop bound is not a variable access nor a constant expression. */ + TLoopBoundIsNonConstExpr(ForStmt forLoop, Expr loopBound) { + loopBound = forLoop.getCondition().(LegacyForLoopCondition).getLoopBound() and + (not loopBound instanceof VariableAccess and not loopBound.isConstant()) + } or + /* 5-2-1. The loop step is a variable that is mutated in the for loop. */ + TLoopStepIsMutatedVariableAccess( + ForStmt forLoop, VariableAccess variableAccess, VariableAccess mutatedVariableAccess + ) { + exists(Expr loopStepExpr, Expr mutatingExpr | + loopStepExpr = getLoopStepOfForStmt(forLoop) and + ( + /* 1. The mutating expression may be in the loop body. */ + mutatingExpr = forLoop.getStmt().getChildStmt().getAChild*() + or + /* 2. The mutating expression may be in the loop updating expression. */ + mutatingExpr = forLoop.getUpdate().getAChild*() + or + /* 3. The mutating expression may be in the loop condition */ + mutatingExpr = forLoop.getCondition().getAChild*() + or + /* 4. The mutating expression may be in the loop initializer */ + mutatingExpr = forLoop.getInitialization().getAChild*() + ) and + variableAccess = loopStepExpr.getAChild*() and + mutatedVariableAccess = variableAccess.getTarget().getAnAccess() and + variableModifiedInExpression(mutatingExpr, mutatedVariableAccess) + ) + } or + /* 5-2-2. The loop step is not a variable access nor a constant expression. */ + TLoopStepIsNonConstExpr(ForStmt forLoop, Expr loopStep) { + loopStep = getLoopStepOfForStmt(forLoop) and + (not loopStep instanceof VariableAccess and not loopStep.isConstant()) + } or + /* + * 6-1. The loop counter is taken as a mutable reference or its address to a mutable pointer. + */ + + TLoopCounterIsTakenNonConstAddress(ForStmt forLoop, VariableAccess loopVariableAccessInCondition) { + loopVariableAccessInCondition = forLoop.getCondition().(LegacyForLoopCondition).getLoopCounter() and + ( + loopVariableAssignedToNonConstPointerOrReferenceType(forLoop, loopVariableAccessInCondition) + or + loopVariablePassedAsArgumentToNonConstReferenceParameter(forLoop, + loopVariableAccessInCondition) + ) + } or + /* + * 6-2. The loop bound is taken as a mutable reference or its address to a mutable pointer. + */ + + TLoopBoundIsTakenNonConstAddress(ForStmt forLoop, Expr loopBoundExpr) { + loopBoundExpr = forLoop.getCondition().(LegacyForLoopCondition).getLoopBound() and + exists(VariableAccess variableAccess | + variableAccess = loopBoundExpr.getAChild*() and + ( + loopVariableAssignedToNonConstPointerOrReferenceType(forLoop, variableAccess) + or + loopVariablePassedAsArgumentToNonConstReferenceParameter(forLoop, variableAccess) + ) + ) + } or + /* + * 6-3. The loop step is taken as a mutable reference or its address to a mutable pointer. + */ + + TLoopStepIsTakenNonConstAddress(ForStmt forLoop, Expr loopVariableAccessInCondition) { + loopVariableAccessInCondition = getLoopStepOfForStmt(forLoop) and + ( + loopVariableAssignedToNonConstPointerOrReferenceType(forLoop, loopVariableAccessInCondition) + or + loopVariablePassedAsArgumentToNonConstReferenceParameter(forLoop, + loopVariableAccessInCondition) + ) + } + +class AlertType extends TAlertType { + /** + * Extract the primary location depending on the case of this instance. + */ + Location getLocation() { result = this.asElement().getLocation() } + + Element asElement() { + this = TNonIntegerTypeCounterVariable(result, _) or + this = TNoRelationalOperatorInLoopCondition(result, _) or + this = TLoopCounterMutatedInLoopBody(result, _) or + this = TLoopCounterUpdatedNotByCrementOrAddSubAssignmentExpr(result, _, _) or + this = TLoopCounterSmallerThanLoopBound(result, _) or + this = TLoopBoundIsMutatedVariableAccess(result, _, _) or + this = TLoopBoundIsNonConstExpr(result, _) or + this = TLoopStepIsMutatedVariableAccess(result, _, _) or + this = TLoopStepIsNonConstExpr(result, _) or + this = TLoopCounterIsTakenNonConstAddress(result, _) or + this = TLoopBoundIsTakenNonConstAddress(result, _) or + this = TLoopStepIsTakenNonConstAddress(result, _) + } + + /** + * Gets the target the link leads to depending on the case of this instance. + */ + Locatable getLinkTarget1() { + this = TNonIntegerTypeCounterVariable(_, result) + or + this = TNoRelationalOperatorInLoopCondition(_, result) + or + this = TLoopCounterMutatedInLoopBody(_, result) + or + this = TLoopCounterUpdatedNotByCrementOrAddSubAssignmentExpr(_, result, _) + or + exists(LegacyForLoopCondition forLoopCondition | + this = TLoopCounterSmallerThanLoopBound(_, forLoopCondition) and + result = forLoopCondition.getLoopCounter() + ) + or + this = TLoopBoundIsNonConstExpr(_, result) + or + this = TLoopBoundIsMutatedVariableAccess(_, result, _) + or + this = TLoopStepIsNonConstExpr(_, result) + or + this = TLoopStepIsMutatedVariableAccess(_, result, _) + or + this = TLoopCounterIsTakenNonConstAddress(_, result) + or + this = TLoopBoundIsTakenNonConstAddress(_, result) + or + this = TLoopStepIsTakenNonConstAddress(_, result) + } + + /** + * Gets the text of the link depending on the case of this instance. + */ + string getLinkText1() { + this = TNonIntegerTypeCounterVariable(_, _) and + result = "counter variable" + or + this = TNoRelationalOperatorInLoopCondition(_, _) and + result = "loop condition" + or + this = TLoopCounterMutatedInLoopBody(_, _) and + result = "counter variable" + or + this = TLoopCounterUpdatedNotByCrementOrAddSubAssignmentExpr(_, _, _) and + result = "counter variable" + or + this = TLoopCounterSmallerThanLoopBound(_, _) and + result = "counter variable" + or + this = TLoopBoundIsMutatedVariableAccess(_, _, _) and + result = "loop bound" + or + this = TLoopBoundIsNonConstExpr(_, _) and + result = "loop bound" + or + this = TLoopStepIsMutatedVariableAccess(_, _, _) and + result = "loop step" + or + this = TLoopStepIsNonConstExpr(_, _) and + result = "loop step" + or + this = TLoopCounterIsTakenNonConstAddress(_, _) and + result = "loop counter" + or + this = TLoopBoundIsTakenNonConstAddress(_, _) and + result = "loop bound" + or + this = TLoopStepIsTakenNonConstAddress(_, _) and + result = "loop step" + } + + /** + * Gets the message with a placeholder, depending on the case of this instance. + */ + string getMessage() { + this = TNonIntegerTypeCounterVariable(_, _) and + result = "The $@ is not of an integer type." + or + this = TNoRelationalOperatorInLoopCondition(_, _) and + result = + "The $@ does not determine termination based only on a comparison against the value of the counter variable." + or + this = TLoopCounterMutatedInLoopBody(_, _) and + result = "The $@ may be mutated in a location other than its update expression." + or + this = TLoopCounterUpdatedNotByCrementOrAddSubAssignmentExpr(_, _, _) and + result = "The $@ is not updated with an $@ other than addition or subtraction." + or + this = TLoopCounterSmallerThanLoopBound(_, _) and + result = "The $@ has a smaller type than that of the $@." + or + this = TLoopBoundIsNonConstExpr(_, _) and + result = "The $@ is a $@." + or + this = TLoopBoundIsMutatedVariableAccess(_, _, _) and + result = "The $@ is a non-const expression, or a variable that may be $@ in the loop." + or + this = TLoopStepIsNonConstExpr(_, _) and + result = "The $@ is a $@." + or + this = TLoopStepIsMutatedVariableAccess(_, _, _) and + result = "The $@ is a non-const expression, or a variable that may be $@ in the loop." + or + this = TLoopCounterIsTakenNonConstAddress(_, _) and + result = "The $@ is taken as a mutable reference or its address to a mutable pointer." + or + this = TLoopBoundIsTakenNonConstAddress(_, _) and + result = "The $@ is taken as a mutable reference or its address to a mutable pointer." + or + this = TLoopStepIsTakenNonConstAddress(_, _) and + result = "The $@ is taken as a mutable reference or its address to a mutable pointer." + } + + Locatable getLinkTarget2() { + this = TNonIntegerTypeCounterVariable(_, result) // Throwaway + or + this = TNoRelationalOperatorInLoopCondition(_, result) // Throwaway + or + this = TLoopCounterMutatedInLoopBody(_, result) // Throwaway + or + this = TLoopCounterUpdatedNotByCrementOrAddSubAssignmentExpr(_, _, result) + or + exists(LegacyForLoopCondition forLoopCondition | + this = TLoopCounterSmallerThanLoopBound(_, forLoopCondition) and + result = forLoopCondition.getLoopBound() + ) + or + this = TLoopBoundIsNonConstExpr(_, result) + or + this = TLoopBoundIsMutatedVariableAccess(_, _, result) + or + this = TLoopStepIsNonConstExpr(_, result) + or + this = TLoopStepIsMutatedVariableAccess(_, _, result) + or + this = TLoopCounterIsTakenNonConstAddress(_, result) // Throwaway + or + this = TLoopBoundIsTakenNonConstAddress(_, result) // Throwaway + or + this = TLoopStepIsTakenNonConstAddress(_, result) // Throwaway + } + + string getLinkText2() { + this = TNonIntegerTypeCounterVariable(_, _) and + result = "N/A" // Throwaway + or + this = TNoRelationalOperatorInLoopCondition(_, _) and + result = "N/A" // Throwaway + or + this = TLoopCounterMutatedInLoopBody(_, _) and + result = "N/A" // Throwaway + or + this = TLoopCounterUpdatedNotByCrementOrAddSubAssignmentExpr(_, _, _) and + result = "expression" + or + this = TLoopCounterSmallerThanLoopBound(_, _) and + result = "loop bound" + or + this = TLoopBoundIsNonConstExpr(_, _) and + result = "non-const expression" + or + this = TLoopBoundIsMutatedVariableAccess(_, _, _) and + result = "mutated" + or + this = TLoopStepIsNonConstExpr(_, _) and + result = "non-const expression" + or + this = TLoopStepIsMutatedVariableAccess(_, _, _) and + result = "mutated" + or + this = TLoopCounterIsTakenNonConstAddress(_, _) and + result = "N/A" // Throwaway + or + this = TLoopBoundIsTakenNonConstAddress(_, _) and + result = "N/A" // Throwaway + or + this = TLoopStepIsTakenNonConstAddress(_, _) and + result = "N/A" // Throwaway + } + + string toString() { result = this.asElement().toString() } +} + +from AlertType alert +where not isExcluded(alert.asElement(), StatementsPackage::legacyForStatementsShouldBeSimpleQuery()) +select alert, alert.getMessage(), alert.getLinkTarget1(), alert.getLinkText1(), + alert.getLinkTarget2(), alert.getLinkText2() diff --git a/cpp/misra/src/rules/RULE-9-5-2/ForRangeInitializerAtMostOneFunctionCall.ql b/cpp/misra/src/rules/RULE-9-5-2/ForRangeInitializerAtMostOneFunctionCall.ql new file mode 100644 index 000000000..e6b8df9f8 --- /dev/null +++ b/cpp/misra/src/rules/RULE-9-5-2/ForRangeInitializerAtMostOneFunctionCall.ql @@ -0,0 +1,28 @@ +/** + * @id cpp/misra/for-range-initializer-at-most-one-function-call + * @name RULE-9-5-2: A for-range-initializer shall contain at most one function call + * @description Multiple function calls in a for-range-initializer can lead to unclear iteration + * behavior and potential side effects that make the code harder to understand and + * debug. + * @kind problem + * @precision very-high + * @problem.severity error + * @tags external/misra/id/rule-9-5-2 + * correctness + * maintainability + * readability + * external/misra/allocated-target/single-translation-unit + * external/misra/enforcement/decidable + * external/misra/obligation/required + */ + +import cpp +import codingstandards.cpp.misra + +from RangeBasedForStmt foreach, Expr initializer +where + not isExcluded(foreach, StatementsPackage::forRangeInitializerAtMostOneFunctionCallQuery()) and + initializer = foreach.getRange() and + count(Call call | call = initializer.getAChild*() | call) >= 2 +select foreach, "Range-based for loop has nested call expression in its $@.", initializer, + "initializer" diff --git a/cpp/misra/test/rules/RULE-9-4-2/AppropriateStructureOfSwitchStatement.expected b/cpp/misra/test/rules/RULE-9-4-2/AppropriateStructureOfSwitchStatement.expected new file mode 100644 index 000000000..673e57063 --- /dev/null +++ b/cpp/misra/test/rules/RULE-9-4-2/AppropriateStructureOfSwitchStatement.expected @@ -0,0 +1,12 @@ +| test.cpp:28:3:37:3 | switch (...) ... | Switch statement contains a statement that is not a simple declaration. | +| test.cpp:51:3:60:3 | switch (...) ... | Switch statement contains a switch label that is not directly within the switch body. | +| test.cpp:62:3:71:3 | switch (...) ... | Switch statement contains a switch label that is not directly within the switch body. | +| test.cpp:75:3:85:3 | switch (...) ... | Switch statement contains a statement label that is not a case label. | +| test.cpp:89:3:97:3 | switch (...) ... | Switch statement has a statement that is not a case label as its first element. | +| test.cpp:147:3:154:3 | switch (...) ... | Switch statement is missing a terminator that moves the control out of its body. | +| test.cpp:188:3:192:3 | switch (...) ... | Switch statement contains less than two branches. | +| test.cpp:194:3:200:3 | switch (...) ... | Switch statement contains less than two branches. | +| test.cpp:215:3:219:3 | switch (...) ... | Switch statement contains less than two branches. | +| test.cpp:215:3:219:3 | switch (...) ... | Switch statement lacks a default case. | +| test.cpp:223:3:229:3 | switch (...) ... | Switch statement lacks a case for one of its variants. | +| test.cpp:231:3:239:3 | switch (...) ... | Switch statement lacks a case for one of its variants. | diff --git a/cpp/misra/test/rules/RULE-9-4-2/AppropriateStructureOfSwitchStatement.qlref b/cpp/misra/test/rules/RULE-9-4-2/AppropriateStructureOfSwitchStatement.qlref new file mode 100644 index 000000000..9f475afbe --- /dev/null +++ b/cpp/misra/test/rules/RULE-9-4-2/AppropriateStructureOfSwitchStatement.qlref @@ -0,0 +1 @@ +rules/RULE-9-4-2/AppropriateStructureOfSwitchStatement.ql \ No newline at end of file diff --git a/cpp/misra/test/rules/RULE-9-4-2/test.cpp b/cpp/misra/test/rules/RULE-9-4-2/test.cpp new file mode 100644 index 000000000..0e8c2e2ec --- /dev/null +++ b/cpp/misra/test/rules/RULE-9-4-2/test.cpp @@ -0,0 +1,253 @@ +#include + +int i = 0; + +/** + * Test the initializer of a switch statement. + */ +void testInitializer(int expr) { + switch (expr) { // COMPLIANT: No initializer + case 1: + i++; + break; + default: + i--; + break; + } + + switch (int j = 0; + expr) { // COMPLIANT: Only declaration statement can be an initializer + case 1: + j++; + break; + default: + j--; + break; + } + + switch ( + i = 1; + expr) { // NON_COMPLIANT: Only declaration statement can be an initializer + case 1: + i++; + break; + default: + i--; + break; + } +} + +void testNestedCaseLabels(int expr) { + switch (expr) { // COMPLIANT: Consecutive case labels are allowed + case 1: + case 2: + i++; + break; + default: + i--; + break; + } + + switch (expr) { // NON_COMPLIANT: Statements with case labels should all be at + // the same level + case 1: { + case 2: + i++; + break; + } + default: + break; + } + + switch (expr) { // NON_COMPLIANT: Statements with case labels should all be at + // the same level + case 1: + i++; + break; + case 2: { + default: + break; + } + } +} + +void testOtherLabelsInBranch(int expr) { + switch (expr) { // NON_COMPLIANT: Non-case labels appearing in a switch branch + case 1: { + i++; + goto someLabel; + someLabel: + i++; + break; + } + default: + break; + } +} + +void testLeadingNonCaseStatement(int expr) { + switch (expr) { // NON_COMPLIANT: Non-case statement is the first statement in + // the switch body + int x = 1; + case 1: + i++; + break; + default: + break; + } +} + +[[noreturn]] void f() { exit(0); } +void g() {} + +void testSwitchBranchTerminator(int expr) { + switch (expr) { // COMPLIANT: Break is allowed as a branch terminator + case 1: + i++; + break; + default: + break; + } + + for (int j = 0; j++; j < 10) { + switch (expr) { // COMPLIANT: Continue is allowed as a branch terminator + case 1: + i++; + continue; + default: + continue; + } + } + + switch (expr) { // COMPLIANT: Goto is allowed as a branch terminator + case 1: + i++; + goto error; + default: + goto error; + } + + switch (expr) { // COMPLIANT: Throw is allowed as a branch terminator + case 1: + i++; + throw; + default: + throw; + } + + switch (expr) { // COMPLIANT: Call to a `[[noreturn]]` function is allowed as + // a branch terminator + case 1: + i++; + f(); + default: + f(); + } + + switch (expr) { // NON_COMPLIANT: Branch ends with a call to a function that + // is not `[[noreturn]]` + case 1: + i++; + g(); + default: + g(); + } + + switch (expr) { // COMPLIANT: Return is allowed as a branch terminator + case 1: + i++; + return; + default: + return; + } + + switch (expr) { // COMPLIANT: Empty statement with `[[fallthrough]]` is + // allowed as a branch terminator + case 1: + i++; + [[fallthrough]]; + default: + i++; + break; + } + +error: + return; +} + +void testSwitchBranchCount(int expr) { + switch (expr) { // COMPLIANT: Branch count is 2 + case 1: + i++; + break; + default: + i++; + break; + } + + switch (expr) { // NON_COMPLIANT: Branch count is 1 + default: + i++; + break; + } + + switch (expr) { // NON_COMPLIANT: Branch count is 1 + case 1: + case 2: + default: + i++; + break; + } +} + +enum E { V1, V2, V3 }; + +void testDefaultLabelPresence(int expr) { + switch (expr) { // COMPLIANT: There is a default branch + case 1: + i++; + break; + default: + i++; + break; + } + + switch (expr) { // NON_COMPLIANT: Default branch is missing + case 1: + i++; + break; + } + + E e; + + switch (e) { // COMPLIANT: There is a default branch + case V1: + i++; + break; + default: + break; + } + + switch (e) { // NON_COMPLIANT: Default branch is missing on a non-exhaustive + // enum switch + case V1: + i++; + break; + case V2: + i++; + break; + } + + switch (e) { // COMPLIANT: Default branch can be omitted on an exhaustive enum + // switch + case V1: + i++; + break; + case V2: + i++; + break; + case V3: + i++; + break; + } +} \ No newline at end of file diff --git a/cpp/misra/test/rules/RULE-9-5-1/LegacyForStatementsShouldBeSimple.expected b/cpp/misra/test/rules/RULE-9-5-1/LegacyForStatementsShouldBeSimple.expected new file mode 100644 index 000000000..a2b79b724 --- /dev/null +++ b/cpp/misra/test/rules/RULE-9-5-1/LegacyForStatementsShouldBeSimple.expected @@ -0,0 +1,45 @@ +| test.cpp:24:3:26:3 | for(...;...;...) ... | The $@ is not of an integer type. | test.cpp:24:14:24:14 | i | counter variable | test.cpp:24:14:24:14 | i | N/A | +| test.cpp:33:3:35:3 | for(...;...;...) ... | The $@ does not determine termination based only on a comparison against the value of the counter variable. | test.cpp:33:19:33:25 | ... == ... | loop condition | test.cpp:33:19:33:25 | ... == ... | N/A | +| test.cpp:37:3:39:3 | for(...;...;...) ... | The $@ does not determine termination based only on a comparison against the value of the counter variable. | test.cpp:37:19:37:24 | ... < ... | loop condition | test.cpp:37:19:37:24 | ... < ... | N/A | +| test.cpp:78:3:80:3 | for(...;...;...) ... | The $@ is not updated with an $@ other than addition or subtraction. | test.cpp:78:12:78:12 | i | counter variable | test.cpp:79:8:79:13 | ... *= ... | expression | +| test.cpp:92:3:95:3 | for(...;...;...) ... | The $@ has a smaller type than that of the $@. | test.cpp:92:21:92:21 | i | counter variable | test.cpp:92:25:92:53 | call to max | loop bound | +| test.cpp:117:3:120:3 | for(...;...;...) ... | The $@ is a non-const expression, or a variable that may be $@ in the loop. | test.cpp:118:13:118:13 | j | loop step | test.cpp:119:5:119:5 | j | mutated | +| test.cpp:122:3:124:3 | for(...;...;...) ... | The $@ is a non-const expression, or a variable that may be $@ in the loop. | test.cpp:123:13:123:13 | j | loop step | test.cpp:123:16:123:16 | j | mutated | +| test.cpp:122:3:124:3 | for(...;...;...) ... | The $@ is not updated with an $@ other than addition or subtraction. | test.cpp:122:12:122:12 | i | counter variable | test.cpp:123:8:123:18 | ... , ... | expression | +| test.cpp:134:3:137:3 | for(...;...;...) ... | The $@ is a non-const expression, or a variable that may be $@ in the loop. | test.cpp:134:23:134:23 | j | loop bound | test.cpp:136:5:136:5 | j | mutated | +| test.cpp:139:3:141:3 | for(...;...;...) ... | The $@ is a $@. | test.cpp:139:23:139:25 | ... ++ | loop bound | test.cpp:139:23:139:25 | ... ++ | non-const expression | +| test.cpp:139:3:141:3 | for(...;...;...) ... | The $@ is a non-const expression, or a variable that may be $@ in the loop. | test.cpp:139:23:139:23 | j | loop bound | test.cpp:139:23:139:23 | j | mutated | +| test.cpp:143:3:145:3 | for(...;...;...) ... | The $@ is a $@. | test.cpp:143:25:143:27 | ... ++ | loop bound | test.cpp:143:25:143:27 | ... ++ | non-const expression | +| test.cpp:143:3:145:3 | for(...;...;...) ... | The $@ is a non-const expression, or a variable that may be $@ in the loop. | test.cpp:143:25:143:25 | j | loop bound | test.cpp:143:25:143:25 | j | mutated | +| test.cpp:143:3:145:3 | for(...;...;...) ... | The $@ may be mutated in a location other than its update expression. | test.cpp:143:12:143:12 | i | counter variable | test.cpp:143:12:143:12 | i | N/A | +| test.cpp:149:3:152:3 | for(...;...;...) ... | The $@ is a non-const expression, or a variable that may be $@ in the loop. | test.cpp:149:23:149:23 | k | loop bound | test.cpp:151:15:151:15 | k | mutated | +| test.cpp:154:3:157:3 | for(...;...;...) ... | The $@ is a non-const expression, or a variable that may be $@ in the loop. | test.cpp:155:13:155:13 | l | loop step | test.cpp:156:15:156:15 | l | mutated | +| test.cpp:159:3:161:3 | for(...;...;...) ... | The $@ is a $@. | test.cpp:159:23:159:24 | call to h1 | loop bound | test.cpp:159:23:159:24 | call to h1 | non-const expression | +| test.cpp:167:3:169:3 | for(...;...;...) ... | The $@ is a $@. | test.cpp:168:13:168:14 | call to h1 | loop step | test.cpp:168:13:168:14 | call to h1 | non-const expression | +| test.cpp:182:3:185:3 | for(...;...;...) ... | The $@ is taken as a mutable reference or its address to a mutable pointer. | test.cpp:182:19:182:19 | i | loop counter | test.cpp:182:19:182:19 | i | N/A | +| test.cpp:187:3:190:3 | for(...;...;...) ... | The $@ is taken as a mutable reference or its address to a mutable pointer. | test.cpp:187:19:187:19 | i | loop counter | test.cpp:187:19:187:19 | i | N/A | +| test.cpp:202:3:205:3 | for(...;...;...) ... | The $@ is taken as a mutable reference or its address to a mutable pointer. | test.cpp:202:19:202:19 | i | loop counter | test.cpp:202:19:202:19 | i | N/A | +| test.cpp:207:3:210:3 | for(...;...;...) ... | The $@ is taken as a mutable reference or its address to a mutable pointer. | test.cpp:207:23:207:23 | k | loop bound | test.cpp:207:23:207:23 | k | N/A | +| test.cpp:212:3:215:3 | for(...;...;...) ... | The $@ is taken as a mutable reference or its address to a mutable pointer. | test.cpp:212:23:212:23 | k | loop bound | test.cpp:212:23:212:23 | k | N/A | +| test.cpp:227:3:230:3 | for(...;...;...) ... | The $@ is taken as a mutable reference or its address to a mutable pointer. | test.cpp:227:23:227:23 | k | loop bound | test.cpp:227:23:227:23 | k | N/A | +| test.cpp:232:3:235:3 | for(...;...;...) ... | The $@ is taken as a mutable reference or its address to a mutable pointer. | test.cpp:232:31:232:31 | l | loop step | test.cpp:232:31:232:31 | l | N/A | +| test.cpp:237:3:240:3 | for(...;...;...) ... | The $@ is taken as a mutable reference or its address to a mutable pointer. | test.cpp:237:31:237:31 | l | loop step | test.cpp:237:31:237:31 | l | N/A | +| test.cpp:252:3:255:3 | for(...;...;...) ... | The $@ is taken as a mutable reference or its address to a mutable pointer. | test.cpp:252:31:252:31 | l | loop step | test.cpp:252:31:252:31 | l | N/A | +| test.cpp:257:3:260:3 | for(...;...;...) ... | The $@ is taken as a mutable reference or its address to a mutable pointer. | test.cpp:257:19:257:19 | i | loop counter | test.cpp:257:19:257:19 | i | N/A | +| test.cpp:257:3:260:3 | for(...;...;...) ... | The $@ may be mutated in a location other than its update expression. | test.cpp:257:12:257:12 | i | counter variable | test.cpp:257:12:257:12 | i | N/A | +| test.cpp:262:3:265:3 | for(...;...;...) ... | The $@ is taken as a mutable reference or its address to a mutable pointer. | test.cpp:262:19:262:19 | i | loop counter | test.cpp:262:19:262:19 | i | N/A | +| test.cpp:262:3:265:3 | for(...;...;...) ... | The $@ may be mutated in a location other than its update expression. | test.cpp:262:12:262:12 | i | counter variable | test.cpp:262:12:262:12 | i | N/A | +| test.cpp:277:3:280:3 | for(...;...;...) ... | The $@ is taken as a mutable reference or its address to a mutable pointer. | test.cpp:277:19:277:19 | i | loop counter | test.cpp:277:19:277:19 | i | N/A | +| test.cpp:277:3:280:3 | for(...;...;...) ... | The $@ may be mutated in a location other than its update expression. | test.cpp:277:12:277:12 | i | counter variable | test.cpp:277:12:277:12 | i | N/A | +| test.cpp:282:3:285:3 | for(...;...;...) ... | The $@ is a non-const expression, or a variable that may be $@ in the loop. | test.cpp:282:23:282:23 | k | loop bound | test.cpp:284:8:284:8 | k | mutated | +| test.cpp:282:3:285:3 | for(...;...;...) ... | The $@ is taken as a mutable reference or its address to a mutable pointer. | test.cpp:282:23:282:23 | k | loop bound | test.cpp:282:23:282:23 | k | N/A | +| test.cpp:287:3:290:3 | for(...;...;...) ... | The $@ is a non-const expression, or a variable that may be $@ in the loop. | test.cpp:287:23:287:23 | k | loop bound | test.cpp:289:9:289:9 | k | mutated | +| test.cpp:287:3:290:3 | for(...;...;...) ... | The $@ is taken as a mutable reference or its address to a mutable pointer. | test.cpp:287:23:287:23 | k | loop bound | test.cpp:287:23:287:23 | k | N/A | +| test.cpp:303:3:306:3 | for(...;...;...) ... | The $@ is a non-const expression, or a variable that may be $@ in the loop. | test.cpp:303:23:303:23 | k | loop bound | test.cpp:305:9:305:9 | k | mutated | +| test.cpp:303:3:306:3 | for(...;...;...) ... | The $@ is taken as a mutable reference or its address to a mutable pointer. | test.cpp:303:23:303:23 | k | loop bound | test.cpp:303:23:303:23 | k | N/A | +| test.cpp:308:3:311:3 | for(...;...;...) ... | The $@ is a non-const expression, or a variable that may be $@ in the loop. | test.cpp:308:31:308:31 | l | loop step | test.cpp:310:8:310:8 | l | mutated | +| test.cpp:308:3:311:3 | for(...;...;...) ... | The $@ is taken as a mutable reference or its address to a mutable pointer. | test.cpp:308:31:308:31 | l | loop step | test.cpp:308:31:308:31 | l | N/A | +| test.cpp:313:3:316:3 | for(...;...;...) ... | The $@ is a non-const expression, or a variable that may be $@ in the loop. | test.cpp:313:31:313:31 | l | loop step | test.cpp:315:9:315:9 | l | mutated | +| test.cpp:313:3:316:3 | for(...;...;...) ... | The $@ is taken as a mutable reference or its address to a mutable pointer. | test.cpp:313:31:313:31 | l | loop step | test.cpp:313:31:313:31 | l | N/A | +| test.cpp:328:3:331:3 | for(...;...;...) ... | The $@ is a non-const expression, or a variable that may be $@ in the loop. | test.cpp:328:31:328:31 | l | loop step | test.cpp:330:9:330:9 | l | mutated | +| test.cpp:328:3:331:3 | for(...;...;...) ... | The $@ is taken as a mutable reference or its address to a mutable pointer. | test.cpp:328:31:328:31 | l | loop step | test.cpp:328:31:328:31 | l | N/A | diff --git a/cpp/misra/test/rules/RULE-9-5-1/LegacyForStatementsShouldBeSimple.qlref b/cpp/misra/test/rules/RULE-9-5-1/LegacyForStatementsShouldBeSimple.qlref new file mode 100644 index 000000000..bf443d6d6 --- /dev/null +++ b/cpp/misra/test/rules/RULE-9-5-1/LegacyForStatementsShouldBeSimple.qlref @@ -0,0 +1 @@ +rules/RULE-9-5-1/LegacyForStatementsShouldBeSimple.ql \ No newline at end of file diff --git a/cpp/misra/test/rules/RULE-9-5-1/test.cpp b/cpp/misra/test/rules/RULE-9-5-1/test.cpp new file mode 100644 index 000000000..59f077c5b --- /dev/null +++ b/cpp/misra/test/rules/RULE-9-5-1/test.cpp @@ -0,0 +1,332 @@ +#include +#include +#include + +void f1(int &x) {} // Function that takes a non-const integer reference +void g1(int *x) {} // Function that takes a non-const integer pointer +void f2(const int &x) {} // Function that takes a non-const integer reference +void g2(const int *x) {} // Function that takes a non-const integer pointer +void f3(int *const x) {} + +int h1() { return 1; } +constexpr int h2() { return 1; } + +int main() { + int j = 5; + int k = 10; + int l = 2; + + /* ========== 1. Type of the initialized counter variable ========== */ + + for (int i = 0; i < 10; i++) { // COMPLIANT: `i` has an integer type + } + + for (float i = 0.0; i < 10; + i++) { // NON_COMPLIANT: `i` has a non-integer type + } + + /* ========== 2. Termination condition ========== */ + + for (int i = 0; i < 10; i++) { // COMPLIANT: `<` is a relational operator + } + + for (int i = 0; i == 10; + i++) { // NON_COMPLIANT: `==` is not a relational operator + } + + for (int i = 0; j < 10; i++) { // NON_COMPLIANT: `j` is not the loop counter + j++; + } + + /* ========== 3. Updating expression ========== */ + + for (int i = 0; i < 10; + ++i) { // COMPLIANT: Pre-increment operator used as the update expression + } + + for (int i = 0; i < 10; i++) { // COMPLIANT: Post-increment operator used as + // the update expression + } + + for (int i = 20; i > 10; + --i) { // COMPLIANT: Pre-increment operator used as the update expression + } + + for (int i = 20; i > 10; i--) { // COMPLIANT: Post-increment operator used as + // the update expression + } + + for (int i = 0; i < 10; i += 3) { // COMPLIANT: Add-and-assign operator used + // as the update expression with loop step 3 + } + + for (int i = 20; i > 10; + i -= 3) { // COMPLIANT: Add-and-assign operator used + // as the update expression with loop step 3 + } + + for (int i = 0; i < 10; + i = i + + 3) { // COMPLIANT: Direct assignment with addition with loop step 3 + } + + for (int i = 20; i < 10; + i = i - + 3) { // COMPLIANT: Direct assignment with addition with loop step 3 + } + + for (int i = 0; i < 10; + i *= 2) { // NON_COMPLIANT: Mutiplication is not incrementing + } + + /* ========== 4. Type of the loop counter and the loop bound ========== */ + + for (int i = 0; i < 10; i++) { // COMPLIANT: 0 and 10 are of same type + } + + for (unsigned long long int i = 0; i < 10; + i++) { // COMPLIANT: The loop counter has type bigger than that of the + // loop bound + } + + for (short i = 0; i < std::numeric_limits::max(); + i++) { // NON_COMPLIANT: The type of the loop counter is not bigger + // than that of the loop bound + } + + for (int i = 0; i < j; + i++) { // COMPLIANT: The loop step and the loop bound has the same type + } + + /* ========== 5. Immutability of the loop bound and the loop step ========== + */ + + for (int i = 0; i < 10; + i++) { // COMPLIANT: The update expression is an post-increment operation + // and its loop step is always 1 + } + + for (int i = 0; i < 10; i += 2) { // COMPLIANT: The loop step is always 2 + } + + for (int i = 0; i < 10; + i += + j) { // COMPLIANT: The loop step `j` is not mutated anywhere in the loop + } + + for (int i = 0; i < 10; + i += j) { // NON_COMPLIANT: The loop step `j` is mutated in the loop + j++; + } + + for (int i = 0; i < 10; + i += j, j++) { // NON_COMPLIANT: The loop step `j` is mutated in the loop + } + + for (int i = 0; i < j; i++) { // COMPLIANT: The loop bound `j` is not mutated + // anywhere in the loop + } + + for (int i = 0; i < j; i++) { // COMPLIANT: The loop bound `j` is not mutated + // anywhere in the loop + } + + for (int i = 0; i < j; + i++) { // NON_COMPLIANT: The loop bound `j` is mutated in the loop + j++; + } + + for (int i = 0; i < j++; + i++) { // NON_COMPLIANT: The loop bound `j` is mutated in the loop + } + + for (int i = 0; i++ < j++; + i++) { // NON_COMPLIANT: The loop bound `j` is mutated in the loop + } + + int n = 0; + + for (int i = 0; i < k; + i += l) { // NON_COMPLIANT: The loop bound is mutated through an address + *(true ? &k : &n) += 1; + } + + for (int i = 0; i < k; + i += l) { // NON_COMPLIANT: The loop step is mutated through an address + *(true ? &l : &n) += 1; + } + + for (int i = 0; i < h1(); + i++) { // NON_COMPLIANT: The loop bound is not a constant expression + } + + for (int i = 0; i < h2(); + i++) { // COMPLIANT: The loop bound is a constant expression + } + + for (int i = 0; i < j; + i += h1()) { // NON_COMPLIANT: The loop step is not a constant expression + } + + for (int i = 0; i < j; + i += h2()) { // COMPLIANT: The loop step is a constant expression + } + + /* ========== 6. Existence of pointers to the loop counter, loop bound, and + * loop step ========== */ + + for (int i = 0; i < k; i += l) { // COMPLIANT: The loop counter, bound, and + // step are not taken addresses of + } + + for (int i = j; i < k; i += l) { // NON_COMPLIANT: The loop counter is taken + // as a non-const reference + int &m = i; + } + + for (int i = j; i < k; i += l) { // NON_COMPLIANT: The loop counter is taken + // as a non-const pointer + int *m = &i; + } + + for (int i = j; i < k; i += l) { // COMPLIANT: The loop counter is taken + // as a const reference + const int &m = i; + } + + for (int i = j; i < k; i += l) { // COMPLIANT: The loop counter is taken + // as a const pointer + const int *m = &i; + } + + for (int i = j; i < k; i += l) { // NON-COMPLIANT: The loop counter is taken + // as a const but mutable pointer + int *const m = &i; + } + + for (int i = j; i < k; i += l) { // NON_COMPLIANT: The loop bound is taken as + // a non-const reference + int &m = k; + } + + for (int i = j; i < k; i += l) { // NON_COMPLIANT: The loop bound is taken as + // a non-const pointer + int *m = &k; + } + + for (int i = j; i < k; i += l) { // COMPLIANT: The loop bound is taken + // as a const reference + const int &m = k; + } + + for (int i = j; i < k; i += l) { // COMPLIANT: The loop bound is taken + // as a const pointer + const int *m = &k; + } + + for (int i = j; i < k; i += l) { // NON-COMPLIANT: The loop bound is taken as + // a const but mutable pointer + int *const m = &k; + } + + for (int i = j; i < k; i += l) { // NON_COMPLIANT: The loop step is taken as + // a non-const reference + int &m = l; + } + + for (int i = j; i < k; i += l) { // NON_COMPLIANT: The loop step is taken as + // a non-const pointer + int *m = &l; + } + + for (int i = j; i < k; i += l) { // COMPLIANT: The loop step is taken as + // a const reference + const int &m = l; + } + + for (int i = j; i < k; i += l) { // COMPLIANT: The loop step is taken as + // a const pointer + const int *m = &l; + } + + for (int i = j; i < k; i += l) { // NON-COMPLIANT: The loop step is taken as + // a const but mutable pointer + int *const m = &l; + } + + for (int i = j; i < k; i += l) { // NON_COMPLIANT: The loop counter is passed + // to a non-const reference parameter + f1(i); + } + + for (int i = j; i < k; i += l) { // NON_COMPLIANT: The loop counter is passed + // to a non-const pointer parameter + g1(&i); + } + + for (int i = j; i < k; i += l) { // COMPLIANT: The loop counter is passed + // to a const reference parameter + f2(i); + } + + for (int i = j; i < k; i += l) { // COMPLIANT: The loop counter is passed + // to a const pointer parameter + g2(&i); + } + + for (int i = j; i < k; i += l) { // NON_COMPLIANT: The loop counter is passed + // to a const but mutable pointer parameter + f3(&i); + } + + for (int i = j; i < k; i += l) { // NON_COMPLIANT: The loop bound is passed to + // a non-const reference parameter + f1(k); + } + + for (int i = j; i < k; i += l) { // NON_COMPLIANT: The loop bound is passed to + // a non-const pointer parameter + g1(&k); + } + + for (int i = j; i < k; i += l) { // COMPLIANT: The loop bound is passed to a + // const reference parameter + f2(k); + } + + for (int i = j; i < k; + i += + l) { // COMPLIANT: The loop bound is passed to a const pointer parameter + g2(&k); + } + + for (int i = j; i < k; i += l) { // NON_COMPLIANT: The loop bound is passed to + // a const but mutable pointer parameter + f3(&k); + } + + for (int i = j; i < k; i += l) { // NON_COMPLIANT: The loop step is passed to + // a non-const reference parameter + f1(l); + } + + for (int i = j; i < k; i += l) { // NON_COMPLIANT: The loop step is passed to + // a non-const pointer parameter + g1(&l); + } + + for (int i = j; i < k; i += l) { // COMPLIANT: The loop step is passed to + // a const reference parameter + f2(l); + } + + for (int i = j; i < k; i += l) { // COMPLIANT: The loop step is passed to + // a const pointer parameter + g2(&l); + } + + for (int i = j; i < k; i += l) { // NON_COMPLIANT: The loop step is passed to + // a const but mutable pointer parameter + f3(&l); + } +} \ No newline at end of file diff --git a/cpp/misra/test/rules/RULE-9-5-2/ForRangeInitializerAtMostOneFunctionCall.expected b/cpp/misra/test/rules/RULE-9-5-2/ForRangeInitializerAtMostOneFunctionCall.expected new file mode 100644 index 000000000..fdd088bf3 --- /dev/null +++ b/cpp/misra/test/rules/RULE-9-5-2/ForRangeInitializerAtMostOneFunctionCall.expected @@ -0,0 +1,8 @@ +| test.cpp:48:3:49:3 | for(...:...) ... | Range-based for loop has nested call expression in its $@. | test.cpp:48:17:48:27 | call to processData | initializer | +| test.cpp:56:3:59:3 | for(...:...) ... | Range-based for loop has nested call expression in its $@. | test.cpp:56:17:57:19 | call to vector | initializer | +| test.cpp:71:3:73:3 | for(...:...) ... | Range-based for loop has nested call expression in its $@. | test.cpp:71:17:72:21 | call to MyContainer | initializer | +| test.cpp:95:3:97:3 | for(...:...) ... | Range-based for loop has nested call expression in its $@. | test.cpp:95:26:95:26 | call to operator+ | initializer | +| test.cpp:99:3:101:3 | for(...:...) ... | Range-based for loop has nested call expression in its $@. | test.cpp:99:31:99:31 | call to operator+ | initializer | +| test.cpp:103:3:107:3 | for(...:...) ... | Range-based for loop has nested call expression in its $@. | test.cpp:104:18:104:18 | call to operator+ | initializer | +| test.cpp:116:3:119:3 | for(...:...) ... | Range-based for loop has nested call expression in its $@. | test.cpp:117:8:117:25 | call to convertToIntVector | initializer | +| test.cpp:121:3:124:3 | for(...:...) ... | Range-based for loop has nested call expression in its $@. | test.cpp:122:8:122:25 | call to convertToIntVector | initializer | diff --git a/cpp/misra/test/rules/RULE-9-5-2/ForRangeInitializerAtMostOneFunctionCall.qlref b/cpp/misra/test/rules/RULE-9-5-2/ForRangeInitializerAtMostOneFunctionCall.qlref new file mode 100644 index 000000000..be7fc0ef1 --- /dev/null +++ b/cpp/misra/test/rules/RULE-9-5-2/ForRangeInitializerAtMostOneFunctionCall.qlref @@ -0,0 +1 @@ +rules/RULE-9-5-2/ForRangeInitializerAtMostOneFunctionCall.ql \ No newline at end of file diff --git a/cpp/misra/test/rules/RULE-9-5-2/test.cpp b/cpp/misra/test/rules/RULE-9-5-2/test.cpp new file mode 100644 index 000000000..bc8c6c6ac --- /dev/null +++ b/cpp/misra/test/rules/RULE-9-5-2/test.cpp @@ -0,0 +1,133 @@ +#include +#include + +/* Helper functions */ +std::vector getData() { return {1, 2, 3}; } +std::vector processData(const std::vector &input) { return input; } +std::vector getContainer() { return {4, 5, 6}; } + +class MyContainer { +public: + MyContainer() = default; + MyContainer(std::vector data) : data_(data) {} + std::vector::iterator begin() { return data_.begin(); } + std::vector::iterator end() { return data_.end(); } + +private: + std::vector data_{7, 8, 9}; +}; + +class ConvertibleToVector { +public: + operator std::vector() const { return {7, 8, 9}; } + std::array::iterator begin() { return data_.begin(); } + std::array::iterator end() { return data_.end(); } + std::array::const_iterator begin() const { return data_.cbegin(); } + std::array::const_iterator end() const { return data_.cend(); } + +private: + std::array data_{7, 8, 9}; +}; + +std::vector operator+(std::vector a, std::vector b) { + std::vector result = a; + result.insert(result.end(), b.begin(), b.end()); + return result; +} + +std::vector convertToIntVector(std::vector vector) { return vector; } + +int main() { + std::vector localVec = {1, 2, 3}; + + /* ========== 1. EXPLICIT FUNCTION CALLS ========== */ + + for (auto x : getContainer()) { // COMPLIANT: 1 function call only + } + + for (auto x : processData(getData())) { // NON_COMPLIANT: 2 function calls + } + + /* ========== 2. OBJECT CREATION (CONSTRUCTOR CALLS) ========== */ + + for (auto x : std::vector(3)) { // COMPLIANT: 1 constructor call only + } + + for (auto x : std::vector{ + 1, 2, 3}) { // NON_COMPLIANT: 2 constructor call to + // `vector` and `initializer_list`, respectively + } + + for (auto x : MyContainer()) { // COMPLIANT: 1 constructor call only + } + + for (auto x : std::string("hello")) { // COMPLIANT: 1 constructor call only + } + + for (auto x : std::vector( + getData())) { // NON-COMPLIANT: 1 constructor + 1 function call + } + + for (auto x : MyContainer(processData( + localVec))) { // NON-COMPLIANT: 1 constructor + 1 function call + } + + auto data = std::vector(getData()); + for (auto x : data) { // NON-COMPLIANT: 1 constructor + 1 function call + } + + MyContainer myContainer = MyContainer(processData(localVec)); + for (auto x : myContainer) { // NON-COMPLIANT: 1 constructor + 1 function call + } + + /* ========== 3. OPERATOR OVERLOADING ========== */ + + std::vector vec1 = {1}, vec2 = {2}, vec3 = {3}; + std::vector appendedVector = (vec1 + vec2) + vec3; + for (auto x : appendedVector) { // COMPLIANT: 0 calls + } + + std::vector appendedVector2 = getData() + processData(localVec); + for (auto x : appendedVector2) { // COMPLIANT: 0 calls + } + + std::vector anotherVec = {4, 5, 6}; + for (auto x : localVec + anotherVec) { // NON_COMPLIANT: 2 calls to vector's + // constructor, 1 operator+ call + } + + for (auto x : (vec1 + vec2) + vec3) { // NON-COMPLIANT: 3 calls to vector's + // constructor, 2 operator+ calls + } + + for (auto x : + getData() + + processData( + localVec)) { // NON-COMPLIANT: 2 function calls + 1 operator call + } + + /* ========== 4. IMPLICIT CONVERSIONS ========== */ + + ConvertibleToVector convertible; + for (int x : + ConvertibleToVector()) { // COMPLIANT: 1 conversion operator call only + } + + for (int x : + convertToIntVector(convertible)) { // NON_COMPLIANT: 1 function call + 1 + // conversion operator call + } + + for (int x : + convertToIntVector(convertible)) { // NON_COMPLIANT: 1 function call + 1 + // conversion operator call + } + + std::vector intVector1 = convertToIntVector(convertible); + for (int x : intVector1) { // COMPLIANT: 0 function calls + } + + std::vector intVector2 = convertToIntVector(convertible); + for (int x : intVector2) { // COMPLIANT: 0 function calls + } +} \ No newline at end of file diff --git a/rule_packages/cpp/Statements.json b/rule_packages/cpp/Statements.json new file mode 100644 index 000000000..91a643253 --- /dev/null +++ b/rule_packages/cpp/Statements.json @@ -0,0 +1,78 @@ +{ + "MISRA-C++-2023": { + "RULE-9-4-2": { + "properties": { + "allocated-target": [ + "Single Translation Unit" + ], + "enforcement": "decidable", + "obligation": "required" + }, + "queries": [ + { + "description": "A switch statement should have an appropriate structure with proper cases, default labels, and break statements to ensure clear control flow and prevent unintended fall-through behavior.", + "kind": "problem", + "name": "The structure of a switch statement shall be appropriate", + "precision": "very-high", + "severity": "error", + "short_name": "AppropriateStructureOfSwitchStatement", + "tags": [ + "correctness", + "maintainability", + "readability" + ] + } + ], + "title": "The structure of a switch statement shall be appropriate" + }, + "RULE-9-5-1": { + "properties": { + "allocated-target": [ + "Single Translation Unit" + ], + "enforcement": "decidable", + "obligation": "advisory" + }, + "queries": [ + { + "description": "Legacy for statements with complex initialization, condition, and increment expressions can be difficult to understand and maintain. Simple for loops are more readable.", + "kind": "problem", + "name": "Legacy for statements should be simple", + "precision": "high", + "severity": "recommendation", + "short_name": "LegacyForStatementsShouldBeSimple", + "tags": [ + "maintainability", + "readability" + ] + } + ], + "title": "Legacy for statements should be simple" + }, + "RULE-9-5-2": { + "properties": { + "allocated-target": [ + "Single Translation Unit" + ], + "enforcement": "decidable", + "obligation": "required" + }, + "queries": [ + { + "description": "Multiple function calls in a for-range-initializer can lead to unclear iteration behavior and potential side effects that make the code harder to understand and debug.", + "kind": "problem", + "name": "A for-range-initializer shall contain at most one function call", + "precision": "very-high", + "severity": "error", + "short_name": "ForRangeInitializerAtMostOneFunctionCall", + "tags": [ + "correctness", + "maintainability", + "readability" + ] + } + ], + "title": "A for-range-initializer shall contain at most one function call" + } + } +}