[clang][analyzer] Add report of NULL stream to StreamChecker.

The report of NULL stream was removed in commit 570bf97.
The old reason is not actual any more because the checker dependencies are changed.
It is not good to eliminate a failure state (where the stream is NULL) without
generating a bug report because other checkers are not able to find it later.
The checker did this with the NULL stream pointer, and because this checker
runs now before other checkers that can detect NULL pointers, the null pointer
bug was not found at all.

Reviewed By: steakhal

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

diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
index 74f3dad..36c742a 100644 (file)
--- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -550,6 +550,7 @@ def PthreadLockChecker : Checker<"PthreadLock">,
 
 def StreamChecker : Checker<"Stream">,
   HelpText<"Check stream handling functions">,
+  WeakDependencies<[NonNullParamChecker]>,
   Documentation<HasDocumentation>;
 
 def SimpleStreamChecker : Checker<"SimpleStream">,
@@ -578,7 +579,7 @@ def StdCLibraryFunctionsChecker : Checker<"StdCLibraryFunctions">,
                   "false",
                   InAlpha>
   ]>,
-  WeakDependencies<[CallAndMessageChecker, NonNullParamChecker]>,
+  WeakDependencies<[CallAndMessageChecker, NonNullParamChecker, StreamChecker]>,
   Documentation<HasDocumentation>;
 
 } // end "alpha.unix"

diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index 3f61dd8..d2ddb5c 100644 (file)
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -210,6 +210,7 @@ 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 +339,7 @@ private:
                          const StreamErrorState &ErrorKind) const;
 
   /// Check that the stream (in StreamVal) is not NULL.
-  /// If it can only be NULL a sink node is generated and nullptr returned.
+  /// If it can only be NULL a fatal error is emitted 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,11 +1040,13 @@ StreamChecker::ensureStreamNonNull(SVal StreamVal, const Expr *StreamE,
   std::tie(StateNotNull, StateNull) = CM.assumeDual(C.getState(), *Stream);
 
   if (!StateNotNull && StateNull) {
-    // 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());
+    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));
+    }
     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 6965c31..2a28d74 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
@@ -17,8 +17,8 @@
 // CHECK-NEXT: core.CallAndMessageModeling
 // CHECK-NEXT: core.CallAndMessage
 // CHECK-NEXT: core.NonNullParamChecker
-// CHECK-NEXT: alpha.unix.StdCLibraryFunctions
 // CHECK-NEXT: alpha.unix.Stream
+// CHECK-NEXT: alpha.unix.StdCLibraryFunctions
 // 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 6f95563..87f07a2 100644 (file)
--- a/clang/test/Analysis/std-c-library-functions-arg-weakdeps.c
+++ b/clang/test/Analysis/std-c-library-functions-arg-weakdeps.c
@@ -3,6 +3,7 @@
 
 // RUN: %clang_analyze_cc1 %s \
 // RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=alpha.unix.Stream \
 // RUN:   -analyzer-checker=alpha.unix.StdCLibraryFunctions \
 // RUN:   -analyzer-config alpha.unix.StdCLibraryFunctions:ModelPOSIX=true \
 // RUN:   -triple x86_64-unknown-linux-gnu \
@@ -53,3 +54,8 @@ 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-note.c b/clang/test/Analysis/stream-note.c
index 61dd174..6466630 100644 (file)
--- a/clang/test/Analysis/stream-note.c
+++ b/clang/test/Analysis/stream-note.c
@@ -83,13 +83,13 @@ void check_note_leak_2(int c) {
 
 void check_track_null(void) {
   FILE *F;
-  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}}
+  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}}
     fclose(F);
     return;
   }
-  fclose(F); // stdargs-warning {{The 1st argument to 'fclose' is NULL but should not be NULL}}
-             // stdargs-note@-1 {{The 1st argument to 'fclose' is NULL but should not be NULL}}
+  fclose(F); // expected-warning {{Stream pointer might be NULL}}
+             // expected-note@-1 {{Stream pointer might 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
index 6180b30..a14befd 100644 (file)
--- a/clang/test/Analysis/stream-stdlibraryfunctionargs.c
+++ b/clang/test/Analysis/stream-stdlibraryfunctionargs.c
@@ -1,11 +1,11 @@
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.unix.Stream,alpha.unix.StdCLibraryFunctions,debug.ExprInspection \
-// RUN:   -analyzer-config alpha.unix.StdCLibraryFunctions:ModelPOSIX=true -verify=stdargs,any %s
+// RUN:   -analyzer-config alpha.unix.StdCLibraryFunctions:ModelPOSIX=true -verify=stream,any %s
 
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.unix.Stream,debug.ExprInspection \
-// RUN:   -analyzer-config alpha.unix.StdCLibraryFunctions:ModelPOSIX=true -verify=any %s
+// RUN:   -analyzer-config alpha.unix.StdCLibraryFunctions:ModelPOSIX=true -verify=stream,any %s
 
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.unix.StdCLibraryFunctions,debug.ExprInspection \
-// RUN:   -analyzer-config alpha.unix.StdCLibraryFunctions:ModelPOSIX=true -verify=stdargs,any %s
+// RUN:   -analyzer-config alpha.unix.StdCLibraryFunctions:ModelPOSIX=true -verify=stdfunc,any %s
 
 #include "Inputs/system-header-simulator.h"
 
@@ -18,31 +18,43 @@ 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{{should not be NULL}}
+  fclose(fp); // \
+  // stream-warning{{Stream pointer might be NULL}} \
+  // stdfunc-warning{{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{{should not be NULL}}
+  fclose(fp); // \
+  // stream-warning{{Stream pointer might be NULL}} \
+  // stdfunc-warning{{should not be NULL}}
 }
 
 void test_fclose(void) {
   FILE *fp = tmpfile();
-  fclose(fp); // stdargs-warning{{should not be NULL}}
+  fclose(fp); // \
+  // stream-warning{{Stream pointer might be NULL}} \
+  // stdfunc-warning{{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{{should not be NULL}}
-  fclose(fp); // stdargs-warning{{should not be NULL}}
+  fp = freopen("file", "w", fp); // \
+  // stream-warning{{Stream pointer might be NULL}} \
+  // stdfunc-warning{{should not be NULL}}
+  fclose(fp); // \
+  // stream-warning{{Stream pointer might be NULL}} \
+  // stdfunc-warning{{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{{The 4th argument to 'fread' is NULL but should not be NULL}}
+  size_t ret = fread(buf, size, n, fp); // \
+  // stream-warning{{Stream pointer might be NULL}} \
+  // stdfunc-warning{{The 4th argument to 'fread' is NULL but 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}}
@@ -52,7 +64,9 @@ void test_fread(void) {
 
 void test_fwrite(void) {
   FILE *fp = tmpfile();
-  size_t ret = fwrite(buf, size, n, fp); // stdargs-warning{{The 4th argument to 'fwrite' is NULL but should not be NULL}}
+  size_t ret = fwrite(buf, size, n, fp); // \
+  // stream-warning{{Stream pointer might be NULL}} \
+  // stdfunc-warning{{The 4th argument to 'fwrite' is NULL but 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}}
@@ -62,21 +76,27 @@ void test_fwrite(void) {
 
 void test_fseek(void) {
   FILE *fp = tmpfile();
-  fseek(fp, 0, 0); // stdargs-warning{{should not be NULL}}
+  fseek(fp, 0, 0); // \
+  // stream-warning{{Stream pointer might be NULL}} \
+  // stdfunc-warning{{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{{should not be NULL}}
+  ftell(fp); // \
+  // stream-warning{{Stream pointer might be NULL}} \
+  // stdfunc-warning{{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{{should not be NULL}}
+  rewind(fp); // \
+  // stream-warning{{Stream pointer might be NULL}} \
+  // stdfunc-warning{{should not be NULL}}
   clang_analyzer_eval(fp != NULL); // any-warning{{TRUE}}
   fclose(fp);
 }
@@ -84,7 +104,9 @@ void test_rewind(void) {
 void test_fgetpos(void) {
   FILE *fp = tmpfile();
   fpos_t pos;
-  fgetpos(fp, &pos); // stdargs-warning{{should not be NULL}}
+  fgetpos(fp, &pos); // \
+  // stream-warning{{Stream pointer might be NULL}} \
+  // stdfunc-warning{{should not be NULL}}
   clang_analyzer_eval(fp != NULL); // any-warning{{TRUE}}
   fclose(fp);
 }
@@ -92,35 +114,45 @@ void test_fgetpos(void) {
 void test_fsetpos(void) {
   FILE *fp = tmpfile();
   fpos_t pos;
-  fsetpos(fp, &pos); // stdargs-warning{{should not be NULL}}
+  fsetpos(fp, &pos); // \
+  // stream-warning{{Stream pointer might be NULL}} \
+  // stdfunc-warning{{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{{should not be NULL}}
+  clearerr(fp); // \
+  // stream-warning{{Stream pointer might be NULL}} \
+  // stdfunc-warning{{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{{should not be NULL}}
+  feof(fp); // \
+  // stream-warning{{Stream pointer might be NULL}} \
+  // stdfunc-warning{{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{{should not be NULL}}
+  ferror(fp); // \
+  // stream-warning{{Stream pointer might be NULL}} \
+  // stdfunc-warning{{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{{should not be NULL}}
+  fileno(fp); // \
+  // stream-warning{{Stream pointer might be NULL}} \
+  // stdfunc-warning{{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 44373e4..a01310c 100644 (file)
--- a/clang/test/Analysis/stream.c
+++ b/clang/test/Analysis/stream.c
@@ -2,6 +2,81 @@
 
 #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)
@@ -86,7 +161,7 @@ void pr8081(FILE *stream, long offset, int whence) {
 }
 
 void check_freopen_1(void) {
-  FILE *f1 = freopen("foo.c", "r", (FILE *)0); // Not reported by the stream checker.
+  FILE *f1 = freopen("foo.c", "r", (FILE *)0); // expected-warning {{Stream pointer might be NULL}}
   f1 = freopen(0, "w", (FILE *)0x123456);      // Do not report this as error.
 }