diff options
Diffstat (limited to 'clang/lib')
-rw-r--r-- | clang/lib/Analysis/BugReporter.cpp | 173 | ||||
-rw-r--r-- | clang/lib/Analysis/CFRefCount.cpp | 15 | ||||
-rw-r--r-- | clang/lib/Analysis/PathDiagnostic.cpp | 2 | ||||
-rw-r--r-- | clang/lib/Frontend/HTMLDiagnostics.cpp | 35 | ||||
-rw-r--r-- | clang/lib/Frontend/PlistDiagnostics.cpp | 35 |
5 files changed, 216 insertions, 44 deletions
diff --git a/clang/lib/Analysis/BugReporter.cpp b/clang/lib/Analysis/BugReporter.cpp index 1d1c2fa7d00..f54200b0407 100644 --- a/clang/lib/Analysis/BugReporter.cpp +++ b/clang/lib/Analysis/BugReporter.cpp @@ -154,6 +154,14 @@ public: 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 @@ PathDiagnosticLocation 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 @@ static void GenerateMinimalPathDiagnostic(PathDiagnostic& PD, 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 @@ static void GenerateMinimalPathDiagnostic(PathDiagnostic& PD, 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 @@ static void GenerateMinimalPathDiagnostic(PathDiagnostic& PD, } //===----------------------------------------------------------------------===// +// "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 @@ static void CompactPathDiagnostic(PathDiagnostic &PD, const SourceManager& SM) { 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 @@ void GRBugReporter::GeneratePathDiagnostic(PathDiagnostic& PD, switch (PDB.getGenerationScheme()) { case PathDiagnosticClient::Extensive: + GenerateExtensivePathDiagnostic(PD,PDB, N); + break; case PathDiagnosticClient::Minimal: GenerateMinimalPathDiagnostic(PD, PDB, N); break; diff --git a/clang/lib/Analysis/CFRefCount.cpp b/clang/lib/Analysis/CFRefCount.cpp index 6e43ec5fafb..c9917da82ee 100644 --- a/clang/lib/Analysis/CFRefCount.cpp +++ b/clang/lib/Analysis/CFRefCount.cpp @@ -2635,13 +2635,8 @@ PathDiagnosticPiece* CFRefReport::VisitNode(const ExplodedNode<GRState>* N, 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 @@ PathDiagnosticPiece* CFRefReport::VisitNode(const ExplodedNode<GRState>* N, 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 @@ CFRefLeakReport::getEndPath(BugReporter& br, const ExplodedNode<GRState>* EndN){ 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/clang/lib/Analysis/PathDiagnostic.cpp b/clang/lib/Analysis/PathDiagnostic.cpp index 02fe1658ad2..69b11fb5dac 100644 --- a/clang/lib/Analysis/PathDiagnostic.cpp +++ b/clang/lib/Analysis/PathDiagnostic.cpp @@ -140,6 +140,7 @@ void PathDiagnosticClient::HandleDiagnostic(Diagnostic::Level DiagLevel, //===----------------------------------------------------------------------===// 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 @@ FullSourceLoc PathDiagnosticLocation::asLocation() const { } 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/clang/lib/Frontend/HTMLDiagnostics.cpp b/clang/lib/Frontend/HTMLDiagnostics.cpp index 0b266cda0f0..fbca057849b 100644 --- a/clang/lib/Frontend/HTMLDiagnostics.cpp +++ b/clang/lib/Frontend/HTMLDiagnostics.cpp @@ -130,12 +130,12 @@ void HTMLDiagnostics::ReportDiag(const PathDiagnostic& D) { 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 @@ void HTMLDiagnostics::ReportDiag(const PathDiagnostic& D) { 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 @@ void HTMLDiagnostics::ReportDiag(const PathDiagnostic& D) { 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 @@ void HTMLDiagnostics::ReportDiag(const PathDiagnostic& D) { 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 @@ void HTMLDiagnostics::HandlePiece(Rewriter& R, FileID BugFileID, // 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 @@ void HTMLDiagnostics::HandlePiece(Rewriter& R, FileID BugFileID, // 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/clang/lib/Frontend/PlistDiagnostics.cpp b/clang/lib/Frontend/PlistDiagnostics.cpp index 0c146e54c26..75fad19806e 100644 --- a/clang/lib/Frontend/PlistDiagnostics.cpp +++ b/clang/lib/Frontend/PlistDiagnostics.cpp @@ -40,7 +40,7 @@ namespace { ~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 @@ clang::CreatePlistDiagnosticClient(const std::string& s, } 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 @@ static void AddFID(FIDMap &FIDs, llvm::SmallVectorImpl<FileID> &V, 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 @@ static llvm::raw_ostream& Indent(llvm::raw_ostream& o, const unsigned indent) { 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 @@ static void EmitLocation(llvm::raw_ostream& o, SourceManager* SM, 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 llvm::raw_ostream& EmitString(llvm::raw_ostream& o, 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 ReportControlFlow(llvm::raw_ostream& o, } 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 @@ static void ReportEvent(llvm::raw_ostream& o, const PathDiagnosticPiece& P, 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 ReportEvent(llvm::raw_ostream& o, const PathDiagnosticPiece& P, 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 ReportMacro(llvm::raw_ostream& o, } 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 @@ PlistDiagnostics::~PlistDiagnostics() { // 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 @@ PlistDiagnostics::~PlistDiagnostics() { 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) { |