Author: Balázs Kéri Date: 2020-04-14T12:35:28+02:00 New Revision: f2b5e60dfd09f99d036197c078179c774f8b4582
URL: https://github.com/llvm/llvm-project/commit/f2b5e60dfd09f99d036197c078179c774f8b4582 DIFF: https://github.com/llvm/llvm-project/commit/f2b5e60dfd09f99d036197c078179c774f8b4582.diff LOG: [Analyzer][StreamChecker] Added evaluation of fseek. Summary: Function `fseek` is now evaluated with setting error return value and error flags. Reviewers: Szelethus, NoQ, xazax.hun, rnkovacs, dcoughlin, baloghadamsoftware, martong Reviewed By: Szelethus Subscribers: ASDenysPetrov, 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/D75851 Added: Modified: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp clang/test/Analysis/stream-error.c Removed: ################################################################################ diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp index b38a645432f4..76dd62d30ddf 100644 --- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp @@ -25,8 +25,13 @@ using namespace ento; 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 @@ struct StreamState { 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 @@ struct StreamState { 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<ErrorKindTy> 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<void(const StreamChecker *, const FnDescription *, const CallEvent &, CheckerContext &)>; @@ -95,8 +122,28 @@ struct FnDescription { 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<StreamState::ErrorKindTy, 3> PossibleErrors = {}; }; +Optional<StreamState::ErrorKindTy> +StreamState::getRemainingPossibleError(ErrorKindTy ErrorKind) const { + assert(ErrorState == Unknown && + "Function to be used only if error is unknown."); + llvm::SmallVector<StreamState::ErrorKindTy, 3> 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 @@ DefinedSVal makeRetVal(CheckerContext &C, const CallExpr *CE) { .castAs<DefinedSVal>(); } +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<check::PreCall, eval::Call, check::DeadSymbols> { mutable std::unique_ptr<BuiltinBug> BT_nullfp, BT_illegalwhence, @@ -130,38 +193,49 @@ class StreamChecker private: CallDescriptionMap<FnDescription> 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<StreamState::FEof>, + 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<StreamState::FError>, + 0, + {StreamState::FEof, StreamState::NoError}}}, + {{"fileno", 1}, {&StreamChecker::preDefault, nullptr, 0, {}}}, }; CallDescriptionMap<FnDescription> FnTestDescriptions = { {{"StreamTesterChecker_make_feof_stream", 1}, - {nullptr, - &StreamChecker::evalSetFeofFerror<&StreamState::getOpenedWithFEof>, 0}}, + {nullptr, &StreamChecker::evalSetFeofFerror<StreamState::FEof>, 0}}, {{"StreamTesterChecker_make_ferror_stream", 1}, - {nullptr, - &StreamChecker::evalSetFeofFerror<&StreamState::getOpenedWithFError>, - 0}}, + {nullptr, &StreamChecker::evalSetFeofFerror<StreamState::FError>, 0}}, }; void evalFopen(const FnDescription *Desc, const CallEvent &Call, @@ -177,6 +251,8 @@ class StreamChecker 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 @@ class StreamChecker void evalClearerr(const FnDescription *Desc, const CallEvent &Call, CheckerContext &C) const; - template <bool (StreamState::*IsOfError)() const> + template <StreamState::ErrorKindTy ErrorKind> void evalFeofFerror(const FnDescription *Desc, const CallEvent &Call, CheckerContext &C) const; - template <StreamState (*GetState)()> + template <StreamState::ErrorKindTy EK> void evalSetFeofFerror(const FnDescription *Desc, const CallEvent &Call, CheckerContext &C) const; @@ -197,7 +273,7 @@ class StreamChecker /// 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 @@ class StreamChecker /// 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 @@ class StreamChecker } return FnDescriptions.lookup(Call); - } + } }; } // end anonymous namespace @@ -278,8 +354,10 @@ void StreamChecker::evalFopen(const FnDescription *Desc, const CallEvent &Call, std::tie(StateNotNull, StateNull) = C.getConstraintManager().assumeDual(State, RetVal); - StateNotNull = StateNotNull->set<StreamMap>(RetSym, StreamState::getOpened()); - StateNull = StateNull->set<StreamMap>(RetSym, StreamState::getOpenFailed()); + StateNotNull = + StateNotNull->set<StreamMap>(RetSym, StreamState::getOpened(Desc)); + StateNull = + StateNull->set<StreamMap>(RetSym, StreamState::getOpenFailed(Desc)); C.addTransition(StateNotNull); C.addTransition(StateNull); @@ -328,9 +406,9 @@ void StreamChecker::evalFreopen(const FnDescription *Desc, C.getSValBuilder().makeNull()); StateRetNotNull = - StateRetNotNull->set<StreamMap>(StreamSym, StreamState::getOpened()); + StateRetNotNull->set<StreamMap>(StreamSym, StreamState::getOpened(Desc)); StateRetNull = - StateRetNull->set<StreamMap>(StreamSym, StreamState::getOpenFailed()); + StateRetNull->set<StreamMap>(StreamSym, StreamState::getOpenFailed(Desc)); C.addTransition(StateRetNotNull); C.addTransition(StateRetNull); @@ -352,7 +430,7 @@ void StreamChecker::evalFclose(const FnDescription *Desc, const CallEvent &Call, // Close the File Descriptor. // Regardless if the close fails or not, stream becomes "closed" // and can not be used any more. - State = State->set<StreamMap>(Sym, StreamState::getClosed()); + State = State->set<StreamMap>(Sym, StreamState::getClosed(Desc)); C.addTransition(State); } @@ -374,6 +452,43 @@ void StreamChecker::preFseek(const FnDescription *Desc, const CallEvent &Call, 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<CallExpr>(Call.getOriginExpr()); + if (!CE) + return; + + // Ignore the call if the stream is not tracked. + if (!State->get<StreamMap>(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<StreamMap>(StreamSym, StreamState::getOpened(Desc)); + // We get error. + StateFailed = StateFailed->set<StreamMap>( + 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 @@ void StreamChecker::evalClearerr(const FnDescription *Desc, assertStreamStateOpened(SS); - if (SS->isNoError()) - return; - - State = State->set<StreamMap>(StreamSym, StreamState::getOpened()); + State = State->set<StreamMap>(StreamSym, StreamState::getOpened(Desc)); C.addTransition(State); } -template <bool (StreamState::*IsOfError)() const> +template <StreamState::ErrorKindTy ErrorKind> void StreamChecker::evalFeofFerror(const FnDescription *Desc, const CallEvent &Call, CheckerContext &C) const { @@ -408,25 +520,43 @@ void StreamChecker::evalFeofFerror(const FnDescription *Desc, if (!CE) return; + auto AddTransition = [&C, Desc, StreamSym](ProgramStateRef State, + StreamState::ErrorKindTy SSError) { + StreamState SSNew = StreamState::getOpened(Desc, SSError); + State = State->set<StreamMap>(StreamSym, SSNew); + C.addTransition(State); + }; + 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."); + 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<StreamState::ErrorKindTy> 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 @@ void StreamChecker::preDefault(const FnDescription *Desc, const CallEvent &Call, C.addTransition(State); } -template <StreamState (*GetState)()> +template <StreamState::ErrorKindTy EK> 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)()); + const StreamState *SS = State->get<StreamMap>(StreamSym); + assert(SS && "Stream should be tracked by the checker."); + State = State->set<StreamMap>(StreamSym, + StreamState::getOpened(SS->LastOperation, EK)); C.addTransition(State); } @@ -597,4 +730,3 @@ void ento::registerStreamTesterChecker(CheckerManager &Mgr) { bool ento::shouldRegisterStreamTesterChecker(const CheckerManager &Mgr) { return true; } - diff --git a/clang/test/Analysis/stream-error.c b/clang/test/Analysis/stream-error.c index e9fb8fdebfab..2bd25ca30fa3 100644 --- a/clang/test/Analysis/stream-error.c +++ b/clang/test/Analysis/stream-error.c @@ -52,3 +52,34 @@ void stream_error_ferror() { 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); +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
