On Oct 30, 2012, at 19:32 , Anna Zaks <[email protected]> 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
> [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
