zaks.anna added a comment.
nit: Please, use proper punctuation in the comments.
================
Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:1616
@@ +1615,3 @@
+ builder.isFeasible(false) && !StFalse &&
+ BldCtx.blockCount() == AMgr.options.maxBlockVisitOnPath - 1) {
+
----------------
Do we loose precision for loops that need to be executed exactly
maxBlockVisitOnPath times?
================
Comment at: lib/StaticAnalyzer/Core/LoopWidening.cpp:10
@@ +9,3 @@
+///
+/// This file contains functions which are used to widen constant bound loops.
+/// A loop may be widened to approximate the exit state(s), without analysing
----------------
This would allow to widen not just the constant bound loops!
I think we should widen any loop that we know we did not fully execute.
================
Comment at: lib/StaticAnalyzer/Core/LoopWidening.cpp:171
@@ +170,3 @@
+
+ // Set the approximate value of the loop variable in the last itteration
+ Loc LVLoc = State->getLValue(LoopVariable, LCtx);
----------------
iteration -> iteration
What do you mean by "approximate"? Note that people rely on the analyzer to
track the exact values of variables. They expect it to know what the value is.
It can be very confusing if the analyzer states that it knows the value of the
variable and it is the wrong value.
I am concerned with changing the value of a single variable of the loop based
on a syntactic check. Also, I am not sure there is a strong need for preserving
the value of the index variable either.
================
Comment at: test/Analysis/constant-bound-loops.c:1
@@ +1,2 @@
+// RUN: %clang_cc1 -analyze
-analyzer-checker=core,unix.Malloc,debug.ExprInspection -analyzer-store=region
-analyzer-max-loop 4 -analyzer-config widen-constant-bound-loops=true -verify %s
+
----------------
Should we test that the loop is still executed n times before it is widened?
================
Comment at: test/Analysis/constant-bound-loops.c:174
@@ +173,3 @@
+
+ clang_analyzer_eval(g_global); // expected-warning {{UNKNOWN}}
+ clang_analyzer_eval(s_arg); // expected-warning {{UNKNOWN}}
----------------
I think we should extend this test case.
Ex: what about heap, what about variables touched by the loop variables
declared in the loop's scope?
http://reviews.llvm.org/D12358
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
