diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td --- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td +++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td @@ -1400,6 +1400,11 @@ HelpText<"Mark tainted symbols as such.">, Documentation; +def StreamTesterChecker : Checker<"StreamTester">, + HelpText<"Add test functions to StreamChecker for test and debugging purposes.">, + Dependencies<[StreamChecker]>, + Documentation; + def ExprInspectionChecker : Checker<"ExprInspection">, HelpText<"Check the analyzer's understanding of expressions">, Documentation; diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp --- 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 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 @@ 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(); +} + class StreamChecker : public Checker { mutable std::unique_ptr BT_nullfp, BT_illegalwhence, @@ -81,6 +125,9 @@ 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 FnDescriptions = { {{"fopen"}, {nullptr, &StreamChecker::evalFopen, ArgNone}}, @@ -96,12 +143,27 @@ {{"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 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 @@ void preDefault(const FnDescription *Desc, const CallEvent &Call, CheckerContext &C) const; + void evalClearerr(const FnDescription *Desc, const CallEvent &Call, + CheckerContext &C) const; + + template + void evalFeofFerror(const FnDescription *Desc, const CallEvent &Call, + CheckerContext &C) const; + + template + 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 @@ 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 @@ 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 @@ 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(Call.getOriginExpr()); + const CallExpr *CE = dyn_cast_or_null(Call.getOriginExpr()); if (!CE) return; - DefinedSVal RetVal = SVB.conjureSymbolVal(nullptr, CE, LCtx, C.blockCount()) - .castAs(); + DefinedSVal RetVal = makeRetVal(C, CE); SymbolRef RetSym = RetVal.getAsSymbol(); assert(RetSym && "RetVal must be a symbol here."); @@ -271,6 +347,8 @@ 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 @@ 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(StreamSym); + if (!SS) + return; + + assertStreamStateOpened(SS); + + if (SS->isNoError()) + return; + + State = State->set(StreamSym, StreamState::getOpened()); + C.addTransition(State); +} + +template +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(Call.getOriginExpr()); + if (!CE) + return; + + const StreamState *SS = State->get(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 @@ C.addTransition(State); } +template +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(StreamSym, (*GetState)()); + C.addTransition(State); +} + ProgramStateRef StreamChecker::ensureStreamNonNull(SVal StreamVal, CheckerContext &C, ProgramStateRef State) const { @@ -437,10 +581,20 @@ } } -void ento::registerStreamChecker(CheckerManager &mgr) { - mgr.registerChecker(); +void ento::registerStreamChecker(CheckerManager &Mgr) { + Mgr.registerChecker(); } -bool ento::shouldRegisterStreamChecker(const CheckerManager &mgr) { +bool ento::shouldRegisterStreamChecker(const CheckerManager &Mgr) { return true; } + +void ento::registerStreamTesterChecker(CheckerManager &Mgr) { + auto *Checker = Mgr.getChecker(); + 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 --- 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 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 __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 @@ #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 --- /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 --- 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();