diff --git a/c/cert/src/codeql-pack.lock.yml b/c/cert/src/codeql-pack.lock.yml index a45ea8f43..9a7d19bc0 100644 --- a/c/cert/src/codeql-pack.lock.yml +++ b/c/cert/src/codeql-pack.lock.yml @@ -1,6 +1,8 @@ --- lockVersion: 1.0.0 dependencies: + advanced-security/qtil: + version: 0.0.3 codeql/cpp-all: version: 4.0.3 codeql/dataflow: diff --git a/c/cert/test/codeql-pack.lock.yml b/c/cert/test/codeql-pack.lock.yml index a45ea8f43..9a7d19bc0 100644 --- a/c/cert/test/codeql-pack.lock.yml +++ b/c/cert/test/codeql-pack.lock.yml @@ -1,6 +1,8 @@ --- lockVersion: 1.0.0 dependencies: + advanced-security/qtil: + version: 0.0.3 codeql/cpp-all: version: 4.0.3 codeql/dataflow: diff --git a/c/common/src/codeql-pack.lock.yml b/c/common/src/codeql-pack.lock.yml index a45ea8f43..9a7d19bc0 100644 --- a/c/common/src/codeql-pack.lock.yml +++ b/c/common/src/codeql-pack.lock.yml @@ -1,6 +1,8 @@ --- lockVersion: 1.0.0 dependencies: + advanced-security/qtil: + version: 0.0.3 codeql/cpp-all: version: 4.0.3 codeql/dataflow: diff --git a/c/common/test/codeql-pack.lock.yml b/c/common/test/codeql-pack.lock.yml index a45ea8f43..9a7d19bc0 100644 --- a/c/common/test/codeql-pack.lock.yml +++ b/c/common/test/codeql-pack.lock.yml @@ -1,6 +1,8 @@ --- lockVersion: 1.0.0 dependencies: + advanced-security/qtil: + version: 0.0.3 codeql/cpp-all: version: 4.0.3 codeql/dataflow: diff --git a/c/misra/src/codeql-pack.lock.yml b/c/misra/src/codeql-pack.lock.yml index a45ea8f43..9a7d19bc0 100644 --- a/c/misra/src/codeql-pack.lock.yml +++ b/c/misra/src/codeql-pack.lock.yml @@ -1,6 +1,8 @@ --- lockVersion: 1.0.0 dependencies: + advanced-security/qtil: + version: 0.0.3 codeql/cpp-all: version: 4.0.3 codeql/dataflow: diff --git a/c/misra/test/codeql-pack.lock.yml b/c/misra/test/codeql-pack.lock.yml index a45ea8f43..9a7d19bc0 100644 --- a/c/misra/test/codeql-pack.lock.yml +++ b/c/misra/test/codeql-pack.lock.yml @@ -1,6 +1,8 @@ --- lockVersion: 1.0.0 dependencies: + advanced-security/qtil: + version: 0.0.3 codeql/cpp-all: version: 4.0.3 codeql/dataflow: diff --git a/cpp/autosar/src/codeql-pack.lock.yml b/cpp/autosar/src/codeql-pack.lock.yml index a45ea8f43..9a7d19bc0 100644 --- a/cpp/autosar/src/codeql-pack.lock.yml +++ b/cpp/autosar/src/codeql-pack.lock.yml @@ -1,6 +1,8 @@ --- lockVersion: 1.0.0 dependencies: + advanced-security/qtil: + version: 0.0.3 codeql/cpp-all: version: 4.0.3 codeql/dataflow: diff --git a/cpp/autosar/test/codeql-pack.lock.yml b/cpp/autosar/test/codeql-pack.lock.yml index a45ea8f43..9a7d19bc0 100644 --- a/cpp/autosar/test/codeql-pack.lock.yml +++ b/cpp/autosar/test/codeql-pack.lock.yml @@ -1,6 +1,8 @@ --- lockVersion: 1.0.0 dependencies: + advanced-security/qtil: + version: 0.0.3 codeql/cpp-all: version: 4.0.3 codeql/dataflow: diff --git a/cpp/cert/src/codeql-pack.lock.yml b/cpp/cert/src/codeql-pack.lock.yml index a45ea8f43..9a7d19bc0 100644 --- a/cpp/cert/src/codeql-pack.lock.yml +++ b/cpp/cert/src/codeql-pack.lock.yml @@ -1,6 +1,8 @@ --- lockVersion: 1.0.0 dependencies: + advanced-security/qtil: + version: 0.0.3 codeql/cpp-all: version: 4.0.3 codeql/dataflow: diff --git a/cpp/cert/test/codeql-pack.lock.yml b/cpp/cert/test/codeql-pack.lock.yml index a45ea8f43..9a7d19bc0 100644 --- a/cpp/cert/test/codeql-pack.lock.yml +++ b/cpp/cert/test/codeql-pack.lock.yml @@ -1,6 +1,8 @@ --- lockVersion: 1.0.0 dependencies: + advanced-security/qtil: + version: 0.0.3 codeql/cpp-all: version: 4.0.3 codeql/dataflow: diff --git a/cpp/common/src/codeql-pack.lock.yml b/cpp/common/src/codeql-pack.lock.yml index a45ea8f43..9a7d19bc0 100644 --- a/cpp/common/src/codeql-pack.lock.yml +++ b/cpp/common/src/codeql-pack.lock.yml @@ -1,6 +1,8 @@ --- lockVersion: 1.0.0 dependencies: + advanced-security/qtil: + version: 0.0.3 codeql/cpp-all: version: 4.0.3 codeql/dataflow: diff --git a/cpp/common/src/codingstandards/cpp/ast/Conditions.qll b/cpp/common/src/codingstandards/cpp/ast/Conditions.qll new file mode 100644 index 000000000..1a5c84c34 --- /dev/null +++ b/cpp/common/src/codingstandards/cpp/ast/Conditions.qll @@ -0,0 +1,29 @@ +import cpp +import codeql.util.Boolean + +/** + * Any type of conditional evaluation, such as if statements, and conditional expressions. + * + * A condition may be: + * - An if statement condition + * - A conditional expression (ternary) condition + * - A short-circuiting logical expression (&& or ||) + */ +class Conditional extends Element { + Expr condition; + + Conditional() { + condition = this.(ConditionalExpr).getCondition() + or + condition = this.(IfStmt).getCondition() + or + condition = this.(LogicalOrExpr).getLeftOperand() + or + condition = this.(LogicalAndExpr).getLeftOperand() + } + + /** + * Get the expression that controls the flow of this conditional. + */ + Expr getCondition() { result = condition } +} diff --git a/cpp/common/src/codingstandards/cpp/ast/Search.qll b/cpp/common/src/codingstandards/cpp/ast/Search.qll new file mode 100644 index 000000000..638593889 --- /dev/null +++ b/cpp/common/src/codingstandards/cpp/ast/Search.qll @@ -0,0 +1,21 @@ +import cpp +import qtil.Qtil + +module OutermostSearch::Type Find> { + Find find(Element e) { + // Do not return a result if there are multiple siblings to different `Find`s. + result = unique(Find found | found = outermostSearchImpl(e)) + } + + private Find outermostSearchImpl(Element e) { + if e instanceof Find + then result = e + else ( + result = outermostSearchImpl(e.(ExprStmt).getExpr()) + or + result = outermostSearchImpl(e.(Stmt).getAChild()) + or + result = outermostSearchImpl(e.(Expr).getAChild()) + ) + } +} diff --git a/cpp/common/src/codingstandards/cpp/exclusions/cpp/Preconditions3.qll b/cpp/common/src/codingstandards/cpp/exclusions/cpp/Preconditions3.qll new file mode 100644 index 000000000..8bc8afb0e --- /dev/null +++ b/cpp/common/src/codingstandards/cpp/exclusions/cpp/Preconditions3.qll @@ -0,0 +1,26 @@ +//** THIS FILE IS AUTOGENERATED, DO NOT MODIFY DIRECTLY. **/ +import cpp +import RuleMetadata +import codingstandards.cpp.exclusions.RuleMetadata + +newtype Preconditions3Query = TAssertMacroUsedWithAConstantExpressionQuery() + +predicate isPreconditions3QueryMetadata(Query query, string queryId, string ruleId, string category) { + query = + // `Query` instance for the `assertMacroUsedWithAConstantExpression` query + Preconditions3Package::assertMacroUsedWithAConstantExpressionQuery() and + queryId = + // `@id` for the `assertMacroUsedWithAConstantExpression` query + "cpp/misra/assert-macro-used-with-a-constant-expression" and + ruleId = "RULE-22-3-1" and + category = "required" +} + +module Preconditions3Package { + Query assertMacroUsedWithAConstantExpressionQuery() { + //autogenerate `Query` type + result = + // `Query` type for `assertMacroUsedWithAConstantExpression` query + TQueryCPP(TPreconditions3PackageQuery(TAssertMacroUsedWithAConstantExpressionQuery())) + } +} diff --git a/cpp/common/src/codingstandards/cpp/exclusions/cpp/RuleMetadata.qll b/cpp/common/src/codingstandards/cpp/exclusions/cpp/RuleMetadata.qll index 2e2403165..cd85f01bd 100644 --- a/cpp/common/src/codingstandards/cpp/exclusions/cpp/RuleMetadata.qll +++ b/cpp/common/src/codingstandards/cpp/exclusions/cpp/RuleMetadata.qll @@ -44,6 +44,7 @@ import Operators import OrderOfEvaluation import OutOfBounds import Pointers +import Preconditions3 import Representation import Scope import SideEffects1 @@ -103,6 +104,7 @@ newtype TCPPQuery = TOrderOfEvaluationPackageQuery(OrderOfEvaluationQuery q) or TOutOfBoundsPackageQuery(OutOfBoundsQuery q) or TPointersPackageQuery(PointersQuery q) or + TPreconditions3PackageQuery(Preconditions3Query q) or TRepresentationPackageQuery(RepresentationQuery q) or TScopePackageQuery(ScopeQuery q) or TSideEffects1PackageQuery(SideEffects1Query q) or @@ -162,6 +164,7 @@ predicate isQueryMetadata(Query query, string queryId, string ruleId, string cat isOrderOfEvaluationQueryMetadata(query, queryId, ruleId, category) or isOutOfBoundsQueryMetadata(query, queryId, ruleId, category) or isPointersQueryMetadata(query, queryId, ruleId, category) or + isPreconditions3QueryMetadata(query, queryId, ruleId, category) or isRepresentationQueryMetadata(query, queryId, ruleId, category) or isScopeQueryMetadata(query, queryId, ruleId, category) or isSideEffects1QueryMetadata(query, queryId, ruleId, category) or diff --git a/cpp/common/src/codingstandards/cpp/standardlibrary/Assert.qll b/cpp/common/src/codingstandards/cpp/standardlibrary/Assert.qll new file mode 100644 index 000000000..4cb48cfee --- /dev/null +++ b/cpp/common/src/codingstandards/cpp/standardlibrary/Assert.qll @@ -0,0 +1,163 @@ +import cpp +import codingstandards.cpp.ast.Conditions +import codingstandards.cpp.ast.Search + +/** + * The assert macro. + */ +class AssertMacro extends Macro { + AssertMacro() { this.getName() = "assert" } +} + +/** + * An assertion via the `assert` macro. + */ +class AssertInvocation extends MacroInvocation { + AssertInvocation() { this.getMacro() instanceof AssertMacro } + + /** + * Get the expression that is being asserted, for example, `x > 0` in `assert(x > 0);`. + * + * Since the assert macro may expand to various forms of conditional statements, this method + * uses a combination of AST inspection and control flow analysis to determine the actual + * condition being asserted. + * + * For example, `assert(x)` may expand to `{ if (!__unlikely(x)) abort(); }`. In this case: + * - We first identify the "outermost conditonal" generated by the macro, which is the `if` + * statement, with the raw condition `!__unlikely(x)`. + * - We then analyze the control flow to see if the abort occurs when this raw condition is true + * or false. In this case, it aborts when the raw condition `!__unlikely(x)` is true. + * - To unwrap the user provided condition `x` from the raw condition, we remove the compiler + * intrinsics such as `__unlikely`. + * - Lastly, we also account for the negation. Since the program aborts on `!x`, we know we are + * asserting `!x` is false, which is equivalent to asserting `x` is true. + * + * Note that the last two bullets are handled in either order, to support `!__unlikely(x)` or + * `__unlikely(!x)`. + */ + Expr getAssertCondition() { + exists(Conditional conditional, Expr condition, Boolean isTrue | + // Get the outermost conditional in the assert body (if, &&, ||, ?:, etc). + conditional = OutermostSearch::find(this.getGeneratedBody()) and + condition = conditional.getCondition() and + // An assertion of form `assert(x)` may expand to a negated form, e.g. `if (!x) abort()`, or + // it may expand to a non-negated form e.g. `x || abort()`. We check whether the condition + // appears to abort when `condition` is true or false to distinguish these cases. + Asserts::appearsToAssert(conditional, isTrue) and + // If we seem to be asserting the condition is both true and false, give no result. + not Asserts::appearsToAssert(conditional, isTrue.booleanNot()) and + // Unwrap compiler inserted calls around the actual asserted condition such as `__unlikely`, + // and unwrap conditions such as `!x` if we found `appearsToAssert(conditional, false)`. + result = Asserts::unwrapAssertRawCondition(condition, isTrue.booleanNot()) + ) + } + + /** + * Helper method to get the body whether it is a statement or an expression. + */ + Element getGeneratedBody() { result = this.getStmt() or result = this.getExpr() } + + /** + * Holds if this is an assertion of the form `assert(false)` or `assert(false && "message")`. + */ + predicate isAssertFalse() { + exists(Expr assertCondition | + assertCondition = this.getAssertCondition() and + ( + // plain `assert(false)` + assertCondition.(Literal).getValue() = "0" + or + // with literal, e.g. `assert(false && "message")` + exists(LogicalAndExpr lAnd | + lAnd = assertCondition and + lAnd.getLeftOperand().(Literal).getValue() = "0" and + lAnd.getRightOperand() instanceof StringLiteral + ) + ) + ) + } +} + +private module Asserts { + /** + * The outermost condition of an assert macro may not be the actual condition passed to assert, as + * the compiler may insert special calls like `__unlikely` or `__builtin_expect` around it. This + * function unwraps those calls to get to the actual condition. + * + * `negated` indicates whether negations were unwrapped. For example, `assert(x)` may expand + * to `if (!x) abort();`, so this predicate would hold for `(x, true)` and `(!x, false)`. + */ + Expr unwrapAssertRawCondition(Expr e, Boolean negated) { + exists(Boolean inner_negated | + result = unwrapAssertRawCondition(e.(NotExpr).getOperand(), inner_negated) and + negated = inner_negated.booleanNot() + ) + or + if e.(FunctionCall).getTarget().getName().matches("__%") + then result = unwrapAssertRawCondition(e.(FunctionCall).getArgument(0), negated) + else ( + result = e and negated = false + ) + } + + /** + * Holds if the given conditional appears to assert its condition to be `isTrue`. + * + * For example, `x || abort();` appears to assert `x` is true, while `if (!x) abort();` + * appears to assert `x` is false. + */ + predicate appearsToAssert(Conditional conditional, Boolean isTrue) { + // Check if an aborting side is reachable via the given boolean value of the condition. + sideAborts(conditional, _, isTrue.booleanNot()) + or + // If the condition is a constant value, then we may not be able to reach the side that aborts + // via control flow. Detect such cases here. + not sideAborts(conditional, _, _) and + ( + // If the false side is unreachable, we presume the false side aborts when reachable + not exists(conditional.getCondition().getAFalseSuccessor()) and + // Therefore this asserts the condition is true + isTrue = true + or + // If the true side is unreachable, we presume the true side aborts when reachable + not exists(conditional.getCondition().getATrueSuccessor()) and + // Therefore this asserts the condition is false + isTrue = false + ) + } + + /** + * Holds if a control flow node contained by the `conditional` ast node appears to abort the + * program when the condition evaluates to `isTrue`. + */ + predicate sideAborts(Conditional conditional, Expr abort, Boolean isTrue) { + abort.getEnclosingElement*() = conditional and + appearsToAbort(abort) and + exists(ControlFlowNode branch | + branch.getASuccessor*() = abort and + ( + isTrue = true and + branch = conditional.getCondition().(ControlFlowNode).getATrueSuccessor() + or + isTrue = false and + branch = conditional.getCondition().(ControlFlowNode).getAFalseSuccessor() + ) + ) + } + + /** + * Holds if a control flow node appears to abort the program. + * + * We currently detect: + * - Calls to functions with "fail" or "abort" in their name. + * - Calls to functions beginning with "__" and containing "assert" in their name. + * - Nodes with no successors (e.g. a call to `std::terminate` or a throw statement). + */ + predicate appearsToAbort(ControlFlowNode node) { + node.(FunctionCall).getTarget().getName().matches(["%fail%", "%abort%", "__%assert"]) + or + not exists(node.getASuccessor()) and not node instanceof Conversion + } +} + +import Asserts diff --git a/cpp/common/src/qlpack.yml b/cpp/common/src/qlpack.yml index 21663a518..222a09606 100644 --- a/cpp/common/src/qlpack.yml +++ b/cpp/common/src/qlpack.yml @@ -3,5 +3,6 @@ version: 2.52.0-dev license: MIT dependencies: codeql/cpp-all: 4.0.3 + advanced-security/qtil: "0.0.3" dataExtensions: - ext/*.model.yml diff --git a/cpp/common/test/codeql-pack.lock.yml b/cpp/common/test/codeql-pack.lock.yml index a45ea8f43..9a7d19bc0 100644 --- a/cpp/common/test/codeql-pack.lock.yml +++ b/cpp/common/test/codeql-pack.lock.yml @@ -1,6 +1,8 @@ --- lockVersion: 1.0.0 dependencies: + advanced-security/qtil: + version: 0.0.3 codeql/cpp-all: version: 4.0.3 codeql/dataflow: diff --git a/cpp/common/test/includes/standard-library/assert.h b/cpp/common/test/includes/standard-library/assert.h index e8ba88d63..20d6528e6 100644 --- a/cpp/common/test/includes/standard-library/assert.h +++ b/cpp/common/test/includes/standard-library/assert.h @@ -1,6 +1,13 @@ #ifndef _GHLIBCPP_ASSERT #define _GHLIBCPP_ASSERT -#define assert(x) (void)0 +#include +#include + +#define __assert(e, file, line) \ + ((void)printf ("%s:%d: failed assertion `%s'\n", file, line, e), abort()) + +#define assert(e) \ + { (__builtin_expect(!(e), 0) ? __assert (#e, __FILE__, __LINE__) : (void)0); } #endif // _GHLIBCPP_ASSERT \ No newline at end of file diff --git a/cpp/common/test/library/codingstandards/cpp/standardlibrary/Assert/AssertTest.expected b/cpp/common/test/library/codingstandards/cpp/standardlibrary/Assert/AssertTest.expected new file mode 100644 index 000000000..e69de29bb diff --git a/cpp/common/test/library/codingstandards/cpp/standardlibrary/Assert/AssertTest.ql b/cpp/common/test/library/codingstandards/cpp/standardlibrary/Assert/AssertTest.ql new file mode 100644 index 000000000..2072f5966 --- /dev/null +++ b/cpp/common/test/library/codingstandards/cpp/standardlibrary/Assert/AssertTest.ql @@ -0,0 +1,25 @@ +import cpp +import codingstandards.cpp.standardlibrary.Assert +import utils.test.InlineExpectationsTest + +module AssertTest implements TestSig { + string getARelevantTag() { result = ["asserts_false", "condition"] } + + predicate hasActualResult(Location location, string element, string tag, string value) { + exists(AssertInvocation assert, Expr condition | + condition = assert.getAssertCondition() and + location = assert.getLocation() and + element = condition.toString() and + ( + tag = "condition" and + value = condition.toString().replaceAll(" ", "") + or + tag = "asserts_false" and + value = "true" and + assert.isAssertFalse() + ) + ) + } +} + +import MakeTest diff --git a/cpp/common/test/library/codingstandards/cpp/standardlibrary/Assert/test.cpp b/cpp/common/test/library/codingstandards/cpp/standardlibrary/Assert/test.cpp new file mode 100644 index 000000000..601784723 --- /dev/null +++ b/cpp/common/test/library/codingstandards/cpp/standardlibrary/Assert/test.cpp @@ -0,0 +1,81 @@ +/** + * C-style assert macros can be implemented in many different structures. + * + * This aims to test that we support as many of these as possible. + */ + +// Disable clang-format for this file to enable inline tests. +// clang-format off + +bool __unlikely(bool condition); +bool __likely(bool condition); +void __assert(const char* expr, const char* file, int line); +bool __custom_abort(const char* expr, const char* file, int line); +void __print(const char* expr, const char* file, int line); +void abort(); +[[noreturn]] void end_control_flow(); + +#define assert(X) \ + { (__builtin_expect(!(X), 0) ? __assert (#X, __FILE__, __LINE__) : (void)0); } + +void g() {} + +void f1(int x) { + assert(x); // $ condition=x + assert(false); // $ condition=0 asserts_false=true + assert(false && "abort here"); // $ condition=...&&... asserts_false=true + assert(0); // $ condition=0 asserts_false=true + assert(1); // $ condition=1 + assert(1 && "always true"); // $ condition=...&&... + assert(x ? 1 : 0); // $ condition=...?...:... +} + +#define assert(X) \ + { __builtin_expect((X), 1) ? (void)0 : __assert (#X, __FILE__, __LINE__); } + +void f2(int x) { + assert(x); // $ condition=x + assert(false); // $ condition=0 asserts_false=true + assert(0); // $ condition=0 asserts_false=true + assert(1); // $ condition=1 +} + +#define assert(X) \ + { if(!(X)) { __print (#X, __FILE__, __LINE__); abort(); } } + +void f3(int x) { + assert(x); // $ condition=x + assert(false); // $ condition=0 asserts_false=true + assert(0); // $ condition=0 asserts_false=true + assert(1); // $ condition=1 +} + +#define assert(X) \ + { __likely(X) || __custom_abort (#X, __FILE__, __LINE__); } + +void f4(int x) { + assert(x); // $ condition=x + assert(false); // $ condition=0 asserts_false=true + assert(0); // $ condition=0 asserts_false=true + assert(1); // $ condition=1 +} + +#define assert(X) \ + { __unlikely(!(X)) && __custom_abort (#X, __FILE__, __LINE__); } + +void f5(int x) { + assert(x); // $ condition=x + assert(false); // $ condition=0 asserts_false=true + assert(0); // $ condition=0 asserts_false=true + assert(1); // $ condition=1 +} + +#define assert(X) \ + { if (__unlikely((X))) { (void) 0; } else { __print( #X, __FILE__, __LINE__); end_control_flow(); } } + +void f6(int x) { + assert(x); // $ condition=x + assert(false); // $ condition=0 asserts_false=true + assert(0); // $ condition=0 asserts_false=true + assert(1); // $ condition=1 +} \ No newline at end of file diff --git a/cpp/misra/src/codeql-pack.lock.yml b/cpp/misra/src/codeql-pack.lock.yml index a45ea8f43..9a7d19bc0 100644 --- a/cpp/misra/src/codeql-pack.lock.yml +++ b/cpp/misra/src/codeql-pack.lock.yml @@ -1,6 +1,8 @@ --- lockVersion: 1.0.0 dependencies: + advanced-security/qtil: + version: 0.0.3 codeql/cpp-all: version: 4.0.3 codeql/dataflow: diff --git a/cpp/misra/src/rules/RULE-22-3-1/AssertMacroUsedWithAConstantExpression.ql b/cpp/misra/src/rules/RULE-22-3-1/AssertMacroUsedWithAConstantExpression.ql new file mode 100644 index 000000000..90aad3517 --- /dev/null +++ b/cpp/misra/src/rules/RULE-22-3-1/AssertMacroUsedWithAConstantExpression.ql @@ -0,0 +1,30 @@ +/** + * @id cpp/misra/assert-macro-used-with-a-constant-expression + * @name RULE-22-3-1: The assert macro shall not be used with a constant-expression + * @description Compile time checking of constant expressions via static_assert is preferred to + * potentially disabled runtime checking via the assert macro. + * @kind problem + * @precision very-high + * @problem.severity error + * @tags external/misra/id/rule-22-3-1 + * scope/single-translation-unit + * correctness + * maintainability + * external/misra/enforcement/decidable + * external/misra/obligation/required + */ + +import cpp +import codingstandards.cpp.misra +import codingstandards.cpp.standardlibrary.Assert + +predicate isConstantExpression(Expr e) { exists(e.getValue()) } + +from AssertInvocation assertion, Expr assertedExpr +where + not isExcluded(assertion, Preconditions3Package::assertMacroUsedWithAConstantExpressionQuery()) and + not assertion.isAssertFalse() and + assertedExpr = assertion.getAssertCondition() and + isConstantExpression(assertedExpr) +select assertion, "Call to 'assert' macro with constant expression value $@.", assertedExpr, + assertedExpr.getValue().toString() diff --git a/cpp/misra/test/codeql-pack.lock.yml b/cpp/misra/test/codeql-pack.lock.yml index a45ea8f43..9a7d19bc0 100644 --- a/cpp/misra/test/codeql-pack.lock.yml +++ b/cpp/misra/test/codeql-pack.lock.yml @@ -1,6 +1,8 @@ --- lockVersion: 1.0.0 dependencies: + advanced-security/qtil: + version: 0.0.3 codeql/cpp-all: version: 4.0.3 codeql/dataflow: diff --git a/cpp/misra/test/rules/RULE-22-3-1/AssertMacroUsedWithAConstantExpression.expected b/cpp/misra/test/rules/RULE-22-3-1/AssertMacroUsedWithAConstantExpression.expected new file mode 100644 index 000000000..b6a5a5002 --- /dev/null +++ b/cpp/misra/test/rules/RULE-22-3-1/AssertMacroUsedWithAConstantExpression.expected @@ -0,0 +1,14 @@ +| test.cpp:21:3:21:26 | assert(e) | Call to 'assert' macro with constant expression value $@. | test.cpp:21:10:21:25 | ... == ... | 1 | +| test.cpp:22:3:22:14 | assert(e) | Call to 'assert' macro with constant expression value $@. | test.cpp:22:10:22:13 | 1 | 1 | +| test.cpp:23:3:23:20 | assert(e) | Call to 'assert' macro with constant expression value $@. | test.cpp:23:10:23:19 | ... == ... | 1 | +| test.cpp:24:3:24:16 | assert(e) | Call to 'assert' macro with constant expression value $@. | test.cpp:24:10:24:15 | ... > ... | 1 | +| test.cpp:28:3:28:42 | assert(e) | Call to 'assert' macro with constant expression value $@. | test.cpp:28:10:28:41 | ... && ... | 1 | +| test.cpp:29:3:29:31 | assert(e) | Call to 'assert' macro with constant expression value $@. | test.cpp:29:10:29:30 | ... && ... | 1 | +| test.cpp:30:3:30:38 | assert(e) | Call to 'assert' macro with constant expression value $@. | test.cpp:30:10:30:37 | ... && ... | 1 | +| test.cpp:63:3:63:18 | assert(e) | Call to 'assert' macro with constant expression value $@. | test.cpp:63:10:63:17 | ... == ... | 1 | +| test.cpp:68:3:68:21 | assert(e) | Call to 'assert' macro with constant expression value $@. | test.cpp:68:10:68:20 | ... == ... | 1 | +| test.cpp:76:5:76:19 | assert(e) | Call to 'assert' macro with constant expression value $@. | test.cpp:76:12:76:18 | ... == ... | 1 | +| test.cpp:77:5:77:29 | assert(e) | Call to 'assert' macro with constant expression value $@. | test.cpp:77:12:77:28 | ... > ... | 1 | +| test.cpp:82:3:82:28 | assert(e) | Call to 'assert' macro with constant expression value $@. | test.cpp:82:10:82:27 | ... == ... | 1 | +| test.cpp:86:3:86:19 | assert(e) | Call to 'assert' macro with constant expression value $@. | test.cpp:86:10:86:18 | ... == ... | 1 | +| test.cpp:87:3:87:19 | assert(e) | Call to 'assert' macro with constant expression value $@. | test.cpp:87:10:87:18 | ... < ... | 1 | diff --git a/cpp/misra/test/rules/RULE-22-3-1/AssertMacroUsedWithAConstantExpression.qlref b/cpp/misra/test/rules/RULE-22-3-1/AssertMacroUsedWithAConstantExpression.qlref new file mode 100644 index 000000000..bca61ba2d --- /dev/null +++ b/cpp/misra/test/rules/RULE-22-3-1/AssertMacroUsedWithAConstantExpression.qlref @@ -0,0 +1 @@ +rules/RULE-22-3-1/AssertMacroUsedWithAConstantExpression.ql \ No newline at end of file diff --git a/cpp/misra/test/rules/RULE-22-3-1/test.cpp b/cpp/misra/test/rules/RULE-22-3-1/test.cpp new file mode 100644 index 000000000..0b10d7c77 --- /dev/null +++ b/cpp/misra/test/rules/RULE-22-3-1/test.cpp @@ -0,0 +1,93 @@ +#include +#include + +// Test cases for MISRA C++ 2023 Rule 22.3.1 +// The assert macro shall not be used with a constant-expression + +void test_assert_with_variable_expression() { + std::int32_t l1 = 42; + assert(l1 > 0); // COMPLIANT + + std::int32_t l2 = 100; + assert(l2 < 1000); // COMPLIANT +} + +void test_assert_with_function_call() { + auto f1 = []() { return true; }; + assert(f1()); // COMPLIANT +} + +void test_assert_with_constant_expression() { + assert(sizeof(int) == 4); // NON_COMPLIANT + assert(true); // NON_COMPLIANT + assert(1 + 1 == 2); // NON_COMPLIANT + assert(42 > 0); // NON_COMPLIANT +} + +void test_assert_with_constant_expression_and_string() { + assert((sizeof(int) == 4) && "Bad size"); // NON_COMPLIANT + assert(true && "Always true"); // NON_COMPLIANT + assert((1 + 1 == 2) && "Math works"); // NON_COMPLIANT +} + +void test_assert_false_exception() { + assert(false); // COMPLIANT + assert(false && "Unexpected path"); // COMPLIANT + assert(false && "Should not reach here"); // COMPLIANT +} + +void test_assert_with_mixed_expressions() { + std::int32_t l1 = 10; + // When dynamic and constant expressions are mixed, we treat the assertion as + // compliant. + // + // Technically, some cases such as `assert(dynamic && static)` is equivalent + // to `assert(dynamic); assert(static)` and we could report them. However, + // other cases such as `assert(dynamic || static)` are *not* equivalent to any + // standalone static assertion. + // + // Distinguishing these cases correctly in all cases is non-trivial, so we do + // not report any mixed cases at this time. + // + // We do not consider the following cases to be false negatives: + assert(l1 > 0 && sizeof(int) == 8); // COMPLIANT + assert(sizeof(int) == 4 && l1 > 0); // COMPLIANT + // Because they are difficult to distinguish from the following, which are + // clearly compliant: + assert(l1 > 0 || sizeof(int) == 4); // COMPLIANT + assert(sizeof(int) == 8 || l1 > 0); // COMPLIANT +} + +void test_assert_with_template_constant() { + constexpr std::int32_t l1 = 42; + assert(l1 == 42); // NON_COMPLIANT +} + +void test_assert_with_enum_constant() { + enum { VALUE = 10 }; + assert(VALUE == 10); // NON_COMPLIANT +} + +class TestClass { +public: + static constexpr std::int32_t m1 = 5; + + void test_assert_with_static_member() { + assert(m1 == 5); // NON_COMPLIANT + assert(TestClass::m1 > 0); // NON_COMPLIANT + } +}; + +void test_assert_with_nullptr_constant() { + assert(nullptr == nullptr); // NON_COMPLIANT +} + +void test_assert_with_character_literal() { + assert('a' == 97); // NON_COMPLIANT + assert('A' < 'Z'); // NON_COMPLIANT +} + +void test_assert_with_string_literal_comparison() { + const char *l1 = "test"; + assert(l1 != nullptr); // COMPLIANT +} \ No newline at end of file diff --git a/rule_packages/cpp/Preconditions3.json b/rule_packages/cpp/Preconditions3.json new file mode 100644 index 000000000..393eb9860 --- /dev/null +++ b/rule_packages/cpp/Preconditions3.json @@ -0,0 +1,26 @@ +{ + "MISRA-C++-2023": { + "RULE-22-3-1": { + "properties": { + "enforcement": "decidable", + "obligation": "required" + }, + "queries": [ + { + "description": "Compile time checking of constant expressions via static_assert is preferred to potentially disabled runtime checking via the assert macro.", + "kind": "problem", + "name": "The assert macro shall not be used with a constant-expression", + "precision": "very-high", + "severity": "error", + "short_name": "AssertMacroUsedWithAConstantExpression", + "tags": [ + "scope/single-translation-unit", + "correctness", + "maintainability" + ] + } + ], + "title": "The assert macro shall not be used with a constant-expression" + } + } +} \ No newline at end of file diff --git a/rules.csv b/rules.csv index 3cb64a0ba..3d53f4d55 100644 --- a/rules.csv +++ b/rules.csv @@ -986,7 +986,7 @@ cpp,MISRA-C++-2023,RULE-21-6-5,Yes,Required,Decidable,Single Translation Unit,A cpp,MISRA-C++-2023,RULE-21-10-1,Yes,Required,Decidable,Single Translation Unit,The features of shall not be used,DCL50-CPP,BannedAPIs,Easy, cpp,MISRA-C++-2023,RULE-21-10-2,Yes,Required,Decidable,Single Translation Unit,The standard header file shall not be used,ERR52-CPP,BannedAPIs,Easy, cpp,MISRA-C++-2023,RULE-21-10-3,Yes,Required,Decidable,Single Translation Unit,The facilities provided by the standard header file shall not be used,M18-7-1,ImportMisra23,Import, -cpp,MISRA-C++-2023,RULE-22-3-1,Yes,Required,Decidable,Single Translation Unit,The assert macro shall not be used with a constant-expression,,Preconditions,Easy, +cpp,MISRA-C++-2023,RULE-22-3-1,Yes,Required,Decidable,Single Translation Unit,The assert macro shall not be used with a constant-expression,,Preconditions3,Easy, cpp,MISRA-C++-2023,RULE-22-4-1,Yes,Required,Decidable,Single Translation Unit,The literal value zero shall be the only value assigned to errno,,Preconditions,Easy, cpp,MISRA-C++-2023,RULE-23-11-1,Yes,Advisory,Decidable,Single Translation Unit,The raw pointer constructors of std::shared_ptr and std::unique_ptr should not be used,,BannedAPIs,Easy, cpp,MISRA-C++-2023,RULE-24-5-1,Yes,Required,Decidable,Single Translation Unit,The character handling functions from and shall not be used,,BannedAPIs,Easy,