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 @@ -30,8 +30,12 @@ namespace { +struct FnDescription; + /// Full state information about a stream pointer. struct StreamState { + const FnDescription *LastOperation; + /// State of a stream symbol. /// FIXME: We need maybe an "escaped" state later. enum KindTy { @@ -49,6 +53,8 @@ FEof, /// Other generic (non-EOF) error (`ferror` is true). FError, + /// Unknown error, the meaning depends on the last operation. + UnknownError } ErrorState = NoError; bool isOpened() const { return State == Opened; } @@ -67,21 +73,35 @@ assert(State == Opened && "Error undefined for closed stream."); return ErrorState == FError; } + bool isUnknownError() const { + assert(State == Opened && "Error undefined for closed stream."); + return ErrorState == UnknownError; + } bool operator==(const StreamState &X) const { // In not opened state error should always NoError. - return State == X.State && ErrorState == X.ErrorState; + return LastOperation == X.LastOperation && 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 getOpenedWithFEof() { return StreamState{Opened, FEof}; } - static StreamState getOpenedWithFError() { - return StreamState{Opened, FError}; + static StreamState getOpened(const FnDescription *L) { + return StreamState{L, Opened}; + } + static StreamState getClosed(const FnDescription *L) { + return StreamState{L, Closed}; + } + static StreamState getOpenFailed(const FnDescription *L) { + return StreamState{L, OpenFailed}; + } + static StreamState getOpened(const FnDescription *L, ErrorKindTy E) { + return StreamState{L, Opened, E}; + } + static StreamState getOpenedWithUnknownError(const FnDescription *L) { + return StreamState{L, Opened, UnknownError}; } void Profile(llvm::FoldingSetNodeID &ID) const { + ID.AddPointer(LastOperation); ID.AddInteger(State); ID.AddInteger(ErrorState); } @@ -99,6 +119,11 @@ FnCheck PreFn; FnCheck EvalFn; ArgNoTy StreamArgNo; + // What errors are possible after this operation. + // Used only if this operation resulted in UnknownError + // (otherwise there is a known single error). + // Must contain 2 or 3 elements, or zero. + llvm::SmallVector PossibleErrors = {}; }; /// Get the value of the stream argument out of the passed call event. @@ -119,6 +144,20 @@ .castAs(); } +ProgramStateRef bindAndAssumeTrue(ProgramStateRef State, CheckerContext &C, const CallExpr *CE) { + 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."); + return State; +} + +ProgramStateRef bindInt(uint64_t Value, ProgramStateRef State, CheckerContext &C, const CallExpr *CE) { + State = State->BindExpr(CE, C.getLocationContext(), + C.getSValBuilder().makeIntVal(Value, false)); + return State; +} + class StreamChecker : public Checker { mutable std::unique_ptr BT_nullfp, BT_illegalwhence, @@ -131,28 +170,42 @@ private: CallDescriptionMap FnDescriptions = { - {{"fopen"}, {nullptr, &StreamChecker::evalFopen, ArgNone}}, + {{"fopen"}, {nullptr, &StreamChecker::evalFopen, ArgNone, {}}}, {{"freopen", 3}, - {&StreamChecker::preFreopen, &StreamChecker::evalFreopen, 2}}, - {{"tmpfile"}, {nullptr, &StreamChecker::evalFopen, ArgNone}}, + {&StreamChecker::preFreopen, &StreamChecker::evalFreopen, 2, {}}}, + {{"tmpfile"}, {nullptr, &StreamChecker::evalFopen, ArgNone, {}}}, {{"fclose", 1}, - {&StreamChecker::preDefault, &StreamChecker::evalFclose, 0}}, - {{"fread", 4}, {&StreamChecker::preDefault, nullptr, 3}}, - {{"fwrite", 4}, {&StreamChecker::preDefault, nullptr, 3}}, - {{"fseek", 3}, {&StreamChecker::preFseek, nullptr, 0}}, - {{"ftell", 1}, {&StreamChecker::preDefault, nullptr, 0}}, - {{"rewind", 1}, {&StreamChecker::preDefault, nullptr, 0}}, - {{"fgetpos", 2}, {&StreamChecker::preDefault, nullptr, 0}}, - {{"fsetpos", 2}, {&StreamChecker::preDefault, nullptr, 0}}, + {&StreamChecker::preDefault, &StreamChecker::evalFclose, 0, {}}}, + {{"fread", 4}, {&StreamChecker::preDefault, nullptr, 3, {}}}, + {{"fwrite", 4}, {&StreamChecker::preDefault, nullptr, 3, {}}}, + {{"fseek", 3}, + {&StreamChecker::preFseek, + &StreamChecker::evalFseek, + 0, + {StreamState::FEof, StreamState::FError, StreamState::NoError}}}, + {{"ftell", 1}, {&StreamChecker::preDefault, nullptr, 0, {}}}, + {{"rewind", 1}, {&StreamChecker::preDefault, nullptr, 0, {}}}, + {{"fgetpos", 2}, {&StreamChecker::preDefault, nullptr, 0, {}}}, + {{"fsetpos", 2}, {&StreamChecker::preDefault, nullptr, 0, {}}}, {{"clearerr", 1}, - {&StreamChecker::preDefault, &StreamChecker::evalClearerr, 0}}, + {&StreamChecker::preDefault, &StreamChecker::evalClearerr, 0, {}}}, + // Note: feof can result in UnknownError if at the call there is a + // PossibleErrors with all 3 error states (including NoError). + // Then if feof is false the remaining error could be FError or NoError. {{"feof", 1}, {&StreamChecker::preDefault, - &StreamChecker::evalFeofFerror<&StreamState::isFEof>, 0}}, + &StreamChecker::evalFeofFerror, + 0, + {StreamState::FError, StreamState::NoError}}}, + // Note: ferror can result in UnknownError if at the call there is a + // PossibleErrors with all 3 error states (including NoError). + // Then if ferror is false the remaining error could be FEof or NoError. {{"ferror", 1}, {&StreamChecker::preDefault, - &StreamChecker::evalFeofFerror<&StreamState::isFError>, 0}}, - {{"fileno", 1}, {&StreamChecker::preDefault, nullptr, 0}}, + &StreamChecker::evalFeofFerror, + 0, + {StreamState::FEof, StreamState::NoError}}}, + {{"fileno", 1}, {&StreamChecker::preDefault, nullptr, 0, {}}}, }; void evalFopen(const FnDescription *Desc, const CallEvent &Call, @@ -168,6 +221,8 @@ void preFseek(const FnDescription *Desc, const CallEvent &Call, CheckerContext &C) const; + void evalFseek(const FnDescription *Desc, const CallEvent &Call, + CheckerContext &C) const; void preDefault(const FnDescription *Desc, const CallEvent &Call, CheckerContext &C) const; @@ -175,7 +230,7 @@ void evalClearerr(const FnDescription *Desc, const CallEvent &Call, CheckerContext &C) const; - template + template void evalFeofFerror(const FnDescription *Desc, const CallEvent &Call, CheckerContext &C) const; @@ -258,8 +313,10 @@ std::tie(StateNotNull, StateNull) = C.getConstraintManager().assumeDual(State, RetVal); - StateNotNull = StateNotNull->set(RetSym, StreamState::getOpened()); - StateNull = StateNull->set(RetSym, StreamState::getOpenFailed()); + StateNotNull = + StateNotNull->set(RetSym, StreamState::getOpened(Desc)); + StateNull = + StateNull->set(RetSym, StreamState::getOpenFailed(Desc)); C.addTransition(StateNotNull); C.addTransition(StateNull); @@ -308,9 +365,9 @@ C.getSValBuilder().makeNull()); StateRetNotNull = - StateRetNotNull->set(StreamSym, StreamState::getOpened()); + StateRetNotNull->set(StreamSym, StreamState::getOpened(Desc)); StateRetNull = - StateRetNull->set(StreamSym, StreamState::getOpenFailed()); + StateRetNull->set(StreamSym, StreamState::getOpenFailed(Desc)); C.addTransition(StateRetNotNull); C.addTransition(StateRetNull); @@ -332,7 +389,7 @@ // Close the File Descriptor. // Regardless if the close fails or not, stream becomes "closed" // and can not be used any more. - State = State->set(Sym, StreamState::getClosed()); + State = State->set(Sym, StreamState::getClosed(Desc)); C.addTransition(State); } @@ -354,6 +411,43 @@ C.addTransition(State); } +void StreamChecker::evalFseek(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; + + // Ignore the call if the stream is not tracked. + if (!State->get(StreamSym)) + return; + + DefinedSVal RetVal = makeRetVal(C, CE); + + // Make expression result. + State = State->BindExpr(CE, C.getLocationContext(), RetVal); + + // Bifurcate the state into failed and non-failed. + // Return zero on success, nonzero on error. + ProgramStateRef StateNotFailed, StateFailed; + std::tie(StateFailed, StateNotFailed) = + C.getConstraintManager().assumeDual(State, RetVal); + + // Reset the state to opened with no error. + StateNotFailed = + StateNotFailed->set(StreamSym, StreamState::getOpened(Desc)); + // We get error. + StateFailed = StateFailed->set( + StreamSym, StreamState::getOpenedWithUnknownError(Desc)); + + C.addTransition(StateNotFailed); + C.addTransition(StateFailed); +} + void StreamChecker::evalClearerr(const FnDescription *Desc, const CallEvent &Call, CheckerContext &C) const { @@ -371,11 +465,11 @@ if (SS->isNoError()) return; - State = State->set(StreamSym, StreamState::getOpened()); + State = State->set(StreamSym, StreamState::getOpened(Desc)); C.addTransition(State); } -template +template void StreamChecker::evalFeofFerror(const FnDescription *Desc, const CallEvent &Call, CheckerContext &C) const { @@ -395,18 +489,51 @@ ASSERT_STREAMSTATE_OPENED; - 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."); + if (SS->isUnknownError()) { + llvm::SmallVector NewPossibleErrors; + for (StreamState::ErrorKindTy E : SS->LastOperation->PossibleErrors) + if (E != ErrorKind) + NewPossibleErrors.push_back(E); + if (NewPossibleErrors.size() == SS->LastOperation->PossibleErrors.size()) { + // This kind of error is not possible, function returns zero. + // Error state remains unknown (we know only that it is not ErrorKind). + State = bindInt(0, State, C, CE); + State = State->set( + StreamSym, StreamState::getOpenedWithUnknownError(Desc)); + C.addTransition(State); + } else { + // The previously unknown error could be ErrorKind. + ProgramStateRef StateTrue = bindAndAssumeTrue(State, C, CE); + ProgramStateRef StateFalse = bindInt(0, State, C, CE); + // If return true, set error state to ErrorKind. + StateTrue = StateTrue->set( + StreamSym, StreamState::getOpened(Desc, ErrorKind)); + // Return false + if (NewPossibleErrors.size() == 1) + // One other error is possible, new error kind is fixed to that. + StateFalse = StateFalse->set( + StreamSym, StreamState::getOpened(Desc, NewPossibleErrors.front())); + else + // Multiple other "errors" possible, new error kind is unknown. + // In this case we known later from the last operation's (will be feof + // or ferror) PossibleErrors list what errors are still possible. This + // should be the same as the content of PossibleErrors at this point. + StateFalse = StateFalse->set( + StreamSym, StreamState::getOpened(Desc, StreamState::UnknownError)); + C.addTransition(StateTrue); + C.addTransition(StateFalse); + } } else { - // Return zero. - State = State->BindExpr(CE, C.getLocationContext(), - C.getSValBuilder().makeIntVal(0, false)); + // We know exactly what the error is. + // Make the return value accordingly to the error. + if (SS->ErrorState == ErrorKind) + State = bindAndAssumeTrue(State, C, CE); + else + State = bindInt(0, State, C, CE); + State = State->set(StreamSym, + StreamState::getOpened(Desc, SS->ErrorState)); + C.addTransition(State); } - C.addTransition(State); } void StreamChecker::preDefault(const FnDescription *Desc, const CallEvent &Call, 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 @@ -38,3 +38,48 @@ } fclose(F); } + +void error_fseek() { + FILE *F = fopen("file", "r"); + if (!F) + return; + int rc = fseek(F, 0, SEEK_SET); + if (rc) { + int Eof = feof(F), Error = ferror(F); + // Get feof or ferror or no error. + clang_analyzer_eval(Eof || Error); // expected-warning {{FALSE}} \ + // expected-warning {{TRUE}} + clang_analyzer_eval(Eof && Error); // expected-warning {{FALSE}} + // Error flags should not change. + if (Eof) + clang_analyzer_eval(feof(F)); // expected-warning {{TRUE}} + else + clang_analyzer_eval(feof(F)); // expected-warning {{FALSE}} + if (Error) + clang_analyzer_eval(ferror(F)); // expected-warning {{TRUE}} + else + clang_analyzer_eval(ferror(F)); // expected-warning {{FALSE}} + } else { + clang_analyzer_eval(feof(F)); // expected-warning {{FALSE}} + clang_analyzer_eval(ferror(F)); // expected-warning {{FALSE}} + // Error flags should not change. + clang_analyzer_eval(feof(F)); // expected-warning {{FALSE}} + clang_analyzer_eval(ferror(F)); // expected-warning {{FALSE}} + } + fclose(F); +} + +void error_fseek_clearerr() { + FILE *F = fopen("file", "r"); + if (!F) + return; + int rc = fseek(F, 0, SEEK_SET); + if (rc && feof(F)) { + clearerr(F); + clang_analyzer_eval(feof(F)); // expected-warning {{FALSE}} + } else if (rc && ferror(F)) { + clearerr(F); + clang_analyzer_eval(ferror(F)); // expected-warning {{FALSE}} + } + fclose(F); +}