[clang] f12808a - [clang][analyzer] Display notes in StdLibraryFunctionsChecker only if interesting

Balázs Kéri via cfe-commits cfe-commits at lists.llvm.org
Tue Jul 18 00:29:50 PDT 2023


Author: Balázs Kéri
Date: 2023-07-18T09:29:15+02:00
New Revision: f12808ab20369c85ddb602e5a78bab40d16bb83f

URL: https://github.com/llvm/llvm-project/commit/f12808ab20369c85ddb602e5a78bab40d16bb83f
DIFF: https://github.com/llvm/llvm-project/commit/f12808ab20369c85ddb602e5a78bab40d16bb83f.diff

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

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.

Reviewed By: donat.nagy

Differential Revision: https://reviews.llvm.org/D153776

Added: 
    

Modified: 
    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

Removed: 
    


################################################################################
diff  --git a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
index 8d4fb12061c3d6..1fb248b8ed0e97 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -835,6 +835,7 @@ class StdLibraryFunctionsChecker
 
     for (ArgNo ArgN : VC->getArgsToTrack()) {
       bugreporter::trackExpressionValue(N, Call.getArgExpr(ArgN), *R);
+      R->markInteresting(Call.getArgSVal(ArgN));
       // All tracked arguments are important, highlight them.
       R->addRange(Call.getArgSourceRange(ArgN));
     }
@@ -1298,6 +1299,7 @@ void StdLibraryFunctionsChecker::checkPostCall(const CallEvent &Call,
     // StdLibraryFunctionsChecker.
     ExplodedNode *Pred = Node;
     if (!Case.getNote().empty()) {
+      const SVal RV = Call.getReturnValue();
       // If there is a description for this execution branch (summary case),
       // use it as a note tag.
       std::string Note =
@@ -1305,7 +1307,7 @@ void StdLibraryFunctionsChecker::checkPostCall(const CallEvent &Call,
                         cast<NamedDecl>(Call.getDecl())->getDeclName());
       if (Summary.getInvalidationKd() == EvalCallAsPure) {
         const NoteTag *Tag = C.getNoteTag(
-            [Node, Note](PathSensitiveBugReport &BR) -> std::string {
+            [Node, Note, RV](PathSensitiveBugReport &BR) -> std::string {
               // Try to omit the note if we know in advance which branch is
               // taken (this means, only one branch exists).
               // This check is performed inside the lambda, after other
@@ -1316,18 +1318,22 @@ void StdLibraryFunctionsChecker::checkPostCall(const CallEvent &Call,
               // split that was performed by another checker (and can not find
               // the successors). This is why this check is only used in the
               // EvalCallAsPure case.
-              if (Node->succ_size() > 1)
+              if (BR.isInteresting(RV) && Node->succ_size() > 1)
                 return Note;
               return "";
-            },
-            /*IsPrunable=*/true);
+            });
         Pred = C.addTransition(NewState, Pred, Tag);
       } else {
-        const NoteTag *Tag = C.getNoteTag(Note, /*IsPrunable=*/true);
+        const NoteTag *Tag =
+            C.getNoteTag([Note, RV](PathSensitiveBugReport &BR) -> std::string {
+              if (BR.isInteresting(RV))
+                return Note;
+              return "";
+            });
         Pred = C.addTransition(NewState, Pred, Tag);
       }
       if (!Pred)
-        break;
+        continue;
     }
 
     // If we can get a note tag for the errno change, add this additionally to

diff  --git a/clang/test/Analysis/errno-stdlibraryfunctions-notes.c b/clang/test/Analysis/errno-stdlibraryfunctions-notes.c
index e520e4fa76497f..991384cc373ef3 100644
--- a/clang/test/Analysis/errno-stdlibraryfunctions-notes.c
+++ b/clang/test/Analysis/errno-stdlibraryfunctions-notes.c
@@ -14,10 +14,8 @@ void clang_analyzer_warnIfReached();
 
 void test1() {
   access("path", 0);
-  // expected-note at -1{{Assuming that 'access' fails}}
   access("path", 0);
-  // expected-note at -1{{Assuming that 'access' is successful}}
-  // expected-note at -2{{'errno' may be undefined after successful call to 'access'}}
+  // expected-note at -1{{'errno' may be undefined after successful call to 'access'}}
   if (errno != 0) {
     // expected-warning at -1{{An undefined value may be read from 'errno'}}
     // expected-note at -2{{An undefined value may be read from 'errno'}}
@@ -26,8 +24,7 @@ void test1() {
 
 void test2() {
   if (access("path", 0) == -1) {
-    // expected-note at -1{{Assuming that 'access' fails}}
-    // expected-note at -2{{Taking true branch}}
+    // expected-note at -1{{Taking true branch}}
     // Failure path.
     if (errno != 0) {
       // expected-note at -1{{'errno' is not equal to 0}}
@@ -42,9 +39,8 @@ void test2() {
 void test3() {
   if (access("path", 0) != -1) {
     // Success path.
-    // expected-note at -2{{Assuming that 'access' is successful}}
-    // expected-note at -3{{'errno' may be undefined after successful call to 'access'}}
-    // expected-note at -4{{Taking true branch}}
+    // expected-note at -2{{'errno' may be undefined after successful call to 'access'}}
+    // expected-note at -3{{Taking true branch}}
     if (errno != 0) {
       // expected-warning at -1{{An undefined value may be read from 'errno'}}
       // expected-note at -2{{An undefined value may be read from 'errno'}}

diff  --git a/clang/test/Analysis/std-c-library-functions-arg-constraints-note-tags.cpp b/clang/test/Analysis/std-c-library-functions-arg-constraints-note-tags.cpp
index fc7116d3e669ce..573b0076a0e731 100644
--- a/clang/test/Analysis/std-c-library-functions-arg-constraints-note-tags.cpp
+++ b/clang/test/Analysis/std-c-library-functions-arg-constraints-note-tags.cpp
@@ -52,7 +52,7 @@ void test_buffer_size_note(char *buf, int y) {
 int __test_case_note();
 
 int test_case_note_1(int y) {
-  int x0 = __test_case_note(); // expected-note{{Function returns 1}}
+  int x0 = __test_case_note();
   int x = __test_case_note(); // expected-note{{Function returns 0}} \
                               // expected-note{{'x' initialized here}}
   return y / x; // expected-warning{{Division by zero}} \
@@ -60,7 +60,7 @@ int test_case_note_1(int y) {
 }
 
 int test_case_note_2(int y) {
-  int x = __test_case_note(); // expected-note{{Function returns 1}}
+  int x = __test_case_note();
   return y / (x - 1); // expected-warning{{Division by zero}} \
                       // expected-note{{Division by zero}}
 }

diff  --git a/clang/test/Analysis/std-c-library-functions-arg-constraints.c b/clang/test/Analysis/std-c-library-functions-arg-constraints.c
index 2fdea1fecd831e..21f1d9f8ad8dd5 100644
--- a/clang/test/Analysis/std-c-library-functions-arg-constraints.c
+++ b/clang/test/Analysis/std-c-library-functions-arg-constraints.c
@@ -35,8 +35,7 @@ void test_alnum_concrete(int v) {
 }
 
 void test_alnum_symbolic(int x) {
-  int ret = isalnum(x); // \
-  // bugpath-note{{Assuming the character is non-alphanumeric}}
+  int ret = isalnum(x);
   (void)ret;
 
   clang_analyzer_eval(EOF <= x && x <= 255); // \
@@ -170,8 +169,7 @@ void test_notnull_concrete(FILE *fp) {
   // bugpath-note{{The 1st argument to 'fread' is NULL but should not be NULL}}
 }
 void test_notnull_symbolic(FILE *fp, int *buf) {
-  fread(buf, sizeof(int), 10, fp); // \
-  // bugpath-note{{'fread' fails}}
+  fread(buf, sizeof(int), 10, fp);
   clang_analyzer_eval(buf != 0); // \
   // report-warning{{TRUE}} \
   // bugpath-warning{{TRUE}} \

diff  --git a/clang/test/Analysis/std-c-library-functions-path-notes.c b/clang/test/Analysis/std-c-library-functions-path-notes.c
index 33d6152376c57c..6b5d1d7bd4eb97 100644
--- a/clang/test/Analysis/std-c-library-functions-path-notes.c
+++ b/clang/test/Analysis/std-c-library-functions-path-notes.c
@@ -22,11 +22,9 @@ char test_getenv() {
 }
 
 int test_isalpha(int *x, char c, char d) {
-  int iad = isalpha(d);// \
-  // expected-note{{Assuming the character is non-alphabetical}}
+  int iad = isalpha(d);
   if (isalpha(c)) {// \
-    // expected-note{{Taking true branch}} \
-    // expected-note{{Assuming the character is alphabetical}}
+    // expected-note{{Taking true branch}}
     x = NULL; // \
     // expected-note{{Null pointer value stored to 'x'}}
   }
@@ -38,7 +36,6 @@ int test_isalpha(int *x, char c, char d) {
 
 int test_isdigit(int *x, char c) {
   if (!isdigit(c)) {// \
-  // expected-note{{Assuming the character is not a digit}} \
   // expected-note{{Taking true branch}}
     x = NULL; // \
     // expected-note{{Null pointer value stored to 'x'}}
@@ -64,8 +61,7 @@ int test_islower(int *x) {
 }
 
 int test_bugpath_notes(FILE *f1, char c, FILE *f2) {
-  int f = fileno(f2); // \
-  // expected-note{{Assuming that 'fileno' is successful}}
+  int f = fileno(f2);
   if (f == -1) // \
     // expected-note{{Taking false branch}}
     return 0;
@@ -77,3 +73,10 @@ int test_bugpath_notes(FILE *f1, char c, FILE *f2) {
   // expected-warning{{The 1st argument to 'dup' is -1 but should be >= 0}} \
   // expected-note{{The 1st argument to 'dup' is -1 but should be >= 0}}
 }
+
+int test_fileno_arg_note(FILE *f1) {
+  return dup(fileno(f1)); // \
+  // expected-warning{{The 1st argument to 'dup' is < 0 but should be >= 0}} \
+  // expected-note{{The 1st argument to 'dup' is < 0 but should be >= 0}} \
+  // expected-note{{Assuming that 'fileno' fails}}
+}

diff  --git a/clang/test/Analysis/stream-errno-note.c b/clang/test/Analysis/stream-errno-note.c
index d8aefbabd0b81c..4ab215a64539de 100644
--- a/clang/test/Analysis/stream-errno-note.c
+++ b/clang/test/Analysis/stream-errno-note.c
@@ -11,7 +11,6 @@
 void check_fopen(void) {
   FILE *F = fopen("xxx", "r");
   // expected-note at -1{{'errno' may be undefined after successful call to 'fopen'}}
-  // expected-note at -2{{'fopen' is successful}}
   // expected-note at +2{{'F' is non-null}}
   // expected-note at +1{{Taking false branch}}
   if (!F)
@@ -24,7 +23,6 @@ void check_fopen(void) {
 void check_tmpfile(void) {
   FILE *F = tmpfile();
   // expected-note at -1{{'errno' may be undefined after successful call to 'tmpfile'}}
-  // expected-note at -2{{'tmpfile' is successful}}
   // expected-note at +2{{'F' is non-null}}
   // expected-note at +1{{Taking false branch}}
   if (!F)
@@ -36,14 +34,12 @@ void check_tmpfile(void) {
 
 void check_freopen(void) {
   FILE *F = tmpfile();
-  // expected-note at -1{{'tmpfile' is successful}}
   // expected-note at +2{{'F' is non-null}}
   // expected-note at +1{{Taking false branch}}
   if (!F)
     return;
   F = freopen("xxx", "w", F);
   // expected-note at -1{{'errno' may be undefined after successful call to 'freopen'}}
-  // expected-note at -2{{'freopen' is successful}}
   // expected-note at +2{{'F' is non-null}}
   // expected-note at +1{{Taking false branch}}
   if (!F)
@@ -55,14 +51,12 @@ void check_freopen(void) {
 
 void check_fclose(void) {
   FILE *F = tmpfile();
-  // expected-note at -1{{'tmpfile' is successful}}
   // expected-note at +2{{'F' is non-null}}
   // expected-note at +1{{Taking false branch}}
   if (!F)
     return;
   (void)fclose(F);
   // expected-note at -1{{'errno' may be undefined after successful call to 'fclose'}}
-  // expected-note at -2{{'fclose' is successful}}
   if (errno) {} // expected-warning{{An undefined value may be read from 'errno'}}
   // expected-note at -1{{An undefined value may be read from 'errno'}}
 }
@@ -70,14 +64,12 @@ void check_fclose(void) {
 void check_fread(void) {
   char Buf[10];
   FILE *F = tmpfile();
-  // expected-note at -1{{'tmpfile' is successful}}
   // expected-note at +2{{'F' is non-null}}
   // expected-note at +1{{Taking false branch}}
   if (!F)
     return;
   (void)fread(Buf, 1, 10, F);
   // expected-note at -1{{'errno' may be undefined after successful call to 'fread'}}
-  // expected-note at -2{{'fread' is successful}}
   if (errno) {} // expected-warning{{An undefined value may be read from 'errno'}}
   // expected-note at -1{{An undefined value may be read from 'errno'}}
   (void)fclose(F);
@@ -86,14 +78,12 @@ void check_fread(void) {
 void check_fread_size0(void) {
   char Buf[10];
   FILE *F = tmpfile();
-  // expected-note at -1{{'tmpfile' is successful}}
   // expected-note at +2{{'F' is non-null}}
   // expected-note at +1{{Taking false branch}}
   if (!F)
     return;
   fread(Buf, 0, 1, F);
   // expected-note at -1{{'errno' may be undefined after successful call to 'fread'}}
-  // expected-note at -2{{'fread' is successful}}
   if (errno) {} // expected-warning{{An undefined value may be read from 'errno'}}
   // expected-note at -1{{An undefined value may be read from 'errno'}}
 }
@@ -101,14 +91,12 @@ void check_fread_size0(void) {
 void check_fread_nmemb0(void) {
   char Buf[10];
   FILE *F = tmpfile();
-  // expected-note at -1{{'tmpfile' is successful}}
   // expected-note at +2{{'F' is non-null}}
   // expected-note at +1{{Taking false branch}}
   if (!F)
     return;
   fread(Buf, 1, 0, F);
   // expected-note at -1{{'errno' may be undefined after successful call to 'fread'}}
-  // expected-note at -2{{'fread' is successful}}
   if (errno) {} // expected-warning{{An undefined value may be read from 'errno'}}
   // expected-note at -1{{An undefined value may be read from 'errno'}}
 }
@@ -116,14 +104,12 @@ void check_fread_nmemb0(void) {
 void check_fwrite(void) {
   char Buf[] = "0123456789";
   FILE *F = tmpfile();
-  // expected-note at -1{{'tmpfile' is successful}}
   // expected-note at +2{{'F' is non-null}}
   // expected-note at +1{{Taking false branch}}
   if (!F)
     return;
   int R = fwrite(Buf, 1, 10, F);
   // expected-note at -1{{'errno' may be undefined after successful call to 'fwrite'}}
-  // expected-note at -2{{'fwrite' is successful}}
   if (errno) {} // expected-warning{{An undefined value may be read from 'errno'}}
   // expected-note at -1{{An undefined value may be read from 'errno'}}
   (void)fclose(F);
@@ -131,14 +117,12 @@ void check_fwrite(void) {
 
 void check_fseek(void) {
   FILE *F = tmpfile();
-  // expected-note at -1{{'tmpfile' is successful}}
   // expected-note at +2{{'F' is non-null}}
   // expected-note at +1{{Taking false branch}}
   if (!F)
     return;
   (void)fseek(F, 11, SEEK_SET);
   // expected-note at -1{{'errno' may be undefined after successful call to 'fseek'}}
-  // expected-note at -2{{'fseek' is successful}}
   if (errno) {} // expected-warning{{An undefined value may be read from 'errno'}}
   // expected-note at -1{{An undefined value may be read from 'errno'}}
   (void)fclose(F);
@@ -146,7 +130,6 @@ void check_fseek(void) {
 
 void check_rewind_errnocheck(void) {
   FILE *F = tmpfile();
-  // expected-note at -1{{'tmpfile' is successful}}
   // expected-note at +2{{'F' is non-null}}
   // expected-note at +1{{Taking false branch}}
   if (!F)
@@ -159,14 +142,12 @@ void check_rewind_errnocheck(void) {
 
 void check_fileno(void) {
   FILE *F = tmpfile();
-  // expected-note at -1{{'tmpfile' is successful}}
   // expected-note at +2{{'F' is non-null}}
   // expected-note at +1{{Taking false branch}}
   if (!F)
     return;
   fileno(F);
-  // expected-note at -1{{Assuming that 'fileno' is successful}}
-  // expected-note at -2{{'errno' may be undefined after successful call to 'fileno'}}
+  // expected-note at -1{{'errno' may be undefined after successful call to 'fileno'}}
   if (errno) {} // expected-warning{{An undefined value may be read from 'errno'}}
   // expected-note at -1{{An undefined value may be read from 'errno'}}
   (void)fclose(F);

diff  --git a/clang/test/Analysis/stream-note.c b/clang/test/Analysis/stream-note.c
index 8d297715b03a3a..257245754daddc 100644
--- a/clang/test/Analysis/stream-note.c
+++ b/clang/test/Analysis/stream-note.c
@@ -13,7 +13,6 @@ void check_note_at_correct_open(void) {
     // expected-note at -2 {{Taking false branch}}
     return;
   FILE *F2 = tmpfile();
-  // stdargs-note at -1 {{'tmpfile' is successful}}
   if (!F2) {
     // expected-note at -1 {{'F2' is non-null}}
     // expected-note at -2 {{Taking false branch}}
@@ -22,7 +21,6 @@ void check_note_at_correct_open(void) {
   }
   rewind(F2);
   fclose(F2);
-  // stdargs-note at -1 {{'fclose' fails}}
   rewind(F1);
 }
 // expected-warning at -1 {{Opened stream never closed. Potential resource leak}}
@@ -59,7 +57,6 @@ void check_note_freopen(void) {
 void check_note_leak_2(int c) {
   FILE *F1 = fopen("foo1.c", "r"); // expected-note {{Stream opened here}}
   // stdargs-note at -1 {{'fopen' is successful}}
-  // stdargs-note at -2 {{'fopen' is successful}}
   if (!F1)
     // expected-note at -1 {{'F1' is non-null}}
     // expected-note at -2 {{Taking false branch}}
@@ -68,7 +65,6 @@ void check_note_leak_2(int c) {
     return;
   FILE *F2 = fopen("foo2.c", "r"); // expected-note {{Stream opened here}}
   // stdargs-note at -1 {{'fopen' is successful}}
-  // stdargs-note at -2 {{'fopen' is successful}}
   if (!F2) {
     // expected-note at -1 {{'F2' is non-null}}
     // expected-note at -2 {{Taking false branch}}
@@ -107,16 +103,13 @@ void check_eof_notes_feof_after_feof(void) {
   FILE *F;
   char Buf[10];
   F = fopen("foo1.c", "r");
-  // stdargs-note at -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 at -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 at -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 at -1 {{Read function called when stream is in EOF state. Function has no effect}}
@@ -129,12 +122,10 @@ void check_eof_notes_feof_after_no_feof(void) {
   FILE *F;
   char Buf[10];
   F = fopen("foo1.c", "r");
-  // stdargs-note at -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 at -1 {{'fread' is successful}}
   if (feof(F)) { // expected-note {{Taking false branch}}
     fclose(F);
     return;
@@ -143,7 +134,6 @@ void check_eof_notes_feof_after_no_feof(void) {
     return;
   }
   fread(Buf, 1, 1, F);   // expected-note {{Assuming stream reaches end-of-file here}}
-  // stdargs-note at -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 at -1 {{Read function called when stream is in EOF state. Function has no effect}}
@@ -155,11 +145,9 @@ void check_eof_notes_feof_or_no_error(void) {
   FILE *F;
   char Buf[10];
   F = fopen("foo1.c", "r");
-  // stdargs-note at -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 at -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}}


        


More information about the cfe-commits mailing list