dcoughlin added a comment.
> There is a loss of precision for loops that need to be executed exactly
> maxBlockVisitOnPath times, as the loop body is executed with the widened
> state instead of the last iteration.
I think this is an acceptable loss of precision because, in general, it is
unlikely that a concrete-bounded loop will be executed *exactly*
maxBlockVisitOnPath times. You could add special syntactic checks to try to
detect this, but I think it is unlikely to make much different in practice.
================
Comment at: lib/StaticAnalyzer/Core/LoopWidening.cpp:64
@@ +63,3 @@
+ }
+ return PrevState->invalidateRegions(Regions, getLoopCondition(LoopStmt),
+ BlockCount, LCtx, false, nullptr,
----------------
Passing `true` here instead of `false` for `CausedByPointerEscape` should fix
the false alarm. This will cause invalidation to tell the MallocChecker that
the pointer has escaped and so shouldn't be treated as a leak.
================
Comment at: test/Analysis/loop-widening.c:3
@@ +2,3 @@
+
+extern void clang_analyzer_eval(_Bool);
+extern void clang_analyzer_warnIfReached();
----------------
There seems to be some weirdness when using `_Bool` in the prototype (vs. int).
It adds an extra `!= 0` to symbolic expressions, which can result in
clang_analyzer_eval() yielding `UNKNOWN` when using `extern void
clang_analyze_eval(int)` would print `TRUE` or `FALSE`.
We should track this down and fix it, but for now I think it is better to use
`extern void clang_analyze_eval(int)`.
================
Comment at: test/Analysis/loop-widening.c:13
@@ +12,3 @@
+ int x = 1;
+ // Check loop isn't widened by checking x isn't invalidated
+ for (i = 0; i < 1; ++i) {}
----------------
This is clever.
================
Comment at: test/Analysis/loop-widening.c:111
@@ +110,3 @@
+ }
+ if (i < 50) {
+ clang_analyzer_warnIfReached(); // no-warning
----------------
After changing the `clang_analyzer_eval()` prototype to take an int, you can
use `clang_analyzer_eval(i >= 50); // expected-warning {{TRUE}}`.
================
Comment at: test/Analysis/loop-widening.c:158
@@ +157,3 @@
+ clang_analyzer_eval(h_ptr); // expected-warning {{UNKNOWN}}
+ free(h_ptr);
+}
----------------
I think it would be good to add some nested loop tests. For example:
```
void nested1() {
int i = 0, j = 0;
for (i = 0; i < 10; i++) {
clang_analyzer_eval(i < 10); // expected-warning {{TRUE}}
for (j = 0; j < 2; j++) {
clang_analyzer_eval(j < 2); // expected-warning {{TRUE}}
}
clang_analyzer_eval(j >= 2); // expected-warning {{TRUE}}
}
clang_analyzer_eval(i >= 10); // expected-warning {{TRUE}}
}
void nested2() {
int i = 0, j = 0;
for (i = 0; i < 2; i++) {
clang_analyzer_eval(i < 2); // expected-warning {{TRUE}}
for (j = 0; j < 10; j++) {
clang_analyzer_eval(j < 10); // expected-warning {{TRUE}}
}
clang_analyzer_eval(j >= 10); // expected-warning {{TRUE}}
}
clang_analyzer_eval(i >= 2); // expected-warning {{TRUE}}
}
```
http://reviews.llvm.org/D12358
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
