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 @@ -452,6 +452,11 @@ HelpText<"Check stream handling functions">, Documentation; +def StreamTesterChecker : Checker<"StreamTester">, + HelpText<"Add test functions to StreamChecker for test and debugging purposes.">, + Dependencies<[StreamChecker]>, + Documentation; + def SimpleStreamChecker : Checker<"SimpleStream">, HelpText<"Check for misuses of stream APIs">, 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,17 +19,20 @@ #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; + +#define ASSERT_STREAMSTATE_OPENED \ + assert(SS->isOpened() && \ + "Create of error node failed (only way to get this state)?"); namespace { /// Full state information about a stream pointer. struct StreamState { /// 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). @@ -37,37 +40,44 @@ } State; /// The error state of a stream. + /// Valid only if the stream is opened. enum ErrorKindTy { - NoError, /// No error flag is set or stream is not open. - EofError, /// EOF condition (`feof` is true). - OtherError, /// Other (non-EOF) error (`ferror` is true). - AnyError /// EofError or OtherError, used if exact error is unknown. + /// 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 { return ErrorState == NoError; } - bool isEofError() const { return ErrorState == EofError; } - bool isOtherError() const { return ErrorState == OtherError; } - bool isAnyError() const { return ErrorState == AnyError; } + 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 { + // 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 getOpenedWithAnyError() { - return StreamState{Opened, AnyError}; - } - static StreamState getOpenedWithEofError() { - return StreamState{Opened, EofError}; - } - static StreamState getOpenedWithOtherError() { - return StreamState{Opened, OtherError}; + static StreamState getOpenedWithFEof() { return StreamState{Opened, FEof}; } + static StreamState getOpenedWithFError() { + return StreamState{Opened, FError}; } void Profile(llvm::FoldingSetNodeID &ID) const { @@ -102,7 +112,7 @@ DefinedSVal makeRetVal(CheckerContext &C, const CallExpr *CE) { assert(CE && "Expecting a call expression."); - const LocationContext *LCtx = C.getPredecessor()->getLocationContext(); + const LocationContext *LCtx = C.getLocationContext(); return C.getSValBuilder() .conjureSymbolVal(nullptr, CE, LCtx, C.blockCount()) .castAs(); @@ -118,6 +128,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}}, @@ -135,12 +148,25 @@ {{"fsetpos", 2}, {&StreamChecker::preDefault, nullptr, 0}}, {{"clearerr", 1}, {&StreamChecker::preDefault, &StreamChecker::evalClearerr, 0}}, - {{"feof", 1}, {&StreamChecker::preDefault, &StreamChecker::evalFeof, 0}}, + {{"feof", 1}, + {&StreamChecker::preDefault, + &StreamChecker::evalFeofFerror<&StreamState::isFEof>, 0}}, {{"ferror", 1}, - {&StreamChecker::preDefault, &StreamChecker::evalFerror, 0}}, + {&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; @@ -161,11 +187,13 @@ void evalClearerr(const FnDescription *Desc, const CallEvent &Call, CheckerContext &C) const; - void evalFeof(const FnDescription *Desc, const CallEvent &Call, - CheckerContext &C) const; + template + void evalFeofFerror(const FnDescription *Desc, const CallEvent &Call, + CheckerContext &C) const; - void evalFerror(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. @@ -219,6 +247,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; @@ -315,6 +345,8 @@ if (!SS) return; + ASSERT_STREAMSTATE_OPENED; + // Close the File Descriptor. // Regardless if the close fails or not, stream becomes "closed" // and can not be used any more. @@ -352,6 +384,8 @@ if (!SS) return; + ASSERT_STREAMSTATE_OPENED; + if (SS->isNoError()) return; @@ -359,8 +393,10 @@ C.addTransition(State); } -void StreamChecker::evalFeof(const FnDescription *Desc, const CallEvent &Call, - CheckerContext &C) const { +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) @@ -375,71 +411,20 @@ if (!SS) return; - DefinedSVal RetVal = makeRetVal(C, CE); - - // Make expression result. - State = State->BindExpr(CE, C.getLocationContext(), RetVal); + ASSERT_STREAMSTATE_OPENED; - if (SS->isAnyError()) { - // We do not know yet what kind of error is set. - // Differentiate between EOF and other error. - ProgramStateRef StateEof, StateOther; - std::tie(StateEof, StateOther) = State->assume(RetVal); - assert(StateEof && StateOther && - "Return value should not be constrained already."); - StateEof = StateEof->set(StreamSym, - StreamState::getOpenedWithEofError()); - StateOther = StateOther->set( - StreamSym, StreamState::getOpenedWithOtherError()); - C.addTransition(StateEof); - C.addTransition(StateOther); + 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 { - // We know what error is set, make the return value accordingly. - State = State->assume(RetVal, SS->isEofError()); - assert(State && "Return value should not be constrained already."); - C.addTransition(State); - } -} - -void StreamChecker::evalFerror(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; - - // Make expression result. - DefinedSVal RetVal = makeRetVal(C, CE); - State = State->BindExpr(CE, C.getLocationContext(), RetVal); - - if (SS->isAnyError()) { - // We do not know yet what kind of error is set. - // Differentiate between EOF and other error. - ProgramStateRef StateEof, StateOther; - std::tie(StateOther, StateEof) = State->assume(RetVal); - assert(StateEof && StateOther && - "Return value should not be constrained already."); - StateEof = StateEof->set(StreamSym, - StreamState::getOpenedWithEofError()); - StateOther = StateOther->set( - StreamSym, StreamState::getOpenedWithOtherError()); - C.addTransition(StateEof); - C.addTransition(StateOther); - } else { - // We know what error is set, make the return value accordingly. - State = State->assume(RetVal, SS->isOtherError()); - assert(State && "Return value should not be constrained already."); - C.addTransition(State); + // 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, @@ -456,6 +441,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 { @@ -583,8 +579,17 @@ } } -void ento::registerStreamChecker(CheckerManager &mgr) { - mgr.registerChecker(); +void ento::registerStreamChecker(CheckerManager &Mgr) { + Mgr.registerChecker(); } bool ento::shouldRegisterStreamChecker(const LangOptions &LO) { return true; } + +void ento::registerStreamTesterChecker(CheckerManager &Mgr) { + auto *Checker = Mgr.getChecker(); + Checker->TestMode = true; +} + +bool ento::shouldRegisterStreamTesterChecker(const LangOptions &LO) { + 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 @@ -1,8 +1,10 @@ -// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.unix.Stream -analyzer-checker=debug.ExprInspection -analyzer-store region -verify %s +// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.unix.StreamTester -analyzer-checker=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"); @@ -25,16 +27,28 @@ fclose(F); } -void error_clearerr() { +void stream_error_feof() { FILE *F = fopen("file", "r"); if (!F) return; - int ch = fputc('a', F); - if (ch == EOF) { - // FIXME: fputc not handled by checker yet, should expect TRUE - clang_analyzer_eval(feof(F) || ferror(F)); // expected-warning {{FALSE}} - clearerr(F); - clang_analyzer_eval(feof(F) || ferror(F)); // expected-warning {{FALSE}} - } + 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); }