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}}
   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}}
   // expected-note@+2{{'F' is non-null}}
   // expected-note@+1{{Taking false branch}}
   if (!F)
@@ -99,14 +91,12 @@
 
 void check_fseek(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)fseek(F, 11, SEEK_SET);
   // expected-note@-1{{'errno' may be undefined after successful call to 'fseek'}}
-  // 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'}}
   (void)fclose(F);
@@ -114,7 +104,6 @@
 
 void check_rewind_errnocheck(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)
@@ -127,14 +116,12 @@
 
 void check_fileno(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;
   fileno(F);
-  // expected-note@-1{{Assuming that the call is successful}}
-  // expected-note@-2{{'errno' may be undefined after successful call to 'fileno'}}
+  // expected-note@-1{{'errno' may be undefined after successful call to 'fileno'}}
   if (errno) {} // expected-warning{{An undefined value may be read from 'errno'}}
   // expected-note@-1{{An undefined value may be read from 'errno'}}
   (void)fclose(F);
Index: clang/test/Analysis/std-c-library-functions-path-notes.c
===================================================================
--- clang/test/Analysis/std-c-library-functions-path-notes.c
+++ clang/test/Analysis/std-c-library-functions-path-notes.c
@@ -22,11 +22,9 @@
 }
 
 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_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_bugpath_notes(FILE *f1, char c, FILE *f2) {
-  int f = fileno(f2); // \
-  // expected-note{{Assuming that the call is successful}}
+  int f = fileno(f2);
   if (f == -1) // \
     // expected-note{{Taking false branch}}
     return 0;
Index: clang/test/Analysis/std-c-library-functions-arg-constraints.c
===================================================================
--- clang/test/Analysis/std-c-library-functions-arg-constraints.c
+++ clang/test/Analysis/std-c-library-functions-arg-constraints.c
@@ -35,8 +35,7 @@
 }
 
 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); // \
Index: clang/test/Analysis/std-c-library-functions-arg-constraints-note-tags.cpp
===================================================================
--- clang/test/Analysis/std-c-library-functions-arg-constraints-note-tags.cpp
+++ clang/test/Analysis/std-c-library-functions-arg-constraints-note-tags.cpp
@@ -52,7 +52,7 @@
 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_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}}
 }
Index: clang/test/Analysis/errno-stdlibraryfunctions-notes.c
===================================================================
--- clang/test/Analysis/errno-stdlibraryfunctions-notes.c
+++ clang/test/Analysis/errno-stdlibraryfunctions-notes.c
@@ -14,10 +14,8 @@
 
 void test1() {
   access("path", 0);
-  // expected-note@-1{{Assuming that the call fails}}
   access("path", 0);
-  // expected-note@-1{{Assuming that the call is successful}}
-  // expected-note@-2{{'errno' may be undefined after successful call to 'access'}}
+  // expected-note@-1{{'errno' may be undefined after successful call to 'access'}}
   if (errno != 0) {
     // expected-warning@-1{{An undefined value may be read from 'errno'}}
     // expected-note@-2{{An undefined value may be read from 'errno'}}
@@ -26,8 +24,7 @@
 
 void test2() {
   if (access("path", 0) == -1) {
-    // expected-note@-1{{Assuming that the call fails}}
-    // expected-note@-2{{Taking true branch}}
+    // expected-note@-1{{Taking true branch}}
     // Failure path.
     if (errno != 0) {
       // expected-note@-1{{'errno' is not equal to 0}}
@@ -42,9 +39,8 @@
 void test3() {
   if (access("path", 0) != -1) {
     // Success path.
-    // expected-note@-2{{Assuming that the call is successful}}
-    // expected-note@-3{{'errno' may be undefined after successful call to 'access'}}
-    // expected-note@-4{{Taking true branch}}
+    // expected-note@-2{{'errno' may be undefined after successful call to 'access'}}
+    // expected-note@-3{{Taking true branch}}
     if (errno != 0) {
       // expected-warning@-1{{An undefined value may be read from 'errno'}}
       // expected-note@-2{{An undefined value may be read from 'errno'}}
Index: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -1298,6 +1298,7 @@
     // StdLibraryFunctionsChecker.
     ExplodedNode *Pred = const_cast<ExplodedNode *>(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.
       // Omit the note if we know in advance which branch is taken (this means,
@@ -1305,19 +1306,23 @@
       StringRef Note = Case.getNote();
       if (Summary.getInvalidationKd() == EvalCallAsPure) {
         const NoteTag *Tag = C.getNoteTag(
-            [Node, Note](PathSensitiveBugReport &BR) -> std::string {
-              if (Node->succ_size() > 1)
+            [Node, Note, RV](PathSensitiveBugReport &BR) -> std::string {
+              if (BR.isInteresting(RV) && Node->succ_size() > 1)
                 return Note.str();
               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.str();
+              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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to