On Oct 29, 2012, at 15:51 , Anna Zaks <[email protected]> wrote: > Author: zaks > Date: Mon Oct 29 17:51:50 2012 > New Revision: 166976 > > URL: http://llvm.org/viewvc/llvm-project?rev=166976&view=rev > Log: > [analyzer] Add SimpleStreamChecker. > > This is an example checker for catching fopen fclose API misuses. > > Added: > cfe/trunk/lib/StaticAnalyzer/Checkers/SimpleStreamChecker.cpp > cfe/trunk/test/Analysis/simple-stream-checks.c > Modified: > cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt > cfe/trunk/lib/StaticAnalyzer/Checkers/Checkers.td > > Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt?rev=166976&r1=166975&r2=166976&view=diff > ============================================================================== > --- cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt (original) > +++ cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt Mon Oct 29 17:51:50 > 2012 > @@ -31,6 +31,7 @@ > DivZeroChecker.cpp > DynamicTypePropagation.cpp > ExprInspectionChecker.cpp > + SimpleStreamChecker.cpp > FixedAddressChecker.cpp > GenericTaintChecker.cpp > IdempotentOperationChecker.cpp
This list is supposed to be alphabetical (for convenience, not correctness). > Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/Checkers.td > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/Checkers.td?rev=166976&r1=166975&r2=166976&view=diff > ============================================================================== > --- cfe/trunk/lib/StaticAnalyzer/Checkers/Checkers.td (original) > +++ cfe/trunk/lib/StaticAnalyzer/Checkers/Checkers.td Mon Oct 29 17:51:50 2012 > @@ -303,6 +303,10 @@ > HelpText<"Check stream handling functions">, > DescFile<"StreamChecker.cpp">; > > +def SimpleStreamChecker : Checker<"SimpleStream">, > + HelpText<"Check for misuses of stream APIs">, > + DescFile<"SimpleStreamChecker.cpp">; > + > } // end "alpha.unix" > > let ParentPackage = CString in { > > Added: cfe/trunk/lib/StaticAnalyzer/Checkers/SimpleStreamChecker.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/SimpleStreamChecker.cpp?rev=166976&view=auto > ============================================================================== > --- cfe/trunk/lib/StaticAnalyzer/Checkers/SimpleStreamChecker.cpp (added) > +++ cfe/trunk/lib/StaticAnalyzer/Checkers/SimpleStreamChecker.cpp Mon Oct 29 > 17:51:50 2012 > @@ -0,0 +1,229 @@ > +//===-- SimpleStreamChecker.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 proper use of fopen/fclose APIs. > +// - If a file has been closed with fclose, it should not be accessed > again. > +// Accessing a closed file results in undefined behavior. > +// - If a file was opened with fopen, it must be closed with fclose before > +// the execution ends. Failing to do so results in a resource leak. > +// > +//===----------------------------------------------------------------------===// > + > +#include "ClangSACheckers.h" > +#include "clang/StaticAnalyzer/Core/Checker.h" > +#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" > +#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" > + > +using namespace clang; > +using namespace ento; > + > +namespace { > +typedef llvm::SmallVector<SymbolRef, 2> SymbolVector; > + > +struct StreamState { > + enum Kind { Opened, Closed } K; > + > + StreamState(Kind InK) : K(InK) { } > + > + 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 SimpleStreamChecker: public Checker<check::PostStmt<CallExpr>, > + check::PreStmt<CallExpr>, > + check::DeadSymbols, > + eval::Assume > { > + > + mutable IdentifierInfo *IIfopen, *IIfclose; > + > + mutable OwningPtr<BugType> DoubleCloseBugType; > + mutable OwningPtr<BugType> LeakBugType; > + > + void initIdentifierInfo(ASTContext &Ctx) const; > + > + void reportDoubleClose(SymbolRef FileDescSym, > + const CallExpr *Call, > + CheckerContext &C) const; > + > + ExplodedNode *reportLeaks(SymbolVector LeakedStreams, > + CheckerContext &C) const; > + > +public: > + SimpleStreamChecker() : IIfopen(0), IIfclose(0) {} > + > + /// Process fopen. > + void checkPostStmt(const CallExpr *Call, CheckerContext &C) const; > + /// Process fclose. > + void checkPreStmt(const CallExpr *Call, CheckerContext &C) const; > + > + void checkDeadSymbols(SymbolReaper &SymReaper, CheckerContext &C) const; > + ProgramStateRef evalAssume(ProgramStateRef state, SVal Cond, > + bool Assumption) const; > + > +}; > + > +} // end anonymous namespace > + > +/// The state of the checker is a map from tracked stream symbols to their > +/// state. Let's store it in the GDM. > +REGISTER_MAP_WITH_GDM(StreamMap, SymbolRef, StreamState) > + > +void SimpleStreamChecker::checkPostStmt(const CallExpr *Call, > + CheckerContext &C) const { > + initIdentifierInfo(C.getASTContext()); > + > + if (C.getCalleeIdentifier(Call) != IIfopen) > + return; > + > + // Get the symbolic value corresponding to the file handle. > + SymbolRef FileDesc = C.getSVal(Call).getAsSymbol(); > + if (!FileDesc) > + return; > + > + // Generate the next transition (an edge in the exploded graph). > + ProgramStateRef State = C.getState(); > + State = State->set<StreamMap>(FileDesc, StreamState::getOpened()); > + C.addTransition(State); > +} > + > +void SimpleStreamChecker::checkPreStmt(const CallExpr *Call, > + CheckerContext &C) const { > + initIdentifierInfo(C.getASTContext()); > + > + if (C.getCalleeIdentifier(Call) != IIfclose) > + return; > + if (Call->getNumArgs() != 1) > + return; > + > + // Get the symbolic value corresponding to the file handle. > + SymbolRef FileDesc = C.getSVal(Call->getArg(0)).getAsSymbol(); > + if (!FileDesc) > + return; > + > + // Check if the stream has already been closed. > + ProgramStateRef State = C.getState(); > + const StreamState *SS = State->get<StreamMap>(FileDesc); > + if (SS && SS->isClosed()) > + reportDoubleClose(FileDesc, Call, C); > + > + // Generate the next transition, in which the stream is closed. > + State = State->set<StreamMap>(FileDesc, StreamState::getClosed()); > + C.addTransition(State); > +} > + > +void SimpleStreamChecker::checkDeadSymbols(SymbolReaper &SymReaper, > + CheckerContext &C) const { > + ProgramStateRef State = C.getState(); > + StreamMap TrackedStreams = State->get<StreamMap>(); > + SymbolVector LeakedStreams; > + for (StreamMap::iterator I = TrackedStreams.begin(), > + E = TrackedStreams.end(); I != E; ++I) { > + SymbolRef Sym = I->first; > + if (SymReaper.isDead(Sym)) { > + const StreamState &SS = I->second; > + if (SS.isOpened()) > + LeakedStreams.push_back(Sym); > + > + // Remove the dead symbol from the streams map. > + State = State->remove<StreamMap>(Sym); > + } > + } > + > + ExplodedNode *N = reportLeaks(LeakedStreams, C); > + C.addTransition(State, N); > +} > + > +// If a symbolic region is assumed to NULL (or another constant), stop > tracking > +// it - assuming that allocation failed on this path. > +ProgramStateRef SimpleStreamChecker::evalAssume(ProgramStateRef State, > + SVal Cond, > + bool Assumption) const { > + StreamMap TrackedStreams = State->get<StreamMap>(); > + SymbolVector LeakedStreams; > + for (StreamMap::iterator I = TrackedStreams.begin(), > + E = TrackedStreams.end(); I != E; ++I) { > + SymbolRef Sym = I->first; > + if (State->getConstraintManager().isNull(State, Sym).isTrue()) > + State = State->remove<StreamMap>(Sym); > + } > + return State; > +} > + > +void SimpleStreamChecker::reportDoubleClose(SymbolRef FileDescSym, > + const CallExpr *CallExpr, > + CheckerContext &C) const { > + // We reached a bug, stop exploring the path here by generating a sink. > + ExplodedNode *ErrNode = C.generateSink(); > + // If this error node already exists, return. > + if (!ErrNode) > + return; > + > + // Initialize the bug type. > + if (!DoubleCloseBugType) > + DoubleCloseBugType.reset(new BugType("Double fclose", > + "Unix Stream API Error")); > + // Generate the report. > + BugReport *R = new BugReport(*DoubleCloseBugType, > + "Closing a previously closed file stream", ErrNode); > + R->addRange(CallExpr->getSourceRange()); > + R->markInteresting(FileDescSym); > + C.EmitReport(R); > +} > + > +ExplodedNode *SimpleStreamChecker::reportLeaks(SymbolVector LeakedStreams, > + CheckerContext &C) const { > + ExplodedNode *Pred = C.getPredecessor(); > + if (LeakedStreams.empty()) > + return Pred; > + > + // Generate an intermediate node representing the leak point. > + static SimpleProgramPointTag Tag("StreamChecker : Leak"); > + ExplodedNode *ErrNode = C.addTransition(Pred->getState(), Pred, &Tag); > + if (!ErrNode) > + return Pred; Why generate an intermediate node at all? I feel like we could just generate the transition in the caller and then report all the leaks here. > + // Initialize the bug type. > + if (!LeakBugType) { > + LeakBugType.reset(new BuiltinBug("Resource Leak", > + "Unix Stream API Error")); > + // Sinks are higher importance bugs as well as calls to assert() or > exit(0). Perhaps: s/are higher importance/represent more important/ …but also assert isn't really a sink. > + LeakBugType->setSuppressOnSink(true); > + } > + > + // Attach bug reports to the leak node. > + for (llvm::SmallVector<SymbolRef, 2>::iterator > + I = LeakedStreams.begin(), E = LeakedStreams.end(); I != E; ++I) { > + BugReport *R = new BugReport(*LeakBugType, > + "Opened file is never closed; potential resource leak", ErrNode); No markInteresting? No attempt to say which file it is? …actually, that's rather hard. Hm. > + C.EmitReport(R); > + } > + > + return ErrNode; > +} > + > +void SimpleStreamChecker::initIdentifierInfo(ASTContext &Ctx) const { > + if (IIfopen) > + return; > + IIfopen = &Ctx.Idents.get("fopen"); > + IIfclose = &Ctx.Idents.get("fclose"); > +} > + > +void ento::registerSimpleStreamChecker(CheckerManager &mgr) { > + mgr.registerChecker<SimpleStreamChecker>(); > +} > > Added: cfe/trunk/test/Analysis/simple-stream-checks.c > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/simple-stream-checks.c?rev=166976&view=auto > ============================================================================== > --- cfe/trunk/test/Analysis/simple-stream-checks.c (added) > +++ cfe/trunk/test/Analysis/simple-stream-checks.c Mon Oct 29 17:51:50 2012 > @@ -0,0 +1,44 @@ > +// RUN: %clang_cc1 -analyze -analyzer-checker=core,alpha.unix.SimpleStream > -verify %s > + > +typedef struct __sFILE { > + unsigned char *_p; > +} FILE; > +FILE *fopen(const char * restrict, const char * restrict) __asm("_" "fopen" > ); > +int fputc(int, FILE *); > +int fputs(const char * restrict, FILE * restrict) __asm("_" "fputs" ); > +int fclose(FILE *); > +void exit(int); > + > +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); > + } > + > + if (Data) // expected-warning {{Opened file is never closed; potential > resource leak}} > + 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); > + } > +} > > > _______________________________________________ > cfe-commits mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
