[analyzer] Refactor: make PathDiagnosticLocation responsible for validation of SourceLocations (commit 2 of ?):
- Modify all PathDiagnosticLocation constructors that take Stmt to also requre LocationContext.
- Add a constructor which should be used in case there is no valid statement/location (it will grab the location of the enclosing function).
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@139763 91177308-0d34-0410-b5e6-96231b3b80d8
diff --git a/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp b/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp
index c2b5476..2607db8 100644
--- a/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp
@@ -622,7 +622,8 @@
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 481a31a..5dd5302 100644
--- a/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
@@ -1875,7 +1875,8 @@
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 @@
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 70efacb..166b6fd 100644
--- a/lib/StaticAnalyzer/Core/BugReporter.cpp
+++ b/lib/StaticAnalyzer/Core/BugReporter.cpp
@@ -83,11 +83,6 @@
default:
break;
}
-
- // Some expressions don't have locations.
- if (S->getLocStart().isInvalid())
- continue;
-
return S;
}
@@ -151,6 +146,10 @@
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 @@
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 @@
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 @@
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 @@
switch (Parent->getStmtClass()) {
case Stmt::ForStmtClass:
case Stmt::ObjCForCollectionStmtClass:
- return PathDiagnosticLocation(Parent, SMgr);
+ return PathDiagnosticLocation(Parent, SMgr, LC);
default:
break;
}
@@ -314,11 +314,11 @@
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 @@
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 @@
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 @@
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 @@
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 @@
}
if (S != Original)
- L = PathDiagnosticLocation(S, L.getManager());
+ L = PathDiagnosticLocation(S, L.getManager(), L.getLocationContext());
}
if (firstCharOnly)
@@ -1091,7 +1092,7 @@
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 @@
// 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 5a56afb..c6fd63f 100644
--- a/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ b/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -90,9 +90,8 @@
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 @@
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 @@
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 @@
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 @@
// 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 @@
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 @@
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::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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 ac9b15e..8d53b2b 100644
--- a/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
+++ b/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
@@ -156,23 +156,20 @@
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 @@
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));
}