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 @@ -10,6 +10,7 @@ // //===----------------------------------------------------------------------===// +#include "clang/AST/ParentMapContext.h" #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h" #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" #include "clang/StaticAnalyzer/Core/Checker.h" @@ -157,6 +158,11 @@ // StreamChecker class and utility functions. //===----------------------------------------------------------------------===// +// This map holds the state of a stream. +// The stream is identified with a SymbolRef that is created when a stream +// opening function is modeled by the checker. +REGISTER_MAP_WITH_PROGRAMSTATE(StreamMap, SymbolRef, StreamState) + namespace { class StreamChecker; @@ -206,8 +212,47 @@ return State; } -class StreamChecker : public Checker { +const VarDecl *findStdStreamDecl(StringRef StdName, CheckerContext &C) { + ASTContext &ACtx = C.getASTContext(); + + IdentifierInfo &II = ACtx.Idents.get(StdName); + auto LookupRes = ACtx.getTranslationUnitDecl()->lookup(&II); + QualType FILEType = ACtx.getFILEType(); + if (!FILEType.isNull()) + FILEType = ACtx.getPointerType(FILEType); + + for (Decl *D : LookupRes) { + D = D->getCanonicalDecl(); + if (!C.getSourceManager().isInSystemHeader(D->getLocation())) + continue; + if (auto *VD = dyn_cast(D)) { + if (!FILEType.isNull() && !ACtx.hasSameType(FILEType, VD->getType())) + continue; + return VD; + } + } + + return nullptr; +} + +SymbolRef findStdStream(StringRef StdName, CheckerContext &C) { + const LocationContext *LCtx = C.getLocationContext(); + + const VarDecl *StdDecl = findStdStreamDecl(StdName, C); + if (!StdDecl) + return nullptr; + + const VarRegion *RegionOfStdStreamVar = + C.getState()->getRegion(StdDecl, LCtx); + if (!RegionOfStdStreamVar) + return nullptr; + SVal StdVal = C.getState()->getSVal(RegionOfStdStreamVar, StdDecl->getType()); + return StdVal.getAsSymbol(); +} + +class StreamChecker + : public Checker { BugType BT_FileNull{this, "NULL stream pointer", "Stream handling error"}; BugType BT_UseAfterClose{this, "Closed stream", "Stream handling error"}; BugType BT_UseAfterOpenFailed{this, "Invalid stream", @@ -221,6 +266,7 @@ /*SuppressOnSink =*/true}; public: + void checkBeginFunction(CheckerContext &C) const; void checkPreCall(const CallEvent &Call, CheckerContext &C) const; bool evalCall(const CallEvent &Call, CheckerContext &C) const; void checkDeadSymbols(SymbolReaper &SymReaper, CheckerContext &C) const; @@ -285,6 +331,10 @@ 0}}, }; + mutable SymbolRef OrigStdin; + mutable SymbolRef OrigStdout; + mutable SymbolRef OrigStderr; + void evalFopen(const FnDescription *Desc, const CallEvent &Call, CheckerContext &C) const; @@ -414,16 +464,15 @@ } // end anonymous namespace -// This map holds the state of a stream. -// The stream is identified with a SymbolRef that is created when a stream -// opening function is modeled by the checker. -REGISTER_MAP_WITH_PROGRAMSTATE(StreamMap, SymbolRef, StreamState) - inline void assertStreamStateOpened(const StreamState *SS) { assert(SS->isOpened() && "Previous create of error node for non-opened stream failed?"); } +//===----------------------------------------------------------------------===// +// Methods of StreamChecker. +//===----------------------------------------------------------------------===// + const ExplodedNode *StreamChecker::getAcquisitionSite(const ExplodedNode *N, SymbolRef StreamSym, CheckerContext &C) { @@ -445,9 +494,14 @@ return nullptr; } -//===----------------------------------------------------------------------===// -// Methods of StreamChecker. -//===----------------------------------------------------------------------===// +void StreamChecker::checkBeginFunction(CheckerContext &C) const { + if (!C.inTopFrame()) + return; + + OrigStdin = findStdStream("stdin", C); + OrigStdout = findStdStream("stdout", C); + OrigStderr = findStdStream("stderr", C); +} void StreamChecker::checkPreCall(const CallEvent &Call, CheckerContext &C) const { @@ -494,6 +548,21 @@ StateNull = StateNull->set(RetSym, StreamState::getOpenFailed(Desc)); + auto AssumeRetValNotEq = [&C, &StateNotNull, RetVal](SymbolRef StdStream) { + SVal NotEqS = C.getSValBuilder().evalBinOp( + StateNotNull, BO_NE, RetVal, + C.getSValBuilder().makeSymbolVal(StdStream), + C.getASTContext().getLogicalOperationType()); + Optional NotEqDef = + NotEqS.getAs(); + if (!NotEqDef) + return; + StateNotNull = StateNotNull->assume(*NotEqDef, true); + }; + AssumeRetValNotEq(OrigStdin); + AssumeRetValNotEq(OrigStdout); + AssumeRetValNotEq(OrigStderr); + C.addTransition(StateNotNull, constructNoteTag(C, RetSym, {&BT_ResourceLeak})); C.addTransition(StateNull, constructNoteTag(C, RetSym, {&BT_FileNull})); 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 @@ -267,3 +267,12 @@ } // expected-warning {{Opened stream never closed. Potential resource leak}} // FIXME: This warning should be placed at the `return` above. // See https://reviews.llvm.org/D83120 about details. + +void check_fopen_is_not_std(FILE *F, int Std) { + if (!Std) + F = fopen("file", "r"); + if (!F) + return; + if (F != stdin && F != stdout && F != stderr) + fclose(F); // no warning: the opened file can not be equal to std streams +}