[PATCH] D64287: [analyzer] Track the right hand side of the last store regardless of its value

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

This revision was automatically updated to reflect the committed changes.
Szelethus marked 2 inline comments as done.
Closed by commit rG0df9c8c57802: [analyzer] Track the right hand side of the 
last store regardless of its value (authored by Szelethus).

Changed prior to commit:
  https://reviews.llvm.org/D64287?vs=210333=214982#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64287

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

Index: clang/test/Analysis/uninit-const.cpp
===
--- clang/test/Analysis/uninit-const.cpp
+++ clang/test/Analysis/uninit-const.cpp
@@ -66,8 +66,8 @@
 
 void f6_2(void) {
   int t;   //expected-note {{'t' declared without an initial value}}
-  int  = t;
-  int  = p;
+  int  = t;  //expected-note {{'p' initialized here}}
+  int  = p;  //expected-note {{'s' initialized here}}
   int  = s;  //expected-note {{'q' initialized here}}
   doStuff6(q); //expected-warning {{1st function call argument is an uninitialized value}}
//expected-note@-1 {{1st function call argument is an uninitialized value}}
@@ -96,7 +96,7 @@
 
 
 void f5(void) {
-  int t;
+  int t;   // expected-note {{'t' declared without an initial value}}
   int* tp = // expected-note {{'tp' initialized here}}
   doStuff_uninit(tp);  // expected-warning {{1st function call argument is a pointer to uninitialized value}}
// expected-note@-1 {{1st function call argument is a pointer to uninitialized value}}
Index: clang/test/Analysis/uninit-const.c
===
--- clang/test/Analysis/uninit-const.c
+++ clang/test/Analysis/uninit-const.c
@@ -24,15 +24,15 @@
 void doStuff_variadic(const int *u, ...){};
 
 void f_1(void) {
-  int t;
+  int t;   // expected-note {{'t' declared without an initial value}}
   int* tp = // expected-note {{'tp' initialized here}}
   doStuff_pointerToConstInt(tp);  // expected-warning {{1st function call argument is a pointer to uninitialized value}}
// expected-note@-1 {{1st function call argument is a pointer to uninitialized value}}
 }
 
 void f_1_1(void) {
-  int t;
-  int* tp1 = 
+  int t; // expected-note {{'t' declared without an initial value}}
+  int *tp1 =  // expected-note {{'tp1' initialized here}}
   int* tp2 = tp1;// expected-note {{'tp2' initialized here}}
   doStuff_pointerToConstInt(tp2);  // expected-warning {{1st function call argument is a pointer to uninitialized value}}
// expected-note@-1 {{1st function call argument is a pointer to uninitialized value}}
@@ -40,12 +40,15 @@
 
 
 int *f_2_sub(int *p) {
-  return p;
+  return p; // expected-note {{Returning pointer (loaded from 'p')}}
 }
 
 void f_2(void) {
-  int t;
-  int* p = f_2_sub();
+  int t;// expected-note {{'t' declared without an initial value}}
+  int *p = f_2_sub(); // expected-note {{Passing value via 1st parameter 'p'}}
+// expected-note@-1{{Calling 'f_2_sub'}}
+// expected-note@-2{{Returning from 'f_2_sub'}}
+// expected-note@-3{{'p' initialized here}}
   int* tp = p; // expected-note {{'tp' initialized here}}
   doStuff_pointerToConstInt(tp); // expected-warning {{1st function call argument is a pointer to uninitialized value}}
   // expected-note@-1 {{1st function call argument is a pointer to uninitialized value}}
@@ -62,7 +65,7 @@
 }
 
 void f_5(void) {
-  int ta[5];
+  int ta[5];   // expected-note {{'ta' initialized here}}
   int* tp = ta;// expected-note {{'tp' initialized here}}
   doStuff_pointerToConstInt(tp);  // expected-warning {{1st function call argument is a pointer to uninitialized value}}
// expected-note@-1 {{1st function call argument is a pointer to uninitialized value}}
@@ -99,7 +102,7 @@
 }
 
 void f_9(void) {
-  int  a[6];
+  int a[6];// expected-note {{'a' initialized here}}
   int const *ptau = a; // expected-note {{'ptau' initialized here}}
   doStuff_arrayOfConstInt(ptau);// expected-warning {{1st function call argument is a pointer to uninitialized value}}
// expected-note@-1 {{1st function call argument is a pointer to uninitialized value}}
@@ -173,7 +176,7 @@
 
 // uninit pointer, uninit val
 void f_variadic_unp_unv(void) {
-  int t;
+  int t; // expected-note {{'t' declared without an initial value}}
   int v;
   int* tp =// expected-note {{'tp' initialized here}}
   doStuff_variadic(tp,v);  // expected-warning {{1st function call argument 

[PATCH] D64287: [analyzer] Track the right hand side of the last store regardless of its value

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

Szelethus added a comment.

In D64287#1589941 , @xazax.hun wrote:

> LGTM!
>
> Since we allow new kinds of SVals to be tracked it would be great to test 
> this first on a larger corpus of projects just to see if there is a crash 
> (due to an unhandled SVal type).


It doesn't! Though that analysis was run with plenty of other patches as well, 
but I saw no crashes regardless.


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

https://reviews.llvm.org/D64287



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

[PATCH] D64287: [analyzer] Track the right hand side of the last store regardless of its value

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

xazax.hun accepted this revision.
xazax.hun added a comment.

LGTM!

Since we allow new kinds of SVals to be tracked it would be great to test this 
first on a larger corpus of projects just to see if there is a crash (due to an 
unhandled SVal type).


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

https://reviews.llvm.org/D64287



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

[PATCH] D64287: [analyzer] Track the right hand side of the last store regardless of its value

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

Szelethus updated this revision to Diff 210333.

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

https://reviews.llvm.org/D64287

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

Index: clang/test/Analysis/uninit-const.cpp
===
--- clang/test/Analysis/uninit-const.cpp
+++ clang/test/Analysis/uninit-const.cpp
@@ -66,8 +66,8 @@
 
 void f6_2(void) {
   int t;   //expected-note {{'t' declared without an initial value}}
-  int  = t;
-  int  = p;
+  int  = t;  //expected-note {{'p' initialized here}}
+  int  = p;  //expected-note {{'s' initialized here}}
   int  = s;  //expected-note {{'q' initialized here}}
   doStuff6(q); //expected-warning {{1st function call argument is an uninitialized value}}
//expected-note@-1 {{1st function call argument is an uninitialized value}}
@@ -96,7 +96,7 @@
 
 
 void f5(void) {
-  int t;
+  int t; // expected-note {{'t' declared without an initial value}}
   int* tp = // expected-note {{'tp' initialized here}}
   doStuff_uninit(tp);  // expected-warning {{1st function call argument is a pointer to uninitialized value}}
// expected-note@-1 {{1st function call argument is a pointer to uninitialized value}}
Index: clang/test/Analysis/uninit-const.c
===
--- clang/test/Analysis/uninit-const.c
+++ clang/test/Analysis/uninit-const.c
@@ -24,15 +24,15 @@
 void doStuff_variadic(const int *u, ...){};
 
 void f_1(void) {
-  int t;
+  int t; // expected-note {{'t' declared without an initial value}}
   int* tp = // expected-note {{'tp' initialized here}}
   doStuff_pointerToConstInt(tp);  // expected-warning {{1st function call argument is a pointer to uninitialized value}}
// expected-note@-1 {{1st function call argument is a pointer to uninitialized value}}
 }
 
 void f_1_1(void) {
-  int t;
-  int* tp1 = 
+  int t; // expected-note {{'t' declared without an initial value}}
+  int* tp1 =  // expected-note {{'tp1' initialized here}}
   int* tp2 = tp1;// expected-note {{'tp2' initialized here}}
   doStuff_pointerToConstInt(tp2);  // expected-warning {{1st function call argument is a pointer to uninitialized value}}
// expected-note@-1 {{1st function call argument is a pointer to uninitialized value}}
@@ -40,12 +40,15 @@
 
 
 int *f_2_sub(int *p) {
-  return p;
+  return p; // expected-note {{Returning pointer (loaded from 'p')}}
 }
 
 void f_2(void) {
-  int t;
-  int* p = f_2_sub();
+  int t; // expected-note {{'t' declared without an initial value}}
+  int* p = f_2_sub(); // expected-note {{Passing value via 1st parameter 'p'}}
+// expected-note@-1{{Calling 'f_2_sub'}}
+// expected-note@-2{{Returning from 'f_2_sub'}}
+// expected-note@-3{{'p' initialized here}}
   int* tp = p; // expected-note {{'tp' initialized here}}
   doStuff_pointerToConstInt(tp); // expected-warning {{1st function call argument is a pointer to uninitialized value}}
   // expected-note@-1 {{1st function call argument is a pointer to uninitialized value}}
@@ -62,7 +65,7 @@
 }
 
 void f_5(void) {
-  int ta[5];
+  int ta[5]; // expected-note {{'ta' initialized here}}
   int* tp = ta;// expected-note {{'tp' initialized here}}
   doStuff_pointerToConstInt(tp);  // expected-warning {{1st function call argument is a pointer to uninitialized value}}
// expected-note@-1 {{1st function call argument is a pointer to uninitialized value}}
@@ -99,7 +102,7 @@
 }
 
 void f_9(void) {
-  int  a[6];
+  int  a[6]; // expected-note {{'a' initialized here}}
   int const *ptau = a; // expected-note {{'ptau' initialized here}}
   doStuff_arrayOfConstInt(ptau);// expected-warning {{1st function call argument is a pointer to uninitialized value}}
// expected-note@-1 {{1st function call argument is a pointer to uninitialized value}}
@@ -173,7 +176,7 @@
 
 // uninit pointer, uninit val
 void f_variadic_unp_unv(void) {
-  int t;
+  int t; // expected-note {{'t' declared without an initial value}}
   int v;
   int* tp =// expected-note {{'tp' initialized here}}
   doStuff_variadic(tp,v);  // expected-warning {{1st function call argument is a pointer to uninitialized value}}
@@ -181,7 +184,7 @@
 }
 // uninit pointer, init val
 void f_variadic_unp_inv(void) {
-  int t;
+  int t; // expected-note {{'t' declared without an initial value}}
   int v = 3;
   int* tp =// expected-note {{'tp' initialized here}}
   doStuff_variadic(tp,v);  // expected-warning {{1st function call argument is a pointer to uninitialized value}}
@@ -216,7 +219,7 @@
 
 

[PATCH] D64287: [analyzer] Track the right hand side of the last store regardless of its 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.

Yeah, i think this is good to go :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64287



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

[PATCH] D64287: [analyzer] Track the right hand side of the last store regardless of its value

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

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

Would you say it's good to go? :)




Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:1436-1437
-}
-ReturnVisitor::addVisitorIfNecessary(StoreSite, InitE->IgnoreParenCasts(),
- BR, EnableNullFPSuppression);
   }

NoQ wrote:
> You're removing this visitor, does it cause any removal of notes?
Nope, `trackExpressionValue` will add it back, if appropriate.



Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:1989-1991
+}
+else if (const auto *AR = L->getRegionAs())
+  CanDereference = false;

NoQ wrote:
> Fair enough. Did we already have a test case for this? I wonder if function 
> pointers might also cause problems here.
> 
> Ugh, this code is so underdebugged.
Yup, `test/Analysis/exercise-ps.c` flopped. And I agree, this looks kinda nasty.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64287



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

[PATCH] D64287: [analyzer] Track the right hand side of the last store regardless of its value

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Whoa, the new notes are amazing. Looks like you've found a pretty bad omission.




Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:1436-1437
-}
-ReturnVisitor::addVisitorIfNecessary(StoreSite, InitE->IgnoreParenCasts(),
- BR, EnableNullFPSuppression);
   }

You're removing this visitor, does it cause any removal of notes?



Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:1989-1991
+}
+else if (const auto *AR = L->getRegionAs())
+  CanDereference = false;

Fair enough. Did we already have a test case for this? I wonder if function 
pointers might also cause problems here.

Ugh, this code is so underdebugged.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64287



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