From 681bc114b51c1198cdec9a165c7d3230abb8f427 Mon Sep 17 00:00:00 2001 From: Ted Kremenek Date: Tue, 16 Aug 2011 01:53:41 +0000 Subject: [PATCH] [analyzer] Enhance ConditionVisitor to understand eagerly evaluated (simple) binary conditions, and teach it to only focus on constraint changes. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@137705 91177308-0d34-0410-b5e6-96231b3b80d8 --- .../Core/BugReporterVisitors.cpp | 143 ++++++++++++++++-- 1 file changed, 129 insertions(+), 14 deletions(-) diff --git a/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp b/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp index 143e633cd6..e13d9c9a0a 100644 --- a/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp +++ b/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp @@ -18,6 +18,7 @@ #include "clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h" #include "clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h" #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h" using namespace clang; using namespace ento; @@ -500,7 +501,15 @@ public: const DeclRefExpr *DR, const bool tookTrue, BugReporterContext &BRC); - + + PathDiagnosticPiece *VisitTrueTest(const Expr *Cond, + const BinaryOperator *BExpr, + const bool tookTrue, + BugReporterContext &BRC); + + void patternMatch(const Expr *Ex, + llvm::raw_ostream &Out, + BugReporterContext &BRC); }; } @@ -509,21 +518,40 @@ PathDiagnosticPiece *ConditionVisitor::VisitNode(const ExplodedNode *N, BugReporterContext &BRC) { const ProgramPoint &progPoint = N->getLocation(); + + const ProgramState *CurrentState = N->getState(); + const ProgramState *PrevState = Prev->getState(); + + // Compare the GDMs of the state, because that is where constraints + // are managed. Note that ensure that we only look at nodes that + // were generated by the analyzer engine proper, not checkers. + if (CurrentState->getGDM().getRoot() == + PrevState->getGDM().getRoot()) + return 0; // If an assumption was made on a branch, it should be caught // here by looking at the state transition. if (const BlockEdge *BE = dyn_cast(&progPoint)) { - const CFGBlock *srcBlk = BE->getSrc(); - - if (const Stmt *term = srcBlk->getTerminator()) { - const ProgramState *CurrentState = N->getState(); - const ProgramState *PrevState = Prev->getState(); - if (CurrentState != PrevState) - return VisitTerminator(term, CurrentState, PrevState, - srcBlk, BE->getDst(), - BRC); - } - + const CFGBlock *srcBlk = BE->getSrc(); + if (const Stmt *term = srcBlk->getTerminator()) + return VisitTerminator(term, CurrentState, PrevState, + srcBlk, BE->getDst(), BRC); + return 0; + } + + if (const PostStmt *PS = dyn_cast(&progPoint)) { + // FIXME: Assuming that BugReporter is a GRBugReporter is a layering + // violation. + const std::pair &tags = + cast(BRC.getBugReporter()). + getEngine().getEagerlyAssumeTags(); + + const ProgramPointTag *tag = PS->getTag(); + if (tag == tags.first) + return VisitTrueTest(cast(PS->getStmt()), true, BRC); + if (tag == tags.second) + return VisitTrueTest(cast(PS->getStmt()), false, BRC); + return 0; } @@ -538,7 +566,6 @@ ConditionVisitor::VisitTerminator(const Stmt *Term, const CFGBlock *dstBlk, BugReporterContext &BRC) { - assert(CurrentState != PrevState); const Expr *Cond = 0; switch (Term->getStmtClass()) { @@ -566,10 +593,13 @@ ConditionVisitor::VisitTrueTest(const Expr *Cond, const Expr *Ex = Cond; - while (true) + while (true) { + Ex = Ex->IgnoreParens(); switch (Ex->getStmtClass()) { default: return 0; + case Stmt::BinaryOperatorClass: + return VisitTrueTest(Cond, cast(Cond), tookTrue, BRC); case Stmt::DeclRefExprClass: return VisitTrueTest(Cond, cast(Ex), tookTrue, BRC); case Stmt::UnaryOperatorClass: { @@ -582,6 +612,91 @@ ConditionVisitor::VisitTrueTest(const Expr *Cond, return 0; } } + } +} + +void ConditionVisitor::patternMatch(const Expr *Ex, llvm::raw_ostream &Out, + BugReporterContext &BRC) { + const Expr *OriginalExpr = Ex; + Ex = Ex->IgnoreParenCasts(); + + if (const DeclRefExpr *DR = dyn_cast(Ex)) { + if (const VarDecl *VD = dyn_cast(DR->getDecl())) + Out << VD->getDeclName().getAsString(); + return; + } + + if (const IntegerLiteral *IL = dyn_cast(Ex)) { + QualType OriginalTy = OriginalExpr->getType(); + if (OriginalTy->isPointerType()) { + if (IL->getValue() == 0) { + Out << "null"; + return; + } + } + else if (OriginalTy->isObjCObjectPointerType()) { + if (IL->getValue() == 0) { + Out << "nil"; + return; + } + } + + Out << IL->getValue(); + return; + } +} + +PathDiagnosticPiece * +ConditionVisitor::VisitTrueTest(const Expr *Cond, + const BinaryOperator *BExpr, + const bool tookTrue, + BugReporterContext &BRC) { + + llvm::SmallString<128> LhsString, RhsString; + { + llvm::raw_svector_ostream OutLHS(LhsString), OutRHS(RhsString); + patternMatch(BExpr->getLHS(), OutLHS, BRC); + patternMatch(BExpr->getRHS(), OutRHS, BRC); + } + + if (LhsString.empty() || RhsString.empty()) + return 0; + + llvm::SmallString<256> buf; + llvm::raw_svector_ostream Out(buf); + Out << "Assuming " << LhsString << " is "; + + // Do we need to invert the opcode? + BinaryOperator::Opcode Op = BExpr->getOpcode(); + + if (!tookTrue) + switch (Op) { + case BO_EQ: Op = BO_NE; break; + case BO_NE: Op = BO_EQ; break; + case BO_LT: Op = BO_GE; break; + case BO_GT: Op = BO_LE; break; + case BO_LE: Op = BO_GT; break; + case BO_GE: Op = BO_GE; break; + default: + return 0; + } + + switch (BExpr->getOpcode()) { + case BO_EQ: + Out << "equal to "; + break; + case BO_NE: + Out << "not equal to "; + break; + default: + Out << BinaryOperator::getOpcodeStr(Op) << ' '; + break; + } + + Out << RhsString; + + PathDiagnosticLocation Loc(Cond, BRC.getSourceManager()); + return new PathDiagnosticEventPiece(Loc, Out.str()); } PathDiagnosticPiece * -- 2.40.0