[PATCH] D12358: [Analyzer] Widening loops which do not exit

[Devin Coughlin via cfe-commits]

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