[PATCH] D64232: [analyzer] Prune calls to functions with linear CFGs that return a non-zero constrained value

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus marked 2 inline comments as done.
Szelethus added a comment.

In D64232#1570938 <https://reviews.llvm.org/D64232#1570938>, @NoQ wrote:

> I'm slightly worried that we're fighting the symptoms rather than the root cause here: why were these values tracked that far in the first place when we already have no interest in tracking them at the end of the function?


Could you please elaborate? Which of the modified test cases (or any other) do you think falls under "being tracked too far" and why? Whenever the CFG where the value isn't linear, I think the information could be valuable, see the inline.

> I.e., i suspect that your "mild tracking mode" would get rid of a lot of those automagically.

A lot of those, correct, all of them, nope. I've been slacking on publishing the moderate tracking I've been working on, I'll get that done during the day.



================
Comment at: clang/test/Analysis/track-control-dependency-conditions.cpp:185
     return true;
   return coin(); // tracking-note{{Returning value}}
 }
----------------
This note is meaningful, the bug would not have occurred if `coin()` wasn't assumed to be false.


================
Comment at: clang/test/Analysis/uninit-vals.c:181
 void testUseHalfPoint() {
-  struct Point p = getHalfPoint(); // expected-note{{Calling 'getHalfPoint'}}
-                                   // expected-note at -1{{Returning from 'getHalfPoint'}}
-                                   // expected-note at -2{{'p' initialized here}}
+  struct Point p = getHalfPoint(); // expected-note{{'p' initialized here}}
   use(p); // expected-warning{{uninitialized}}
----------------
NoQ wrote:
> Huh, so there's not even a note in `getHalfPoint()`, just calling..returning? This definitely needs some attention from `NoStoreFuncVisitor`.
> 
> Generally, i think this is probably the single place where we do really want some info about what happens in `getHalfPoint()`. The report that consists only of "p is initialized..." and "...p is uninitialized" is pretty weird. Btw, could you write down the full warning text in this test? How bad this actually is?
Wild idea: `UninitializedObjectChecker` exposes it's main logic through a header file, how about we use it when we find a read of an uninitialized region?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64232/new/

https://reviews.llvm.org/D64232