zaks.anna added inline comments.
================
Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:1616
@@ +1615,3 @@
+ builder.isFeasible(false) && !StFalse &&
+ BldCtx.blockCount() == AMgr.options.maxBlockVisitOnPath - 1) {
+
----------------
seaneveson wrote:
> zaks.anna wrote:
> > Do we loose precision for loops that need to be executed exactly
> > maxBlockVisitOnPath times?
> Yes.
>
> To be precise, the loss is for loops that need to be executed
> (maxBlockVisitOnPath - 1) times, because processCFGBlockEntrance generates a
> sink if blockCount >= maxBlockVisitOnPath.
>
> With the default value for maxBlockVisitOnPath, a loop that iterated three
> times would be fully analyzed with widen=false, but would be unnecessarily
> widened if widen=true.
>
> This could be fixed by (effectively) incrementing the value of
> maxBlockVisitOnPath, when the widening was enabled.
>
> e.g. replacing the following line from processCFGBlockEntrance:
>
> ```
> if (nodeBuilder.getContext().blockCount() >=
> AMgr.options.maxBlockVisitOnPath) {
> ...
> ```
> With something like:
>
> ```
> int blockLimit = AMgr.options.maxBlockVisitOnPath +
> (AMgr.options.shouldWidenConstantBoundLoops() ? 1 : 0);
> if (nodeBuilder.getContext().blockCount() >= blockLimit) {
> ...
> ```
>
> Does that seem resonable?
This would increment the value for all loops...
I think we should first answer the question below. Do we want to widen all
loops, not just loops with "constant bounds"?
================
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}}
----------------
seaneveson wrote:
> zaks.anna wrote:
> > 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?
> Good point. I actually encountered a false positive while improving this case.
>
>
> ```
> int *h_ptr = malloc(sizeof(int));
> *h_ptr = 3;
> for (int i = 0; i < 10; ++i) {} // WARNING: Potential leak of memory pointed
> to by 'h_ptr'
> ```
>
> The value of h_ptr is invalidated, but I'm not sure about *h_ptr. Is it
> likely this warning is produced because the pointer is invalidated, but not
> the memory?
Could you provide the full example? There is leak in the code segment above.
http://reviews.llvm.org/D12358
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
