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 @@ -25,8 +25,13 @@ namespace { +struct FnDescription; + /// Full state information about a stream pointer. struct StreamState { + /// The last file operation called in the stream. + const FnDescription *LastOperation; + /// State of a stream symbol. /// FIXME: We need maybe an "escaped" state later. enum KindTy { @@ -45,6 +50,9 @@ FEof, /// Other generic (non-EOF) error (`ferror` is true). FError, + /// Unknown error flag is set (or none), the meaning depends on the last + /// operation. + Unknown } ErrorState = NoError; bool isOpened() const { return State == Opened; } @@ -63,28 +71,47 @@ assert(State == Opened && "Error undefined for closed stream."); return ErrorState == FError; } + bool isUnknown() const { + assert(State == Opened && "Error undefined for closed stream."); + return ErrorState == Unknown; + } 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 getOpened(const FnDescription *L, ErrorKindTy E) { + return StreamState{L, Opened, E}; + } + static StreamState getClosed(const FnDescription *L) { + return StreamState{L, Closed}; } + static StreamState getOpenFailed(const FnDescription *L) { + return StreamState{L, OpenFailed}; + } + + /// Return if the specified error kind is possible on the stream in the + /// current state. + /// This depends on the stored `LastOperation` value. + /// If the error is not possible returns empty value. + /// If the error is possible returns the remaining possible error type + /// (after taking out `ErrorKind`). If a single error is possible it will + /// return that value, otherwise unknown error. + Optional getRemainingPossibleError(ErrorKindTy ErrorKind) const; void Profile(llvm::FoldingSetNodeID &ID) const { + ID.AddPointer(LastOperation); ID.AddInteger(State); ID.AddInteger(ErrorState); } }; class StreamChecker; -struct FnDescription; using FnCheck = std::function; @@ -95,8 +122,28 @@ FnCheck PreFn; FnCheck EvalFn; ArgNoTy StreamArgNo; + // What errors are possible after this operation. + // Used only if this operation resulted in Unknown state + // (otherwise there is a known single error). + // Must contain 2 or 3 elements, or zero. + llvm::SmallVector PossibleErrors = {}; }; +Optional +StreamState::getRemainingPossibleError(ErrorKindTy ErrorKind) const { + assert(ErrorState == Unknown && + "Function to be used only if error is unknown."); + llvm::SmallVector NewPossibleErrors; + for (StreamState::ErrorKindTy E : LastOperation->PossibleErrors) + if (E != ErrorKind) + NewPossibleErrors.push_back(E); + if (NewPossibleErrors.size() == LastOperation->PossibleErrors.size()) + return {}; + if (NewPossibleErrors.size() == 1) + return NewPossibleErrors.front(); + return Unknown; +} + /// Get the value of the stream argument out of the passed call event. /// The call should contain a function that is described by Desc. SVal getStreamArg(const FnDescription *Desc, const CallEvent &Call) { @@ -115,6 +162,22 @@ .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, @@ -130,38 +193,49 @@ 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 Unknown 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 Unknown 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, {}}}, }; CallDescriptionMap FnTestDescriptions = { {{"StreamTesterChecker_make_feof_stream", 1}, - {nullptr, - &StreamChecker::evalSetFeofFerror<&StreamState::getOpenedWithFEof>, 0}}, + {nullptr, &StreamChecker::evalSetFeofFerror, 0}}, {{"StreamTesterChecker_make_ferror_stream", 1}, - {nullptr, - &StreamChecker::evalSetFeofFerror<&StreamState::getOpenedWithFError>, - 0}}, + {nullptr, &StreamChecker::evalSetFeofFerror, 0}}, }; void evalFopen(const FnDescription *Desc, const CallEvent &Call, @@ -177,6 +251,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; @@ -184,11 +260,11 @@ void evalClearerr(const FnDescription *Desc, const CallEvent &Call, CheckerContext &C) const; - template + template void evalFeofFerror(const FnDescription *Desc, const CallEvent &Call, CheckerContext &C) const; - template + template void evalSetFeofFerror(const FnDescription *Desc, const CallEvent &Call, CheckerContext &C) const; @@ -197,7 +273,7 @@ /// Otherwise the return value is a new state where the stream is constrained /// to be non-null. ProgramStateRef ensureStreamNonNull(SVal StreamVal, CheckerContext &C, - ProgramStateRef State) const; + ProgramStateRef State) const; /// Check that the stream is the opened state. /// If the stream is known to be not opened an error is generated @@ -210,7 +286,7 @@ /// Otherwise returns the state. /// (State is not changed here because the "whence" value is already known.) ProgramStateRef ensureFseekWhenceCorrect(SVal WhenceVal, CheckerContext &C, - ProgramStateRef State) const; + ProgramStateRef State) const; /// Find the description data of the function called by a call event. /// Returns nullptr if no function is recognized. @@ -226,7 +302,7 @@ } return FnDescriptions.lookup(Call); - } + } }; } // end anonymous namespace @@ -278,8 +354,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); @@ -328,9 +406,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); @@ -352,7 +430,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); } @@ -374,6 +452,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::getOpened(Desc, StreamState::Unknown)); + + C.addTransition(StateNotFailed); + C.addTransition(StateFailed); +} + void StreamChecker::evalClearerr(const FnDescription *Desc, const CallEvent &Call, CheckerContext &C) const { @@ -388,14 +503,11 @@ assertStreamStateOpened(SS); - 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 { @@ -408,25 +520,43 @@ if (!CE) return; + auto AddTransition = [&C, Desc, StreamSym](ProgramStateRef State, + StreamState::ErrorKindTy SSError) { + StreamState SSNew = StreamState::getOpened(Desc, SSError); + State = State->set(StreamSym, SSNew); + C.addTransition(State); + }; + 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."); + if (!SS->isUnknown()) { + // The stream is in exactly known state (error or not). + // Check if it is in error of kind `ErrorKind`. + if (SS->ErrorState == ErrorKind) + State = bindAndAssumeTrue(State, C, CE); + else + State = bindInt(0, State, C, CE); + // Error state is not changed in the new state. + AddTransition(State, SS->ErrorState); } else { - // Return zero. - State = State->BindExpr(CE, C.getLocationContext(), - C.getSValBuilder().makeIntVal(0, false)); + // Stream is in unknown state, check if error `ErrorKind` is possible. + Optional NewError = + SS->getRemainingPossibleError(ErrorKind); + if (!NewError) { + // This kind of error is not possible, function returns zero. + // Error state remains unknown. + AddTransition(bindInt(0, State, C, CE), StreamState::Unknown); + } else { + // Add state with true returned. + AddTransition(bindAndAssumeTrue(State, C, CE), ErrorKind); + // Add state with false returned and the new remaining error type. + AddTransition(bindInt(0, State, C, CE), *NewError); + } } - C.addTransition(State); } void StreamChecker::preDefault(const FnDescription *Desc, const CallEvent &Call, @@ -443,14 +573,17 @@ C.addTransition(State); } -template +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)()); + const StreamState *SS = State->get(StreamSym); + assert(SS && "Stream should be tracked by the checker."); + State = State->set(StreamSym, + StreamState::getOpened(SS->LastOperation, EK)); C.addTransition(State); } @@ -597,4 +730,3 @@ bool ento::shouldRegisterStreamTesterChecker(const CheckerManager &Mgr) { return true; } - 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 @@ -52,3 +52,34 @@ clang_analyzer_eval(ferror(F)); // expected-warning {{FALSE}} fclose(F); } + +void error_fseek() { + FILE *F = fopen("file", "r"); + if (!F) + return; + int rc = fseek(F, 0, SEEK_SET); + if (rc) { + int IsFEof = feof(F), IsFError = ferror(F); + // Get feof or ferror or no error. + clang_analyzer_eval(IsFEof || IsFError); + // expected-warning@-1 {{FALSE}} + // expected-warning@-2 {{TRUE}} + clang_analyzer_eval(IsFEof && IsFError); // expected-warning {{FALSE}} + // Error flags should not change. + if (IsFEof) + clang_analyzer_eval(feof(F)); // expected-warning {{TRUE}} + else + clang_analyzer_eval(feof(F)); // expected-warning {{FALSE}} + if (IsFError) + 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); +}