diff --git a/change_notes/2024-01-31-fix-fp-a7-1-2.md b/change_notes/2024-01-31-fix-fp-a7-1-2.md new file mode 100644 index 0000000000..94a74d463f --- /dev/null +++ b/change_notes/2024-01-31-fix-fp-a7-1-2.md @@ -0,0 +1,2 @@ +`A7-1-2` - `VariableMissingConstexpr.ql`: + - Fix FP reported in #466. Addresses incorrect assumption that calls to `constexpr` functions are always compile-time evaluated. \ No newline at end of file diff --git a/cpp/autosar/src/rules/A7-1-2/VariableMissingConstexpr.ql b/cpp/autosar/src/rules/A7-1-2/VariableMissingConstexpr.ql index e3981b3836..3c2ae9a592 100644 --- a/cpp/autosar/src/rules/A7-1-2/VariableMissingConstexpr.ql +++ b/cpp/autosar/src/rules/A7-1-2/VariableMissingConstexpr.ql @@ -17,6 +17,7 @@ import cpp import codingstandards.cpp.autosar import codingstandards.cpp.TrivialType import codingstandards.cpp.SideEffect +import semmle.code.cpp.controlflow.SSA predicate isZeroInitializable(Variable v) { not exists(v.getInitializer().getExpr()) and @@ -33,6 +34,78 @@ predicate isTypeZeroInitializable(Type t) { t.getUnderlyingType() instanceof ArrayType } +/** + * An optimized set of expressions used to determine the flow through constexpr variables. + */ +class VariableAccessOrCallOrLiteral extends Expr { + VariableAccessOrCallOrLiteral() { + this instanceof VariableAccess or + this instanceof Call or + this instanceof Literal + } +} + +/** + * Holds if the value of source flows through compile time evaluated variables to target. + */ +predicate flowsThroughConstExprVariables( + VariableAccessOrCallOrLiteral source, VariableAccessOrCallOrLiteral target +) { + ( + source = target + or + source != target and + exists(SsaDefinition intermediateDef, StackVariable intermediate | + intermediateDef.getAVariable().getFunction() = source.getEnclosingFunction() and + intermediateDef.getAVariable().getFunction() = target.getEnclosingFunction() and + intermediateDef.getAVariable() = intermediate and + intermediate.isConstexpr() + | + DataFlow::localExprFlow(source, intermediateDef.getDefiningValue(intermediate)) and + flowsThroughConstExprVariables(intermediateDef.getAUse(intermediate), target) + ) + ) +} + +/* + * Returns true if the given call may be evaluated at compile time and is compile time evaluated because + * all its arguments are compile time evaluated and its default values are compile time evaluated. + */ + +predicate isCompileTimeEvaluated(Call call) { + // 1. The call may be evaluated at compile time, because it is constexpr, and + call.getTarget().isConstexpr() and + // 2. all its arguments are compile time evaluated, and + forall(DataFlow::Node ultimateArgSource, DataFlow::Node argSource | + argSource = DataFlow::exprNode(call.getAnArgument()) and + DataFlow::localFlow(ultimateArgSource, argSource) and + not DataFlow::localFlowStep(_, ultimateArgSource) + | + ( + ultimateArgSource.asExpr() instanceof Literal + or + any(Call c | isCompileTimeEvaluated(c)) = ultimateArgSource.asExpr() + ) and + // If the ultimate argument source is not the same as the argument source, then it must flow through + // constexpr variables. + ( + ultimateArgSource != argSource + implies + flowsThroughConstExprVariables(ultimateArgSource.asExpr(), argSource.asExpr()) + ) + ) and + // 3. all the default values used are compile time evaluated. + forall(Expr defaultValue, Parameter parameterUsingDefaultValue, int idx | + parameterUsingDefaultValue = call.getTarget().getParameter(idx) and + not exists(call.getArgument(idx)) and + parameterUsingDefaultValue.getAnAssignedValue() = defaultValue + | + defaultValue instanceof Literal + or + any(Call c | isCompileTimeEvaluated(c)) = defaultValue + ) +} + from Variable v where not isExcluded(v, ConstPackage::variableMissingConstexprQuery()) and @@ -46,7 +119,7 @@ where ( v.getInitializer().getExpr().isConstant() or - v.getInitializer().getExpr().(Call).getTarget().isConstexpr() + any(Call call | isCompileTimeEvaluated(call)) = v.getInitializer().getExpr() or isZeroInitializable(v) or diff --git a/cpp/autosar/test/rules/A7-1-2/VariableMissingConstexpr.expected b/cpp/autosar/test/rules/A7-1-2/VariableMissingConstexpr.expected index 6b6ed61dc8..dbf223e0cf 100644 --- a/cpp/autosar/test/rules/A7-1-2/VariableMissingConstexpr.expected +++ b/cpp/autosar/test/rules/A7-1-2/VariableMissingConstexpr.expected @@ -10,3 +10,14 @@ | test.cpp:55:7:55:8 | m2 | Variable m2 could be marked 'constexpr'. | | test.cpp:130:7:130:8 | m1 | Variable m1 could be marked 'constexpr'. | | test.cpp:141:7:141:8 | m1 | Variable m1 could be marked 'constexpr'. | +| test.cpp:221:7:221:8 | l1 | Variable l1 could be marked 'constexpr'. | +| test.cpp:235:7:235:8 | l6 | Variable l6 could be marked 'constexpr'. | +| test.cpp:237:7:237:8 | l8 | Variable l8 could be marked 'constexpr'. | +| test.cpp:240:7:240:9 | l10 | Variable l10 could be marked 'constexpr'. | +| test.cpp:243:7:243:9 | l12 | Variable l12 could be marked 'constexpr'. | +| test.cpp:248:7:248:9 | l15 | Variable l15 could be marked 'constexpr'. | +| test.cpp:250:7:250:9 | l16 | Variable l16 could be marked 'constexpr'. | +| test.cpp:251:7:251:9 | l17 | Variable l17 could be marked 'constexpr'. | +| test.cpp:257:7:257:9 | l21 | Variable l21 could be marked 'constexpr'. | +| test.cpp:262:7:262:9 | l24 | Variable l24 could be marked 'constexpr'. | +| test.cpp:263:7:263:9 | l25 | Variable l25 could be marked 'constexpr'. | diff --git a/cpp/autosar/test/rules/A7-1-2/test.cpp b/cpp/autosar/test/rules/A7-1-2/test.cpp index 020ba09a2b..a3b7baea83 100644 --- a/cpp/autosar/test/rules/A7-1-2/test.cpp +++ b/cpp/autosar/test/rules/A7-1-2/test.cpp @@ -204,4 +204,64 @@ class ExcludedCases { void operator=(ExcludedCases &) {} // COMPLIANT void operator=(ExcludedCases &&) {} // COMPLIANT -}; \ No newline at end of file +}; + +extern int random(); +constexpr int add(int x, int y) { return x + y; } +// Example with compile time constant literal value as default argument +constexpr int add1(int x, int y = 1) { return x + y; } +// Example with compile time constant function call as default argument +constexpr int add2(int x, int y = add(add1(1), 2)) { return x + y; } +// Example with non compile time constant function call as default argument +constexpr int add3(int x, int y = random()) { return x + y; } +// Example with compile time constant literal value as default arguments +constexpr int add4(int x = 1, int y = 2) { return x + y; } + +constexpr void fp_reported_in_466(int p) { + int l1 = add(1, 2); // NON_COMPLIANT + int l2 = add(1, p); // COMPLIANT + + int l3 = 0; + if (p > 0) { + l3 = 1; + } else { + l3 = p; + } + + constexpr int l4 = add(1, 2); // COMPLIANT + + int l5 = + add(l3, 2); // COMPLIANT - l3 is not compile time constant on all paths + int l6 = add(l4, 2); // NON_COMPLIANT + int l7 = add(l1, 2); // COMPLIANT - l1 is not constexpr + int l8 = + add1(l4, 2); // NON_COMPLIANT - all arguments are compile time constants + int l9 = add1(l1, 2); // COMPLIANT - l1 is not constexpr + int l10 = add1(l4); // NON_COMPLIANT - argument and the default value of the + // second argument are compile time constants + int l11 = add1(l1); // COMPLIANT - l1 is not constexpr + int l12 = add1(1); // NON_COMPLIANT + int l13 = + add1(1, l3); // COMPLIANT - l3 is not compile time constant on all paths + int l14 = + add1(l3); // COMPLIANT - l3 is not compile time constant on all paths + int l15 = add2(1); // NON_COMPLIANT - provided argument and default value are + // compile time constants + int l16 = add2(1, 2); // NON_COMPLIANT + int l17 = add2(l4, 2); // NON_COMPLIANT + int l18 = add2(l1, 2); // COMPLIANT - l1 is not constexpr + int l19 = + add2(l3); // COMPLIANT - l3 is not compile time constant on all paths + int l20 = + add2(l3, 1); // COMPLIANT - l3 is not compile time constant on all paths + int l21 = add3(1, 1); // NON_COMPLIANT + int l22 = add3(1); // COMPLIANT - default value for second argument is not a + // compile time constant + int l23 = + add3(1, l3); // COMPLIANT - l3 is not compile time constant on all paths + int l24 = add4(); // NON_COMPLIANT - default values are compile time constants + int l25 = add4(1); // NON_COMPLIANT - default value for second argument is a + // compile time constant + int l26 = + add4(1, l3); // COMPLIANT - l3 is not compile time constant on all paths +} \ No newline at end of file