[cfe-commits] r166976 - in /cfe/trunk: lib/StaticAnalyzer/Checkers/CMakeLists.txt lib/StaticAnalyzer/Checkers/Checkers.td lib/StaticAnalyzer/Checkers/SimpleStreamChecker.cpp test/Analysis/simple-stream-checks.c
Jordan Rose
jordan_rose at apple.com
Mon Oct 29 16:19:36 PDT 2012
On Oct 29, 2012, at 15:51 , Anna Zaks <ganna at apple.com> 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
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20121029/04ddc527/attachment.html>
More information about the cfe-commits
mailing list