- Changed PathDiagnosticPiece::getLocation() to return a PathDiagnosticLocation
instead of a FullSourceLoc. This resulted in a bunch of small edits in various
clients.
- Updated BugReporter to include an alternate PathDiagnostic generation
algorithm for PathDiagnosticClients desiring more control-flow pieces.
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@68193 91177308-0d34-0410-b5e6-96231b3b80d8
diff --git a/include/clang/Analysis/PathDiagnostic.h b/include/clang/Analysis/PathDiagnostic.h
index 9419240..234fd18 100644
--- a/include/clang/Analysis/PathDiagnostic.h
+++ b/include/clang/Analysis/PathDiagnostic.h
@@ -59,6 +59,9 @@
const Stmt *S;
const SourceManager *SM;
public:
+ PathDiagnosticLocation()
+ : K(SingleLoc), S(0), SM(0) {}
+
PathDiagnosticLocation(FullSourceLoc L)
: K(SingleLoc), R(L, L), S(0), SM(&L.getManager()) {}
@@ -75,10 +78,30 @@
bool operator!=(const PathDiagnosticLocation &X) const {
return K != X.K || R != X.R || S != X.S;
}
+
+ PathDiagnosticLocation& operator=(const PathDiagnosticLocation &X) {
+ K = X.K;
+ R = X.R;
+ S = X.S;
+ SM = X.SM;
+ return *this;
+ }
+
+ bool isValid() const {
+ return SM != 0;
+ }
FullSourceLoc asLocation() const;
SourceRange asRange() const;
- const Stmt *asStmt() const { return S; }
+ const Stmt *asStmt() const { assert(isValid()); return S; }
+
+ bool hasRange() const { return K == Statement || K == Range; }
+
+ void invalidate() {
+ *this = PathDiagnosticLocation();
+ }
+
+ const SourceManager& getManager() const { assert(isValid()); return *SM; }
};
class PathDiagnosticLocationPair {
@@ -130,7 +153,7 @@
/// be displayed by the PathDiagnosticClient.
DisplayHint getDisplayHint() const { return Hint; }
- virtual FullSourceLoc getLocation() const = 0;
+ virtual PathDiagnosticLocation getLocation() const = 0;
Kind getKind() const { return kind; }
@@ -176,13 +199,15 @@
public:
PathDiagnosticSpotPiece(const PathDiagnosticLocation &pos,
const std::string& s,
- PathDiagnosticPiece::Kind k)
+ PathDiagnosticPiece::Kind k,
+ bool addPosRange = true)
: PathDiagnosticPiece(s, k), Pos(pos) {
assert(Pos.asLocation().isValid() &&
"PathDiagnosticSpotPiece's must have a valid location.");
+ if (addPosRange && Pos.hasRange()) addRange(Pos.asRange());
}
- FullSourceLoc getLocation() const { return Pos.asLocation(); }
+ PathDiagnosticLocation getLocation() const { return Pos; }
};
class PathDiagnosticEventPiece : public PathDiagnosticSpotPiece {
@@ -227,21 +252,21 @@
~PathDiagnosticControlFlowPiece();
- FullSourceLoc getStartLocation() const {
+ PathDiagnosticLocation getStartLocation() const {
assert(!LPairs.empty() &&
"PathDiagnosticControlFlowPiece needs at least one location.");
- return LPairs[0].getStart().asLocation();
+ return LPairs[0].getStart();
}
- FullSourceLoc getEndLocation() const {
+ PathDiagnosticLocation getEndLocation() const {
assert(!LPairs.empty() &&
"PathDiagnosticControlFlowPiece needs at least one location.");
- return LPairs[0].getEnd().asLocation();
+ return LPairs[0].getEnd();
}
void push_back(const PathDiagnosticLocationPair &X) { LPairs.push_back(X); }
- virtual FullSourceLoc getLocation() const { return getStartLocation(); }
+ virtual PathDiagnosticLocation getLocation() const { return getStartLocation(); }
typedef std::vector<PathDiagnosticLocationPair>::iterator iterator;
iterator begin() { return LPairs.begin(); }
@@ -313,7 +338,7 @@
void addMeta(const std::string& s) { OtherDesc.push_back(s); }
void addMeta(const char* s) { OtherDesc.push_back(s); }
- FullSourceLoc getLocation() const {
+ PathDiagnosticLocation getLocation() const {
assert(Size > 0 && "getLocation() requires a non-empty PathDiagnostic.");
return rbegin()->getLocation();
}
diff --git a/lib/Analysis/BugReporter.cpp b/lib/Analysis/BugReporter.cpp
index 1d1c2fa..f54200b 100644
--- a/lib/Analysis/BugReporter.cpp
+++ b/lib/Analysis/BugReporter.cpp
@@ -154,6 +154,14 @@
PathDiagnosticLocation getEnclosingStmtLocation(const Stmt *S);
+ PathDiagnosticLocation
+ getEnclosingStmtLocation(const PathDiagnosticLocation &L) {
+ if (const Stmt *S = L.asStmt())
+ return getEnclosingStmtLocation(S);
+
+ return L;
+ }
+
PathDiagnosticClient::PathGenerationScheme getGenerationScheme() const {
return PDC ? PDC->getGenerationScheme() : PathDiagnosticClient::Extensive;
}
@@ -196,13 +204,21 @@
PathDiagnosticBuilder::getEnclosingStmtLocation(const Stmt *S) {
assert(S && "Null Stmt* passed to getEnclosingStmtLocation");
ParentMap &P = getParentMap();
- while (isa<Expr>(S)) {
+
+ while (isa<DeclStmt>(S) || isa<Expr>(S)) {
const Stmt *Parent = P.getParent(S);
if (!Parent)
break;
switch (Parent->getStmtClass()) {
+ case Stmt::BinaryOperatorClass: {
+ const BinaryOperator *B = cast<BinaryOperator>(Parent);
+ if (B->isLogicalOp())
+ return PathDiagnosticLocation(S, SMgr);
+ break;
+ }
+
case Stmt::CompoundStmtClass:
case Stmt::StmtExprClass:
return PathDiagnosticLocation(S, SMgr);
@@ -674,7 +690,7 @@
End = PDB.getEnclosingStmtLocation(S);
PD.push_front(new PathDiagnosticControlFlowPiece(Start, End,
- "Loop condition is true. Entering loop body"));
+ "Loop condition is true. Entering loop body"));
}
break;
@@ -688,10 +704,10 @@
if (*(Src->succ_begin()+1) == Dst)
PD.push_front(new PathDiagnosticControlFlowPiece(Start, End,
- "Taking false branch"));
+ "Taking false branch"));
else
PD.push_front(new PathDiagnosticControlFlowPiece(Start, End,
- "Taking true branch"));
+ "Taking true branch"));
break;
}
@@ -715,6 +731,151 @@
}
//===----------------------------------------------------------------------===//
+// "Extensive" PathDiagnostic generation.
+//===----------------------------------------------------------------------===//
+
+static bool IsControlFlowExpr(const Stmt *S) {
+ const Expr *E = dyn_cast<Expr>(S);
+
+ if (!E)
+ return false;
+
+ E = E->IgnoreParenCasts();
+
+ if (isa<ConditionalOperator>(E))
+ return true;
+
+ if (const BinaryOperator *B = dyn_cast<BinaryOperator>(E))
+ if (B->isLogicalOp())
+ return true;
+
+ return false;
+}
+
+static void GenExtAddEdge(PathDiagnostic& PD,
+ PathDiagnosticBuilder &PDB,
+ PathDiagnosticLocation NewLoc,
+ PathDiagnosticLocation &PrevLoc,
+ PathDiagnosticLocation UpdateLoc) {
+
+ if (const Stmt *S = NewLoc.asStmt()) {
+ if (IsControlFlowExpr(S))
+ return;
+ }
+
+
+ if (!PrevLoc.isValid()) {
+ PrevLoc = NewLoc;
+ return;
+ }
+
+ if (NewLoc == PrevLoc)
+ return;
+
+ PD.push_front(new PathDiagnosticControlFlowPiece(NewLoc, PrevLoc));
+ PrevLoc = UpdateLoc;
+}
+
+static bool IsNestedDeclStmt(const Stmt *S, ParentMap &PM) {
+ const DeclStmt *DS = dyn_cast<DeclStmt>(S);
+
+ if (!DS)
+ return false;
+
+ const Stmt *Parent = PM.getParent(DS);
+ if (!Parent)
+ return false;
+
+ if (const ForStmt *FS = dyn_cast<ForStmt>(Parent))
+ return FS->getInit() == DS;
+
+ // FIXME: In the future IfStmt/WhileStmt may contain DeclStmts in their condition.
+// if (const IfStmt *IF = dyn_cast<IfStmt>(Parent))
+// return IF->getCond() == DS;
+//
+// if (const WhileStmt *WS = dyn_cast<WhileStmt>(Parent))
+// return WS->getCond() == DS;
+
+ return false;
+}
+
+static void GenExtAddEdge(PathDiagnostic& PD,
+ PathDiagnosticBuilder &PDB,
+ const PathDiagnosticLocation &NewLoc,
+ PathDiagnosticLocation &PrevLoc) {
+ GenExtAddEdge(PD, PDB, NewLoc, PrevLoc, NewLoc);
+}
+
+static void GenerateExtensivePathDiagnostic(PathDiagnostic& PD,
+ PathDiagnosticBuilder &PDB,
+ const ExplodedNode<GRState> *N) {
+
+ SourceManager& SMgr = PDB.getSourceManager();
+ const ExplodedNode<GRState>* NextNode = N->pred_empty()
+ ? NULL : *(N->pred_begin());
+
+ PathDiagnosticLocation PrevLoc;
+
+ while (NextNode) {
+ N = NextNode;
+ NextNode = GetPredecessorNode(N);
+ ProgramPoint P = N->getLocation();
+
+ // Block edges.
+ if (const BlockEdge *BE = dyn_cast<BlockEdge>(&P)) {
+ const CFGBlock &Blk = *BE->getSrc();
+ if (const Stmt *Term = Blk.getTerminator()) {
+ const Stmt *Cond = Blk.getTerminatorCondition();
+
+ if (!Cond || !IsControlFlowExpr(Cond)) {
+ GenExtAddEdge(PD, PDB, PathDiagnosticLocation(Term, SMgr), PrevLoc);
+ continue;
+ }
+ }
+
+ // Only handle blocks with more than 1 statement here, as the blocks
+ // with one statement are handled at BlockEntrances.
+ if (Blk.size() > 1) {
+ const Stmt *S = *Blk.rbegin();
+
+ // We don't add control-flow edges for DeclStmt's that appear in
+ // the condition of if/while/for or are control-flow merge expressions.
+ if (!IsControlFlowExpr(S) && !IsNestedDeclStmt(S, PDB.getParentMap())) {
+ GenExtAddEdge(PD, PDB, PathDiagnosticLocation(S, SMgr), PrevLoc);
+ }
+ }
+
+ continue;
+ }
+
+ if (const BlockEntrance *BE = dyn_cast<BlockEntrance>(&P)) {
+ if (const Stmt* S = BE->getFirstStmt()) {
+ if (!IsControlFlowExpr(S) && !IsNestedDeclStmt(S, PDB.getParentMap())) {
+ // Are we jumping with the same enclosing statement?
+ if (PrevLoc.isValid() && PDB.getEnclosingStmtLocation(S) ==
+ PDB.getEnclosingStmtLocation(PrevLoc)) {
+ continue;
+ }
+
+ GenExtAddEdge(PD, PDB, PDB.getEnclosingStmtLocation(S), PrevLoc);
+ }
+ }
+
+ continue;
+ }
+
+ PathDiagnosticPiece* p =
+ PDB.getReport().VisitNode(N, NextNode, PDB.getGraph(),
+ PDB.getBugReporter(), PDB.getNodeMapClosure());
+
+ if (p) {
+ GenExtAddEdge(PD, PDB, p->getLocation(), PrevLoc);
+ PD.push_front(p);
+ }
+ }
+}
+
+//===----------------------------------------------------------------------===//
// Methods for BugType and subclasses.
//===----------------------------------------------------------------------===//
BugType::~BugType() {}
@@ -993,7 +1154,7 @@
for (PathDiagnostic::iterator I = PD.begin(), E = PD.end(); I!=E; ++I) {
// Get the location of the PathDiagnosticPiece.
- const FullSourceLoc Loc = I->getLocation();
+ const FullSourceLoc Loc = I->getLocation().asLocation();
// Determine the instantiation location, which is the location we group
// related PathDiagnosticPieces.
@@ -1114,6 +1275,8 @@
switch (PDB.getGenerationScheme()) {
case PathDiagnosticClient::Extensive:
+ GenerateExtensivePathDiagnostic(PD,PDB, N);
+ break;
case PathDiagnosticClient::Minimal:
GenerateMinimalPathDiagnostic(PD, PDB, N);
break;
diff --git a/lib/Analysis/CFRefCount.cpp b/lib/Analysis/CFRefCount.cpp
index 6e43ec5..c9917da 100644
--- a/lib/Analysis/CFRefCount.cpp
+++ b/lib/Analysis/CFRefCount.cpp
@@ -2635,13 +2635,8 @@
os << "+0 retain count (non-owning reference).";
}
- FullSourceLoc Pos(S->getLocStart(), BR.getContext().getSourceManager());
- PathDiagnosticPiece* P = new PathDiagnosticEventPiece(Pos, os.str());
-
- if (Expr* Exp = dyn_cast<Expr>(S))
- P->addRange(Exp->getSourceRange());
-
- return P;
+ PathDiagnosticLocation Pos(S, BR.getContext().getSourceManager());
+ return new PathDiagnosticEventPiece(Pos, os.str());
}
// Gather up the effects that were performed on the object at this
@@ -2797,7 +2792,7 @@
return 0; // We have nothing to say!
Stmt* S = cast<PostStmt>(N->getLocation()).getStmt();
- FullSourceLoc Pos(S->getLocStart(), BR.getContext().getSourceManager());
+ PathDiagnosticLocation Pos(S, BR.getContext().getSourceManager());
PathDiagnosticPiece* P = new PathDiagnosticEventPiece(Pos, os.str());
// Add the range by scanning the children of the statement for any bindings
@@ -2958,7 +2953,9 @@
assert(LeakN && S && "No leak site found.");
// Generate the diagnostic.
- FullSourceLoc L(S->getLocStart(), SMgr);
+ // FIXME: We need to do a better job at determing the leak site, e.g., at
+ // the end of function bodies.
+ PathDiagnosticLocation L(S, SMgr);
std::string sbuf;
llvm::raw_string_ostream os(sbuf);
diff --git a/lib/Analysis/PathDiagnostic.cpp b/lib/Analysis/PathDiagnostic.cpp
index 02fe165..69b11fb 100644
--- a/lib/Analysis/PathDiagnostic.cpp
+++ b/lib/Analysis/PathDiagnostic.cpp
@@ -140,6 +140,7 @@
//===----------------------------------------------------------------------===//
FullSourceLoc PathDiagnosticLocation::asLocation() const {
+ assert(isValid());
// Note that we want a 'switch' here so that the compiler can warn us in
// case we add more cases.
switch (K) {
@@ -154,6 +155,7 @@
}
SourceRange PathDiagnosticLocation::asRange() const {
+ assert(isValid());
// Note that we want a 'switch' here so that the compiler can warn us in
// case we add more cases.
switch (K) {
diff --git a/lib/Frontend/HTMLDiagnostics.cpp b/lib/Frontend/HTMLDiagnostics.cpp
index 0b266cd..fbca057 100644
--- a/lib/Frontend/HTMLDiagnostics.cpp
+++ b/lib/Frontend/HTMLDiagnostics.cpp
@@ -130,12 +130,12 @@
if (noDir)
return;
- SourceManager &SMgr = D.begin()->getLocation().getManager();
+ const SourceManager &SMgr = D.begin()->getLocation().getManager();
FileID FID;
// Verify that the entire path is from the same FileID.
for (PathDiagnostic::const_iterator I = D.begin(), E = D.end(); I != E; ++I) {
- FullSourceLoc L = I->getLocation().getInstantiationLoc();
+ FullSourceLoc L = I->getLocation().asLocation().getInstantiationLoc();
if (FID.isInvalid()) {
FID = SMgr.getFileID(L);
@@ -162,7 +162,7 @@
return; // FIXME: Emit a warning?
// Create a new rewriter to generate HTML.
- Rewriter R(SMgr);
+ Rewriter R(const_cast<SourceManager&>(SMgr));
// Process the path.
unsigned n = D.size();
@@ -215,18 +215,18 @@
llvm::raw_string_ostream os(s);
os << "<!-- REPORTHEADER -->\n"
- << "<h3>Bug Summary</h3>\n<table class=\"simpletable\">\n"
+ << "<h3>Bug Summary</h3>\n<table class=\"simpletable\">\n"
"<tr><td class=\"rowname\">File:</td><td>"
- << html::EscapeText(DirName)
- << html::EscapeText(Entry->getName())
- << "</td></tr>\n<tr><td class=\"rowname\">Location:</td><td>"
- "<a href=\"#EndPath\">line "
- << (*D.rbegin()).getLocation().getInstantiationLineNumber()
- << ", column "
- << (*D.rbegin()).getLocation().getInstantiationColumnNumber()
- << "</a></td></tr>\n"
- "<tr><td class=\"rowname\">Description:</td><td>"
- << D.getDescription() << "</td></tr>\n";
+ << html::EscapeText(DirName)
+ << html::EscapeText(Entry->getName())
+ << "</td></tr>\n<tr><td class=\"rowname\">Location:</td><td>"
+ "<a href=\"#EndPath\">line "
+ << (*D.rbegin()).getLocation().asLocation().getInstantiationLineNumber()
+ << ", column "
+ << (*D.rbegin()).getLocation().asLocation().getInstantiationColumnNumber()
+ << "</a></td></tr>\n"
+ "<tr><td class=\"rowname\">Description:</td><td>"
+ << D.getDescription() << "</td></tr>\n";
// Output any other meta data.
@@ -280,7 +280,8 @@
std::string s;
llvm::raw_string_ostream os(s);
os << "\n<!-- BUGLINE "
- << D.back()->getLocation().getInstantiationLineNumber() << " -->\n";
+ << D.back()->getLocation().asLocation().getInstantiationLineNumber()
+ << " -->\n";
R.InsertStrBefore(SMgr.getLocForStartOfFile(FID), os.str());
}
@@ -336,7 +337,7 @@
// For now, just draw a box above the line in question, and emit the
// warning.
- FullSourceLoc Pos = P.getLocation();
+ FullSourceLoc Pos = P.getLocation().asLocation();
if (!Pos.isValid())
return;
@@ -460,7 +461,7 @@
// Get the name of the macro by relexing it.
{
- FullSourceLoc L = MP->getLocation().getInstantiationLoc();
+ FullSourceLoc L = MP->getLocation().asLocation().getInstantiationLoc();
assert(L.isFileID());
std::pair<const char*, const char*> BufferInfo = L.getBufferData();
const char* MacroName = L.getDecomposedLoc().second + BufferInfo.first;
diff --git a/lib/Frontend/PlistDiagnostics.cpp b/lib/Frontend/PlistDiagnostics.cpp
index 0c146e5..75fad19 100644
--- a/lib/Frontend/PlistDiagnostics.cpp
+++ b/lib/Frontend/PlistDiagnostics.cpp
@@ -40,7 +40,7 @@
~PlistDiagnostics();
void HandlePathDiagnostic(const PathDiagnostic* D);
- PathGenerationScheme getGenerationScheme() const { return Extensive; }
+ PathGenerationScheme getGenerationScheme() const { return Minimal; }
bool supportsLogicalOpControlFlow() const { return true; }
bool supportsAllBlockEdges() const { return true; }
};
@@ -56,7 +56,7 @@
}
static void AddFID(FIDMap &FIDs, llvm::SmallVectorImpl<FileID> &V,
- SourceManager* SM, SourceLocation L) {
+ const SourceManager* SM, SourceLocation L) {
FileID FID = SM->getFileID(SM->getInstantiationLoc(L));
FIDMap::iterator I = FIDs.find(FID);
@@ -65,7 +65,8 @@
V.push_back(FID);
}
-static unsigned GetFID(const FIDMap& FIDs, SourceManager* SM, SourceLocation L){
+static unsigned GetFID(const FIDMap& FIDs, const SourceManager* SM,
+ SourceLocation L) {
FileID FID = SM->getFileID(SM->getInstantiationLoc(L));
FIDMap::const_iterator I = FIDs.find(FID);
assert(I != FIDs.end());
@@ -77,7 +78,7 @@
return o;
}
-static void EmitLocation(llvm::raw_ostream& o, SourceManager* SM,
+static void EmitLocation(llvm::raw_ostream& o, const SourceManager* SM,
SourceLocation L, const FIDMap& FM,
const unsigned indent) {
@@ -91,8 +92,15 @@
Indent(o, indent) << "</dict>\n";
}
-static void EmitRange(llvm::raw_ostream& o, SourceManager* SM, SourceRange R,
- const FIDMap& FM, const unsigned indent) {
+static void EmitLocation(llvm::raw_ostream& o, const SourceManager* SM,
+ const PathDiagnosticLocation &L, const FIDMap& FM,
+ const unsigned indent) {
+ EmitLocation(o, SM, L.asLocation(), FM, indent);
+}
+
+static void EmitRange(llvm::raw_ostream& o, const SourceManager* SM,
+ SourceRange R, const FIDMap& FM,
+ const unsigned indent) {
Indent(o, indent) << "<array>\n";
EmitLocation(o, SM, R.getBegin(), FM, indent+1);
@@ -120,7 +128,7 @@
static void ReportControlFlow(llvm::raw_ostream& o,
const PathDiagnosticControlFlowPiece& P,
- const FIDMap& FM, SourceManager *SM,
+ const FIDMap& FM, const SourceManager *SM,
unsigned indent) {
Indent(o, indent) << "<dict>\n";
@@ -167,7 +175,8 @@
}
static void ReportEvent(llvm::raw_ostream& o, const PathDiagnosticPiece& P,
- const FIDMap& FM, SourceManager* SM, unsigned indent) {
+ const FIDMap& FM, const SourceManager* SM,
+ unsigned indent) {
Indent(o, indent) << "<dict>\n";
++indent;
@@ -175,7 +184,7 @@
Indent(o, indent) << "<key>kind</key><string>event</string>\n";
// Output the location.
- FullSourceLoc L = P.getLocation();
+ FullSourceLoc L = P.getLocation().asLocation();
Indent(o, indent) << "<key>location</key>\n";
EmitLocation(o, SM, L, FM, indent);
@@ -211,7 +220,7 @@
static void ReportMacro(llvm::raw_ostream& o,
const PathDiagnosticMacroPiece& P,
- const FIDMap& FM, SourceManager *SM,
+ const FIDMap& FM, const SourceManager *SM,
unsigned indent) {
for (PathDiagnosticMacroPiece::const_iterator I=P.begin(), E=P.end();
@@ -231,7 +240,7 @@
}
static void ReportDiag(llvm::raw_ostream& o, const PathDiagnosticPiece& P,
- const FIDMap& FM, SourceManager* SM) {
+ const FIDMap& FM, const SourceManager* SM) {
unsigned indent = 4;
@@ -267,7 +276,7 @@
// ranges of the diagnostics.
FIDMap FM;
llvm::SmallVector<FileID, 10> Fids;
- SourceManager* SM = 0;
+ const SourceManager* SM = 0;
if (!BatchedDiags.empty())
SM = &(*BatchedDiags.begin())->begin()->getLocation().getManager();
@@ -278,7 +287,7 @@
const PathDiagnostic *D = *DI;
for (PathDiagnostic::const_iterator I=D->begin(), E=D->end(); I!=E; ++I) {
- AddFID(FM, Fids, SM, I->getLocation());
+ AddFID(FM, Fids, SM, I->getLocation().asLocation());
for (PathDiagnosticPiece::range_iterator RI=I->ranges_begin(),
RE=I->ranges_end(); RI!=RE; ++RI) {