Fix FP reported in #466

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.

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

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'. |

cpp/autosar/test/rules/A7-1-2/test.cpp

@@ -204,4 +204,64 @@ class ExcludedCases {
 
   void operator=(ExcludedCases &) {}  // COMPLIANT
   void operator=(ExcludedCases &&) {} // COMPLIANT
-};
+};
+
+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
+}