The Clang Static Analyzer uses the CFG for walking the expressions.
In the case of DeclStmts, the CFG will emit the subexpressions prior to the declaration itself.
This is in line with the expectation that we need to evaluate those expressions prior to binding any value to the declaration.
However, in constexpr variable declarations, we can assume that the initialized expression is free of undefined behavior; if the code compiles.
Consequently, (most of) the Static Analyzer reports are false-positives in the given calling context at least. Some code-smell warnings such as deadstore or similar warnings might be still justified though, but that’s a different topic.
So, we have false-positives in constexpr initializers, which we want to get rid of.
The D117553 patch would suppress these reports, but that solution is less than ideal.
We should not evaluate any of the initializer subexpressions in the first place if all the reports coming from there will be suppressed.
The idea is that we (probably) shouldn’t emit such initializer expressions in the CFG in the first place.
One can easily omit these expressions by modifying the CFG construction in just a few lines, but this would result in losing precision since we no longer bind anything to the VarDecl in question.
We need to get the result of Expr::EvaluateAsInitializer() of the constexpr initializer, transform that value into an SVal and bind it to the VarDecl.
Other tools consuming the CFG might need to do this as well, or we should introduce an option for turning this feature on/off.
There are different kinds of clang::APValues:
-
None,Indeterminate: I think, we shouldn’t expect these if the code compiles. -
Int: Take theAPSIntand construct a concrete integer sval from this. -
Float,FixedPoint,ComplexInt,ComplexFloat,LValue,Vector,AddrLabelDiff,Union: IDK, probably just returnUnknownVal()for now. -
Array: Take the individual initializerAPValueelements and transform them intoSVals recursively, then create a compound value from thisSVallist. -
Struct: I was thinking about constructing aCXXTempObjectRegionto which I could getFieldRegions and bind the initializerSVals to there; and return aMemRegionVal{tmpregion}SVal. -
MemberPointer: I haven’t thought about it yet. I don’t expect it to be more difficult than thestructcase.
I’ve got a slightly confused, that in each case I’m constructing an SVal directly, without binding anything to the store; but in case of structs I suddenly need to add bindings to the store. I guess it’s because the this should point somewhere right?
I just wanted to have some discussion about this topic before I dive into the implementation details.
PS: We could take this idea one step further. Beyond constexpr initializer expressions, we could say that for any function call of which we know all the arguments, we could create an argument substitution and call the Exp::EvaluateWithSubstitution(); than convert the resulting clang::APValue back to the SVals domain.
However, I would expect this to happen sooo rarely, that honestly, I’m not too excited about it.
What do you think, do you think it is worth the effort to explore the constexpr initializer case?
@NoQ @Xazax-hun @martong @Szelethus
Example code:
constexpr int inc(int x) { return x + 1; }
void fn() {
constexpr int a[] = {0, inc(1), 1, inc(2)};
}
CFG previously:
[B2 (ENTRY)]
Succs (1): B1
[B1]
1: 0
2: inc
3: [B1.2] (ImplicitCastExpr, FunctionToPointerDecay, int (*)(int))
4: 1
5: [B1.3]([B1.4])
6: 1
7: inc
8: [B1.7] (ImplicitCastExpr, FunctionToPointerDecay, int (*)(int))
9: 2
10: [B1.8]([B1.9])
11: {[B1.1], [B1.5], [B1.6], [B1.10]}
12: constexpr int a[] = {0, inc(1), 1, inc(2)};
Preds (1): B2
Succs (1): B0
[B0 (EXIT)]
Preds (1): B
CFG after omitting all the initializer expressions of constexpr variables:
[B2 (ENTRY)]
Succs (1): B1
[B1]
1: constexpr int a[] = {0, inc(1), 1, inc(2)};
Preds (1): B2
Succs (1): B0
[B0 (EXIT)]
Preds (1): B
It might make sense rework this behavior though.