[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]

This revision was automatically updated to reflect the committed changes.
Szelethus marked 5 inline comments as done.
Closed by commit rL368771: [analyzer] Prune calls to functions with linear CFGs 
that return a non-zero… (authored by Szelethus, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D64232?vs=212801=214969#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D64232

Files:
  cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
  cfe/trunk/test/Analysis/diagnostics/find_last_store.c
  cfe/trunk/test/Analysis/track-control-dependency-conditions.cpp
  cfe/trunk/test/Analysis/uninit-vals.c

Index: cfe/trunk/test/Analysis/diagnostics/find_last_store.c
===
--- cfe/trunk/test/Analysis/diagnostics/find_last_store.c
+++ cfe/trunk/test/Analysis/diagnostics/find_last_store.c
@@ -2,13 +2,11 @@
 typedef struct { float b; } c;
 void *a();
 void *d() {
-  return a(); // expected-note{{Returning pointer}}
+  return a();
 }
 
 void no_find_last_store() {
-  c *e = d(); // expected-note{{Calling 'd'}}
-  // expected-note@-1{{Returning from 'd'}}
-  // expected-note@-2{{'e' initialized here}}
+  c *e = d(); // expected-note{{'e' initialized here}}
 
   (void)(e || e->b); // expected-note{{Assuming 'e' is null}}
   // expected-note@-1{{Left side of '||' is false}}
Index: cfe/trunk/test/Analysis/track-control-dependency-conditions.cpp
===
--- cfe/trunk/test/Analysis/track-control-dependency-conditions.cpp
+++ cfe/trunk/test/Analysis/track-control-dependency-conditions.cpp
@@ -119,7 +119,7 @@
 bool coin();
 
 bool foo() {
-  return coin(); // tracking-note{{Returning value}}
+  return coin();
 }
 
 int bar;
@@ -127,12 +127,10 @@
 void test() {
   int *x = 0; // expected-note{{'x' initialized to a null pointer value}}
 
-  if (int flag = foo()) // tracking-note{{Calling 'foo'}}
-// tracking-note@-1{{Returning from 'foo'}}
-// tracking-note@-2{{'flag' initialized here}}
-// debug-note@-3{{Tracking condition 'flag'}}
-// expected-note@-4{{Assuming 'flag' is not equal to 0}}
-// expected-note@-5{{Taking true branch}}
+  if (int flag = foo()) // tracking-note{{'flag' initialized here}}
+// debug-note@-1{{Tracking condition 'flag'}}
+// expected-note@-2{{Assuming 'flag' is not equal to 0}}
+// expected-note@-3{{Taking true branch}}
 
 *x = 5; // expected-warning{{Dereference of null pointer}}
 // expected-note@-1{{Dereference of null pointer}}
@@ -143,39 +141,114 @@
 bool coin();
 
 struct ConvertsToBool {
-  operator bool() const { return coin(); } // tracking-note{{Returning value}}
+  operator bool() const { return coin(); }
 };
 
 void test() {
   int *x = 0; // expected-note{{'x' initialized to a null pointer value}}
 
   if (ConvertsToBool())
-// tracking-note@-1 {{Calling 'ConvertsToBool::operator bool'}}
-// tracking-note@-2{{Returning from 'ConvertsToBool::operator bool'}}
-// debug-note@-3{{Tracking condition 'ConvertsToBool()'}}
-// expected-note@-4{{Assuming the condition is true}}
-// expected-note@-5{{Taking true branch}}
+// debug-note@-1{{Tracking condition 'ConvertsToBool()'}}
+// expected-note@-2{{Assuming the condition is true}}
+// expected-note@-3{{Taking true branch}}
 *x = 5; // expected-warning{{Dereference of null pointer}}
 // expected-note@-1{{Dereference of null pointer}}
 }
 
 } // end of namespace variable_declaration_in_condition
 
+namespace important_returning_pointer_loaded_from {
+bool coin();
+
+int *getIntPtr();
+
+void storeValue(int **i) {
+  *i = getIntPtr(); // tracking-note{{Value assigned to 'i'}}
+}
+
+int *conjurePointer() {
+  int *i;
+  storeValue(); // tracking-note{{Calling 'storeValue'}}
+  // tracking-note@-1{{Returning from 'storeValue'}}
+  return i;   // tracking-note{{Returning pointer (loaded from 'i')}}
+}
+
+void f(int *ptr) {
+  if (ptr) // expected-note{{Assuming 'ptr' is null}}
+   // expected-note@-1{{Taking false branch}}
+;
+  if (!conjurePointer())
+// tracking-note@-1{{Calling 'conjurePointer'}}
+// tracking-note@-2{{Returning from 'conjurePointer'}}
+// debug-note@-3{{Tracking condition '!conjurePointer()'}}
+// expected-note@-4{{Assuming the condition is true}}
+// expected-note@-5{{Taking true branch}}
+*ptr = 5; // expected-warning{{Dereference of null pointer}}
+  // expected-note@-1{{Dereference of null pointer}}
+}
+} // end of namespace important_returning_pointer_loaded_from
+
+namespace 

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

[Artem Dergachev via Phabricator via cfe-commits]

NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.

Sry, should have approved this ages ago >.<




Comment at: clang/test/Analysis/uninit-vals.c:181
 void testUseHalfPoint() {
-  struct Point p = getHalfPoint(); // expected-note{{Calling 'getHalfPoint'}}
-   // expected-note@-1{{Returning from 
'getHalfPoint'}}
-   // expected-note@-2{{'p' initialized here}}
+  struct Point p = getHalfPoint(); // expected-note{{'p' initialized here}}
   use(p); // expected-warning{{uninitialized}}

Szelethus wrote:
> NoQ wrote:
> > Szelethus wrote:
> > > Szelethus wrote:
> > > > 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?
> > > Passed-by-value struct argument contains uninitialized data (e.g., field: 
> > > 'y')
> > > 
> > > Quite good imo.
> > What specific logic are you talking about? You mean it'd scan the structure 
> > for uninitialized fields and present the results of the scan to the user in 
> > a note?
> >What specific logic are you talking about? You mean it'd scan the structure 
> >for uninitialized fields and present the results of the scan to the user in 
> >a note?
> Nvm, looked at the code, realized that what I said made no sense. What we are 
> really missing here is a `trackRegionValue()` function :^)
> 
> Btw, I wasted s much time on figuring out that you don't get ANY notes 
> whatsoever if you make this a cpp file rather than a c file, only the 
> warning... Is this intended?
> Btw, I wasted s much time on figuring out that you don't get ANY notes 
> whatsoever if you make this a cpp file rather than a c file, only the 
> warning... Is this intended?

At a glance, the behavior of this code in C and C++ is ridiculously different. 
The way structures are returned-by-value is one of the glaring differences 
between C and C++. Even at runtime / ABI / calling convention level: in C it's 
acceptable to simply pass the structure through registers (or put it on the 
stack, wherever the return value normally lives), however in C++ the calling 
convention usually requres you to pass return address as a separate hidden 
parameter so that to use it as "this" in the structure's constructor (and then 
later use it for RVO and NRVO which may span multiple stack frames) . And the 
Static Analyzer also deals with completely different ASTs. So i'm not surprised 
that there's a difference.

But i would also probably not want there to be a difference. If you could turn 
it into a sensible bug report i guess it could be helpful.


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

https://reviews.llvm.org/D64232



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[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 updated this revision to Diff 212801.
Szelethus added a comment.

Use `size() == 3` instead if `isLinear()`, add a related test case.


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

https://reviews.llvm.org/D64232

Files:
  clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
  clang/test/Analysis/diagnostics/find_last_store.c
  clang/test/Analysis/track-control-dependency-conditions.cpp
  clang/test/Analysis/uninit-vals.c

Index: clang/test/Analysis/uninit-vals.c
===
--- clang/test/Analysis/uninit-vals.c
+++ clang/test/Analysis/uninit-vals.c
@@ -149,8 +149,6 @@
   RetVoidFuncType f = foo_radar12278788_fp;
   return ((RetIntFuncType)f)(); //expected-warning {{Undefined or garbage value returned to caller}}
 //expected-note@-1 {{Undefined or garbage value returned to caller}}
-//expected-note@-2 {{Calling 'foo_radar12278788_fp'}}
-//expected-note@-3 {{Returning from 'foo_radar12278788_fp'}}
 }
 
 void rdar13665798() {
@@ -164,8 +162,6 @@
 RetVoidFuncType f = foo_radar12278788_fp;
 return ((RetIntFuncType)f)(); //expected-warning {{Undefined or garbage value returned to caller}}
   //expected-note@-1 {{Undefined or garbage value returned to caller}}
-  //expected-note@-2 {{Calling 'foo_radar12278788_fp'}}
-  //expected-note@-3 {{Returning from 'foo_radar12278788_fp'}}
   }();
 }
 
@@ -182,18 +178,14 @@
 void use(struct Point p); 
 
 void testUseHalfPoint() {
-  struct Point p = getHalfPoint(); // expected-note{{Calling 'getHalfPoint'}}
-   // expected-note@-1{{Returning from 'getHalfPoint'}}
-   // expected-note@-2{{'p' initialized here}}
+  struct Point p = getHalfPoint(); // expected-note{{'p' initialized here}}
   use(p); // expected-warning{{uninitialized}}
   // expected-note@-1{{uninitialized}}
 }
 
 void testUseHalfPoint2() {
   struct Point p;
-  p = getHalfPoint(); // expected-note{{Calling 'getHalfPoint'}}
-  // expected-note@-1{{Returning from 'getHalfPoint'}}
-  // expected-note@-2{{Value assigned to 'p'}}
+  p = getHalfPoint(); // expected-note{{Value assigned to 'p'}}
   use(p); // expected-warning{{uninitialized}}
   // expected-note@-1{{uninitialized}}
 }
Index: clang/test/Analysis/track-control-dependency-conditions.cpp
===
--- clang/test/Analysis/track-control-dependency-conditions.cpp
+++ clang/test/Analysis/track-control-dependency-conditions.cpp
@@ -119,7 +119,7 @@
 bool coin();
 
 bool foo() {
-  return coin(); // tracking-note{{Returning value}}
+  return coin();
 }
 
 int bar;
@@ -127,12 +127,10 @@
 void test() {
   int *x = 0; // expected-note{{'x' initialized to a null pointer value}}
 
-  if (int flag = foo()) // tracking-note{{Calling 'foo'}}
-// tracking-note@-1{{Returning from 'foo'}}
-// tracking-note@-2{{'flag' initialized here}}
-// debug-note@-3{{Tracking condition 'flag'}}
-// expected-note@-4{{Assuming 'flag' is not equal to 0}}
-// expected-note@-5{{Taking true branch}}
+  if (int flag = foo()) // tracking-note{{'flag' initialized here}}
+// debug-note@-1{{Tracking condition 'flag'}}
+// expected-note@-2{{Assuming 'flag' is not equal to 0}}
+// expected-note@-3{{Taking true branch}}
 
 *x = 5; // expected-warning{{Dereference of null pointer}}
 // expected-note@-1{{Dereference of null pointer}}
@@ -143,39 +141,114 @@
 bool coin();
 
 struct ConvertsToBool {
-  operator bool() const { return coin(); } // tracking-note{{Returning value}}
+  operator bool() const { return coin(); }
 };
 
 void test() {
   int *x = 0; // expected-note{{'x' initialized to a null pointer value}}
 
   if (ConvertsToBool())
-// tracking-note@-1 {{Calling 'ConvertsToBool::operator bool'}}
-// tracking-note@-2{{Returning from 'ConvertsToBool::operator bool'}}
-// debug-note@-3{{Tracking condition 'ConvertsToBool()'}}
-// expected-note@-4{{Assuming the condition is true}}
-// expected-note@-5{{Taking true branch}}
+// debug-note@-1{{Tracking condition 'ConvertsToBool()'}}
+// expected-note@-2{{Assuming the condition is true}}
+// expected-note@-3{{Taking true branch}}
 *x = 5; // expected-warning{{Dereference of null pointer}}
 // expected-note@-1{{Dereference of null pointer}}
 }
 
 } // end of namespace variable_declaration_in_condition
 
+namespace important_returning_pointer_loaded_from {
+bool coin();
+
+int *getIntPtr();
+
+void storeValue(int **i) {
+  *i = getIntPtr(); 

[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 updated this revision to Diff 211744.
Szelethus added a comment.

- Be even more strict on the rule: mark the note as prunable even if the note 
message says `.*(loaded from '')`. This is okay, because 
`ReturnVisitor` will track the return value anyways, and if at least a single 
non-prunable note is added to the function, it won't be pruned (see the test 
case in the namespace `important_returning_pointer_loaded_from`).
- Add test cases for when the return value is function-local 
(`unimportant_returning_pointer_loaded_from`), and when it's the parameter 
(`unimportant_returning_pointer_loaded_from_through_cast`, creduced and then 
manually reduced from an ASTImporter.cpp 

 report).


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

https://reviews.llvm.org/D64232

Files:
  clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
  clang/test/Analysis/diagnostics/find_last_store.c
  clang/test/Analysis/track-control-dependency-conditions.cpp
  clang/test/Analysis/uninit-vals.c

Index: clang/test/Analysis/uninit-vals.c
===
--- clang/test/Analysis/uninit-vals.c
+++ clang/test/Analysis/uninit-vals.c
@@ -149,8 +149,6 @@
   RetVoidFuncType f = foo_radar12278788_fp;
   return ((RetIntFuncType)f)(); //expected-warning {{Undefined or garbage value returned to caller}}
 //expected-note@-1 {{Undefined or garbage value returned to caller}}
-//expected-note@-2 {{Calling 'foo_radar12278788_fp'}}
-//expected-note@-3 {{Returning from 'foo_radar12278788_fp'}}
 }
 
 void rdar13665798() {
@@ -164,8 +162,6 @@
 RetVoidFuncType f = foo_radar12278788_fp;
 return ((RetIntFuncType)f)(); //expected-warning {{Undefined or garbage value returned to caller}}
   //expected-note@-1 {{Undefined or garbage value returned to caller}}
-  //expected-note@-2 {{Calling 'foo_radar12278788_fp'}}
-  //expected-note@-3 {{Returning from 'foo_radar12278788_fp'}}
   }();
 }
 
@@ -182,18 +178,14 @@
 void use(struct Point p); 
 
 void testUseHalfPoint() {
-  struct Point p = getHalfPoint(); // expected-note{{Calling 'getHalfPoint'}}
-   // expected-note@-1{{Returning from 'getHalfPoint'}}
-   // expected-note@-2{{'p' initialized here}}
+  struct Point p = getHalfPoint(); // expected-note{{'p' initialized here}}
   use(p); // expected-warning{{uninitialized}}
   // expected-note@-1{{uninitialized}}
 }
 
 void testUseHalfPoint2() {
   struct Point p;
-  p = getHalfPoint(); // expected-note{{Calling 'getHalfPoint'}}
-  // expected-note@-1{{Returning from 'getHalfPoint'}}
-  // expected-note@-2{{Value assigned to 'p'}}
+  p = getHalfPoint(); // expected-note{{Value assigned to 'p'}}
   use(p); // expected-warning{{uninitialized}}
   // expected-note@-1{{uninitialized}}
 }
Index: clang/test/Analysis/track-control-dependency-conditions.cpp
===
--- clang/test/Analysis/track-control-dependency-conditions.cpp
+++ clang/test/Analysis/track-control-dependency-conditions.cpp
@@ -119,7 +119,7 @@
 bool coin();
 
 bool foo() {
-  return coin(); // tracking-note{{Returning value}}
+  return coin();
 }
 
 int bar;
@@ -127,12 +127,10 @@
 void test() {
   int *x = 0; // expected-note{{'x' initialized to a null pointer value}}
 
-  if (int flag = foo()) // tracking-note{{Calling 'foo'}}
-// tracking-note@-1{{Returning from 'foo'}}
-// tracking-note@-2{{'flag' initialized here}}
-// debug-note@-3{{Tracking condition 'flag'}}
-// expected-note@-4{{Assuming 'flag' is not equal to 0}}
-// expected-note@-5{{Taking true branch}}
+  if (int flag = foo()) // tracking-note{{'flag' initialized here}}
+// debug-note@-1{{Tracking condition 'flag'}}
+// expected-note@-2{{Assuming 'flag' is not equal to 0}}
+// expected-note@-3{{Taking true branch}}
 
 *x = 5; // expected-warning{{Dereference of null pointer}}
 // expected-note@-1{{Dereference of null pointer}}
@@ -143,39 +141,114 @@
 bool coin();
 
 struct ConvertsToBool {
-  operator bool() const { return coin(); } // tracking-note{{Returning value}}
+  operator bool() const { return coin(); }
 };
 
 void test() {
   int *x = 0; 

[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 added a comment.

LLVM+Clang (A total of 207 reports changed)

| with "track-conditions" set to true (bug length)  







  | with 
"track-conditions" set to true and this patch applied (bug length)  







   |
| InstrTypes.cpp 

 (9)
 | InstrTypes.cpp 

 (5)
 |
| APInt.h 

 (17)   
| 
APInt.h 

 (13)   
|
| ASTDiagnostic.cpp 

 (19)   
  | 
ASTDiagnostic.cpp 

 (15)   
   

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

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:1052
+// to return that every time.
+if (N->getCFG().isLinear())
+  WouldEventBeMeaningless = true;

NoQ wrote:
> This time i'd rather go for "has exactly 3 blocks", because we're mostly 
> interested in the CFG being "syntactically linear" rather than "semantically 
> linear". I.e., if the CFG has an if-statement with a compile-time-constant 
> condition, i'd rather show the path, because who knows what does the 
> programmer think about this condition.
Yeah, this is a hard question, both sides can have its own reasons. 

One of my concern is that functions that does seem to be linear to people are 
actually not linear CFG wise. One example is single one liner functions with an 
assert preceding the line in question. 

Other interesting note is that once we add exception support and edges for 
exception handling in the CFG, all the analyzer budgets and CFG node count 
based heuristics need to be redone. But this is not something we should worry 
about at this point. 




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

https://reviews.llvm.org/D64232



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[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 updated this revision to Diff 210330.
Szelethus added a comment.

Rebase on top master. Putting this on the bottom of the patch stack because 
this really deserves it's own analysis. (Side note, I completely messed up like 
~40 hrs worth of analysis because I didn't check which branches do I have 
stacked on top of each other, so this might take a while...)


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

https://reviews.llvm.org/D64232

Files:
  clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
  clang/test/Analysis/diagnostics/find_last_store.c
  clang/test/Analysis/track-control-dependency-conditions.cpp
  clang/test/Analysis/uninit-vals.c

Index: clang/test/Analysis/uninit-vals.c
===
--- clang/test/Analysis/uninit-vals.c
+++ clang/test/Analysis/uninit-vals.c
@@ -149,8 +149,6 @@
   RetVoidFuncType f = foo_radar12278788_fp;
   return ((RetIntFuncType)f)(); //expected-warning {{Undefined or garbage value returned to caller}}
 //expected-note@-1 {{Undefined or garbage value returned to caller}}
-//expected-note@-2 {{Calling 'foo_radar12278788_fp'}}
-//expected-note@-3 {{Returning from 'foo_radar12278788_fp'}}
 }
 
 void rdar13665798() {
@@ -164,8 +162,6 @@
 RetVoidFuncType f = foo_radar12278788_fp;
 return ((RetIntFuncType)f)(); //expected-warning {{Undefined or garbage value returned to caller}}
   //expected-note@-1 {{Undefined or garbage value returned to caller}}
-  //expected-note@-2 {{Calling 'foo_radar12278788_fp'}}
-  //expected-note@-3 {{Returning from 'foo_radar12278788_fp'}}
   }();
 }
 
@@ -182,18 +178,14 @@
 void use(struct Point p); 
 
 void testUseHalfPoint() {
-  struct Point p = getHalfPoint(); // expected-note{{Calling 'getHalfPoint'}}
-   // expected-note@-1{{Returning from 'getHalfPoint'}}
-   // expected-note@-2{{'p' initialized here}}
+  struct Point p = getHalfPoint(); // expected-note{{'p' initialized here}}
   use(p); // expected-warning{{uninitialized}}
   // expected-note@-1{{uninitialized}}
 }
 
 void testUseHalfPoint2() {
   struct Point p;
-  p = getHalfPoint(); // expected-note{{Calling 'getHalfPoint'}}
-  // expected-note@-1{{Returning from 'getHalfPoint'}}
-  // expected-note@-2{{Value assigned to 'p'}}
+  p = getHalfPoint(); // expected-note{{Value assigned to 'p'}}
   use(p); // expected-warning{{uninitialized}}
   // expected-note@-1{{uninitialized}}
 }
Index: clang/test/Analysis/track-control-dependency-conditions.cpp
===
--- clang/test/Analysis/track-control-dependency-conditions.cpp
+++ clang/test/Analysis/track-control-dependency-conditions.cpp
@@ -119,7 +119,7 @@
 bool coin();
 
 bool foo() {
-  return coin(); // tracking-note{{Returning value}}
+  return coin();
 }
 
 int bar;
@@ -127,12 +127,10 @@
 void test() {
   int *x = 0; // expected-note{{'x' initialized to a null pointer value}}
 
-  if (int flag = foo()) // tracking-note{{Calling 'foo'}}
-// tracking-note@-1{{Returning from 'foo'}}
-// tracking-note@-2{{'flag' initialized here}}
-// debug-note@-3{{Tracking condition 'flag'}}
-// expected-note@-4{{Assuming 'flag' is not equal to 0}}
-// expected-note@-5{{Taking true branch}}
+  if (int flag = foo()) // tracking-note{{'flag' initialized here}}
+// debug-note@-1{{Tracking condition 'flag'}}
+// expected-note@-2{{Assuming 'flag' is not equal to 0}}
+// expected-note@-3{{Taking true branch}}
 
 *x = 5; // expected-warning{{Dereference of null pointer}}
 // expected-note@-1{{Dereference of null pointer}}
@@ -143,18 +141,16 @@
 bool coin();
 
 struct ConvertsToBool {
-  operator bool() const { return coin(); } // tracking-note{{Returning value}}
+  operator bool() const { return coin(); }
 };
 
 void test() {
   int *x = 0; // expected-note{{'x' initialized to a null pointer value}}
 
   if (ConvertsToBool())
-// tracking-note@-1 {{Calling 'ConvertsToBool::operator bool'}}
-// tracking-note@-2{{Returning from 'ConvertsToBool::operator bool'}}
-// debug-note@-3{{Tracking condition 'ConvertsToBool()'}}
-// expected-note@-4{{Assuming the condition is true}}
-// expected-note@-5{{Taking true branch}}
+// debug-note@-1{{Tracking condition 'ConvertsToBool()'}}
+// expected-note@-2{{Assuming the condition is true}}
+// expected-note@-3{{Taking true branch}}
 *x = 5; // expected-warning{{Dereference of null pointer}}
 // 

[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 inline comments.



Comment at: clang/test/Analysis/track-control-dependency-conditions.cpp:185
 return true;
   return coin(); // tracking-note{{Returning value}}
 }

NoQ wrote:
> Szelethus wrote:
> > This note is meaningful, the bug would not have occurred if `coin()` wasn't 
> > assumed to be false.
> M. M. Dunno, this is getting complicated very quickly. Say, if you 
> replace `return true` with `return false` on the previous line, the note 
> becomes meaningless again. I don't see a direct connection between 
> meaningfulness and linearness.
> 
> The note in this example is relatively meaningful indeed, but i'm not sure 
> it's so much meaningful that it's worth the noise. It's still not surprising 
> for me that `flipCoin()` occasionally returns false. I do believe that it may 
> be sometimes surprising that `flipCoin()` may return false on the *current 
> path*, which would make the note meaningful. However, note that we don't 
> prove that it may return false, we only push the assumption one step deeper, 
> i.e. put the blame on `coin()` instead of `flipCoin()`, but we still fully 
> trust our assumption about `coin()` which may have the same problem. And if 
> we had an actual proof that it may return false, we would have had a 
> concrete-false return value, so the patch wouldn't apply.
> 
> I guess some experimental data would be great to have. 
Oh yea, I see where you are going. As I understand correctly, these are two 
separate problems: trying to somehow argue about other execution paths and 
whether dropping all constraints on a symbol is a good approach.

I should have all the results by tomorrow if all goes according to plan.



Comment at: clang/test/Analysis/uninit-vals.c:181
 void testUseHalfPoint() {
-  struct Point p = getHalfPoint(); // expected-note{{Calling 'getHalfPoint'}}
-   // expected-note@-1{{Returning from 
'getHalfPoint'}}
-   // expected-note@-2{{'p' initialized here}}
+  struct Point p = getHalfPoint(); // expected-note{{'p' initialized here}}
   use(p); // expected-warning{{uninitialized}}

NoQ wrote:
> Szelethus wrote:
> > Szelethus wrote:
> > > 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?
> > Passed-by-value struct argument contains uninitialized data (e.g., field: 
> > 'y')
> > 
> > Quite good imo.
> What specific logic are you talking about? You mean it'd scan the structure 
> for uninitialized fields and present the results of the scan to the user in a 
> note?
>What specific logic are you talking about? You mean it'd scan the structure 
>for uninitialized fields and present the results of the scan to the user in a 
>note?
Nvm, looked at the code, realized that what I said made no sense. What we are 
really missing here is a `trackRegionValue()` function :^)

Btw, I wasted s much time on figuring out that you don't get ANY notes 
whatsoever if you make this a cpp file rather than a c file, only the 
warning... Is this intended?


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

https://reviews.llvm.org/D64232



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:1052
+// to return that every time.
+if (N->getCFG().isLinear())
+  WouldEventBeMeaningless = true;

This time i'd rather go for "has exactly 3 blocks", because we're mostly 
interested in the CFG being "syntactically linear" rather than "semantically 
linear". I.e., if the CFG has an if-statement with a compile-time-constant 
condition, i'd rather show the path, because who knows what does the programmer 
think about this condition.



Comment at: clang/test/Analysis/track-control-dependency-conditions.cpp:185
 return true;
   return coin(); // tracking-note{{Returning value}}
 }

Szelethus wrote:
> This note is meaningful, the bug would not have occurred if `coin()` wasn't 
> assumed to be false.
M. M. Dunno, this is getting complicated very quickly. Say, if you 
replace `return true` with `return false` on the previous line, the note 
becomes meaningless again. I don't see a direct connection between 
meaningfulness and linearness.

The note in this example is relatively meaningful indeed, but i'm not sure it's 
so much meaningful that it's worth the noise. It's still not surprising for me 
that `flipCoin()` occasionally returns false. I do believe that it may be 
sometimes surprising that `flipCoin()` may return false on the *current path*, 
which would make the note meaningful. However, note that we don't prove that it 
may return false, we only push the assumption one step deeper, i.e. put the 
blame on `coin()` instead of `flipCoin()`, but we still fully trust our 
assumption about `coin()` which may have the same problem. And if we had an 
actual proof that it may return false, we would have had a concrete-false 
return value, so the patch wouldn't apply.

I guess some experimental data would be great to have. 



Comment at: clang/test/Analysis/uninit-vals.c:181
 void testUseHalfPoint() {
-  struct Point p = getHalfPoint(); // expected-note{{Calling 'getHalfPoint'}}
-   // expected-note@-1{{Returning from 
'getHalfPoint'}}
-   // expected-note@-2{{'p' initialized here}}
+  struct Point p = getHalfPoint(); // expected-note{{'p' initialized here}}
   use(p); // expected-warning{{uninitialized}}

Szelethus wrote:
> Szelethus wrote:
> > 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?
> Passed-by-value struct argument contains uninitialized data (e.g., field: 'y')
> 
> Quite good imo.
What specific logic are you talking about? You mean it'd scan the structure for 
uninitialized fields and present the results of the scan to the user in a note?


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

https://reviews.llvm.org/D64232



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[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 an inline comment as done.
Szelethus added inline comments.



Comment at: clang/test/Analysis/uninit-vals.c:181
 void testUseHalfPoint() {
-  struct Point p = getHalfPoint(); // expected-note{{Calling 'getHalfPoint'}}
-   // expected-note@-1{{Returning from 
'getHalfPoint'}}
-   // expected-note@-2{{'p' initialized here}}
+  struct Point p = getHalfPoint(); // expected-note{{'p' initialized here}}
   use(p); // expected-warning{{uninitialized}}

Szelethus wrote:
> 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?
Passed-by-value struct argument contains uninitialized data (e.g., field: 'y')

Quite good imo.


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

https://reviews.llvm.org/D64232



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[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 added a comment.

Gentle ping.


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

https://reviews.llvm.org/D64232



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[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 updated this revision to Diff 208276.
Szelethus added a comment.

Rebase.


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

https://reviews.llvm.org/D64232

Files:
  clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
  clang/test/Analysis/diagnostics/find_last_store.c
  clang/test/Analysis/track-control-dependency-conditions.cpp
  clang/test/Analysis/uninit-vals.c

Index: clang/test/Analysis/uninit-vals.c
===
--- clang/test/Analysis/uninit-vals.c
+++ clang/test/Analysis/uninit-vals.c
@@ -149,8 +149,6 @@
   RetVoidFuncType f = foo_radar12278788_fp;
   return ((RetIntFuncType)f)(); //expected-warning {{Undefined or garbage value returned to caller}}
 //expected-note@-1 {{Undefined or garbage value returned to caller}}
-//expected-note@-2 {{Calling 'foo_radar12278788_fp'}}
-//expected-note@-3 {{Returning from 'foo_radar12278788_fp'}}
 }
 
 void rdar13665798() {
@@ -164,8 +162,6 @@
 RetVoidFuncType f = foo_radar12278788_fp;
 return ((RetIntFuncType)f)(); //expected-warning {{Undefined or garbage value returned to caller}}
   //expected-note@-1 {{Undefined or garbage value returned to caller}}
-  //expected-note@-2 {{Calling 'foo_radar12278788_fp'}}
-  //expected-note@-3 {{Returning from 'foo_radar12278788_fp'}}
   }();
 }
 
@@ -182,18 +178,14 @@
 void use(struct Point p); 
 
 void testUseHalfPoint() {
-  struct Point p = getHalfPoint(); // expected-note{{Calling 'getHalfPoint'}}
-   // expected-note@-1{{Returning from 'getHalfPoint'}}
-   // expected-note@-2{{'p' initialized here}}
+  struct Point p = getHalfPoint(); // expected-note{{'p' initialized here}}
   use(p); // expected-warning{{uninitialized}}
   // expected-note@-1{{uninitialized}}
 }
 
 void testUseHalfPoint2() {
   struct Point p;
-  p = getHalfPoint(); // expected-note{{Calling 'getHalfPoint'}}
-  // expected-note@-1{{Returning from 'getHalfPoint'}}
-  // expected-note@-2{{Value assigned to 'p'}}
+  p = getHalfPoint(); // expected-note{{Value assigned to 'p'}}
   use(p); // expected-warning{{uninitialized}}
   // expected-note@-1{{uninitialized}}
 }
Index: clang/test/Analysis/track-control-dependency-conditions.cpp
===
--- clang/test/Analysis/track-control-dependency-conditions.cpp
+++ clang/test/Analysis/track-control-dependency-conditions.cpp
@@ -119,7 +119,7 @@
 bool coin();
 
 bool foo() {
-  return coin(); // tracking-note{{Returning value}}
+  return coin();
 }
 
 int bar;
@@ -127,11 +127,9 @@
 void test() {
   int *x = 0; // expected-note{{'x' initialized to a null pointer value}}
 
-  if (int flag = foo()) // tracking-note{{Calling 'foo'}}
-// tracking-note@-1{{Returning from 'foo'}}
-// debug-note@-2{{Tracking condition 'flag'}}
-// expected-note@-3{{Assuming 'flag' is not equal to 0}}
-// expected-note@-4{{Taking true branch}}
+  if (int flag = foo()) // debug-note{{Tracking condition 'flag'}}
+// expected-note@-1{{Assuming 'flag' is not equal to 0}}
+// expected-note@-2{{Taking true branch}}
 
 *x = 5; // expected-warning{{Dereference of null pointer}}
 // expected-note@-1{{Dereference of null pointer}}
@@ -142,18 +140,16 @@
 bool coin();
 
 struct ConvertsToBool {
-  operator bool() const { return coin(); } // tracking-note{{Returning value}}
+  operator bool() const { return coin(); }
 };
 
 void test() {
   int *x = 0; // expected-note{{'x' initialized to a null pointer value}}
 
   if (ConvertsToBool())
-// tracking-note@-1 {{Calling 'ConvertsToBool::operator bool'}}
-// tracking-note@-2{{Returning from 'ConvertsToBool::operator bool'}}
-// debug-note@-3{{Tracking condition 'ConvertsToBool()'}}
-// expected-note@-4{{Assuming the condition is true}}
-// expected-note@-5{{Taking true branch}}
+// debug-note@-1{{Tracking condition 'ConvertsToBool()'}}
+// expected-note@-2{{Assuming the condition is true}}
+// expected-note@-3{{Taking true branch}}
 *x = 5; // expected-warning{{Dereference of null pointer}}
 // expected-note@-1{{Dereference of null pointer}}
 }
@@ -163,18 +159,16 @@
 namespace unimportant_returning_value_note {
 bool coin();
 
-bool flipCoin() { return coin(); } // tracking-note{{Returning value}}
+bool flipCoin() { return coin(); }
 
 void i(int *ptr) {
   if (ptr) // expected-note{{Assuming 'ptr' is null}}
// expected-note@-1{{Taking false branch}}
 ;
   if (!flipCoin())
-// tracking-note@-1{{Calling 'flipCoin'}}
-// 

[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 , @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@-1{{Returning from 
'getHalfPoint'}}
-   // expected-note@-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



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

I guess this makes sense. The results look good. 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? I.e., i suspect that your "mild 
tracking mode" would get rid of a lot of those automagically.




Comment at: clang/test/Analysis/uninit-vals.c:181
 void testUseHalfPoint() {
-  struct Point p = getHalfPoint(); // expected-note{{Calling 'getHalfPoint'}}
-   // expected-note@-1{{Returning from 
'getHalfPoint'}}
-   // expected-note@-2{{'p' initialized here}}
+  struct Point p = getHalfPoint(); // expected-note{{'p' initialized here}}
   use(p); // expected-warning{{uninitialized}}

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?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64232



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[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 created this revision.
Szelethus added reviewers: NoQ, xazax.hun, rnkovacs, baloghadamsoftware, 
dcoughlin, Charusso.
Szelethus added a project: clang.
Herald added subscribers: cfe-commits, gamesh411, dkrupp, donat.nagy, 
mikhail.ramalho, a.sidorin, szepet, whisperity.

During the evaluation of D62883 , I noticed a 
bunch of totally meaningless notes with the pattern of "Calling 'A'" -> 
"Returning value" -> "Returning from 'A'", which added no value to the report 
at all.

This patch (not only affecting tracked conditions mind you) prunes diagnostic 
messages to functions that return a value not constrained to be 0, and are also 
linear.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D64232

Files:
  clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
  clang/test/Analysis/diagnostics/find_last_store.c
  clang/test/Analysis/track-control-dependency-conditions.cpp
  clang/test/Analysis/uninit-vals.c

Index: clang/test/Analysis/uninit-vals.c
===
--- clang/test/Analysis/uninit-vals.c
+++ clang/test/Analysis/uninit-vals.c
@@ -149,8 +149,6 @@
   RetVoidFuncType f = foo_radar12278788_fp;
   return ((RetIntFuncType)f)(); //expected-warning {{Undefined or garbage value returned to caller}}
 //expected-note@-1 {{Undefined or garbage value returned to caller}}
-//expected-note@-2 {{Calling 'foo_radar12278788_fp'}}
-//expected-note@-3 {{Returning from 'foo_radar12278788_fp'}}
 }
 
 void rdar13665798() {
@@ -164,8 +162,6 @@
 RetVoidFuncType f = foo_radar12278788_fp;
 return ((RetIntFuncType)f)(); //expected-warning {{Undefined or garbage value returned to caller}}
   //expected-note@-1 {{Undefined or garbage value returned to caller}}
-  //expected-note@-2 {{Calling 'foo_radar12278788_fp'}}
-  //expected-note@-3 {{Returning from 'foo_radar12278788_fp'}}
   }();
 }
 
@@ -182,18 +178,14 @@
 void use(struct Point p); 
 
 void testUseHalfPoint() {
-  struct Point p = getHalfPoint(); // expected-note{{Calling 'getHalfPoint'}}
-   // expected-note@-1{{Returning from 'getHalfPoint'}}
-   // expected-note@-2{{'p' initialized here}}
+  struct Point p = getHalfPoint(); // expected-note{{'p' initialized here}}
   use(p); // expected-warning{{uninitialized}}
   // expected-note@-1{{uninitialized}}
 }
 
 void testUseHalfPoint2() {
   struct Point p;
-  p = getHalfPoint(); // expected-note{{Calling 'getHalfPoint'}}
-  // expected-note@-1{{Returning from 'getHalfPoint'}}
-  // expected-note@-2{{Value assigned to 'p'}}
+  p = getHalfPoint(); // expected-note{{Value assigned to 'p'}}
   use(p); // expected-warning{{uninitialized}}
   // expected-note@-1{{uninitialized}}
 }
Index: clang/test/Analysis/track-control-dependency-conditions.cpp
===
--- clang/test/Analysis/track-control-dependency-conditions.cpp
+++ clang/test/Analysis/track-control-dependency-conditions.cpp
@@ -140,18 +140,16 @@
 bool coin();
 
 struct ConvertsToBool {
-  operator bool() const { return coin(); } // tracking-note{{Returning value}}
+  operator bool() const { return coin(); }
 };
 
 void test() {
   int *x = 0; // expected-note{{'x' initialized to a null pointer value}}
 
   if (ConvertsToBool())
-// tracking-note@-1 {{Calling 'ConvertsToBool::operator bool'}}
-// tracking-note@-2{{Returning from 'ConvertsToBool::operator bool'}}
-// debug-note@-3{{Tracking condition 'ConvertsToBool()'}}
-// expected-note@-4{{Assuming the condition is true}}
-// expected-note@-5{{Taking true branch}}
+// debug-note@-1{{Tracking condition 'ConvertsToBool()'}}
+// expected-note@-2{{Assuming the condition is true}}
+// expected-note@-3{{Taking true branch}}
 *x = 5; // expected-warning{{Dereference of null pointer}}
 // expected-note@-1{{Dereference of null pointer}}
 }
@@ -161,18 +159,16 @@
 namespace unimportant_returning_value_note {
 bool coin();
 
-bool flipCoin() { return coin(); } // tracking-note{{Returning value}}
+bool flipCoin() { return coin(); }
 
 void i(int *ptr) {
   if (ptr) // expected-note{{Assuming 'ptr' is null}}
// expected-note@-1{{Taking false branch}}
 ;
   if (!flipCoin())
-// tracking-note@-1{{Calling 'flipCoin'}}
-// tracking-note@-2{{Returning from 'flipCoin'}}
-// debug-note@-3{{Tracking condition '!flipCoin()'}}
-// expected-note@-4{{Assuming the condition is true}}
-// expected-note@-5{{Taking true branch}}
+// debug-note@-1{{Tracking condition '!flipCoin()'}}
+// expected-note@-2{{Assuming the condition is true}}
+// expected-note@-3{{Taking true