[clang] 11bd3e5 - [Analyzer][StreamChecker] Introduction of stream error handling.
Balázs Kéri via cfe-commits
cfe-commits at lists.llvm.org
Wed Apr 8 02:32:04 PDT 2020
Author: Balázs Kéri
Date: 2020-04-08T11:30:19+02:00
New Revision: 11bd3e5c6549a4983be454ccfbeb16e88c9532db
URL: https://github.com/llvm/llvm-project/commit/11bd3e5c6549a4983be454ccfbeb16e88c9532db
DIFF: https://github.com/llvm/llvm-project/commit/11bd3e5c6549a4983be454ccfbeb16e88c9532db.diff
LOG: [Analyzer][StreamChecker] Introduction of stream error handling.
Summary:
Store the error flags (EOF or error) of a stream.
Support the functions feof, ferror, clearerr.
Added a test checker for setting the error flags.
Reviewers: Szelethus, NoQ, Charusso, baloghadamsoftware, xazax.hun
Reviewed By: Szelethus
Subscribers: steakhal, ASDenysPetrov, rnkovacs, xazax.hun, baloghadamsoftware, szepet, a.sidorin, mikhail.ramalho, Szelethus, donat.nagy, dkrupp, gamesh411, Charusso, martong, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D75682
Added:
clang/test/Analysis/stream-error.c
Modified:
clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
clang/test/Analysis/Inputs/system-header-simulator.h
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 6a577940e313..31e7e02c192b 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -1400,6 +1400,11 @@ def TaintTesterChecker : Checker<"TaintTest">,
HelpText<"Mark tainted symbols as such.">,
Documentation<NotDocumented>;
+def StreamTesterChecker : Checker<"StreamTester">,
+ HelpText<"Add test functions to StreamChecker for test and debugging purposes.">,
+ Dependencies<[StreamChecker]>,
+ Documentation<NotDocumented>;
+
def ExprInspectionChecker : Checker<"ExprInspection">,
HelpText<"Check the analyzer's understanding of expressions">,
Documentation<NotDocumented>;
diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index 4b20fcdc9a15..b38a645432f4 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -19,33 +19,67 @@
#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h"
-#include <functional>
using namespace clang;
using namespace ento;
-using namespace std::placeholders;
namespace {
+/// Full state information about a stream pointer.
struct StreamState {
- enum Kind { Opened, Closed, OpenFailed, Escaped } K;
-
- StreamState(Kind k) : K(k) {}
-
- bool isOpened() const { return K == Opened; }
- bool isClosed() const { return K == Closed; }
- bool isOpenFailed() const { return K == OpenFailed; }
- //bool isEscaped() const { return K == Escaped; }
+ /// State of a stream symbol.
+ /// FIXME: We need maybe an "escaped" state later.
+ enum KindTy {
+ Opened, /// Stream is opened.
+ Closed, /// Closed stream (an invalid stream pointer after it was closed).
+ OpenFailed /// The last open operation has failed.
+ } State;
+
+ /// The error state of a stream.
+ /// Valid only if the stream is opened.
+ /// It is assumed that feof and ferror flags are never true at the same time.
+ enum ErrorKindTy {
+ /// No error flag is set (or stream is not open).
+ NoError,
+ /// EOF condition (`feof` is true).
+ FEof,
+ /// Other generic (non-EOF) error (`ferror` is true).
+ FError,
+ } ErrorState = NoError;
+
+ bool isOpened() const { return State == Opened; }
+ bool isClosed() const { return State == Closed; }
+ bool isOpenFailed() const { return State == OpenFailed; }
+
+ bool isNoError() const {
+ assert(State == Opened && "Error undefined for closed stream.");
+ return ErrorState == NoError;
+ }
+ bool isFEof() const {
+ assert(State == Opened && "Error undefined for closed stream.");
+ return ErrorState == FEof;
+ }
+ bool isFError() const {
+ assert(State == Opened && "Error undefined for closed stream.");
+ return ErrorState == FError;
+ }
- bool operator==(const StreamState &X) const { return K == X.K; }
+ bool operator==(const StreamState &X) const {
+ // In not opened state error should always NoError.
+ return State == X.State && ErrorState == X.ErrorState;
+ }
- static StreamState getOpened() { return StreamState(Opened); }
- static StreamState getClosed() { return StreamState(Closed); }
- static StreamState getOpenFailed() { return StreamState(OpenFailed); }
- static StreamState getEscaped() { return StreamState(Escaped); }
+ static StreamState getOpened() { return StreamState{Opened}; }
+ static StreamState getClosed() { return StreamState{Closed}; }
+ static StreamState getOpenFailed() { return StreamState{OpenFailed}; }
+ static StreamState getOpenedWithFEof() { return StreamState{Opened, FEof}; }
+ static StreamState getOpenedWithFError() {
+ return StreamState{Opened, FError};
+ }
void Profile(llvm::FoldingSetNodeID &ID) const {
- ID.AddInteger(K);
+ ID.AddInteger(State);
+ ID.AddInteger(ErrorState);
}
};
@@ -71,6 +105,16 @@ SVal getStreamArg(const FnDescription *Desc, const CallEvent &Call) {
return Call.getArgSVal(Desc->StreamArgNo);
}
+/// Create a conjured symbol return value for a call expression.
+DefinedSVal makeRetVal(CheckerContext &C, const CallExpr *CE) {
+ assert(CE && "Expecting a call expression.");
+
+ const LocationContext *LCtx = C.getLocationContext();
+ return C.getSValBuilder()
+ .conjureSymbolVal(nullptr, CE, LCtx, C.blockCount())
+ .castAs<DefinedSVal>();
+}
+
class StreamChecker
: public Checker<check::PreCall, eval::Call, check::DeadSymbols> {
mutable std::unique_ptr<BuiltinBug> BT_nullfp, BT_illegalwhence,
@@ -81,6 +125,9 @@ class StreamChecker
bool evalCall(const CallEvent &Call, CheckerContext &C) const;
void checkDeadSymbols(SymbolReaper &SymReaper, CheckerContext &C) const;
+ /// If true, evaluate special testing stream functions.
+ bool TestMode = false;
+
private:
CallDescriptionMap<FnDescription> FnDescriptions = {
{{"fopen"}, {nullptr, &StreamChecker::evalFopen, ArgNone}},
@@ -96,12 +143,27 @@ class StreamChecker
{{"rewind", 1}, {&StreamChecker::preDefault, nullptr, 0}},
{{"fgetpos", 2}, {&StreamChecker::preDefault, nullptr, 0}},
{{"fsetpos", 2}, {&StreamChecker::preDefault, nullptr, 0}},
- {{"clearerr", 1}, {&StreamChecker::preDefault, nullptr, 0}},
- {{"feof", 1}, {&StreamChecker::preDefault, nullptr, 0}},
- {{"ferror", 1}, {&StreamChecker::preDefault, nullptr, 0}},
+ {{"clearerr", 1},
+ {&StreamChecker::preDefault, &StreamChecker::evalClearerr, 0}},
+ {{"feof", 1},
+ {&StreamChecker::preDefault,
+ &StreamChecker::evalFeofFerror<&StreamState::isFEof>, 0}},
+ {{"ferror", 1},
+ {&StreamChecker::preDefault,
+ &StreamChecker::evalFeofFerror<&StreamState::isFError>, 0}},
{{"fileno", 1}, {&StreamChecker::preDefault, nullptr, 0}},
};
+ CallDescriptionMap<FnDescription> FnTestDescriptions = {
+ {{"StreamTesterChecker_make_feof_stream", 1},
+ {nullptr,
+ &StreamChecker::evalSetFeofFerror<&StreamState::getOpenedWithFEof>, 0}},
+ {{"StreamTesterChecker_make_ferror_stream", 1},
+ {nullptr,
+ &StreamChecker::evalSetFeofFerror<&StreamState::getOpenedWithFError>,
+ 0}},
+ };
+
void evalFopen(const FnDescription *Desc, const CallEvent &Call,
CheckerContext &C) const;
@@ -119,6 +181,17 @@ class StreamChecker
void preDefault(const FnDescription *Desc, const CallEvent &Call,
CheckerContext &C) const;
+ void evalClearerr(const FnDescription *Desc, const CallEvent &Call,
+ CheckerContext &C) const;
+
+ template <bool (StreamState::*IsOfError)() const>
+ void evalFeofFerror(const FnDescription *Desc, const CallEvent &Call,
+ CheckerContext &C) const;
+
+ template <StreamState (*GetState)()>
+ void evalSetFeofFerror(const FnDescription *Desc, const CallEvent &Call,
+ CheckerContext &C) const;
+
/// Check that the stream (in StreamVal) is not NULL.
/// 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
@@ -160,6 +233,11 @@ class StreamChecker
REGISTER_MAP_WITH_PROGRAMSTATE(StreamMap, SymbolRef, StreamState)
+inline void assertStreamStateOpened(const StreamState *SS) {
+ assert(SS->isOpened() &&
+ "Previous create of error node for non-opened stream failed?");
+}
+
void StreamChecker::checkPreCall(const CallEvent &Call,
CheckerContext &C) const {
const FnDescription *Desc = lookupFn(Call);
@@ -171,6 +249,8 @@ void StreamChecker::checkPreCall(const CallEvent &Call,
bool StreamChecker::evalCall(const CallEvent &Call, CheckerContext &C) const {
const FnDescription *Desc = lookupFn(Call);
+ if (!Desc && TestMode)
+ Desc = FnTestDescriptions.lookup(Call);
if (!Desc || !Desc->EvalFn)
return false;
@@ -182,15 +262,11 @@ bool StreamChecker::evalCall(const CallEvent &Call, CheckerContext &C) const {
void StreamChecker::evalFopen(const FnDescription *Desc, const CallEvent &Call,
CheckerContext &C) const {
ProgramStateRef State = C.getState();
- SValBuilder &SVB = C.getSValBuilder();
- const LocationContext *LCtx = C.getPredecessor()->getLocationContext();
-
- auto *CE = dyn_cast_or_null<CallExpr>(Call.getOriginExpr());
+ const CallExpr *CE = dyn_cast_or_null<CallExpr>(Call.getOriginExpr());
if (!CE)
return;
- DefinedSVal RetVal = SVB.conjureSymbolVal(nullptr, CE, LCtx, C.blockCount())
- .castAs<DefinedSVal>();
+ DefinedSVal RetVal = makeRetVal(C, CE);
SymbolRef RetSym = RetVal.getAsSymbol();
assert(RetSym && "RetVal must be a symbol here.");
@@ -271,6 +347,8 @@ void StreamChecker::evalFclose(const FnDescription *Desc, const CallEvent &Call,
if (!SS)
return;
+ assertStreamStateOpened(SS);
+
// Close the File Descriptor.
// Regardless if the close fails or not, stream becomes "closed"
// and can not be used any more.
@@ -296,6 +374,61 @@ void StreamChecker::preFseek(const FnDescription *Desc, const CallEvent &Call,
C.addTransition(State);
}
+void StreamChecker::evalClearerr(const FnDescription *Desc,
+ const CallEvent &Call,
+ CheckerContext &C) const {
+ ProgramStateRef State = C.getState();
+ SymbolRef StreamSym = getStreamArg(Desc, Call).getAsSymbol();
+ if (!StreamSym)
+ return;
+
+ const StreamState *SS = State->get<StreamMap>(StreamSym);
+ if (!SS)
+ return;
+
+ assertStreamStateOpened(SS);
+
+ if (SS->isNoError())
+ return;
+
+ State = State->set<StreamMap>(StreamSym, StreamState::getOpened());
+ C.addTransition(State);
+}
+
+template <bool (StreamState::*IsOfError)() const>
+void StreamChecker::evalFeofFerror(const FnDescription *Desc,
+ const CallEvent &Call,
+ CheckerContext &C) const {
+ ProgramStateRef State = C.getState();
+ SymbolRef StreamSym = getStreamArg(Desc, Call).getAsSymbol();
+ if (!StreamSym)
+ return;
+
+ const CallExpr *CE = dyn_cast_or_null<CallExpr>(Call.getOriginExpr());
+ if (!CE)
+ return;
+
+ const StreamState *SS = State->get<StreamMap>(StreamSym);
+ // Ignore the call if the stream is not tracked.
+ if (!SS)
+ return;
+
+ assertStreamStateOpened(SS);
+
+ if ((SS->*IsOfError)()) {
+ // Function returns nonzero.
+ DefinedSVal RetVal = makeRetVal(C, CE);
+ State = State->BindExpr(CE, C.getLocationContext(), RetVal);
+ State = State->assume(RetVal, true);
+ assert(State && "Assumption on new value should not fail.");
+ } else {
+ // Return zero.
+ State = State->BindExpr(CE, C.getLocationContext(),
+ C.getSValBuilder().makeIntVal(0, false));
+ }
+ C.addTransition(State);
+}
+
void StreamChecker::preDefault(const FnDescription *Desc, const CallEvent &Call,
CheckerContext &C) const {
ProgramStateRef State = C.getState();
@@ -310,6 +443,17 @@ void StreamChecker::preDefault(const FnDescription *Desc, const CallEvent &Call,
C.addTransition(State);
}
+template <StreamState (*GetState)()>
+void StreamChecker::evalSetFeofFerror(const FnDescription *Desc,
+ const CallEvent &Call,
+ CheckerContext &C) const {
+ ProgramStateRef State = C.getState();
+ SymbolRef StreamSym = getStreamArg(Desc, Call).getAsSymbol();
+ assert(StreamSym && "Operation not permitted on non-symbolic stream value.");
+ State = State->set<StreamMap>(StreamSym, (*GetState)());
+ C.addTransition(State);
+}
+
ProgramStateRef
StreamChecker::ensureStreamNonNull(SVal StreamVal, CheckerContext &C,
ProgramStateRef State) const {
@@ -437,10 +581,20 @@ void StreamChecker::checkDeadSymbols(SymbolReaper &SymReaper,
}
}
-void ento::registerStreamChecker(CheckerManager &mgr) {
- mgr.registerChecker<StreamChecker>();
+void ento::registerStreamChecker(CheckerManager &Mgr) {
+ Mgr.registerChecker<StreamChecker>();
}
-bool ento::shouldRegisterStreamChecker(const CheckerManager &mgr) {
+bool ento::shouldRegisterStreamChecker(const CheckerManager &Mgr) {
return true;
}
+
+void ento::registerStreamTesterChecker(CheckerManager &Mgr) {
+ auto *Checker = Mgr.getChecker<StreamChecker>();
+ Checker->TestMode = true;
+}
+
+bool ento::shouldRegisterStreamTesterChecker(const CheckerManager &Mgr) {
+ return true;
+}
+
diff --git a/clang/test/Analysis/Inputs/system-header-simulator.h b/clang/test/Analysis/Inputs/system-header-simulator.h
index afbaeddd88a8..a98546c7056c 100644
--- a/clang/test/Analysis/Inputs/system-header-simulator.h
+++ b/clang/test/Analysis/Inputs/system-header-simulator.h
@@ -9,7 +9,16 @@
#define restrict /*restrict*/
#endif
+typedef __typeof(sizeof(int)) size_t;
+typedef long long __int64_t;
+typedef __int64_t __darwin_off_t;
+typedef __darwin_off_t fpos_t;
+
typedef struct _FILE FILE;
+#define SEEK_SET 0 /* Seek from beginning of file. */
+#define SEEK_CUR 1 /* Seek from current position. */
+#define SEEK_END 2 /* Seek from end of file. */
+
extern FILE *stdin;
extern FILE *stdout;
extern FILE *stderr;
@@ -24,11 +33,35 @@ int printf(const char *restrict format, ...);
int fprintf(FILE *restrict, const char *restrict, ...);
int getchar(void);
+void setbuf(FILE *restrict, char *restrict);
+int setvbuf(FILE *restrict, char *restrict, int, size_t);
+
+FILE *funopen(const void *,
+ int (*)(void *, char *, int),
+ int (*)(void *, const char *, int),
+ fpos_t (*)(void *, fpos_t, int),
+ int (*)(void *));
+
+FILE *fopen(const char *path, const char *mode);
+FILE *tmpfile(void);
+FILE *freopen(const char *pathname, const char *mode, FILE *stream);
+int fclose(FILE *fp);
+size_t fread(void *ptr, size_t size, size_t nmemb, FILE *stream);
+size_t fwrite(const void *ptr, size_t size, size_t nmemb, FILE *stream);
+int fputc(int ch, FILE *stream);
+int fseek(FILE *__stream, long int __off, int __whence);
+long int ftell(FILE *__stream);
+void rewind(FILE *__stream);
+int fgetpos(FILE *stream, fpos_t *pos);
+int fsetpos(FILE *stream, const fpos_t *pos);
+void clearerr(FILE *stream);
+int feof(FILE *stream);
+int ferror(FILE *stream);
+int fileno(FILE *stream);
+
// Note, on some platforms errno macro gets replaced with a function call.
extern int errno;
-typedef __typeof(sizeof(int)) size_t;
-
size_t strlen(const char *);
char *strcpy(char *restrict, const char *restrict);
@@ -39,21 +72,6 @@ typedef unsigned long __darwin_pthread_key_t;
typedef __darwin_pthread_key_t pthread_key_t;
int pthread_setspecific(pthread_key_t, const void *);
-typedef long long __int64_t;
-typedef __int64_t __darwin_off_t;
-typedef __darwin_off_t fpos_t;
-
-void setbuf(FILE * restrict, char * restrict);
-int setvbuf(FILE * restrict, char * restrict, int, size_t);
-
-FILE *fopen(const char * restrict, const char * restrict);
-int fclose(FILE *);
-FILE *funopen(const void *,
- int (*)(void *, char *, int),
- int (*)(void *, const char *, int),
- fpos_t (*)(void *, fpos_t, int),
- int (*)(void *));
-
int sqlite3_bind_text_my(int, const char*, int n, void(*)(void*));
typedef void (*freeCallback) (void*);
@@ -117,5 +135,6 @@ void _Exit(int status) __attribute__ ((__noreturn__));
#define __DARWIN_NULL 0
#define NULL __DARWIN_NULL
#endif
+#define EOF (-1)
-#define offsetof(t, d) __builtin_offsetof(t, d)
\ No newline at end of file
+#define offsetof(t, d) __builtin_offsetof(t, d)
diff --git a/clang/test/Analysis/stream-error.c b/clang/test/Analysis/stream-error.c
new file mode 100644
index 000000000000..e9fb8fdebfab
--- /dev/null
+++ b/clang/test/Analysis/stream-error.c
@@ -0,0 +1,54 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core -analyzer-checker=debug.StreamTester,debug.ExprInspection -analyzer-store region -verify %s
+
+#include "Inputs/system-header-simulator.h"
+
+void clang_analyzer_eval(int);
+void StreamTesterChecker_make_feof_stream(FILE *);
+void StreamTesterChecker_make_ferror_stream(FILE *);
+
+void error_fopen() {
+ FILE *F = fopen("file", "r");
+ if (!F)
+ return;
+ clang_analyzer_eval(feof(F)); // expected-warning {{FALSE}}
+ clang_analyzer_eval(ferror(F)); // expected-warning {{FALSE}}
+ fclose(F);
+}
+
+void error_freopen() {
+ FILE *F = fopen("file", "r");
+ if (!F)
+ return;
+ F = freopen(0, "w", F);
+ if (!F)
+ return;
+ clang_analyzer_eval(feof(F)); // expected-warning {{FALSE}}
+ clang_analyzer_eval(ferror(F)); // expected-warning {{FALSE}}
+ fclose(F);
+}
+
+void stream_error_feof() {
+ FILE *F = fopen("file", "r");
+ if (!F)
+ return;
+ StreamTesterChecker_make_feof_stream(F);
+ clang_analyzer_eval(feof(F)); // expected-warning {{TRUE}}
+ clang_analyzer_eval(ferror(F)); // expected-warning {{FALSE}}
+ clearerr(F);
+ clang_analyzer_eval(feof(F)); // expected-warning {{FALSE}}
+ clang_analyzer_eval(ferror(F)); // expected-warning {{FALSE}}
+ fclose(F);
+}
+
+void stream_error_ferror() {
+ FILE *F = fopen("file", "r");
+ if (!F)
+ return;
+ StreamTesterChecker_make_ferror_stream(F);
+ clang_analyzer_eval(feof(F)); // expected-warning {{FALSE}}
+ clang_analyzer_eval(ferror(F)); // expected-warning {{TRUE}}
+ clearerr(F);
+ clang_analyzer_eval(feof(F)); // expected-warning {{FALSE}}
+ clang_analyzer_eval(ferror(F)); // expected-warning {{FALSE}}
+ fclose(F);
+}
diff --git a/clang/test/Analysis/stream.c b/clang/test/Analysis/stream.c
index 6a22e9ee722e..554a234c20d5 100644
--- a/clang/test/Analysis/stream.c
+++ b/clang/test/Analysis/stream.c
@@ -1,26 +1,6 @@
// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.unix.Stream -analyzer-store region -verify %s
-typedef __typeof__(sizeof(int)) size_t;
-typedef __typeof__(sizeof(int)) fpos_t;
-typedef struct _IO_FILE FILE;
-#define SEEK_SET 0 /* Seek from beginning of file. */
-#define SEEK_CUR 1 /* Seek from current position. */
-#define SEEK_END 2 /* Seek from end of file. */
-extern FILE *fopen(const char *path, const char *mode);
-extern FILE *tmpfile(void);
-extern int fclose(FILE *fp);
-extern size_t fread(void *ptr, size_t size, size_t nmemb, FILE *stream);
-extern size_t fwrite(const void *ptr, size_t size, size_t nmemb, FILE *stream);
-extern int fseek (FILE *__stream, long int __off, int __whence);
-extern long int ftell (FILE *__stream);
-extern void rewind (FILE *__stream);
-extern int fgetpos(FILE *stream, fpos_t *pos);
-extern int fsetpos(FILE *stream, const fpos_t *pos);
-extern void clearerr(FILE *stream);
-extern int feof(FILE *stream);
-extern int ferror(FILE *stream);
-extern int fileno(FILE *stream);
-extern FILE *freopen(const char *pathname, const char *mode, FILE *stream);
+#include "Inputs/system-header-simulator.h"
void check_fread() {
FILE *fp = tmpfile();
More information about the cfe-commits
mailing list