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 report of NULL stream was removed in commit 570bf97
<https://reviews.llvm.org/rG570bf972f5adf05438c7e08d693bf4b96bfd510a>.
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.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D152169
Files:
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
Index: clang/test/Analysis/stream.c
===================================================================
--- clang/test/Analysis/stream.c
+++ 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 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.
}
Index: clang/test/Analysis/stream-stdlibraryfunctionargs.c
===================================================================
--- clang/test/Analysis/stream-stdlibraryfunctionargs.c
+++ 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 @@
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_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_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_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_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);
}
Index: clang/test/Analysis/stream-note.c
===================================================================
--- clang/test/Analysis/stream-note.c
+++ clang/test/Analysis/stream-note.c
@@ -83,13 +83,13 @@
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) {
Index: clang/test/Analysis/std-c-library-functions-arg-weakdeps.c
===================================================================
--- clang/test/Analysis/std-c-library-functions-arg-weakdeps.c
+++ 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 @@
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]}}
+}
Index: clang/test/Analysis/std-c-library-functions-arg-enabled-checkers.c
===================================================================
--- clang/test/Analysis/std-c-library-functions-arg-enabled-checkers.c
+++ 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
Index: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -210,6 +210,7 @@
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 @@
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 @@
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;
}
Index: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
===================================================================
--- clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -550,6 +550,7 @@
def StreamChecker : Checker<"Stream">,
HelpText<"Check stream handling functions">,
+ WeakDependencies<[NonNullParamChecker]>,
Documentation<HasDocumentation>;
def SimpleStreamChecker : Checker<"SimpleStream">,
@@ -578,7 +579,7 @@
"false",
InAlpha>
]>,
- WeakDependencies<[CallAndMessageChecker, NonNullParamChecker]>,
+ WeakDependencies<[CallAndMessageChecker, NonNullParamChecker, StreamChecker]>,
Documentation<HasDocumentation>;
} // end "alpha.unix"
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
