[clang][analyzer] Remove report of null stream from StreamChecker.

The case of NULL stream passed to stream functions was reported by StreamChecker.
The same condition is checked already by StdLibraryFunctionsChecker and it is
enough to check at one place. The StreamChecker stops now analysis if a passed NULL
stream is encountered but generates no report.
This change removes a dependency between StdCLibraryFunctionArgs checker and
StreamChecker. There is now no more specific message reported by StreamChecker,
the previous weak-dependency is not needed. And StreamChecker can be used
without StdCLibraryFunctions checker or its ModelPOSIX option.

Reviewed By: Szelethus

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

diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
index 3dd2c7c..094b3a6 100644 (file)
--- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -588,7 +588,7 @@ def StdCLibraryFunctionArgsChecker : Checker<"StdCLibraryFunctionArgs">,
            "such as whether the parameter of isalpha is in the range [0, 255] "
            "or is EOF.">,
   Dependencies<[StdCLibraryFunctionsChecker]>,
-  WeakDependencies<[CallAndMessageChecker, NonNullParamChecker, StreamChecker]>,
+  WeakDependencies<[CallAndMessageChecker, NonNullParamChecker]>,
   Documentation<HasDocumentation>;
 
 } // end "alpha.unix"

diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index e02ec4d..4af6a69 100644 (file)
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -209,7 +209,6 @@ ProgramStateRef bindInt(uint64_t Value, ProgramStateRef State,
 
 class StreamChecker : public Checker<check::PreCall, eval::Call,
                                      check::DeadSymbols, check::PointerEscape> {
-  BugType BT_FileNull{this, "NULL stream pointer", "Stream handling error"};
   BugType BT_UseAfterClose{this, "Closed stream", "Stream handling error"};
   BugType BT_UseAfterOpenFailed{this, "Invalid stream",
                                 "Stream handling error"};
@@ -338,7 +337,7 @@ private:
                          const StreamErrorState &ErrorKind) const;
 
   /// Check that the stream (in StreamVal) is not NULL.
-  /// If it can only be NULL a fatal error is emitted and nullptr returned.
+  /// If it can only be NULL a sink node is generated and nullptr returned.
   /// Otherwise the return value is a new state where the stream is constrained
   /// to be non-null.
   ProgramStateRef ensureStreamNonNull(SVal StreamVal, const Expr *StreamE,
@@ -1039,13 +1038,11 @@ StreamChecker::ensureStreamNonNull(SVal StreamVal, const Expr *StreamE,
   std::tie(StateNotNull, StateNull) = CM.assumeDual(C.getState(), *Stream);
 
   if (!StateNotNull && StateNull) {
-    if (ExplodedNode *N = C.generateErrorNode(StateNull)) {
-      auto R = std::make_unique<PathSensitiveBugReport>(
-          BT_FileNull, "Stream pointer might be NULL.", N);
-      if (StreamE)
-        bugreporter::trackExpressionValue(N, StreamE, *R);
-      C.emitReport(std::move(R));
-    }
+    // Stream argument is NULL, stop analysis on this path.
+    // This case should occur only if StdLibraryFunctionsChecker (or ModelPOSIX
+    // option of it) is not turned on, otherwise that checker ensures non-null
+    // argument.
+    C.generateSink(StateNull, C.getPredecessor());
     return nullptr;
   }
 

diff --git a/clang/test/Analysis/std-c-library-functions-arg-enabled-checkers.c b/clang/test/Analysis/std-c-library-functions-arg-enabled-checkers.c
index f00e10a..4e5c66a 100644 (file)
--- a/clang/test/Analysis/std-c-library-functions-arg-enabled-checkers.c
+++ b/clang/test/Analysis/std-c-library-functions-arg-enabled-checkers.c
@@ -18,9 +18,9 @@
 // CHECK-NEXT: core.CallAndMessageModeling
 // CHECK-NEXT: core.CallAndMessage
 // CHECK-NEXT: core.NonNullParamChecker
-// CHECK-NEXT: alpha.unix.Stream
 // CHECK-NEXT: apiModeling.StdCLibraryFunctions
 // CHECK-NEXT: alpha.unix.StdCLibraryFunctionArgs
+// CHECK-NEXT: alpha.unix.Stream
 // CHECK-NEXT: apiModeling.Errno
 // CHECK-NEXT: apiModeling.TrustNonnull
 // CHECK-NEXT: apiModeling.TrustReturnsNonnull

diff --git a/clang/test/Analysis/std-c-library-functions-arg-weakdeps.c b/clang/test/Analysis/std-c-library-functions-arg-weakdeps.c
index 1dead73..3d2d5a6 100644 (file)
--- a/clang/test/Analysis/std-c-library-functions-arg-weakdeps.c
+++ b/clang/test/Analysis/std-c-library-functions-arg-weakdeps.c
@@ -6,7 +6,6 @@
 // RUN:   -analyzer-checker=apiModeling.StdCLibraryFunctions \
 // RUN:   -analyzer-config apiModeling.StdCLibraryFunctions:ModelPOSIX=true \
 // RUN:   -analyzer-checker=alpha.unix.StdCLibraryFunctionArgs \
-// RUN:   -analyzer-checker=alpha.unix.Stream \
 // RUN:   -triple x86_64-unknown-linux-gnu \
 // RUN:   -verify
 
@@ -18,7 +17,6 @@
 // RUN:   -analyzer-checker=apiModeling.StdCLibraryFunctions \
 // RUN:   -analyzer-config apiModeling.StdCLibraryFunctions:ModelPOSIX=true \
 // RUN:   -analyzer-checker=alpha.unix.StdCLibraryFunctionArgs \
-// RUN:   -analyzer-checker=alpha.unix.Stream \
 // RUN:   -analyzer-config apiModeling.StdCLibraryFunctions:DisplayLoadedSummaries=true \
 // RUN:   -triple x86_64-unknown-linux 2>&1 | FileCheck %s
 
@@ -57,8 +55,3 @@ void test_notnull_arg(FILE *F) {
   fread(p, sizeof(int), 5, F); // \
   expected-warning{{Null pointer passed to 1st parameter expecting 'nonnull' [core.NonNullParamChecker]}}
 }
-
-void test_notnull_stream_arg(void) {
-  fileno(0); // \
-  // expected-warning{{Stream pointer might be NULL [alpha.unix.Stream]}}
-}

diff --git a/clang/test/Analysis/stream-noopen.c b/clang/test/Analysis/stream-noopen.c
index 003923c..bc1d768 100644 (file)
--- a/clang/test/Analysis/stream-noopen.c
+++ b/clang/test/Analysis/stream-noopen.c
@@ -33,9 +33,11 @@ void test_freopen(FILE *F) {
 
 void test_fread(FILE *F) {
   size_t Ret = fread(RBuf, 1, 10, F);
+  clang_analyzer_eval(F != NULL); // expected-warning {{TRUE}}
   if (Ret == 10) {
     if (errno) {} // expected-warning{{undefined}}
   } else {
+    clang_analyzer_eval(Ret < 10); // expected-warning {{TRUE}}
     clang_analyzer_eval(errno != 0); // expected-warning {{TRUE}}
   }
   clang_analyzer_eval(feof(F)); // expected-warning {{UNKNOWN}}
@@ -44,9 +46,11 @@ void test_fread(FILE *F) {
 
 void test_fwrite(FILE *F) {
   size_t Ret = fwrite(WBuf, 1, 10, F);
+  clang_analyzer_eval(F != NULL); // expected-warning {{TRUE}}
   if (Ret == 10) {
     if (errno) {} // expected-warning{{undefined}}
   } else {
+    clang_analyzer_eval(Ret < 10); // expected-warning {{TRUE}}
     clang_analyzer_eval(errno != 0); // expected-warning {{TRUE}}
   }
   clang_analyzer_eval(feof(F)); // expected-warning {{UNKNOWN}}
@@ -55,6 +59,7 @@ void test_fwrite(FILE *F) {
 
 void test_fclose(FILE *F) {
   int Ret = fclose(F);
+  clang_analyzer_eval(F != NULL); // expected-warning {{TRUE}}
   if (Ret == 0) {
     if (errno) {} // expected-warning{{undefined}}
   } else {
@@ -67,6 +72,7 @@ void test_fclose(FILE *F) {
 
 void test_fseek(FILE *F) {
   int Ret = fseek(F, SEEK_SET, 1);
+  clang_analyzer_eval(F != NULL); // expected-warning {{TRUE}}
   if (Ret == 0) {
     if (errno) {} // expected-warning{{undefined}}
   } else {
@@ -81,6 +87,7 @@ void check_fgetpos(FILE *F) {
   errno = 0;
   fpos_t Pos;
   int Ret = fgetpos(F, &Pos);
+  clang_analyzer_eval(F != NULL); // expected-warning {{TRUE}}
   if (Ret)
     clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}}
   else
@@ -95,6 +102,7 @@ void check_fsetpos(FILE *F) {
   errno = 0;
   fpos_t Pos;
   int Ret = fsetpos(F, &Pos);
+  clang_analyzer_eval(F != NULL); // expected-warning {{TRUE}}
   if (Ret)
     clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}}
   else
@@ -108,6 +116,7 @@ void check_fsetpos(FILE *F) {
 void check_ftell(FILE *F) {
   errno = 0;
   long Ret = ftell(F);
+  clang_analyzer_eval(F != NULL); // expected-warning {{TRUE}}
   if (Ret == -1) {
     clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}}
   } else {
@@ -120,6 +129,42 @@ void check_ftell(FILE *F) {
   clang_analyzer_eval(ferror(F)); // expected-warning {{UNKNOWN}}
 }
 
+void test_rewind(FILE *F) {
+  errno = 0;
+  rewind(F);
+  clang_analyzer_eval(F != NULL); // expected-warning{{TRUE}}
+  clang_analyzer_eval(errno == 0); // expected-warning{{TRUE}}
+                                   // expected-warning@-1{{FALSE}}
+  rewind(F);
+}
+
+void test_feof(FILE *F) {
+  errno = 0;
+  feof(F);
+  clang_analyzer_eval(F != NULL); // expected-warning{{TRUE}}
+  if (errno) {} // no-warning
+  clang_analyzer_eval(errno == 0); // expected-warning{{TRUE}}
+                                   // expected-warning@-1{{FALSE}}
+}
+
+void test_ferror(FILE *F) {
+  errno = 0;
+  ferror(F);
+  clang_analyzer_eval(F != NULL); // expected-warning{{TRUE}}
+  if (errno) {} // no-warning
+  clang_analyzer_eval(errno == 0); // expected-warning{{TRUE}}
+                                   // expected-warning@-1{{FALSE}}
+}
+
+void test_clearerr(FILE *F) {
+  errno = 0;
+  clearerr(F);
+  clang_analyzer_eval(F != NULL); // expected-warning{{TRUE}}
+  if (errno) {} // no-warning
+  clang_analyzer_eval(errno == 0); // expected-warning{{TRUE}}
+                                   // expected-warning@-1{{FALSE}}
+}
+
 void freadwrite_zerosize(FILE *F) {
   fwrite(WBuf, 1, 0, F);
   clang_analyzer_eval(feof(F)); // expected-warning {{UNKNOWN}}

diff --git a/clang/test/Analysis/stream-note.c b/clang/test/Analysis/stream-note.c
index 1ed9bda..42e46d4 100644 (file)
--- a/clang/test/Analysis/stream-note.c
+++ b/clang/test/Analysis/stream-note.c
@@ -1,4 +1,7 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.unix.Stream -analyzer-output text -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.unix.Stream -analyzer-output text \
+// RUN:   -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.unix.Stream,alpha.unix.StdCLibraryFunctionArgs -analyzer-output text \
+// RUN:   -analyzer-config apiModeling.StdCLibraryFunctions:ModelPOSIX=true -verify=expected,stdargs %s
 
 #include "Inputs/system-header-simulator.h"
 
@@ -80,13 +83,14 @@ void check_note_leak_2(int c) {
 
 void check_track_null(void) {
   FILE *F;
-  F = fopen("foo1.c", "r"); // expected-note {{Value assigned to 'F'}} expected-note {{Assuming pointer value is null}}
-  if (F != NULL) {          // expected-note {{Taking false branch}} expected-note {{'F' is equal to NULL}}
+  F = fopen("foo1.c", "r"); // stdargs-note {{Value assigned to 'F'}} stdargs-note {{Assuming pointer value is null}}
+  if (F != NULL) {          // stdargs-note {{Taking false branch}} stdargs-note {{'F' is equal to NULL}}
     fclose(F);
     return;
   }
-  fclose(F); // expected-warning {{Stream pointer might be NULL}}
-  // expected-note@-1 {{Stream pointer might be NULL}}
+  fclose(F); // stdargs-warning {{Function argument constraint is not satisfied}}
+             // stdargs-note@-1 {{Function argument constraint is not satisfied}}
+             // stdargs-note@-2 {{The 1st argument should not be NULL}}
 }
 
 void check_eof_notes_feof_after_feof(void) {

diff --git a/clang/test/Analysis/stream-stdlibraryfunctionargs.c b/clang/test/Analysis/stream-stdlibraryfunctionargs.c
new file mode 100644 (file)
index 0000000..469d40b
--- /dev/null
+++ b/clang/test/Analysis/stream-stdlibraryfunctionargs.c
@@ -0,0 +1,142 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.unix.Stream,alpha.unix.StdCLibraryFunctionArgs,debug.ExprInspection \
+// RUN:   -analyzer-config apiModeling.StdCLibraryFunctions:ModelPOSIX=true -verify=stdargs,any %s
+
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.unix.Stream,debug.ExprInspection \
+// RUN:   -analyzer-config apiModeling.StdCLibraryFunctions:ModelPOSIX=true -verify=any %s
+
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.unix.StdCLibraryFunctionArgs,debug.ExprInspection \
+// RUN:   -analyzer-config apiModeling.StdCLibraryFunctions:ModelPOSIX=true -verify=stdargs,any %s
+
+#include "Inputs/system-header-simulator.h"
+
+extern void clang_analyzer_eval(int);
+
+void *buf;
+size_t size;
+size_t n;
+
+void test_fopen(void) {
+  FILE *fp = fopen("path", "r");
+  clang_analyzer_eval(fp != NULL); // any-warning{{TRUE}} any-warning{{FALSE}}
+  fclose(fp); // stdargs-warning{{Function argument constraint is not satisfied}} \
+              // stdargs-note{{The 1st argument should not be NULL}}
+}
+
+void test_tmpfile(void) {
+  FILE *fp = tmpfile();
+  clang_analyzer_eval(fp != NULL); // any-warning{{TRUE}} any-warning{{FALSE}}
+  fclose(fp); // stdargs-warning{{Function argument constraint is not satisfied}} \
+              // stdargs-note{{The 1st argument should not be NULL}}
+}
+
+void test_fclose(void) {
+  FILE *fp = tmpfile();
+  fclose(fp); // stdargs-warning{{Function argument constraint is not satisfied}} \
+              // stdargs-note{{The 1st argument should not be NULL}}
+  clang_analyzer_eval(fp != NULL); // any-warning{{TRUE}}
+}
+
+void test_freopen(void) {
+  FILE *fp = tmpfile();
+  fp = freopen("file", "w", fp); // stdargs-warning{{Function argument constraint is not satisfied}} \
+                                 // stdargs-note{{The 3rd argument should not be NULL}}
+  fclose(fp); // stdargs-warning{{Function argument constraint is not satisfied}} \
+              // stdargs-note{{The 1st argument should not be NULL}}
+  clang_analyzer_eval(fp != NULL); // any-warning{{TRUE}}
+}
+
+void test_fread(void) {
+  FILE *fp = tmpfile();
+  size_t ret = fread(buf, size, n, fp); // stdargs-warning{{Function argument constraint is not satisfied}} \
+                                        // stdargs-note{{The 4th argument should not be NULL}}
+  clang_analyzer_eval(fp != NULL); // any-warning{{TRUE}}
+  clang_analyzer_eval(ret <= n); // any-warning{{TRUE}}
+  clang_analyzer_eval(ret == n); // any-warning{{TRUE}} any-warning{{FALSE}}
+
+  fclose(fp);
+}
+
+void test_fwrite(void) {
+  FILE *fp = tmpfile();
+  size_t ret = fwrite(buf, size, n, fp); // stdargs-warning{{Function argument constraint is not satisfied}} \
+                                        // stdargs-note{{The 4th argument should not be NULL}}
+  clang_analyzer_eval(fp != NULL); // any-warning{{TRUE}}
+  clang_analyzer_eval(ret <= n); // any-warning{{TRUE}}
+  clang_analyzer_eval(ret == n); // any-warning{{TRUE}} any-warning{{FALSE}}
+
+  fclose(fp);
+}
+
+void test_fseek(void) {
+  FILE *fp = tmpfile();
+  fseek(fp, 0, 0); // stdargs-warning{{Function argument constraint is not satisfied}} \
+                   // stdargs-note{{The 1st argument should not be NULL}}
+  clang_analyzer_eval(fp != NULL); // any-warning{{TRUE}}
+  fclose(fp);
+}
+
+void test_ftell(void) {
+  FILE *fp = tmpfile();
+  ftell(fp); // stdargs-warning{{Function argument constraint is not satisfied}} \
+             // stdargs-note{{The 1st argument should not be NULL}}
+  clang_analyzer_eval(fp != NULL); // any-warning{{TRUE}}
+  fclose(fp);
+}
+
+void test_rewind(void) {
+  FILE *fp = tmpfile();
+  rewind(fp); // stdargs-warning{{Function argument constraint is not satisfied}} \
+              // stdargs-note{{The 1st argument should not be NULL}}
+  clang_analyzer_eval(fp != NULL); // any-warning{{TRUE}}
+  fclose(fp);
+}
+
+void test_fgetpos(void) {
+  FILE *fp = tmpfile();
+  fpos_t pos;
+  fgetpos(fp, &pos); // stdargs-warning{{Function argument constraint is not satisfied}} \
+                     // stdargs-note{{The 1st argument should not be NULL}}
+  clang_analyzer_eval(fp != NULL); // any-warning{{TRUE}}
+  fclose(fp);
+}
+
+void test_fsetpos(void) {
+  FILE *fp = tmpfile();
+  fpos_t pos;
+  fsetpos(fp, &pos); // stdargs-warning{{Function argument constraint is not satisfied}} \
+                     // stdargs-note{{The 1st argument should not be NULL}}
+  clang_analyzer_eval(fp != NULL); // any-warning{{TRUE}}
+  fclose(fp);
+}
+
+void test_clearerr(void) {
+  FILE *fp = tmpfile();
+  clearerr(fp); // stdargs-warning{{Function argument constraint is not satisfied}} \
+                // stdargs-note{{The 1st argument should not be NULL}}
+  clang_analyzer_eval(fp != NULL); // any-warning{{TRUE}}
+  fclose(fp);
+}
+
+void test_feof(void) {
+  FILE *fp = tmpfile();
+  feof(fp); // stdargs-warning{{Function argument constraint is not satisfied}} \
+            // stdargs-note{{The 1st argument should not be NULL}}
+  clang_analyzer_eval(fp != NULL); // any-warning{{TRUE}}
+  fclose(fp);
+}
+
+void test_ferror(void) {
+  FILE *fp = tmpfile();
+  ferror(fp); // stdargs-warning{{Function argument constraint is not satisfied}} \
+              // stdargs-note{{The 1st argument should not be NULL}}
+  clang_analyzer_eval(fp != NULL); // any-warning{{TRUE}}
+  fclose(fp);
+}
+
+void test_fileno(void) {
+  FILE *fp = tmpfile();
+  fileno(fp); // stdargs-warning{{Function argument constraint is not satisfied}} \
+              // stdargs-note{{The 1st argument should not be NULL}}
+  clang_analyzer_eval(fp != NULL); // any-warning{{TRUE}}
+  fclose(fp);
+}

diff --git a/clang/test/Analysis/stream.c b/clang/test/Analysis/stream.c
index a01310c..44373e4 100644 (file)
--- a/clang/test/Analysis/stream.c
+++ b/clang/test/Analysis/stream.c
@@ -2,81 +2,6 @@
 
 #include "Inputs/system-header-simulator.h"
 
-void check_fread(void) {
-  FILE *fp = tmpfile();
-  fread(0, 0, 0, fp); // expected-warning {{Stream pointer might be NULL}}
-  fclose(fp);
-}
-
-void check_fwrite(void) {
-  FILE *fp = tmpfile();
-  fwrite(0, 0, 0, fp); // expected-warning {{Stream pointer might be NULL}}
-  fclose(fp);
-}
-
-void check_fseek(void) {
-  FILE *fp = tmpfile();
-  fseek(fp, 0, 0); // expected-warning {{Stream pointer might be NULL}}
-  fclose(fp);
-}
-
-void check_ftell(void) {
-  FILE *fp = tmpfile();
-  ftell(fp); // expected-warning {{Stream pointer might be NULL}}
-  fclose(fp);
-}
-
-void check_rewind(void) {
-  FILE *fp = tmpfile();
-  rewind(fp); // expected-warning {{Stream pointer might be NULL}}
-  fclose(fp);
-}
-
-void check_fgetpos(void) {
-  FILE *fp = tmpfile();
-  fpos_t pos;
-  fgetpos(fp, &pos); // expected-warning {{Stream pointer might be NULL}}
-  fclose(fp);
-}
-
-void check_fsetpos(void) {
-  FILE *fp = tmpfile();
-  fpos_t pos;
-  fsetpos(fp, &pos); // expected-warning {{Stream pointer might be NULL}}
-  fclose(fp);
-}
-
-void check_clearerr(void) {
-  FILE *fp = tmpfile();
-  clearerr(fp); // expected-warning {{Stream pointer might be NULL}}
-  fclose(fp);
-}
-
-void check_feof(void) {
-  FILE *fp = tmpfile();
-  feof(fp); // expected-warning {{Stream pointer might be NULL}}
-  fclose(fp);
-}
-
-void check_ferror(void) {
-  FILE *fp = tmpfile();
-  ferror(fp); // expected-warning {{Stream pointer might be NULL}}
-  fclose(fp);
-}
-
-void check_fileno(void) {
-  FILE *fp = tmpfile();
-  fileno(fp); // expected-warning {{Stream pointer might be NULL}}
-  fclose(fp);
-}
-
-void f_open(void) {
-  FILE *p = fopen("foo", "r");
-  char buf[1024];
-  fread(buf, 1, 1, p); // expected-warning {{Stream pointer might be NULL}}
-  fclose(p);
-}
-
 void f_seek(void) {
   FILE *p = fopen("foo", "r");
   if (!p)
@@ -161,7 +86,7 @@ void pr8081(FILE *stream, long offset, int whence) {
 }
 
 void check_freopen_1(void) {
-  FILE *f1 = freopen("foo.c", "r", (FILE *)0); // expected-warning {{Stream pointer might be NULL}}
+  FILE *f1 = freopen("foo.c", "r", (FILE *)0); // Not reported by the stream checker.
   f1 = freopen(0, "w", (FILE *)0x123456);      // Do not report this as error.
 }