[cfe-commits] r167099 - in /cfe/trunk: lib/StaticAnalyzer/Checkers/SimpleStreamChecker.cpp test/Analysis/simple-stream-checks.c
Jordan Rose
jordan_rose at apple.com
Wed Oct 31 09:09:17 PDT 2012
On Oct 30, 2012, at 19:32 , Anna Zaks <ganna at apple.com> wrote:
> Author: zaks
> Date: Tue Oct 30 21:32:41 2012
> New Revision: 167099
>
> URL: http://llvm.org/viewvc/llvm-project?rev=167099&view=rev
> Log:
> [analyzer] SimpleStreamChecker - remove evalAssume and other refinements
>
> Modified:
> cfe/trunk/lib/StaticAnalyzer/Checkers/SimpleStreamChecker.cpp
> cfe/trunk/test/Analysis/simple-stream-checks.c
>
> Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/SimpleStreamChecker.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/SimpleStreamChecker.cpp?rev=167099&r1=167098&r2=167099&view=diff
> ==============================================================================
> --- cfe/trunk/lib/StaticAnalyzer/Checkers/SimpleStreamChecker.cpp (original)
> +++ cfe/trunk/lib/StaticAnalyzer/Checkers/SimpleStreamChecker.cpp Tue Oct 30 21:32:41 2012
> @@ -27,10 +27,11 @@
> typedef llvm::SmallVector<SymbolRef, 2> SymbolVector;
>
> struct StreamState {
> +private:
> enum Kind { Opened, Closed } K;
> -
> StreamState(Kind InK) : K(InK) { }
>
> +public:
> bool isOpened() const { return K == Opened; }
> bool isClosed() const { return K == Closed; }
>
> @@ -47,8 +48,7 @@
>
> class SimpleStreamChecker: public Checker<check::PostStmt<CallExpr>,
> check::PreStmt<CallExpr>,
> - check::DeadSymbols,
> - eval::Assume > {
> + check::DeadSymbols > {
>
> mutable IdentifierInfo *IIfopen, *IIfclose;
>
> @@ -61,11 +61,12 @@
> const CallExpr *Call,
> CheckerContext &C) const;
>
> - ExplodedNode *reportLeaks(SymbolVector LeakedStreams,
> - CheckerContext &C) const;
> + void reportLeaks(SymbolVector LeakedStreams,
> + CheckerContext &C,
> + ExplodedNode *ErrNode) const;
>
> public:
> - SimpleStreamChecker() : IIfopen(0), IIfclose(0) {}
> + SimpleStreamChecker();
>
> /// Process fopen.
> void checkPostStmt(const CallExpr *Call, CheckerContext &C) const;
> @@ -73,9 +74,6 @@
> 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
> @@ -84,6 +82,17 @@
> /// state. Let's store it in the ProgramState.
> REGISTER_MAP_WITH_PROGRAMSTATE(StreamMap, SymbolRef, StreamState)
>
> +SimpleStreamChecker::SimpleStreamChecker() : 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);
> +}
Is it possible to put the BugTypes in the initializer list, or is that too messy? reset() is ever so slightly more expensive than using the OwningPtr constructor.
Actually, hang on. There's no need to use an OwningPtr if they're not constructed lazily. They could just be members.
> void SimpleStreamChecker::checkPostStmt(const CallExpr *Call,
> CheckerContext &C) const {
> initIdentifierInfo(C.getASTContext());
> @@ -135,32 +144,20 @@
> SymbolRef Sym = I->first;
> if (SymReaper.isDead(Sym)) {
> const StreamState &SS = I->second;
> - if (SS.isOpened())
> - LeakedStreams.push_back(Sym);
> + if (SS.isOpened()) {
> + // If a symbolic region is NULL, assume that allocation failed on
> + // this path and do not report a leak.
> + if (!State->getConstraintManager().isNull(State, Sym).isTrue())
> + LeakedStreams.push_back(Sym);
> + }
!...isTrue() is hard to read and makes me think it's the same as isFalse(), which it isn't. Can we add another convenience accessor, isFalseOrUnderconstrained() or something?
> // 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 {
> - StreamMapTy TrackedStreams = State->get<StreamMap>();
> - SymbolVector LeakedStreams;
> - for (StreamMapTy::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;
> + ExplodedNode *N = C.addTransition(State);
> + reportLeaks(LeakedStreams, C, N);
> }
>
> void SimpleStreamChecker::reportDoubleClose(SymbolRef FileDescSym,
> @@ -168,14 +165,10 @@
> 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 we've already reached this node on another path, 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);
> @@ -184,26 +177,9 @@
> 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;
> -
> - // 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).
> - LeakBugType->setSuppressOnSink(true);
> - }
> -
> +void SimpleStreamChecker::reportLeaks(SymbolVector LeakedStreams,
> + CheckerContext &C,
> + ExplodedNode *ErrNode) const {
> // Attach bug reports to the leak node.
> // TODO: Identify the leaked file descriptor.
> for (llvm::SmallVector<SymbolRef, 2>::iterator
> @@ -213,8 +189,6 @@
> R->markInteresting(*I);
> C.EmitReport(R);
> }
> -
> - return ErrNode;
> }
>
> void SimpleStreamChecker::initIdentifierInfo(ASTContext &Ctx) const {
>
> Modified: cfe/trunk/test/Analysis/simple-stream-checks.c
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/simple-stream-checks.c?rev=167099&r1=167098&r2=167099&view=diff
> ==============================================================================
> --- cfe/trunk/test/Analysis/simple-stream-checks.c (original)
> +++ cfe/trunk/test/Analysis/simple-stream-checks.c Tue Oct 30 21:32:41 2012
> @@ -42,3 +42,10 @@
> fclose(F);
> }
> }
> +
> +void CloseOnlyOnValidFileHandle() {
> + FILE *F = fopen("myfile.txt", "w");
> + if (F)
> + fclose(F);
> + int x = 0; // no warning
> +}
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
More information about the cfe-commits
mailing list