Skip to content

Commit

Permalink
Exclude flows through non constexpr variables
Browse files Browse the repository at this point in the history
Before the analysis only considered whether the source of an argument
passed to a function was computed at compile time. Now we consider
whether intermediate variables are also constexpr even though their
values are compile time constants, because otherwise the compiler
will accept the variable receiving the compiled time constant to be a
constexpr variable.
  • Loading branch information
rvermeulen committed Feb 6, 2024
1 parent fe630e9 commit 1a47115
Show file tree
Hide file tree
Showing 3 changed files with 98 additions and 46 deletions.
51 changes: 46 additions & 5 deletions cpp/autosar/src/rules/A7-1-2/VariableMissingConstexpr.ql
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -33,6 +34,36 @@ 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.
Expand All @@ -42,13 +73,23 @@ 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::localFlow(ultimateArgSource, DataFlow::exprNode(call.getAnArgument())) 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()
(
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 |
Expand Down
22 changes: 11 additions & 11 deletions cpp/autosar/test/rules/A7-1-2/VariableMissingConstexpr.expected
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +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:7 | x | Variable x could be marked 'constexpr'. |
| test.cpp:234:7:234:7 | v | Variable v could be marked 'constexpr'. |
| test.cpp:235:7:235:7 | w | Variable w could be marked 'constexpr'. |
| test.cpp:237:7:237:7 | a | Variable a could be marked 'constexpr'. |
| test.cpp:239:7:239:7 | b | Variable b could be marked 'constexpr'. |
| test.cpp:242:7:242:7 | e | Variable e could be marked 'constexpr'. |
| test.cpp:244:7:244:7 | f | Variable f could be marked 'constexpr'. |
| test.cpp:245:7:245:7 | g | Variable g could be marked 'constexpr'. |
| test.cpp:248:7:248:7 | j | Variable j could be marked 'constexpr'. |
| test.cpp:252:7:252:7 | m | Variable m could be marked 'constexpr'. |
| test.cpp:253:7:253:7 | n | Variable n 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'. |
71 changes: 41 additions & 30 deletions cpp/autosar/test/rules/A7-1-2/test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -218,39 +218,50 @@ constexpr int add3(int x, int y = random()) { return x + y; }
constexpr int add4(int x = 1, int y = 2) { return x + y; }

constexpr void fp_reported_in_466(int p) {
int x = add(1, 2); // NON_COMPLIANT
int y = add(1, p); // COMPLIANT
int l1 = add(1, 2); // NON_COMPLIANT
int l2 = add(1, p); // COMPLIANT

int z = 0;
int l3 = 0;
if (p > 0) {
z = 1;
l3 = 1;
} else {
z = p;
l3 = p;
}

constexpr int t = add(1, 2); // COMPLIANT

int u = add(z, 2); // COMPLIANT - z is not compile time constant on all paths
int v = add(t, 2); // NON_COMPLIANT
int w =
add1(t, 2); // NON_COMPLIANT - all arguments are compile time constants
int a = add1(t); // NON_COMPLIANT - s and the default value of the second
// argument are compile time constants
int b = add1(1); // NON_COMPLIANT
int c = add1(1, z); // COMPLIANT - z is not compile time constant on all paths
int d = add1(z); // COMPLIANT - z is not compile time constant on all paths
int e = add2(1); // NON_COMPLIANT - provided argument and default value are
// compile time constants
int f = add2(1, 2); // NON_COMPLIANT
int g = add2(t, 2); // NON_COMPLIANT
int h = add2(z); // COMPLIANT - z is not compile time constant on all paths
int i = add2(z, 1); // COMPLIANT - z is not compile time constant on all paths
int j = add3(1, 1); // NON_COMPLIANT
int k = add3(1); // COMPLIANT - default value for second argument is not a
// compile time constant
int l = add3(1, z); // COMPLIANT - z is not compile time constant on all paths
int m = add4(); // NON_COMPLIANT - default values are compile time constants
int n = add4(1); // NON_COMPLIANT - default value for second argument is a
// compile time constant
int o = add4(1, z); // COMPLIANT - z is not compile time constant on all paths
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 - s 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
}

0 comments on commit 1a47115

Please sign in to comment.