diff options
7 files changed, 87 insertions, 69 deletions
diff --git a/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitor.h b/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitor.h index 85ac9820db..41c0a3a46b 100644 --- a/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitor.h +++ b/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitor.h @@ -138,25 +138,27 @@ public: BugReport &BR); PathDiagnosticPiece *VisitTerminator(const Stmt *Term, - const ProgramState *CurrentState, - const ProgramState *PrevState, + const ExplodedNode *N, const CFGBlock *srcBlk, const CFGBlock *dstBlk, BugReporterContext &BRC); PathDiagnosticPiece *VisitTrueTest(const Expr *Cond, bool tookTrue, - BugReporterContext &BRC); + BugReporterContext &BRC, + const LocationContext *LC); PathDiagnosticPiece *VisitTrueTest(const Expr *Cond, const DeclRefExpr *DR, const bool tookTrue, - BugReporterContext &BRC); + BugReporterContext &BRC, + const LocationContext *LC); PathDiagnosticPiece *VisitTrueTest(const Expr *Cond, const BinaryOperator *BExpr, const bool tookTrue, - BugReporterContext &BRC); + BugReporterContext &BRC, + const LocationContext *LC); bool patternMatch(const Expr *Ex, llvm::raw_ostream &Out, diff --git a/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h b/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h index bc783a69b2..c6edfab7b5 100644 --- a/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h +++ b/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h @@ -97,21 +97,26 @@ public: PathDiagnosticLocation(FullSourceLoc L) : K(SingleLocK), R(L, L), S(0), D(0), SM(&L.getManager()), LC(0) {} + /// Constructs a location when no specific statement is available. + /// Defaults to end of brace for the enclosing function body. + PathDiagnosticLocation(const LocationContext *lc, const SourceManager &sm) + : K(SingleLocK), S(0), D(0), SM(&sm), LC(lc) {} + PathDiagnosticLocation(const Stmt *s, const SourceManager &sm, - const LocationContext *lc = 0) + const LocationContext *lc) : K(StmtK), S(s), D(0), SM(&sm), LC(lc) {} - /// Create a location corresponding to the next valid ExplodedNode. - static PathDiagnosticLocation create(const ExplodedNode* N, - const SourceManager &SM); - PathDiagnosticLocation(SourceRange r, const SourceManager &sm) : K(RangeK), R(r), S(0), D(0), SM(&sm), LC(0) {} PathDiagnosticLocation(const Decl *d, const SourceManager &sm) : K(DeclK), S(0), D(d), SM(&sm), LC(0) {} + /// Create a location corresponding to the next valid ExplodedNode. + static PathDiagnosticLocation create(const ExplodedNode* N, + const SourceManager &SM); + bool operator==(const PathDiagnosticLocation &X) const { return K == X.K && R == X.R && S == X.S && D == X.D && LC == X.LC; } @@ -138,6 +143,7 @@ public: void flatten(); const SourceManager& getManager() const { assert(isValid()); return *SM; } + const LocationContext* getLocationContext() const { return LC; } void Profile(llvm::FoldingSetNodeID &ID) const; }; diff --git a/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp b/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp index c2b547612c..2607db80ba 100644 --- a/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp @@ -622,7 +622,8 @@ PathDiagnosticPiece *MacOSKeychainAPIChecker::SecKeychainBugVisitor::VisitNode( unsigned Idx = getTrackedFunctionIndex(funName, true); assert(Idx != InvalidIdx && "This should be a call to an allocator."); const Expr *ArgExpr = CE->getArg(FunctionsToTrack[Idx].Param); - PathDiagnosticLocation Pos(ArgExpr, BRC.getSourceManager()); + PathDiagnosticLocation Pos(ArgExpr, BRC.getSourceManager(), + N->getLocationContext()); return new PathDiagnosticEventPiece(Pos, "Data is allocated here."); } diff --git a/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp b/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp index 481a31a045..5dd53024b9 100644 --- a/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp @@ -1875,7 +1875,8 @@ PathDiagnosticPiece *CFRefReportVisitor::VisitNode(const ExplodedNode *N, os << "+0 retain count"; } - PathDiagnosticLocation Pos(S, BRC.getSourceManager()); + PathDiagnosticLocation Pos(S, BRC.getSourceManager(), + N->getLocationContext()); return new PathDiagnosticEventPiece(Pos, os.str()); } @@ -2039,7 +2040,8 @@ PathDiagnosticPiece *CFRefReportVisitor::VisitNode(const ExplodedNode *N, return 0; // We have nothing to say! const Stmt *S = cast<StmtPoint>(N->getLocation()).getStmt(); - PathDiagnosticLocation Pos(S, BRC.getSourceManager()); + PathDiagnosticLocation Pos(S, BRC.getSourceManager(), + N->getLocationContext()); PathDiagnosticPiece *P = new PathDiagnosticEventPiece(Pos, os.str()); // Add the range by scanning the children of the statement for any bindings diff --git a/lib/StaticAnalyzer/Core/BugReporter.cpp b/lib/StaticAnalyzer/Core/BugReporter.cpp index 70efacbd34..166b6fdd09 100644 --- a/lib/StaticAnalyzer/Core/BugReporter.cpp +++ b/lib/StaticAnalyzer/Core/BugReporter.cpp @@ -83,11 +83,6 @@ static const Stmt *GetNextStmt(const ExplodedNode *N) { default: break; } - - // Some expressions don't have locations. - if (S->getLocStart().isInvalid()) - continue; - return S; } @@ -151,6 +146,10 @@ public: Decl const &getCodeDecl() { return R->getErrorNode()->getCodeDecl(); } + const LocationContext* getLocationContext() { + return R->getErrorNode()->getLocationContext(); + } + ParentMap& getParentMap() { return R->getErrorNode()->getParentMap(); } const Stmt *getParent(const Stmt *S) { @@ -174,7 +173,7 @@ public: PathDiagnosticLocation PathDiagnosticBuilder::ExecutionContinues(const ExplodedNode *N) { if (const Stmt *S = GetNextStmt(N)) - return PathDiagnosticLocation(S, getSourceManager()); + return PathDiagnosticLocation(S, getSourceManager(), getLocationContext()); return FullSourceLoc(N->getLocationContext()->getDecl()->getBodyRBrace(), getSourceManager()); @@ -235,6 +234,7 @@ PathDiagnosticBuilder::getEnclosingStmtLocation(const Stmt *S) { assert(S && "Null Stmt *passed to getEnclosingStmtLocation"); ParentMap &P = getParentMap(); SourceManager &SMgr = getSourceManager(); + const LocationContext *LC = getLocationContext(); while (IsNested(S, P)) { const Stmt *Parent = P.getParentIgnoreParens(S); @@ -246,44 +246,44 @@ PathDiagnosticBuilder::getEnclosingStmtLocation(const Stmt *S) { case Stmt::BinaryOperatorClass: { const BinaryOperator *B = cast<BinaryOperator>(Parent); if (B->isLogicalOp()) - return PathDiagnosticLocation(S, SMgr); + return PathDiagnosticLocation(S, SMgr, LC); break; } case Stmt::CompoundStmtClass: case Stmt::StmtExprClass: - return PathDiagnosticLocation(S, SMgr); + return PathDiagnosticLocation(S, SMgr, LC); case Stmt::ChooseExprClass: // Similar to '?' if we are referring to condition, just have the edge // point to the entire choose expression. if (cast<ChooseExpr>(Parent)->getCond() == S) - return PathDiagnosticLocation(Parent, SMgr); + return PathDiagnosticLocation(Parent, SMgr, LC); else - return PathDiagnosticLocation(S, SMgr); + return PathDiagnosticLocation(S, SMgr, LC); case Stmt::BinaryConditionalOperatorClass: case Stmt::ConditionalOperatorClass: // For '?', if we are referring to condition, just have the edge point // to the entire '?' expression. if (cast<AbstractConditionalOperator>(Parent)->getCond() == S) - return PathDiagnosticLocation(Parent, SMgr); + return PathDiagnosticLocation(Parent, SMgr, LC); else - return PathDiagnosticLocation(S, SMgr); + return PathDiagnosticLocation(S, SMgr, LC); case Stmt::DoStmtClass: - return PathDiagnosticLocation(S, SMgr); + return PathDiagnosticLocation(S, SMgr, LC); case Stmt::ForStmtClass: if (cast<ForStmt>(Parent)->getBody() == S) - return PathDiagnosticLocation(S, SMgr); + return PathDiagnosticLocation(S, SMgr, LC); break; case Stmt::IfStmtClass: if (cast<IfStmt>(Parent)->getCond() != S) - return PathDiagnosticLocation(S, SMgr); + return PathDiagnosticLocation(S, SMgr, LC); break; case Stmt::ObjCForCollectionStmtClass: if (cast<ObjCForCollectionStmt>(Parent)->getBody() == S) - return PathDiagnosticLocation(S, SMgr); + return PathDiagnosticLocation(S, SMgr, LC); break; case Stmt::WhileStmtClass: if (cast<WhileStmt>(Parent)->getCond() != S) - return PathDiagnosticLocation(S, SMgr); + return PathDiagnosticLocation(S, SMgr, LC); break; default: break; @@ -301,7 +301,7 @@ PathDiagnosticBuilder::getEnclosingStmtLocation(const Stmt *S) { switch (Parent->getStmtClass()) { case Stmt::ForStmtClass: case Stmt::ObjCForCollectionStmtClass: - return PathDiagnosticLocation(Parent, SMgr); + return PathDiagnosticLocation(Parent, SMgr, LC); default: break; } @@ -314,11 +314,11 @@ PathDiagnosticBuilder::getEnclosingStmtLocation(const Stmt *S) { if (const ForStmt *FS = dyn_cast_or_null<ForStmt>(P.getParentIgnoreParens(S))) { if (FS->getInit() == S) - return PathDiagnosticLocation(FS, SMgr); + return PathDiagnosticLocation(FS, SMgr, LC); } } - return PathDiagnosticLocation(S, SMgr); + return PathDiagnosticLocation(S, SMgr, LC); } //===----------------------------------------------------------------------===// @@ -516,6 +516,7 @@ static void GenerateMinimalPathDiagnostic(PathDiagnostic& PD, const ExplodedNode *N) { SourceManager& SMgr = PDB.getSourceManager(); + const LocationContext *LC = PDB.getLocationContext(); const ExplodedNode *NextNode = N->pred_empty() ? NULL : *(N->pred_begin()); while (NextNode) { @@ -562,7 +563,7 @@ static void GenerateMinimalPathDiagnostic(PathDiagnostic& PD, llvm::raw_string_ostream os(sbuf); if (const Stmt *S = Dst->getLabel()) { - PathDiagnosticLocation End(S, SMgr); + PathDiagnosticLocation End(S, SMgr, LC); switch (S->getStmtClass()) { default: @@ -663,14 +664,14 @@ static void GenerateMinimalPathDiagnostic(PathDiagnostic& PD, if (*(Src->succ_begin()+1) == Dst) { os << "false"; - PathDiagnosticLocation End(B->getLHS(), SMgr); + PathDiagnosticLocation End(B->getLHS(), SMgr, LC); PathDiagnosticLocation Start(B->getOperatorLoc(), SMgr); PD.push_front(new PathDiagnosticControlFlowPiece(Start, End, os.str())); } else { os << "true"; - PathDiagnosticLocation Start(B->getLHS(), SMgr); + PathDiagnosticLocation Start(B->getLHS(), SMgr, LC); PathDiagnosticLocation End = PDB.ExecutionContinues(N); PD.push_front(new PathDiagnosticControlFlowPiece(Start, End, os.str())); @@ -682,14 +683,14 @@ static void GenerateMinimalPathDiagnostic(PathDiagnostic& PD, if (*(Src->succ_begin()+1) == Dst) { os << "false"; - PathDiagnosticLocation Start(B->getLHS(), SMgr); + PathDiagnosticLocation Start(B->getLHS(), SMgr, LC); PathDiagnosticLocation End = PDB.ExecutionContinues(N); PD.push_front(new PathDiagnosticControlFlowPiece(Start, End, os.str())); } else { os << "true"; - PathDiagnosticLocation End(B->getLHS(), SMgr); + PathDiagnosticLocation End(B->getLHS(), SMgr, LC); PathDiagnosticLocation Start(B->getOperatorLoc(), SMgr); PD.push_front(new PathDiagnosticControlFlowPiece(Start, End, os.str())); @@ -874,7 +875,7 @@ class EdgeBuilder { } if (S != Original) - L = PathDiagnosticLocation(S, L.getManager()); + L = PathDiagnosticLocation(S, L.getManager(), L.getLocationContext()); } if (firstCharOnly) @@ -1091,7 +1092,7 @@ void EdgeBuilder::addContext(const Stmt *S) { if (!S) return; - PathDiagnosticLocation L(S, PDB.getSourceManager()); + PathDiagnosticLocation L(S, PDB.getSourceManager(), PDB.getLocationContext()); while (!CLocs.empty()) { const PathDiagnosticLocation &TopContextLoc = CLocs.back(); @@ -1131,7 +1132,8 @@ static void GenerateExtensivePathDiagnostic(PathDiagnostic& PD, // Are we jumping to the head of a loop? Add a special diagnostic. if (const Stmt *Loop = BE->getDst()->getLoopTarget()) { - PathDiagnosticLocation L(Loop, PDB.getSourceManager()); + PathDiagnosticLocation L(Loop, PDB.getSourceManager(), + PDB.getLocationContext()); const CompoundStmt *CS = NULL; if (!Term) { diff --git a/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp b/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp index 5a56afb056..c6fd63f3bd 100644 --- a/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp +++ b/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp @@ -90,9 +90,8 @@ BugReporterVisitor::getDefaultEndPath(BugReporterContext &BRC, if (const BlockEntrance *BE = dyn_cast<BlockEntrance>(&PP)) { const CFGBlock *block = BE->getBlock(); if (block->getBlockID() == 0) { - L = PathDiagnosticLocation( - EndPathNode->getLocationContext()->getDecl()->getBodyRBrace(), - BRC.getSourceManager()); + L = PathDiagnosticLocation(PP.getLocationContext(), + BRC.getSourceManager()); } } @@ -102,7 +101,8 @@ BugReporterVisitor::getDefaultEndPath(BugReporterContext &BRC, if (!S) return NULL; - L = PathDiagnosticLocation(S, BRC.getSourceManager()); + L = PathDiagnosticLocation(S, BRC.getSourceManager(), + PP.getLocationContext()); } BugReport::ranges_iterator Beg, End; @@ -254,7 +254,7 @@ PathDiagnosticPiece *FindLastStoreBRVisitor::VisitNode(const ExplodedNode *N, return NULL; // Construct a new PathDiagnosticPiece. - PathDiagnosticLocation L(S, BRC.getSourceManager()); + PathDiagnosticLocation L(S, BRC.getSourceManager(), P.getLocationContext()); return new PathDiagnosticEventPiece(L, os.str()); } @@ -314,7 +314,7 @@ TrackConstraintBRVisitor::VisitNode(const ExplodedNode *N, return NULL; // Construct a new PathDiagnosticPiece. - PathDiagnosticLocation L(S, BRC.getSourceManager()); + PathDiagnosticLocation L(S, BRC.getSourceManager(), P.getLocationContext()); return new PathDiagnosticEventPiece(L, os.str()); } @@ -424,7 +424,8 @@ PathDiagnosticPiece *NilReceiverBRVisitor::VisitNode(const ExplodedNode *N, // the receiver was null. BR.addVisitor(bugreporter::getTrackNullOrUndefValueVisitor(N, Receiver)); // Issue a message saying that the method was skipped. - PathDiagnosticLocation L(Receiver, BRC.getSourceManager()); + PathDiagnosticLocation L(Receiver, BRC.getSourceManager(), + N->getLocationContext()); return new PathDiagnosticEventPiece(L, "No method actually called " "because the receiver is nil"); } @@ -489,8 +490,7 @@ PathDiagnosticPiece *ConditionBRVisitor::VisitNode(const ExplodedNode *N, if (const BlockEdge *BE = dyn_cast<BlockEdge>(&progPoint)) { const CFGBlock *srcBlk = BE->getSrc(); if (const Stmt *term = srcBlk->getTerminator()) - return VisitTerminator(term, CurrentState, PrevState, - srcBlk, BE->getDst(), BRC); + return VisitTerminator(term, N, srcBlk, BE->getDst(), BRC); return 0; } @@ -503,9 +503,11 @@ PathDiagnosticPiece *ConditionBRVisitor::VisitNode(const ExplodedNode *N, const ProgramPointTag *tag = PS->getTag(); if (tag == tags.first) - return VisitTrueTest(cast<Expr>(PS->getStmt()), true, BRC); + return VisitTrueTest(cast<Expr>(PS->getStmt()), true, + BRC, N->getLocationContext()); if (tag == tags.second) - return VisitTrueTest(cast<Expr>(PS->getStmt()), false, BRC); + return VisitTrueTest(cast<Expr>(PS->getStmt()), false, + BRC, N->getLocationContext()); return 0; } @@ -515,8 +517,7 @@ PathDiagnosticPiece *ConditionBRVisitor::VisitNode(const ExplodedNode *N, PathDiagnosticPiece * ConditionBRVisitor::VisitTerminator(const Stmt *Term, - const ProgramState *CurrentState, - const ProgramState *PrevState, + const ExplodedNode *N, const CFGBlock *srcBlk, const CFGBlock *dstBlk, BugReporterContext &BRC) { @@ -537,13 +538,14 @@ ConditionBRVisitor::VisitTerminator(const Stmt *Term, assert(srcBlk->succ_size() == 2); const bool tookTrue = *(srcBlk->succ_begin()) == dstBlk; return VisitTrueTest(Cond->IgnoreParenNoopCasts(BRC.getASTContext()), - tookTrue, BRC); + tookTrue, BRC, N->getLocationContext()); } PathDiagnosticPiece * ConditionBRVisitor::VisitTrueTest(const Expr *Cond, bool tookTrue, - BugReporterContext &BRC) { + BugReporterContext &BRC, + const LocationContext *LC) { const Expr *Ex = Cond; @@ -553,9 +555,9 @@ ConditionBRVisitor::VisitTrueTest(const Expr *Cond, default: return 0; case Stmt::BinaryOperatorClass: - return VisitTrueTest(Cond, cast<BinaryOperator>(Ex), tookTrue, BRC); + return VisitTrueTest(Cond, cast<BinaryOperator>(Ex), tookTrue, BRC, LC); case Stmt::DeclRefExprClass: - return VisitTrueTest(Cond, cast<DeclRefExpr>(Ex), tookTrue, BRC); + return VisitTrueTest(Cond, cast<DeclRefExpr>(Ex), tookTrue, BRC, LC); case Stmt::UnaryOperatorClass: { const UnaryOperator *UO = cast<UnaryOperator>(Ex); if (UO->getOpcode() == UO_LNot) { @@ -610,7 +612,8 @@ PathDiagnosticPiece * ConditionBRVisitor::VisitTrueTest(const Expr *Cond, const BinaryOperator *BExpr, const bool tookTrue, - BugReporterContext &BRC) { + BugReporterContext &BRC, + const LocationContext *LC) { bool shouldInvert = false; @@ -670,7 +673,7 @@ ConditionBRVisitor::VisitTrueTest(const Expr *Cond, Out << (shouldInvert ? LhsString : RhsString); - PathDiagnosticLocation Loc(Cond, BRC.getSourceManager()); + PathDiagnosticLocation Loc(Cond, BRC.getSourceManager(), LC); return new PathDiagnosticEventPiece(Loc, Out.str()); } @@ -678,7 +681,8 @@ PathDiagnosticPiece * ConditionBRVisitor::VisitTrueTest(const Expr *Cond, const DeclRefExpr *DR, const bool tookTrue, - BugReporterContext &BRC) { + BugReporterContext &BRC, + const LocationContext *LC) { const VarDecl *VD = dyn_cast<VarDecl>(DR->getDecl()); if (!VD) @@ -702,6 +706,6 @@ ConditionBRVisitor::VisitTrueTest(const Expr *Cond, else return 0; - PathDiagnosticLocation Loc(Cond, BRC.getSourceManager()); + PathDiagnosticLocation Loc(Cond, BRC.getSourceManager(), LC); return new PathDiagnosticEventPiece(Loc, Out.str()); } diff --git a/lib/StaticAnalyzer/Core/PathDiagnostic.cpp b/lib/StaticAnalyzer/Core/PathDiagnostic.cpp index ac9b15e743..8d53b2b380 100644 --- a/lib/StaticAnalyzer/Core/PathDiagnostic.cpp +++ b/lib/StaticAnalyzer/Core/PathDiagnostic.cpp @@ -156,23 +156,20 @@ PathDiagnosticLocation PathDiagnosticLocation::create(const ExplodedNode* N, static SourceLocation getValidSourceLocation(const Stmt* S, const LocationContext *LC) { + assert(LC); SourceLocation L = S->getLocStart(); // S might be a temporary statement that does not have a location in the // source code, so find an enclosing statement and use it's location. - if (!L.isValid() && LC) { - assert(LC); + if (!L.isValid()) { ParentMap & PM = LC->getParentMap(); - const Stmt *PS = S; while (!L.isValid()) { - PS = PM.getParent(PS); - L = PS->getLocStart(); + S = PM.getParent(S); + L = S->getLocStart(); } } - // TODO: either change the name or uncomment the assert. - //assert(L.isValid()); return L; } @@ -191,6 +188,10 @@ FullSourceLoc PathDiagnosticLocation::asLocation() const { return FullSourceLoc(D->getLocation(), const_cast<SourceManager&>(*SM)); } + if (!R.isValid()) + return FullSourceLoc(LC->getDecl()->getBodyRBrace(), + const_cast<SourceManager&>(*SM)); + return FullSourceLoc(R.getBegin(), const_cast<SourceManager&>(*SM)); } |