[clang] 2c60f9c - [clang][analyzer] Add report of NULL stream to StreamChecker.
Balázs Kéri via cfe-commits
cfe-commits at lists.llvm.org
Tue Jun 6 02:51:56 PDT 2023
Author: Balázs Kéri
Date: 2023-06-06T11:51:33+02:00
New Revision: 2c60f9c8a4fdfce7f33493f874893e1c8c8143c1
URL: https://github.com/llvm/llvm-project/commit/2c60f9c8a4fdfce7f33493f874893e1c8c8143c1
DIFF: https://github.com/llvm/llvm-project/commit/2c60f9c8a4fdfce7f33493f874893e1c8c8143c1.diff
LOG: [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
Added:
Modified:
clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
clang/test/Analysis/std-c-library-functions-arg-enabled-checkers.c
clang/test/Analysis/std-c-library-functions-arg-weakdeps.c
clang/test/Analysis/stream-note.c
clang/test/Analysis/stream-stdlibraryfunctionargs.c
clang/test/Analysis/stream.c
Removed:
################################################################################
diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
index 74f3dad585ee7..36c742a5221fa 100644
--- 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 3f61dd8239405..d2ddb5c06f588 100644
--- 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 @@ class StreamChecker : public Checker<check::PreCall, eval::Call,
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 6965c311eeae4..2a28d74bd49d3 100644
--- 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 6f95563d045fb..87f07a2d90a14 100644
--- 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 61dd17446da43..6466630782f5d 100644
--- 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 at -1 {{The 1st argument to 'fclose' is NULL but should not be NULL}}
+ fclose(F); // expected-warning {{Stream pointer might be NULL}}
+ // expected-note at -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 6180b30ae1788..a14befde51038 100644
--- 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 44373e4ad9f6c..a01310cfef5dd 100644
--- 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.
}
More information about the cfe-commits
mailing list