NoQ added a comment.
In D75698#1909541 <https://reviews.llvm.org/D75698#1909541>, @martong wrote:
> > While invalidation is a fundamental part of static analysis, it is
> > unfortunately not an under-approximation (resulting in fewer but more
> > precise paths of execution)
>
> +1 for saying this out :)
It //would have been// a correct under-approximation assuming that other parts
of the static analyzer were working correctly. The reason why invalidation
causes false positives is because we're making state splits on every
if-statement and never join back, which causes us to explore more execution
paths than we're entitled to by the presumption of no dead code. Eg.,
class C {
bool x;
void do_not_change_x();
void foo() {
if (x) { // assuming 'x' is true
do_not_change_x(); // 'x' is invalidated
}
// We are not entitled to a state split here
// over the invalidated value of 'x'.
if (x) {
...
}
}
};
Normally such eager state splits don't cause any problems because we tell the
users to "simply add an assert, the code is obscure anyway". But when
invalidation kicks in, like in this example, such asserts become very ugly.
I.e., in this case you'd have to do something like this to suppress the warning
and you'll have to do it every time you call `do_not_change_x()` for every
variable that it //doesn't// change:
bool old_x = x;
do_not_change_x();
assert(x == old_x && "x changed!");
// Also suppress unused variable warning in release builds.
(void)old_x;
P.S. So, like, we could try to emit the warning only if we covered enough
execution paths to prove that there's either dead code or the warning is true.
Then we would no longer care about invalidation problems. Unfortunately, i
don't have any specific suggestion of how to prove such facts for an arbitrary
CFG.
P.P.S. Actually you know what, maybe we should only drop the report if the
constraint over the invalidated value contradicts the constraint over the old
value. That'll make things a bit more complicated and will require a visitor
indeed, though hopefully not as complicated as concrete value tracking, as
we're still interested in only one region at a time.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D75698/new/
https://reviews.llvm.org/D75698
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
