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 @@ -31,6 +31,9 @@ struct StreamState { /// The last file operation called in the stream. const FnDescription *LastOperation; + /// Was a opening function encountered for this stream? + /// (False if the stream was first encountered in a non-opening function.) + bool OpenEncountered; /// State of a stream symbol. /// FIXME: We need maybe an "escaped" state later. @@ -82,17 +85,23 @@ ErrorState == X.ErrorState; } - static StreamState getOpened(const FnDescription *L) { - return StreamState{L, Opened}; + static StreamState getOpened(const FnDescription *L, + const StreamState *PrevState, + ErrorKindTy NewError) { + if (PrevState) + return StreamState{L, PrevState->OpenEncountered, Opened, NewError}; + else + return StreamState{L, false, Opened, NewError}; } - static StreamState getOpened(const FnDescription *L, ErrorKindTy E) { - return StreamState{L, Opened, E}; + static StreamState getOpened(const FnDescription *L, + bool OpenEncountered = true) { + return StreamState{L, OpenEncountered, Opened, NoError}; } static StreamState getClosed(const FnDescription *L) { - return StreamState{L, Closed}; + return StreamState{L, false, Closed}; } static StreamState getOpenFailed(const FnDescription *L) { - return StreamState{L, OpenFailed}; + return StreamState{L, false, OpenFailed}; } /// Return if the specified error kind is possible on the stream in the @@ -106,6 +115,7 @@ void Profile(llvm::FoldingSetNodeID &ID) const { ID.AddPointer(LastOperation); + ID.AddBoolean(OpenEncountered); ID.AddInteger(State); ID.AddInteger(ErrorState); } @@ -268,6 +278,9 @@ void evalSetFeofFerror(const FnDescription *Desc, const CallEvent &Call, CheckerContext &C) const; + void evalDebugDumpState(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 @@ -310,7 +323,7 @@ REGISTER_MAP_WITH_PROGRAMSTATE(StreamMap, SymbolRef, StreamState) inline void assertStreamStateOpened(const StreamState *SS) { - assert(SS->isOpened() && + assert((!SS || SS->isOpened()) && "Previous create of error node for non-opened stream failed?"); } @@ -394,6 +407,10 @@ if (!StreamSym) return; + bool OpenEncountered = false; + if (const StreamState *SS = State->get(StreamSym)) + OpenEncountered = SS->OpenEncountered; + // Generate state for non-failed case. // Return value is the passed stream pointer. // According to the documentations, the stream is closed first @@ -405,8 +422,8 @@ ProgramStateRef StateRetNull = State->BindExpr(CE, C.getLocationContext(), C.getSValBuilder().makeNull()); - StateRetNotNull = - StateRetNotNull->set(StreamSym, StreamState::getOpened(Desc)); + StateRetNotNull = StateRetNotNull->set( + StreamSym, StreamState::getOpened(Desc, OpenEncountered)); StateRetNull = StateRetNull->set(StreamSym, StreamState::getOpenFailed(Desc)); @@ -422,8 +439,6 @@ return; const StreamState *SS = State->get(Sym); - if (!SS) - return; assertStreamStateOpened(SS); @@ -463,13 +478,12 @@ if (!CE) return; - // Ignore the call if the stream is not tracked. - if (!State->get(StreamSym)) - return; + const StreamState *SS = State->get(StreamSym); - DefinedSVal RetVal = makeRetVal(C, CE); + assertStreamStateOpened(SS); // Make expression result. + DefinedSVal RetVal = makeRetVal(C, CE); State = State->BindExpr(CE, C.getLocationContext(), RetVal); // Bifurcate the state into failed and non-failed. @@ -479,11 +493,11 @@ C.getConstraintManager().assumeDual(State, RetVal); // Reset the state to opened with no error. - StateNotFailed = - StateNotFailed->set(StreamSym, StreamState::getOpened(Desc)); + StateNotFailed = StateNotFailed->set( + StreamSym, StreamState::getOpened(Desc, SS, StreamState::NoError)); // We get error. StateFailed = StateFailed->set( - StreamSym, StreamState::getOpened(Desc, StreamState::Unknown)); + StreamSym, StreamState::getOpened(Desc, SS, StreamState::Unknown)); C.addTransition(StateNotFailed); C.addTransition(StateFailed); @@ -498,12 +512,12 @@ return; const StreamState *SS = State->get(StreamSym); - if (!SS) - return; assertStreamStateOpened(SS); - State = State->set(StreamSym, StreamState::getOpened(Desc)); + State = State->set( + StreamSym, StreamState::getOpened(Desc, SS, StreamState::NoError)); + C.addTransition(State); } @@ -520,18 +534,23 @@ if (!CE) return; - auto AddTransition = [&C, Desc, StreamSym](ProgramStateRef State, - StreamState::ErrorKindTy SSError) { - StreamState SSNew = StreamState::getOpened(Desc, SSError); + const StreamState *SS = State->get(StreamSym); + + assertStreamStateOpened(SS); + + auto AddTransition = [&C, Desc, StreamSym, + SS](ProgramStateRef State, + StreamState::ErrorKindTy NewError) { + StreamState SSNew = StreamState::getOpened(Desc, SS, NewError); State = State->set(StreamSym, SSNew); C.addTransition(State); }; - const StreamState *SS = State->get(StreamSym); - if (!SS) + if (!SS) { + AddTransition(bindInt(0, State, C, CE), StreamState::Unknown); + AddTransition(bindAndAssumeTrue(State, C, CE), ErrorKind); return; - - assertStreamStateOpened(SS); + } if (!SS->isUnknown()) { // The stream is in exactly known state (error or not). @@ -582,8 +601,8 @@ assert(StreamSym && "Operation not permitted on non-symbolic stream value."); const StreamState *SS = State->get(StreamSym); assert(SS && "Stream should be tracked by the checker."); - State = State->set(StreamSym, - StreamState::getOpened(SS->LastOperation, EK)); + State = State->set( + StreamSym, StreamState::getOpened(SS->LastOperation, SS, EK)); C.addTransition(State); } @@ -698,7 +717,7 @@ for (const auto &I : Map) { SymbolRef Sym = I.first; const StreamState &SS = I.second; - if (!SymReaper.isDead(Sym) || !SS.isOpened()) + if (!SymReaper.isDead(Sym) || !SS.isOpened() || !SS.OpenEncountered) continue; ExplodedNode *N = C.generateErrorNode(); diff --git a/clang/test/Analysis/stream-error.c b/clang/test/Analysis/stream-error.c --- a/clang/test/Analysis/stream-error.c +++ b/clang/test/Analysis/stream-error.c @@ -3,6 +3,7 @@ #include "Inputs/system-header-simulator.h" void clang_analyzer_eval(int); +void clang_analyzer_warnIfReached(); void StreamTesterChecker_make_feof_stream(FILE *); void StreamTesterChecker_make_ferror_stream(FILE *); @@ -15,16 +16,12 @@ fclose(F); } -void error_freopen() { - FILE *F = fopen("file", "r"); - if (!F) - return; +void error_freopen(FILE *F) { 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() { @@ -40,6 +37,15 @@ fclose(F); } +void stream_error_feof_noopen(FILE *F) { + if (feof(F)) + clang_analyzer_eval(ferror(F)); // expected-warning {{FALSE}} + else if (ferror(F)) + clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}} + else + clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}} +} + void stream_error_ferror() { FILE *F = fopen("file", "r"); if (!F) @@ -53,10 +59,16 @@ fclose(F); } -void error_fseek() { - FILE *F = fopen("file", "r"); - if (!F) - return; +void stream_error_ferror_noopen(FILE *F) { + if (ferror(F)) + clang_analyzer_eval(feof(F)); // expected-warning {{FALSE}} + else if (feof(F)) + clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}} + else + clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}} +} + +void error_fseek(FILE *F) { int rc = fseek(F, 0, SEEK_SET); if (rc) { int IsFEof = feof(F), IsFError = ferror(F); @@ -81,5 +93,4 @@ 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 @@ -8,30 +8,50 @@ fclose(fp); } +void check_feread_noopen(FILE *fp) { + fread(0, 0, 0, fp); +} + void check_fwrite() { FILE *fp = tmpfile(); fwrite(0, 0, 0, fp); // expected-warning {{Stream pointer might be NULL}} fclose(fp); } +void check_fwrite_noopen(FILE *fp) { + fwrite(0, 0, 0, fp); +} + void check_fseek() { FILE *fp = tmpfile(); fseek(fp, 0, 0); // expected-warning {{Stream pointer might be NULL}} fclose(fp); } +void check_fseek_noopen(FILE *fp) { + fseek(fp, 0, 0); +} + void check_ftell() { FILE *fp = tmpfile(); ftell(fp); // expected-warning {{Stream pointer might be NULL}} fclose(fp); } +void check_ftell_noopen(FILE *fp) { + ftell(fp); +} + void check_rewind() { FILE *fp = tmpfile(); rewind(fp); // expected-warning {{Stream pointer might be NULL}} fclose(fp); } +void check_rewind_noopen(FILE *fp) { + rewind(fp); +} + void check_fgetpos() { FILE *fp = tmpfile(); fpos_t pos; @@ -39,6 +59,11 @@ fclose(fp); } +void check_fgetpos_noopen(FILE *fp) { + fpos_t pos; + fgetpos(fp, &pos); +} + void check_fsetpos() { FILE *fp = tmpfile(); fpos_t pos; @@ -46,30 +71,51 @@ fclose(fp); } +void check_fsetpos_noopen(FILE *fp) { + fpos_t pos; + fsetpos(fp, &pos); +} + void check_clearerr() { FILE *fp = tmpfile(); clearerr(fp); // expected-warning {{Stream pointer might be NULL}} fclose(fp); } +void check_clearerr_noopen(FILE *fp) { + clearerr(fp); +} + void check_feof() { FILE *fp = tmpfile(); feof(fp); // expected-warning {{Stream pointer might be NULL}} fclose(fp); } +void check_feof_noopen(FILE *fp) { + feof(fp); +} + void check_ferror() { FILE *fp = tmpfile(); ferror(fp); // expected-warning {{Stream pointer might be NULL}} fclose(fp); } +void check_ferror_noopen(FILE *fp) { + ferror(fp); +} + void check_fileno() { FILE *fp = tmpfile(); fileno(fp); // expected-warning {{Stream pointer might be NULL}} fclose(fp); } +void check_fileno_noopen(FILE *fp) { + fileno(fp); +} + void f_open(void) { FILE *p = fopen("foo", "r"); char buf[1024];