[PATCH] D153776: [clang][analyzer] Display notes in StdLibraryFunctionsChecker only if interesting

[Balázs Kéri via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGf12808ab2036: [clang][analyzer] Display notes in 
StdLibraryFunctionsChecker only if… (authored by balazske).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153776

Files:
  clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
  clang/test/Analysis/errno-stdlibraryfunctions-notes.c
  clang/test/Analysis/std-c-library-functions-arg-constraints-note-tags.cpp
  clang/test/Analysis/std-c-library-functions-arg-constraints.c
  clang/test/Analysis/std-c-library-functions-path-notes.c
  clang/test/Analysis/stream-errno-note.c
  clang/test/Analysis/stream-note.c

Index: clang/test/Analysis/stream-note.c
===
--- clang/test/Analysis/stream-note.c
+++ clang/test/Analysis/stream-note.c
@@ -13,7 +13,6 @@
 // expected-note@-2 {{Taking false branch}}
 return;
   FILE *F2 = tmpfile();
-  // stdargs-note@-1 {{'tmpfile' is successful}}
   if (!F2) {
 // expected-note@-1 {{'F2' is non-null}}
 // expected-note@-2 {{Taking false branch}}
@@ -22,7 +21,6 @@
   }
   rewind(F2);
   fclose(F2);
-  // stdargs-note@-1 {{'fclose' fails}}
   rewind(F1);
 }
 // expected-warning@-1 {{Opened stream never closed. Potential resource leak}}
@@ -59,7 +57,6 @@
 void check_note_leak_2(int c) {
   FILE *F1 = fopen("foo1.c", "r"); // expected-note {{Stream opened here}}
   // stdargs-note@-1 {{'fopen' is successful}}
-  // stdargs-note@-2 {{'fopen' is successful}}
   if (!F1)
 // expected-note@-1 {{'F1' is non-null}}
 // expected-note@-2 {{Taking false branch}}
@@ -68,7 +65,6 @@
 return;
   FILE *F2 = fopen("foo2.c", "r"); // expected-note {{Stream opened here}}
   // stdargs-note@-1 {{'fopen' is successful}}
-  // stdargs-note@-2 {{'fopen' is successful}}
   if (!F2) {
 // expected-note@-1 {{'F2' is non-null}}
 // expected-note@-2 {{Taking false branch}}
@@ -107,16 +103,13 @@
   FILE *F;
   char Buf[10];
   F = fopen("foo1.c", "r");
-  // stdargs-note@-1 {{'fopen' is successful}}
   if (F == NULL) { // expected-note {{Taking false branch}} expected-note {{'F' is not equal to NULL}}
 return;
   }
   fread(Buf, 1, 1, F);
-  // stdargs-note@-1 {{'fread' fails}}
   if (feof(F)) { // expected-note {{Taking true branch}}
 clearerr(F);
 fread(Buf, 1, 1, F);   // expected-note {{Assuming stream reaches end-of-file here}}
-// stdargs-note@-1 {{'fread' fails}}
 if (feof(F)) { // expected-note {{Taking true branch}}
   fread(Buf, 1, 1, F); // expected-warning {{Read function called when stream is in EOF state. Function has no effect}}
   // expected-note@-1 {{Read function called when stream is in EOF state. Function has no effect}}
@@ -129,12 +122,10 @@
   FILE *F;
   char Buf[10];
   F = fopen("foo1.c", "r");
-  // stdargs-note@-1 {{'fopen' is successful}}
   if (F == NULL) { // expected-note {{Taking false branch}} expected-note {{'F' is not equal to NULL}}
 return;
   }
   fread(Buf, 1, 1, F);
-  // stdargs-note@-1 {{'fread' is successful}}
   if (feof(F)) { // expected-note {{Taking false branch}}
 fclose(F);
 return;
@@ -143,7 +134,6 @@
 return;
   }
   fread(Buf, 1, 1, F);   // expected-note {{Assuming stream reaches end-of-file here}}
-  // stdargs-note@-1 {{'fread' fails}}
   if (feof(F)) { // expected-note {{Taking true branch}}
 fread(Buf, 1, 1, F); // expected-warning {{Read function called when stream is in EOF state. Function has no effect}}
 // expected-note@-1 {{Read function called when stream is in EOF state. Function has no effect}}
@@ -155,11 +145,9 @@
   FILE *F;
   char Buf[10];
   F = fopen("foo1.c", "r");
-  // stdargs-note@-1 {{'fopen' is successful}}
   if (F == NULL) // expected-note {{Taking false branch}} expected-note {{'F' is not equal to NULL}}
 return;
   int RRet = fread(Buf, 1, 1, F); // expected-note {{Assuming stream reaches end-of-file here}}
-  // stdargs-note@-1 {{'fread' fails}}
   if (ferror(F)) {// expected-note {{Taking false branch}}
   } else {
 fread(Buf, 1, 1, F); // expected-warning {{Read function called when stream is in EOF state. Function has no effect}}
Index: clang/test/Analysis/stream-errno-note.c
===
--- clang/test/Analysis/stream-errno-note.c
+++ clang/test/Analysis/stream-errno-note.c
@@ -11,7 +11,6 @@
 void check_fopen(void) {
   FILE *F = fopen("xxx", "r");
   // expected-note@-1{{'errno' may be undefined after successful call to 'fopen'}}
-  // expected-note@-2{{'fopen' is successful}}
   // expected-note@+2{{'F' is non-null}}
   // expected-note@+1{{Taking false branch}}
   if (!F)
@@ -24,7 +23,6 @@
 void check_tmpfile(void) {
   FILE *F = tmpfile();
   // expected-note@-1{{'errno' may be undefined 

[PATCH] D153776: [clang][analyzer] Display notes in StdLibraryFunctionsChecker only if interesting

[Donát Nagy via Phabricator via cfe-commits]

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

LGTM, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153776

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

[PATCH] D153776: [clang][analyzer] Display notes in StdLibraryFunctionsChecker only if interesting

[Balázs Kéri via Phabricator via cfe-commits]

balazske updated this revision to Diff 537003.
balazske added a comment.

rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153776

Files:
  clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
  clang/test/Analysis/errno-stdlibraryfunctions-notes.c
  clang/test/Analysis/std-c-library-functions-arg-constraints-note-tags.cpp
  clang/test/Analysis/std-c-library-functions-arg-constraints.c
  clang/test/Analysis/std-c-library-functions-path-notes.c
  clang/test/Analysis/stream-errno-note.c
  clang/test/Analysis/stream-note.c

Index: clang/test/Analysis/stream-note.c
===
--- clang/test/Analysis/stream-note.c
+++ clang/test/Analysis/stream-note.c
@@ -13,7 +13,6 @@
 // expected-note@-2 {{Taking false branch}}
 return;
   FILE *F2 = tmpfile();
-  // stdargs-note@-1 {{'tmpfile' is successful}}
   if (!F2) {
 // expected-note@-1 {{'F2' is non-null}}
 // expected-note@-2 {{Taking false branch}}
@@ -22,7 +21,6 @@
   }
   rewind(F2);
   fclose(F2);
-  // stdargs-note@-1 {{'fclose' fails}}
   rewind(F1);
 }
 // expected-warning@-1 {{Opened stream never closed. Potential resource leak}}
@@ -59,7 +57,6 @@
 void check_note_leak_2(int c) {
   FILE *F1 = fopen("foo1.c", "r"); // expected-note {{Stream opened here}}
   // stdargs-note@-1 {{'fopen' is successful}}
-  // stdargs-note@-2 {{'fopen' is successful}}
   if (!F1)
 // expected-note@-1 {{'F1' is non-null}}
 // expected-note@-2 {{Taking false branch}}
@@ -68,7 +65,6 @@
 return;
   FILE *F2 = fopen("foo2.c", "r"); // expected-note {{Stream opened here}}
   // stdargs-note@-1 {{'fopen' is successful}}
-  // stdargs-note@-2 {{'fopen' is successful}}
   if (!F2) {
 // expected-note@-1 {{'F2' is non-null}}
 // expected-note@-2 {{Taking false branch}}
@@ -107,16 +103,13 @@
   FILE *F;
   char Buf[10];
   F = fopen("foo1.c", "r");
-  // stdargs-note@-1 {{'fopen' is successful}}
   if (F == NULL) { // expected-note {{Taking false branch}} expected-note {{'F' is not equal to NULL}}
 return;
   }
   fread(Buf, 1, 1, F);
-  // stdargs-note@-1 {{'fread' fails}}
   if (feof(F)) { // expected-note {{Taking true branch}}
 clearerr(F);
 fread(Buf, 1, 1, F);   // expected-note {{Assuming stream reaches end-of-file here}}
-// stdargs-note@-1 {{'fread' fails}}
 if (feof(F)) { // expected-note {{Taking true branch}}
   fread(Buf, 1, 1, F); // expected-warning {{Read function called when stream is in EOF state. Function has no effect}}
   // expected-note@-1 {{Read function called when stream is in EOF state. Function has no effect}}
@@ -129,12 +122,10 @@
   FILE *F;
   char Buf[10];
   F = fopen("foo1.c", "r");
-  // stdargs-note@-1 {{'fopen' is successful}}
   if (F == NULL) { // expected-note {{Taking false branch}} expected-note {{'F' is not equal to NULL}}
 return;
   }
   fread(Buf, 1, 1, F);
-  // stdargs-note@-1 {{'fread' is successful}}
   if (feof(F)) { // expected-note {{Taking false branch}}
 fclose(F);
 return;
@@ -143,7 +134,6 @@
 return;
   }
   fread(Buf, 1, 1, F);   // expected-note {{Assuming stream reaches end-of-file here}}
-  // stdargs-note@-1 {{'fread' fails}}
   if (feof(F)) { // expected-note {{Taking true branch}}
 fread(Buf, 1, 1, F); // expected-warning {{Read function called when stream is in EOF state. Function has no effect}}
 // expected-note@-1 {{Read function called when stream is in EOF state. Function has no effect}}
@@ -155,11 +145,9 @@
   FILE *F;
   char Buf[10];
   F = fopen("foo1.c", "r");
-  // stdargs-note@-1 {{'fopen' is successful}}
   if (F == NULL) // expected-note {{Taking false branch}} expected-note {{'F' is not equal to NULL}}
 return;
   int RRet = fread(Buf, 1, 1, F); // expected-note {{Assuming stream reaches end-of-file here}}
-  // stdargs-note@-1 {{'fread' fails}}
   if (ferror(F)) {// expected-note {{Taking false branch}}
   } else {
 fread(Buf, 1, 1, F); // expected-warning {{Read function called when stream is in EOF state. Function has no effect}}
Index: clang/test/Analysis/stream-errno-note.c
===
--- clang/test/Analysis/stream-errno-note.c
+++ clang/test/Analysis/stream-errno-note.c
@@ -11,7 +11,6 @@
 void check_fopen(void) {
   FILE *F = fopen("xxx", "r");
   // expected-note@-1{{'errno' may be undefined after successful call to 'fopen'}}
-  // expected-note@-2{{'fopen' is successful}}
   // expected-note@+2{{'F' is non-null}}
   // expected-note@+1{{Taking false branch}}
   if (!F)
@@ -24,7 +23,6 @@
 void check_tmpfile(void) {
   FILE *F = tmpfile();
   // expected-note@-1{{'errno' may be undefined after successful call to 'tmpfile'}}
-  // expected-note@-2{{'tmpfile' is successful}}
   // expected-note@+2{{'F' is non-null}}
   // expected-note@+1{{Taking false branch}}
   

[PATCH] D153776: [clang][analyzer] Display notes in StdLibraryFunctionsChecker only if interesting

[Donát Nagy via Phabricator via cfe-commits]

donat.nagy added a comment.

In D153776#4467627 , @balazske wrote:

> The success/failure note tags are not added to all functions, `dup` is one of 
> these, this means at `dup` no note tag is shown. A next patch can be made to 
> add these messages to all functions. The other places look good, but 
> CodeChecker is a bit tricky, you must select 
> //*_stdclf_notetag_interesting_test_2// at the small arrow after the "found 
> in:" text (upper right corner). The link is good but not that instance of the 
> bug is displayed because only the note tags are different.

I think we should definitely add success/failure note tags to all functions 
where this checker can suddenly assume that "Oops, this failed". If this 
"Assuming that 'foo' fails" message is not there, it's a very natural reaction 
to search for some earlier note that would let the checker deduce that this 
function will fail here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153776

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

[PATCH] D153776: [clang][analyzer] Display notes in StdLibraryFunctionsChecker only if interesting

[Balázs Kéri via Phabricator via cfe-commits]

balazske added a comment.

The success/failure note tags are not added to all functions, `dup` is one of 
these, this means at `dup` no note tag is shown. A next patch can be made to 
add these messages to all functions. The other places look good, but 
CodeChecker is a bit tricky, you must select 
//*_stdclf_notetag_interesting_test_2// at the small arrow after the "found 
in:" text (upper right corner). The link is good but not that instance of the 
bug is displayed because only the note tags are different.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153776

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

[PATCH] D153776: [clang][analyzer] Display notes in StdLibraryFunctionsChecker only if interesting

[Donát Nagy via Phabricator via cfe-commits]

donat.nagy requested changes to this revision.
donat.nagy added a comment.
This revision now requires changes to proceed.

Thanks for attaching the test results! Unfortunately it seems that it was 
important to look at them, because I noticed that in the posgresql codebase 
there are many bug reports on calls like `func1(func2(...), ...)` where the 
checker does not print the expected "Assuming that 'func2' fails" note: 
fdopen/dup 
,
 fstat/fileno 
,
 isatty/fileno 
,
 another isatty/fileno 
,
 a third isatty/fileno 
,
 a second dup/fileno 

 (this is not an exhaustive list, but I think I listed the majority of them).

Please investigate these before merging your commits.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153776

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

[PATCH] D153776: [clang][analyzer] Display notes in StdLibraryFunctionsChecker only if interesting

[Balázs Kéri via Phabricator via cfe-commits]

balazske added a comment.

| results with the latest version:   |
| memcached_1.6.8_stdclf_notetag_interesting_test_2  | Reports 

  |
| tmux_2.6_stdclf_notetag_interesting_test_2 | Reports 

 |
| curl_curl-7_66_0_stdclf_notetag_interesting_test_2 | Reports 

 |
| twin_v0.8.1_stdclf_notetag_interesting_test_2  | Reports 

  |
| vim_v8.2.1920_stdclf_notetag_interesting_test_2| Reports 

|
| openssl_openssl-3.0.0-alpha7_stdclf_notetag_interesting_test_2 | Reports 

 |
| sqlite_version-3.33.0_stdclf_notetag_interesting_test_2| Reports 

|
| ffmpeg_n4.3.1_stdclf_notetag_interesting_test_2| Reports 

|
| postgres_REL_13_0_stdclf_notetag_interesting_test_2| Reports 

|
| xerces_v3.2.3_stdclf_notetag_interesting_test_2| Reports 

|
| bitcoin_v0.20.1_stdclf_notetag_interesting_test_2  | Reports 

  |


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153776

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

[PATCH] D153776: [clang][analyzer] Display notes in StdLibraryFunctionsChecker only if interesting

[Balázs Kéri via Phabricator via cfe-commits]

balazske updated this revision to Diff 536133.
balazske added a comment.

Rebase to newest parent review version.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153776

Files:
  clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
  clang/test/Analysis/errno-stdlibraryfunctions-notes.c
  clang/test/Analysis/std-c-library-functions-arg-constraints-note-tags.cpp
  clang/test/Analysis/std-c-library-functions-arg-constraints.c
  clang/test/Analysis/std-c-library-functions-path-notes.c
  clang/test/Analysis/stream-errno-note.c
  clang/test/Analysis/stream-note.c

Index: clang/test/Analysis/stream-note.c
===
--- clang/test/Analysis/stream-note.c
+++ clang/test/Analysis/stream-note.c
@@ -13,7 +13,6 @@
 // expected-note@-2 {{Taking false branch}}
 return;
   FILE *F2 = tmpfile();
-  // stdargs-note@-1 {{'tmpfile' is successful}}
   if (!F2) {
 // expected-note@-1 {{'F2' is non-null}}
 // expected-note@-2 {{Taking false branch}}
@@ -22,7 +21,6 @@
   }
   rewind(F2);
   fclose(F2);
-  // stdargs-note@-1 {{'fclose' fails}}
   rewind(F1);
 }
 // expected-warning@-1 {{Opened stream never closed. Potential resource leak}}
@@ -59,7 +57,6 @@
 void check_note_leak_2(int c) {
   FILE *F1 = fopen("foo1.c", "r"); // expected-note {{Stream opened here}}
   // stdargs-note@-1 {{'fopen' is successful}}
-  // stdargs-note@-2 {{'fopen' is successful}}
   if (!F1)
 // expected-note@-1 {{'F1' is non-null}}
 // expected-note@-2 {{Taking false branch}}
@@ -68,7 +65,6 @@
 return;
   FILE *F2 = fopen("foo2.c", "r"); // expected-note {{Stream opened here}}
   // stdargs-note@-1 {{'fopen' is successful}}
-  // stdargs-note@-2 {{'fopen' is successful}}
   if (!F2) {
 // expected-note@-1 {{'F2' is non-null}}
 // expected-note@-2 {{Taking false branch}}
@@ -107,16 +103,13 @@
   FILE *F;
   char Buf[10];
   F = fopen("foo1.c", "r");
-  // stdargs-note@-1 {{'fopen' is successful}}
   if (F == NULL) { // expected-note {{Taking false branch}} expected-note {{'F' is not equal to NULL}}
 return;
   }
   fread(Buf, 1, 1, F);
-  // stdargs-note@-1 {{'fread' fails}}
   if (feof(F)) { // expected-note {{Taking true branch}}
 clearerr(F);
 fread(Buf, 1, 1, F);   // expected-note {{Assuming stream reaches end-of-file here}}
-// stdargs-note@-1 {{'fread' fails}}
 if (feof(F)) { // expected-note {{Taking true branch}}
   fread(Buf, 1, 1, F); // expected-warning {{Read function called when stream is in EOF state. Function has no effect}}
   // expected-note@-1 {{Read function called when stream is in EOF state. Function has no effect}}
@@ -129,12 +122,10 @@
   FILE *F;
   char Buf[10];
   F = fopen("foo1.c", "r");
-  // stdargs-note@-1 {{'fopen' is successful}}
   if (F == NULL) { // expected-note {{Taking false branch}} expected-note {{'F' is not equal to NULL}}
 return;
   }
   fread(Buf, 1, 1, F);
-  // stdargs-note@-1 {{'fread' is successful}}
   if (feof(F)) { // expected-note {{Taking false branch}}
 fclose(F);
 return;
@@ -143,7 +134,6 @@
 return;
   }
   fread(Buf, 1, 1, F);   // expected-note {{Assuming stream reaches end-of-file here}}
-  // stdargs-note@-1 {{'fread' fails}}
   if (feof(F)) { // expected-note {{Taking true branch}}
 fread(Buf, 1, 1, F); // expected-warning {{Read function called when stream is in EOF state. Function has no effect}}
 // expected-note@-1 {{Read function called when stream is in EOF state. Function has no effect}}
@@ -155,11 +145,9 @@
   FILE *F;
   char Buf[10];
   F = fopen("foo1.c", "r");
-  // stdargs-note@-1 {{'fopen' is successful}}
   if (F == NULL) // expected-note {{Taking false branch}} expected-note {{'F' is not equal to NULL}}
 return;
   int RRet = fread(Buf, 1, 1, F); // expected-note {{Assuming stream reaches end-of-file here}}
-  // stdargs-note@-1 {{'fread' fails}}
   if (ferror(F)) {// expected-note {{Taking false branch}}
   } else {
 fread(Buf, 1, 1, F); // expected-warning {{Read function called when stream is in EOF state. Function has no effect}}
Index: clang/test/Analysis/stream-errno-note.c
===
--- clang/test/Analysis/stream-errno-note.c
+++ clang/test/Analysis/stream-errno-note.c
@@ -11,7 +11,6 @@
 void check_fopen(void) {
   FILE *F = fopen("xxx", "r");
   // expected-note@-1{{'errno' may be undefined after successful call to 'fopen'}}
-  // expected-note@-2{{'fopen' is successful}}
   // expected-note@+2{{'F' is non-null}}
   // expected-note@+1{{Taking false branch}}
   if (!F)
@@ -24,7 +23,6 @@
 void check_tmpfile(void) {
   FILE *F = tmpfile();
   // expected-note@-1{{'errno' may be undefined after successful call to 'tmpfile'}}
-  // expected-note@-2{{'tmpfile' is successful}}
   // expected-note@+2{{'F' is non-null}}
   // 

[PATCH] D153776: [clang][analyzer] Display notes in StdLibraryFunctionsChecker only if interesting

[Donát Nagy via Phabricator via cfe-commits]

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

LGTM if you rebase these changes onto the most recent version of D153612 
. Thanks for adding this interestingness 
check!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153776

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

[PATCH] D153776: [clang][analyzer] Display notes in StdLibraryFunctionsChecker only if interesting

[Balázs Kéri via Phabricator via cfe-commits]

balazske added inline comments.



Comment at: 
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:1325
   if (!Pred)
-break;
+continue;
 }

steakhal wrote:
> Why do you continue here? Do you have a case for this?
This is only because the same loop has other failure places (if the state can 
not be applied) where `continue` is used already. The reason is that if one 
`addTransition` fails a next one may succeed. It is probably better to use 
`break` at all places. Even then what was already added to the state can not be 
removed, so it may be not wrong to add all states that can be added.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153776

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

[PATCH] D153776: [clang][analyzer] Display notes in StdLibraryFunctionsChecker only if interesting

[Balázs Kéri via Phabricator via cfe-commits]

balazske added a comment.

Here 

 are results for vim, still the fileno problem is not fixed. This run was made 
before the last update where `markInteresting` is added, then it should work.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153776

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

[PATCH] D153776: [clang][analyzer] Display notes in StdLibraryFunctionsChecker only if interesting

[Balázs Kéri via Phabricator via cfe-commits]

balazske updated this revision to Diff 534868.
balazske added a comment.

`markInteresting` must be used at arguments found to be invalid


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153776

Files:
  clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
  clang/test/Analysis/errno-stdlibraryfunctions-notes.c
  clang/test/Analysis/std-c-library-functions-arg-constraints-note-tags.cpp
  clang/test/Analysis/std-c-library-functions-arg-constraints.c
  clang/test/Analysis/std-c-library-functions-path-notes.c
  clang/test/Analysis/stream-errno-note.c
  clang/test/Analysis/stream-note.c

Index: clang/test/Analysis/stream-note.c
===
--- clang/test/Analysis/stream-note.c
+++ clang/test/Analysis/stream-note.c
@@ -13,7 +13,6 @@
 // expected-note@-2 {{Taking false branch}}
 return;
   FILE *F2 = tmpfile();
-  // stdargs-note@-1 {{call is successful}}
   if (!F2) {
 // expected-note@-1 {{'F2' is non-null}}
 // expected-note@-2 {{Taking false branch}}
@@ -22,7 +21,6 @@
   }
   rewind(F2);
   fclose(F2);
-  // stdargs-note@-1 {{call fails}}
   rewind(F1);
 }
 // expected-warning@-1 {{Opened stream never closed. Potential resource leak}}
@@ -59,7 +57,6 @@
 void check_note_leak_2(int c) {
   FILE *F1 = fopen("foo1.c", "r"); // expected-note {{Stream opened here}}
   // stdargs-note@-1 {{call is successful}}
-  // stdargs-note@-2 {{call is successful}}
   if (!F1)
 // expected-note@-1 {{'F1' is non-null}}
 // expected-note@-2 {{Taking false branch}}
@@ -68,7 +65,6 @@
 return;
   FILE *F2 = fopen("foo2.c", "r"); // expected-note {{Stream opened here}}
   // stdargs-note@-1 {{call is successful}}
-  // stdargs-note@-2 {{call is successful}}
   if (!F2) {
 // expected-note@-1 {{'F2' is non-null}}
 // expected-note@-2 {{Taking false branch}}
@@ -107,7 +103,6 @@
   FILE *F;
   char Buf[10];
   F = fopen("foo1.c", "r");
-  // stdargs-note@-1 {{call is successful}}
   if (F == NULL) { // expected-note {{Taking false branch}} expected-note {{'F' is not equal to NULL}}
 return;
   }
@@ -127,7 +122,6 @@
   FILE *F;
   char Buf[10];
   F = fopen("foo1.c", "r");
-  // stdargs-note@-1 {{call is successful}}
   if (F == NULL) { // expected-note {{Taking false branch}} expected-note {{'F' is not equal to NULL}}
 return;
   }
@@ -151,7 +145,6 @@
   FILE *F;
   char Buf[10];
   F = fopen("foo1.c", "r");
-  // stdargs-note@-1 {{call is successful}}
   if (F == NULL) // expected-note {{Taking false branch}} expected-note {{'F' is not equal to NULL}}
 return;
   int RRet = fread(Buf, 1, 1, F); // expected-note {{Assuming stream reaches end-of-file here}}
Index: clang/test/Analysis/stream-errno-note.c
===
--- clang/test/Analysis/stream-errno-note.c
+++ clang/test/Analysis/stream-errno-note.c
@@ -11,7 +11,6 @@
 void check_fopen(void) {
   FILE *F = fopen("xxx", "r");
   // expected-note@-1{{'errno' may be undefined after successful call to 'fopen'}}
-  // expected-note@-2{{call is successful}}
   // expected-note@+2{{'F' is non-null}}
   // expected-note@+1{{Taking false branch}}
   if (!F)
@@ -24,7 +23,6 @@
 void check_tmpfile(void) {
   FILE *F = tmpfile();
   // expected-note@-1{{'errno' may be undefined after successful call to 'tmpfile'}}
-  // expected-note@-2{{call is successful}}
   // expected-note@+2{{'F' is non-null}}
   // expected-note@+1{{Taking false branch}}
   if (!F)
@@ -36,14 +34,12 @@
 
 void check_freopen(void) {
   FILE *F = tmpfile();
-  // expected-note@-1{{call is successful}}
   // expected-note@+2{{'F' is non-null}}
   // expected-note@+1{{Taking false branch}}
   if (!F)
 return;
   F = freopen("xxx", "w", F);
   // expected-note@-1{{'errno' may be undefined after successful call to 'freopen'}}
-  // expected-note@-2{{call is successful}}
   // expected-note@+2{{'F' is non-null}}
   // expected-note@+1{{Taking false branch}}
   if (!F)
@@ -55,14 +51,12 @@
 
 void check_fclose(void) {
   FILE *F = tmpfile();
-  // expected-note@-1{{call is successful}}
   // expected-note@+2{{'F' is non-null}}
   // expected-note@+1{{Taking false branch}}
   if (!F)
 return;
   (void)fclose(F);
   // expected-note@-1{{'errno' may be undefined after successful call to 'fclose'}}
-  // expected-note@-2{{call is successful}}
   if (errno) {} // expected-warning{{An undefined value may be read from 'errno'}}
   // expected-note@-1{{An undefined value may be read from 'errno'}}
 }
@@ -70,7 +64,6 @@
 void check_fread(void) {
   char Buf[10];
   FILE *F = tmpfile();
-  // expected-note@-1{{call is successful}}
   // expected-note@+2{{'F' is non-null}}
   // expected-note@+1{{Taking false branch}}
   if (!F)
@@ -85,7 +78,6 @@
 void check_fwrite(void) {
   char Buf[] = "0123456789";
   FILE *F = tmpfile();
-  // expected-note@-1{{call is successful}}
   // 

[PATCH] D153776: [clang][analyzer] Display notes in StdLibraryFunctionsChecker only if interesting

[Balázs Benics via Phabricator via cfe-commits]

steakhal added inline comments.



Comment at: 
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:1325
   if (!Pred)
-break;
+continue;
 }

Why do you continue here? Do you have a case for this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153776

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

[PATCH] D153776: [clang][analyzer] Display notes in StdLibraryFunctionsChecker only if interesting

[Balázs Kéri via Phabricator via cfe-commits]

balazske created this revision.
Herald added subscribers: steakhal, manas, ASDenysPetrov, martong, gamesh411, 
dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, 
baloghadamsoftware, xazax.hun.
Herald added a reviewer: Szelethus.
Herald added a reviewer: NoQ.
Herald added a project: All.
balazske requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The note tag that was previously added in all cases when a standard function 
call
is found is displayed now only if the function call (return value) is 
"interesting".
This results in less unneeded notes but some of the previously good notes 
disappear
too. This is because interestingness is not always set as it should be.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D153776

Files:
  clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
  clang/test/Analysis/errno-stdlibraryfunctions-notes.c
  clang/test/Analysis/std-c-library-functions-arg-constraints-note-tags.cpp
  clang/test/Analysis/std-c-library-functions-arg-constraints.c
  clang/test/Analysis/std-c-library-functions-path-notes.c
  clang/test/Analysis/stream-errno-note.c
  clang/test/Analysis/stream-note.c

Index: clang/test/Analysis/stream-note.c
===
--- clang/test/Analysis/stream-note.c
+++ clang/test/Analysis/stream-note.c
@@ -13,7 +13,6 @@
 // expected-note@-2 {{Taking false branch}}
 return;
   FILE *F2 = tmpfile();
-  // stdargs-note@-1 {{call is successful}}
   if (!F2) {
 // expected-note@-1 {{'F2' is non-null}}
 // expected-note@-2 {{Taking false branch}}
@@ -22,7 +21,6 @@
   }
   rewind(F2);
   fclose(F2);
-  // stdargs-note@-1 {{call fails}}
   rewind(F1);
 }
 // expected-warning@-1 {{Opened stream never closed. Potential resource leak}}
@@ -59,7 +57,6 @@
 void check_note_leak_2(int c) {
   FILE *F1 = fopen("foo1.c", "r"); // expected-note {{Stream opened here}}
   // stdargs-note@-1 {{call is successful}}
-  // stdargs-note@-2 {{call is successful}}
   if (!F1)
 // expected-note@-1 {{'F1' is non-null}}
 // expected-note@-2 {{Taking false branch}}
@@ -68,7 +65,6 @@
 return;
   FILE *F2 = fopen("foo2.c", "r"); // expected-note {{Stream opened here}}
   // stdargs-note@-1 {{call is successful}}
-  // stdargs-note@-2 {{call is successful}}
   if (!F2) {
 // expected-note@-1 {{'F2' is non-null}}
 // expected-note@-2 {{Taking false branch}}
@@ -107,7 +103,6 @@
   FILE *F;
   char Buf[10];
   F = fopen("foo1.c", "r");
-  // stdargs-note@-1 {{call is successful}}
   if (F == NULL) { // expected-note {{Taking false branch}} expected-note {{'F' is not equal to NULL}}
 return;
   }
@@ -127,7 +122,6 @@
   FILE *F;
   char Buf[10];
   F = fopen("foo1.c", "r");
-  // stdargs-note@-1 {{call is successful}}
   if (F == NULL) { // expected-note {{Taking false branch}} expected-note {{'F' is not equal to NULL}}
 return;
   }
@@ -151,7 +145,6 @@
   FILE *F;
   char Buf[10];
   F = fopen("foo1.c", "r");
-  // stdargs-note@-1 {{call is successful}}
   if (F == NULL) // expected-note {{Taking false branch}} expected-note {{'F' is not equal to NULL}}
 return;
   int RRet = fread(Buf, 1, 1, F); // expected-note {{Assuming stream reaches end-of-file here}}
Index: clang/test/Analysis/stream-errno-note.c
===
--- clang/test/Analysis/stream-errno-note.c
+++ clang/test/Analysis/stream-errno-note.c
@@ -11,7 +11,6 @@
 void check_fopen(void) {
   FILE *F = fopen("xxx", "r");
   // expected-note@-1{{'errno' may be undefined after successful call to 'fopen'}}
-  // expected-note@-2{{call is successful}}
   // expected-note@+2{{'F' is non-null}}
   // expected-note@+1{{Taking false branch}}
   if (!F)
@@ -24,7 +23,6 @@
 void check_tmpfile(void) {
   FILE *F = tmpfile();
   // expected-note@-1{{'errno' may be undefined after successful call to 'tmpfile'}}
-  // expected-note@-2{{call is successful}}
   // expected-note@+2{{'F' is non-null}}
   // expected-note@+1{{Taking false branch}}
   if (!F)
@@ -36,14 +34,12 @@
 
 void check_freopen(void) {
   FILE *F = tmpfile();
-  // expected-note@-1{{call is successful}}
   // expected-note@+2{{'F' is non-null}}
   // expected-note@+1{{Taking false branch}}
   if (!F)
 return;
   F = freopen("xxx", "w", F);
   // expected-note@-1{{'errno' may be undefined after successful call to 'freopen'}}
-  // expected-note@-2{{call is successful}}
   // expected-note@+2{{'F' is non-null}}
   // expected-note@+1{{Taking false branch}}
   if (!F)
@@ -55,14 +51,12 @@
 
 void check_fclose(void) {
   FILE *F = tmpfile();
-  // expected-note@-1{{call is successful}}
   // expected-note@+2{{'F' is non-null}}
   // expected-note@+1{{Taking false branch}}
   if (!F)
 return;
   (void)fclose(F);
   // expected-note@-1{{'errno' may be undefined after successful call to 'fclose'}}
-  // expected-note@-2{{call is successful}}