Index: include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h =================================================================== --- include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h (revision 180035) +++ include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h (working copy) @@ -186,6 +186,50 @@ return 0; } + typedef std::pair AllocSite; + + /// \brief This utility function finds the location of the allocation of + /// Sym on the path leading to the exploded node N. + template + AllocSite getAllocationSite(const ExplodedNode *N, SymbolRef Sym) const { + const LocationContext *LeakContext = N->getLocationContext(); + + // This utility functions walks an exploded graph backwards to find + // the first node that refers to a tracked symbol. + const ExplodedNode *AllocNode = N; + const MemRegion *ReferenceRegion = 0; + + while (N) { + ProgramStateRef State = N->getState(); + if (!State->get(Sym)) + break; + + // Find the most recent expression bound to the symbol in the current + // context. + if (!ReferenceRegion) { + if (const MemRegion *MR = getLocationRegionIfPostStore(N)) { + SVal Val = State->getSVal(MR); + if (Val.getAsLocSymbol() == Sym) { + const VarRegion* VR = MR->getBaseRegion()->getAs(); + // Do not show local variables belonging to a function other than + // where the error is reported. + if (!VR || + (VR->getStackFrame() == LeakContext->getCurrentStackFrame())) + ReferenceRegion = MR; + } + } + } + + // Allocation node, is the last node in the current context in which the + // symbol was tracked. + if (N->getLocationContext() == LeakContext) + AllocNode = N; + N = N->pred_empty() ? NULL : *(N->pred_begin()); + } + + return AllocSite(AllocNode, ReferenceRegion); + } + /// \brief Get the value of arbitrary expressions at this point in the path. SVal getSVal(const Stmt *S) const { return getState()->getSVal(S, getLocationContext()); Index: lib/StaticAnalyzer/Checkers/Checkers.td =================================================================== --- lib/StaticAnalyzer/Checkers/Checkers.td (revision 180035) +++ lib/StaticAnalyzer/Checkers/Checkers.td (working copy) @@ -319,6 +319,10 @@ HelpText<"Check stream handling functions">, DescFile<"StreamChecker.cpp">; +def StreamCheckerV2 : Checker<"StreamV2">, + HelpText<"Check stream handling functions">, + DescFile<"StreamCheckerV2.cpp">; + def SimpleStreamChecker : Checker<"SimpleStream">, HelpText<"Check for misuses of stream APIs">, DescFile<"SimpleStreamChecker.cpp">; Index: lib/StaticAnalyzer/Checkers/CMakeLists.txt =================================================================== --- lib/StaticAnalyzer/Checkers/CMakeLists.txt (revision 180035) +++ lib/StaticAnalyzer/Checkers/CMakeLists.txt (working copy) @@ -60,6 +60,7 @@ SimpleStreamChecker.cpp StackAddrEscapeChecker.cpp StreamChecker.cpp + StreamCheckerV2.cpp TaintTesterChecker.cpp TraversalChecker.cpp UndefBranchChecker.cpp Index: lib/StaticAnalyzer/Checkers/MallocChecker.cpp =================================================================== --- lib/StaticAnalyzer/Checkers/MallocChecker.cpp (revision 180035) +++ lib/StaticAnalyzer/Checkers/MallocChecker.cpp (working copy) @@ -141,8 +141,6 @@ } }; -typedef std::pair LeakInfo; - class MallocChecker : public CheckergetLocationContext(); - // Walk the ExplodedGraph backwards and find the first node that referred to - // the tracked symbol. - const ExplodedNode *AllocNode = N; - const MemRegion *ReferenceRegion = 0; - - while (N) { - ProgramStateRef State = N->getState(); - if (!State->get(Sym)) - break; - - // Find the most recent expression bound to the symbol in the current - // context. - if (!ReferenceRegion) { - if (const MemRegion *MR = C.getLocationRegionIfPostStore(N)) { - SVal Val = State->getSVal(MR); - if (Val.getAsLocSymbol() == Sym) { - const VarRegion* VR = MR->getBaseRegion()->getAs(); - // Do not show local variables belonging to a function other than - // where the error is reported. - if (!VR || - (VR->getStackFrame() == LeakContext->getCurrentStackFrame())) - ReferenceRegion = MR; - } - } - } - - // Allocation node, is the last node in the current context in which the - // symbol was tracked. - if (N->getLocationContext() == LeakContext) - AllocNode = N; - N = N->pred_empty() ? NULL : *(N->pred_begin()); - } - - return LeakInfo(AllocNode, ReferenceRegion); -} - void MallocChecker::reportLeak(SymbolRef Sym, ExplodedNode *N, CheckerContext &C) const { @@ -1590,7 +1543,7 @@ PathDiagnosticLocation LocUsedForUniqueing; const ExplodedNode *AllocNode = 0; const MemRegion *Region = 0; - llvm::tie(AllocNode, Region) = getAllocationSite(N, Sym, C); + llvm::tie(AllocNode, Region) = C.getAllocationSite(N, Sym); ProgramPoint P = AllocNode->getLocation(); const Stmt *AllocationStmt = 0; Index: lib/StaticAnalyzer/Checkers/StreamCheckerV2.cpp =================================================================== --- lib/StaticAnalyzer/Checkers/StreamCheckerV2.cpp (revision 0) +++ lib/StaticAnalyzer/Checkers/StreamCheckerV2.cpp (working copy) @@ -0,0 +1,301 @@ +//===-- StreamCheckerV2.cpp ---------------------------------------*- C++ -*--// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// +/// Defines a checker for the proper use of stream APIs. +//===----------------------------------------------------------------------===// + + +#include "ClangSACheckers.h" +#include "InterCheckerAPI.h" +#include "clang/AST/Attr.h" +#include "clang/AST/Type.h" +#include "clang/Basic/SourceManager.h" +#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" +#include "clang/StaticAnalyzer/Core/Checker.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" +#include "llvm/ADT/ImmutableMap.h" +#include "llvm/ADT/STLExtras.h" +#include "llvm/ADT/SmallString.h" +#include "llvm/ADT/StringExtras.h" + +using namespace clang; +using namespace ento; + +namespace { + +typedef SmallVector SymbolVector; + +struct StreamState { + enum Kind { Opened, Closed } K; + StreamState(Kind InK) : K(InK) {} + +public: + bool isOpened() const { return K == Opened; } + bool isClosed() const { return K == Closed; } + + static StreamState getOpened() { return StreamState(Opened); } + static StreamState getClosed() { return StreamState(Closed); } + + bool operator==(const StreamState &X) const { return K == X.K; } + void Profile(llvm::FoldingSetNodeID &ID) const { ID.AddInteger(K); } +}; + +class StreamCheckerV2 : public Checker { + mutable IdentifierInfo *IIfopen, *IIfclose; + + OwningPtr DoubleCloseBugType; + OwningPtr LeakBugType; + + void initIdentifierInfo(ASTContext &Ctx) const; + + void reportDoubleClose(SymbolRef FileDescSym, const CallEvent &Call, + CheckerContext &C) const; + + void reportLeaks(SymbolVector LeakedStreams, CheckerContext &C, + ExplodedNode *ErrNode) const; + + bool guaranteedNotToCloseFile(const CallEvent &Call) const; + +public: + StreamCheckerV2(); + + void checkPostCall(const CallEvent &Call, CheckerContext &C) const; + + void checkPreCall(const CallEvent &Call, CheckerContext &C) const; + + void checkDeadSymbols(SymbolReaper &SymReaper, CheckerContext &C) const; + + /// Stop tracking addresses which escape. + ProgramStateRef checkPointerEscape(ProgramStateRef State, + const InvalidatedSymbols &Escaped, + const CallEvent *Call, + PointerEscapeKind Kind) const; +}; +} // end anonymous namespace + +/// The state of the checker is a map from tracked stream symbols to their +/// state. +REGISTER_MAP_WITH_PROGRAMSTATE(StreamMap, SymbolRef, StreamState) + +namespace { +class StopTrackingCallback : public SymbolVisitor { + ProgramStateRef state; + +public: + StopTrackingCallback(ProgramStateRef st) : state(st) {} + ProgramStateRef getState() const { return state; } + + bool VisitSymbol(SymbolRef sym) { + state = state->remove(sym); + return true; + } +}; +} // end anonymous namespace + +StreamCheckerV2::StreamCheckerV2() : IIfopen(0), IIfclose(0) { + // Initialize the bug types. + DoubleCloseBugType.reset( + new BugType("Double fclose", "Unix Stream API Error")); + + LeakBugType.reset(new BugType("Resource Leak", "Unix Stream API Error")); + // Sinks are higher importance bugs as well as calls to assert() or exit(0). + LeakBugType->setSuppressOnSink(true); +} + +void StreamCheckerV2::checkPostCall(const CallEvent &Call, + CheckerContext &C) const { + initIdentifierInfo(C.getASTContext()); + + if (!Call.isGlobalCFunction()) + return; + + if (Call.getCalleeIdentifier() != IIfopen) + return; + + // Get the symbolic value corresponding to the file handle. + SymbolRef FileDesc = Call.getReturnValue().getAsSymbol(); + if (!FileDesc) + return; + + // Generate the next transition (an edge in the exploded graph). + ProgramStateRef State = C.getState(); + State = State->set(FileDesc, StreamState::getOpened()); + C.addTransition(State); +} + +void StreamCheckerV2::checkPreCall(const CallEvent &Call, + CheckerContext &C) const { + initIdentifierInfo(C.getASTContext()); + + if (!Call.isGlobalCFunction()) + return; + + if (Call.getCalleeIdentifier() != IIfclose) + return; + + if (Call.getNumArgs() != 1) + return; + + // Get the symbolic value corresponding to the file handle. + SymbolRef FileDesc = Call.getArgSVal(0).getAsSymbol(); + if (!FileDesc) + return; + + // Check if the stream has already been closed. + ProgramStateRef State = C.getState(); + const StreamState *SS = State->get(FileDesc); + if (SS && SS->isClosed()) { + reportDoubleClose(FileDesc, Call, C); + return; + } + + // Generate the next transition, in which the stream is closed. + State = State->set(FileDesc, StreamState::getClosed()); + C.addTransition(State); +} + +static bool isLeaked(SymbolRef Sym, const StreamState &SS, bool IsSymDead, + ProgramStateRef State) { + if (IsSymDead && SS.isOpened()) { + // If a symbol is NULL, assume that fopen failed on this path. + // A symbol should only be considered leaked if it is non-null. + ConstraintManager &CMgr = State->getConstraintManager(); + ConditionTruthVal OpenFailed = CMgr.isNull(State, Sym); + return !OpenFailed.isConstrainedTrue(); + } + return false; +} + +void StreamCheckerV2::checkDeadSymbols(SymbolReaper &SymReaper, + CheckerContext &C) const { + ProgramStateRef State = C.getState(); + StreamMapTy TrackedStreams = State->get(); + StreamMapTy::Factory &F = State->get_context(); + + SymbolVector LeakedStreams; + for (StreamMapTy::iterator I = TrackedStreams.begin(), + E = TrackedStreams.end(); I != E; ++I) { + SymbolRef Sym = I->first; + bool IsSymDead = SymReaper.isDead(Sym); + + if (isLeaked(Sym, I->second, IsSymDead, State)) + LeakedStreams.push_back(Sym); + if (IsSymDead) + TrackedStreams = F.remove(TrackedStreams, Sym); + } + + ExplodedNode *N = C.addTransition(State); + reportLeaks(LeakedStreams, C, N); + State->set(TrackedStreams); +} + +void StreamCheckerV2::reportDoubleClose(SymbolRef FileDescSym, + const CallEvent &Call, + CheckerContext &C) const { + // We reached a bug, stop exploring the path here by generating a sink. + ExplodedNode *ErrNode = C.generateSink(); + // If we've already reached this node on another path, return. + if (!ErrNode) + return; + + // Generate the report. + BugReport *R = new BugReport( + *DoubleCloseBugType, "Closing a previously closed file stream", ErrNode); + R->addRange(Call.getSourceRange()); + R->markInteresting(FileDescSym); + C.emitReport(R); +} + +void StreamCheckerV2::reportLeaks(SymbolVector LeakedStreams, CheckerContext &C, + ExplodedNode *ErrNode) const { + // Attach bug reports to the leak node. + for (SmallVector::iterator I = LeakedStreams.begin(), + E = LeakedStreams.end(); + I != E; ++I) { + // Get the node where the stream was opened. + const ExplodedNode *OpenNode = 0; + const MemRegion *Region = 0; + llvm::tie(OpenNode, Region) = C.getAllocationSite(ErrNode, *I); + + const Stmt *AllocationStmt = 0; + ProgramPoint P = OpenNode->getLocation(); + if (Optional SP = P.getAs()) + AllocationStmt = SP->getStmt(); + + PathDiagnosticLocation LocUsedForUniqueing; + if (AllocationStmt) + LocUsedForUniqueing = PathDiagnosticLocation::createBegin( + AllocationStmt, C.getSourceManager(), + OpenNode->getLocationContext()); + + SmallString<200> buf; + llvm::raw_svector_ostream os(buf); + if (Region && Region->canPrintPretty()) { + os << "Opened file is never closed; potential resource leak of "; + Region->printPretty(os); + } else { + os << "Opened file is never closed; potential resource leak"; + } + + BugReport *R = new BugReport(*LeakBugType, os.str(), ErrNode, + LocUsedForUniqueing, + OpenNode->getLocationContext()->getDecl()); + R->markInteresting(*I); + C.emitReport(R); + } +} + +bool StreamCheckerV2::guaranteedNotToCloseFile(const CallEvent &Call) const { + // If it's not in a system header, assume it might close a file. + if (!Call.isInSystemHeader()) + return false; + + // Handle cases where we know a buffer's /address/ can escape. + if (Call.argumentsMayEscape()) + return false; + + // Note, even though fclose closes the file, we do not list it here + // since the checker is modeling the call. + return true; +} + +// If the pointer we are tracking escaped, do not track the symbol as +// we cannot reason about it anymore. +ProgramStateRef StreamCheckerV2::checkPointerEscape( + ProgramStateRef State, const InvalidatedSymbols &Escaped, + const CallEvent *Call, PointerEscapeKind Kind) const { + // If we know that the call cannot close a file, there is nothing to do. + if ((Kind == PSK_DirectEscapeOnCall || Kind == PSK_IndirectEscapeOnCall) && + guaranteedNotToCloseFile(*Call)) { + return State; + } + + for (InvalidatedSymbols::const_iterator I = Escaped.begin(), + E = Escaped.end(); + I != E; ++I) { + // The symbol escaped. Optimistically, assume that the corresponding file + // handle will be closed somewhere else. + State = State->remove(*I); + } + return State; +} + +void StreamCheckerV2::initIdentifierInfo(ASTContext &Ctx) const { + if (IIfopen) + return; + IIfopen = &Ctx.Idents.get("fopen"); + IIfclose = &Ctx.Idents.get("fclose"); +} + +void ento::registerStreamCheckerV2(CheckerManager &mgr) { + mgr.registerChecker(); +} Index: test/Analysis/streamV2-checks.c =================================================================== --- test/Analysis/streamV2-checks.c (revision 0) +++ test/Analysis/streamV2-checks.c (working copy) @@ -0,0 +1,91 @@ +// RUN: %clang_cc1 -analyze -analyzer-checker=core,alpha.unix.StreamV2 -verify %s + +#include "Inputs/system-header-simulator-for-simple-stream.h" + +void checkDoubleFClose(int *Data) { + FILE *F = fopen("myfile.txt", "w"); + if (F != 0) { + fputs ("fopen example", F); + if (!Data) + fclose(F); + else + fputc(*Data, F); + fclose(F); // expected-warning {{Closing a previously closed file stream}} + } +} + +int checkLeak(int *Data) { + FILE *F = fopen("myfile.txt", "w"); + if (F != 0) { + fputs ("fopen example", F); // expected-warning {{Opened file is never closed; potential resource leak of 'F'}} + } + + if (Data) + return *Data; + else + return 0; +} + +void checkLeakFollowedByAssert(int *Data) { + FILE *F = fopen("myfile.txt", "w"); + if (F != 0) { + fputs ("fopen example", F); + if (!Data) + exit(0); + fclose(F); + } +} + +void CloseOnlyOnValidFileHandle() { + FILE *F = fopen("myfile.txt", "w"); + if (F) + fclose(F); + int x = 0; // no warning +} + +void leakOnEnfOfPath1(int *Data) { + FILE *F = fopen("myfile.txt", "w"); // expected-warning {{Opened file is never closed; potential resource leak of 'F'}} +} + +void leakOnEnfOfPath2(int *Data) { + FILE *F = fopen("myfile.txt", "w"); // expected-warning {{Opened file is never closed; potential resource leak of 'F'}} + return; +} + +FILE *leakOnEnfOfPath3(int *Data) { + FILE *F = fopen("myfile.txt", "w"); + return F; +} + +void myfclose(FILE *F); +void SymbolEscapedThroughFunctionCall() { + FILE *F = fopen("myfile.txt", "w"); + myfclose(F); + return; // no warning +} + +FILE *GlobalF; +void SymbolEscapedThroughAssignmentToGloabl() { + FILE *F = fopen("myfile.txt", "w"); + GlobalF = F; + return; // no warning +} + +void SymbolDoesNotEscapeThoughStringAPIs(char *Data) { + FILE *F = fopen("myfile.txt", "w"); + fputc(*Data, F); // expected-warning {{Opened file is never closed; potential resource leak of 'F'}} + return; +} + +void passConstPointer(const FILE * F); +void testPassConstPointer() { + FILE *F = fopen("myfile.txt", "w"); + passConstPointer(F); // expected-warning {{Opened file is never closed; potential resource leak of 'F'}} + return; +} + +void testPassToSystemHeaderFunctionIndirectly() { + FileStruct fs; + fs.p = fopen("myfile.txt", "w"); + fakeSystemHeaderCall(&fs); // expected-warning {{Opened file is never closed; potential resource leak of 'fs.p'}} +}