Skip to content

Commit 5ff2a10

Browse files
rvermeulenrvermeulen
authored
Merge pull request #520 from rvermeulen/rvermeulen/fix-466
Fix FP reported in #466
2 parents 08fe14e + 8ea015a commit 5ff2a10

File tree

4 files changed

+148
-2
lines changed

4 files changed

+148
-2
lines changed

‎change_notes/2024-01-31-fix-fp-a7-1-2.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
`A7-1-2` - `VariableMissingConstexpr.ql`:
2+
- 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

Lines changed: 74 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import cpp
1717
import codingstandards.cpp.autosar
1818
import codingstandards.cpp.TrivialType
1919
import codingstandards.cpp.SideEffect
20+
import semmle.code.cpp.controlflow.SSA
2021

2122
predicate isZeroInitializable(Variable v) {
2223
not exists(v.getInitializer().getExpr()) and
@@ -33,6 +34,78 @@ predicate isTypeZeroInitializable(Type t) {
3334
t.getUnderlyingType() instanceof ArrayType
3435
}
3536

37+
/**
38+
* An optimized set of expressions used to determine the flow through constexpr variables.
39+
*/
40+
class VariableAccessOrCallOrLiteral extends Expr {
41+
VariableAccessOrCallOrLiteral() {
42+
this instanceof VariableAccess or
43+
this instanceof Call or
44+
this instanceof Literal
45+
}
46+
}
47+
48+
/**
49+
* Holds if the value of source flows through compile time evaluated variables to target.
50+
*/
51+
predicate flowsThroughConstExprVariables(
52+
VariableAccessOrCallOrLiteral source, VariableAccessOrCallOrLiteral target
53+
) {
54+
(
55+
source = target
56+
or
57+
source != target and
58+
exists(SsaDefinition intermediateDef, StackVariable intermediate |
59+
intermediateDef.getAVariable().getFunction() = source.getEnclosingFunction() and
60+
intermediateDef.getAVariable().getFunction() = target.getEnclosingFunction() and
61+
intermediateDef.getAVariable() = intermediate and
62+
intermediate.isConstexpr()
63+
|
64+
DataFlow::localExprFlow(source, intermediateDef.getDefiningValue(intermediate)) and
65+
flowsThroughConstExprVariables(intermediateDef.getAUse(intermediate), target)
66+
)
67+
)
68+
}
69+
70+
/*
71+
* Returns true if the given call may be evaluated at compile time and is compile time evaluated because
72+
* all its arguments are compile time evaluated and its default values are compile time evaluated.
73+
*/
74+
75+
predicate isCompileTimeEvaluated(Call call) {
76+
// 1. The call may be evaluated at compile time, because it is constexpr, and
77+
call.getTarget().isConstexpr() and
78+
// 2. all its arguments are compile time evaluated, and
79+
forall(DataFlow::Node ultimateArgSource, DataFlow::Node argSource |
80+
argSource = DataFlow::exprNode(call.getAnArgument()) and
81+
DataFlow::localFlow(ultimateArgSource, argSource) and
82+
not DataFlow::localFlowStep(_, ultimateArgSource)
83+
|
84+
(
85+
ultimateArgSource.asExpr() instanceof Literal
86+
or
87+
any(Call c | isCompileTimeEvaluated(c)) = ultimateArgSource.asExpr()
88+
) and
89+
// If the ultimate argument source is not the same as the argument source, then it must flow through
90+
// constexpr variables.
91+
(
92+
ultimateArgSource != argSource
93+
implies
94+
flowsThroughConstExprVariables(ultimateArgSource.asExpr(), argSource.asExpr())
95+
)
96+
) and
97+
// 3. all the default values used are compile time evaluated.
98+
forall(Expr defaultValue, Parameter parameterUsingDefaultValue, int idx |
99+
parameterUsingDefaultValue = call.getTarget().getParameter(idx) and
100+
not exists(call.getArgument(idx)) and
101+
parameterUsingDefaultValue.getAnAssignedValue() = defaultValue
102+
|
103+
defaultValue instanceof Literal
104+
or
105+
any(Call c | isCompileTimeEvaluated(c)) = defaultValue
106+
)
107+
}
108+
36109
from Variable v
37110
where
38111
not isExcluded(v, ConstPackage::variableMissingConstexprQuery()) and
@@ -46,7 +119,7 @@ where
46119
(
47120
v.getInitializer().getExpr().isConstant()
48121
or
49-
v.getInitializer().getExpr().(Call).getTarget().isConstexpr()
122+
any(Call call | isCompileTimeEvaluated(call)) = v.getInitializer().getExpr()
50123
or
51124
isZeroInitializable(v)
52125
or

‎cpp/autosar/test/rules/A7-1-2/VariableMissingConstexpr.expected

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,3 +10,14 @@
1010
| test.cpp:55:7:55:8 | m2 | Variable m2 could be marked 'constexpr'. |
1111
| test.cpp:130:7:130:8 | m1 | Variable m1 could be marked 'constexpr'. |
1212
| test.cpp:141:7:141:8 | m1 | Variable m1 could be marked 'constexpr'. |
13+
| test.cpp:221:7:221:8 | l1 | Variable l1 could be marked 'constexpr'. |
14+
| test.cpp:235:7:235:8 | l6 | Variable l6 could be marked 'constexpr'. |
15+
| test.cpp:237:7:237:8 | l8 | Variable l8 could be marked 'constexpr'. |
16+
| test.cpp:240:7:240:9 | l10 | Variable l10 could be marked 'constexpr'. |
17+
| test.cpp:243:7:243:9 | l12 | Variable l12 could be marked 'constexpr'. |
18+
| test.cpp:248:7:248:9 | l15 | Variable l15 could be marked 'constexpr'. |
19+
| test.cpp:250:7:250:9 | l16 | Variable l16 could be marked 'constexpr'. |
20+
| test.cpp:251:7:251:9 | l17 | Variable l17 could be marked 'constexpr'. |
21+
| test.cpp:257:7:257:9 | l21 | Variable l21 could be marked 'constexpr'. |
22+
| test.cpp:262:7:262:9 | l24 | Variable l24 could be marked 'constexpr'. |
23+
| test.cpp:263:7:263:9 | l25 | Variable l25 could be marked 'constexpr'. |

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

Lines changed: 61 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -204,4 +204,64 @@ class ExcludedCases {
204204

205205
void operator=(ExcludedCases &) {} // COMPLIANT
206206
void operator=(ExcludedCases &&) {} // COMPLIANT
207-
};
207+
};
208+
209+
extern int random();
210+
constexpr int add(int x, int y) { return x + y; }
211+
// Example with compile time constant literal value as default argument
212+
constexpr int add1(int x, int y = 1) { return x + y; }
213+
// Example with compile time constant function call as default argument
214+
constexpr int add2(int x, int y = add(add1(1), 2)) { return x + y; }
215+
// Example with non compile time constant function call as default argument
216+
constexpr int add3(int x, int y = random()) { return x + y; }
217+
// Example with compile time constant literal value as default arguments
218+
constexpr int add4(int x = 1, int y = 2) { return x + y; }
219+
220+
constexpr void fp_reported_in_466(int p) {
221+
int l1 = add(1, 2); // NON_COMPLIANT
222+
int l2 = add(1, p); // COMPLIANT
223+
224+
int l3 = 0;
225+
if (p > 0) {
226+
l3 = 1;
227+
} else {
228+
l3 = p;
229+
}
230+
231+
constexpr int l4 = add(1, 2); // COMPLIANT
232+
233+
int l5 =
234+
add(l3, 2); // COMPLIANT - l3 is not compile time constant on all paths
235+
int l6 = add(l4, 2); // NON_COMPLIANT
236+
int l7 = add(l1, 2); // COMPLIANT - l1 is not constexpr
237+
int l8 =
238+
add1(l4, 2); // NON_COMPLIANT - all arguments are compile time constants
239+
int l9 = add1(l1, 2); // COMPLIANT - l1 is not constexpr
240+
int l10 = add1(l4); // NON_COMPLIANT - argument and the default value of the
241+
// second argument are compile time constants
242+
int l11 = add1(l1); // COMPLIANT - l1 is not constexpr
243+
int l12 = add1(1); // NON_COMPLIANT
244+
int l13 =
245+
add1(1, l3); // COMPLIANT - l3 is not compile time constant on all paths
246+
int l14 =
247+
add1(l3); // COMPLIANT - l3 is not compile time constant on all paths
248+
int l15 = add2(1); // NON_COMPLIANT - provided argument and default value are
249+
// compile time constants
250+
int l16 = add2(1, 2); // NON_COMPLIANT
251+
int l17 = add2(l4, 2); // NON_COMPLIANT
252+
int l18 = add2(l1, 2); // COMPLIANT - l1 is not constexpr
253+
int l19 =
254+
add2(l3); // COMPLIANT - l3 is not compile time constant on all paths
255+
int l20 =
256+
add2(l3, 1); // COMPLIANT - l3 is not compile time constant on all paths
257+
int l21 = add3(1, 1); // NON_COMPLIANT
258+
int l22 = add3(1); // COMPLIANT - default value for second argument is not a
259+
// compile time constant
260+
int l23 =
261+
add3(1, l3); // COMPLIANT - l3 is not compile time constant on all paths
262+
int l24 = add4(); // NON_COMPLIANT - default values are compile time constants
263+
int l25 = add4(1); // NON_COMPLIANT - default value for second argument is a
264+
// compile time constant
265+
int l26 =
266+
add4(1, l3); // COMPLIANT - l3 is not compile time constant on all paths
267+
}

0 commit comments

Comments
 (0)