diff --git a/clang/include/clang/Analysis/PathDiagnostic.h b/clang/include/clang/Analysis/PathDiagnostic.h index f9e595f64cfb..02b19b8f27a0 100644 --- a/clang/include/clang/Analysis/PathDiagnostic.h +++ b/clang/include/clang/Analysis/PathDiagnostic.h @@ -186,7 +186,6 @@ private: const Kind kind; const DisplayHint Hint; std::vector ranges; - std::vector SubPieces; // Do not implement: PathDiagnosticPiece(); @@ -195,10 +194,12 @@ private: protected: PathDiagnosticPiece(FullSourceLoc pos, const std::string& s, - Kind k = Event, DisplayHint hint = Below); + Kind k, DisplayHint hint = Below); PathDiagnosticPiece(FullSourceLoc pos, const char* s, - Kind k = Event, DisplayHint hint = Below); + Kind k, DisplayHint hint = Below); + + PathDiagnosticPiece(FullSourceLoc pos, Kind k, DisplayHint hint = Below); public: virtual ~PathDiagnosticPiece(); @@ -269,15 +270,24 @@ public: }; class PathDiagnosticControlFlowPiece : public PathDiagnosticPiece { + const SourceLocation EndPos; public: - PathDiagnosticControlFlowPiece(FullSourceLoc pos, const std::string& s) - : PathDiagnosticPiece(pos, s, ControlFlow) {} + PathDiagnosticControlFlowPiece(FullSourceLoc startPos, SourceLocation endPos, + const std::string& s) + : PathDiagnosticPiece(startPos, s, ControlFlow), EndPos(endPos) {} - PathDiagnosticControlFlowPiece(FullSourceLoc pos, const char* s) - : PathDiagnosticPiece(pos, s, ControlFlow) {} + PathDiagnosticControlFlowPiece(FullSourceLoc startPos, SourceLocation endPos, + const char* s) + : PathDiagnosticPiece(startPos, s, ControlFlow), EndPos(endPos) {} + + PathDiagnosticControlFlowPiece(FullSourceLoc startPos, SourceLocation endPos) + : PathDiagnosticPiece(startPos, ControlFlow), EndPos(endPos) {} ~PathDiagnosticControlFlowPiece(); + SourceLocation getStartLocation() const { return getLocation(); } + SourceLocation getEndLocation() const { return EndPos; } + static inline bool classof(const PathDiagnosticPiece* P) { return P->getKind() == ControlFlow; } diff --git a/clang/lib/Analysis/BugReporter.cpp b/clang/lib/Analysis/BugReporter.cpp index 86ee484f36cf..8719be7fddd4 100644 --- a/clang/lib/Analysis/BugReporter.cpp +++ b/clang/lib/Analysis/BugReporter.cpp @@ -84,21 +84,40 @@ static inline Stmt* GetCurrentOrNextStmt(const ExplodedNode* N) { // Diagnostics for 'execution continues on line XXX'. //===----------------------------------------------------------------------===// -static inline void ExecutionContinues(llvm::raw_string_ostream& os, - SourceManager& SMgr, - const ExplodedNode* N, - const Decl& D) { +static SourceLocation ExecutionContinues(SourceManager& SMgr, + const ExplodedNode* N, + const Decl& D, + bool* OutHasStmt = 0) { + if (Stmt *S = GetNextStmt(N)) { + if (OutHasStmt) *OutHasStmt = true; + return S->getLocStart(); + } + else { + if (OutHasStmt) *OutHasStmt = false; + return D.getBody()->getRBracLoc(); + } +} + +static SourceLocation ExecutionContinues(llvm::raw_string_ostream& os, + SourceManager& SMgr, + const ExplodedNode* N, + const Decl& D) { // Slow, but probably doesn't matter. if (os.str().empty()) os << ' '; - if (Stmt *S = GetNextStmt(N)) + bool hasStmt; + SourceLocation Loc = ExecutionContinues(SMgr, N, D, &hasStmt); + + if (hasStmt) os << "Execution continues on line " - << SMgr.getInstantiationLineNumber(S->getLocStart()) << '.'; + << SMgr.getInstantiationLineNumber(Loc) << '.'; else os << "Execution jumps to the end of the " << (isa(D) ? "method" : "function") << '.'; + + return Loc; } //===----------------------------------------------------------------------===// @@ -706,36 +725,33 @@ void GRBugReporter::GeneratePathDiagnostic(PathDiagnostic& PD, ProgramPoint P = N->getLocation(); if (const BlockEdge* BE = dyn_cast(&P)) { - CFGBlock* Src = BE->getSrc(); CFGBlock* Dst = BE->getDst(); - Stmt* T = Src->getTerminator(); if (!T) continue; - FullSourceLoc L(T->getLocStart(), SMgr); + FullSourceLoc Start(T->getLocStart(), SMgr); switch (T->getStmtClass()) { default: break; case Stmt::GotoStmtClass: - case Stmt::IndirectGotoStmtClass: { - + case Stmt::IndirectGotoStmtClass: { Stmt* S = GetNextStmt(N); if (!S) continue; std::string sbuf; - llvm::raw_string_ostream os(sbuf); + llvm::raw_string_ostream os(sbuf); + SourceLocation End = S->getLocStart(); - os << "Control jumps to line " - << SMgr.getInstantiationLineNumber(S->getLocStart()) << ".\n"; - - PD.push_front(new PathDiagnosticControlFlowPiece(L, os.str())); + os << "Control jumps to line " << SMgr.getInstantiationLineNumber(End); + PD.push_front(new PathDiagnosticControlFlowPiece(Start, End, + os.str())); break; } @@ -743,67 +759,70 @@ void GRBugReporter::GeneratePathDiagnostic(PathDiagnostic& PD, // Figure out what case arm we took. std::string sbuf; llvm::raw_string_ostream os(sbuf); - - if (Stmt* S = Dst->getLabel()) + SourceLocation End; + + if (Stmt* S = Dst->getLabel()) { + End = S->getLocStart(); + switch (S->getStmtClass()) { - default: - os << "No cases match in the switch statement. " - "Control jumps to line " - << SMgr.getInstantiationLineNumber(S->getLocStart()) << ".\n"; - break; - case Stmt::DefaultStmtClass: - os << "Control jumps to the 'default' case at line " - << SMgr.getInstantiationLineNumber(S->getLocStart()) << ".\n"; - break; - - case Stmt::CaseStmtClass: { - os << "Control jumps to 'case "; - - CaseStmt* Case = cast(S); - Expr* LHS = Case->getLHS()->IgnoreParenCasts(); - - // Determine if it is an enum. - - bool GetRawInt = true; - - if (DeclRefExpr* DR = dyn_cast(LHS)) { + default: + os << "No cases match in the switch statement. " + "Control jumps to line " + << SMgr.getInstantiationLineNumber(End); + break; + case Stmt::DefaultStmtClass: + os << "Control jumps to the 'default' case at line " + << SMgr.getInstantiationLineNumber(End); + break; + + case Stmt::CaseStmtClass: { + os << "Control jumps to 'case "; + CaseStmt* Case = cast(S); + Expr* LHS = Case->getLHS()->IgnoreParenCasts(); + + // Determine if it is an enum. + bool GetRawInt = true; + + if (DeclRefExpr* DR = dyn_cast(LHS)) { + // FIXME: Maybe this should be an assertion. Are there cases + // were it is not an EnumConstantDecl? + EnumConstantDecl* D = + dyn_cast(DR->getDecl()); - // FIXME: Maybe this should be an assertion. Are there cases - // were it is not an EnumConstantDecl? - EnumConstantDecl* D = dyn_cast(DR->getDecl()); - if (D) { - GetRawInt = false; - os << D->getNameAsString(); + if (D) { + GetRawInt = false; + os << D->getNameAsString(); + } } + + if (GetRawInt) { + + // Not an enum. + Expr* CondE = cast(T)->getCond(); + unsigned bits = Ctx.getTypeSize(CondE->getType()); + llvm::APSInt V(bits, false); + + if (!LHS->isIntegerConstantExpr(V, Ctx, 0, true)) { + assert (false && "Case condition must be constant."); + continue; + } + + os << V; + } + + os << ":' at line " << SMgr.getInstantiationLineNumber(End); + break; } - - if (GetRawInt) { - - // Not an enum. - Expr* CondE = cast(T)->getCond(); - unsigned bits = Ctx.getTypeSize(CondE->getType()); - llvm::APSInt V(bits, false); - - if (!LHS->isIntegerConstantExpr(V, Ctx, 0, true)) { - assert (false && "Case condition must be constant."); - continue; - } - - os << V; - } - - os << ":' at line " - << SMgr.getInstantiationLineNumber(S->getLocStart()) << ".\n"; - - break; } } else { os << "'Default' branch taken. "; - ExecutionContinues(os, SMgr, N, getStateManager().getCodeDecl()); + End = ExecutionContinues(os, SMgr, N, + getStateManager().getCodeDecl()); } - PD.push_front(new PathDiagnosticControlFlowPiece(L, os.str())); + PD.push_front(new PathDiagnosticControlFlowPiece(Start, End, + os.str())); break; } @@ -811,8 +830,10 @@ void GRBugReporter::GeneratePathDiagnostic(PathDiagnostic& PD, case Stmt::ContinueStmtClass: { std::string sbuf; llvm::raw_string_ostream os(sbuf); - ExecutionContinues(os, SMgr, N, getStateManager().getCodeDecl()); - PD.push_front(new PathDiagnosticControlFlowPiece(L, os.str())); + SourceLocation End = ExecutionContinues(os, SMgr, N, + getStateManager().getCodeDecl()); + PD.push_front(new PathDiagnosticControlFlowPiece(Start, End, + os.str())); break; } @@ -822,58 +843,73 @@ void GRBugReporter::GeneratePathDiagnostic(PathDiagnostic& PD, os << "'?' condition evaluates to "; if (*(Src->succ_begin()+1) == Dst) - os << "false."; + os << "false"; else - os << "true."; + os << "true"; - PD.push_front(new PathDiagnosticControlFlowPiece(L, os.str())); + SourceLocation End = + ExecutionContinues(SMgr, N, getStateManager().getCodeDecl()); + + PD.push_front(new PathDiagnosticControlFlowPiece(Start, End, + os.str())); break; } - case Stmt::DoStmtClass: { - + case Stmt::DoStmtClass: { if (*(Src->succ_begin()) == Dst) { std::string sbuf; llvm::raw_string_ostream os(sbuf); os << "Loop condition is true. "; - ExecutionContinues(os, SMgr, N, getStateManager().getCodeDecl()); - - PD.push_front(new PathDiagnosticControlFlowPiece(L, os.str())); + SourceLocation End = + ExecutionContinues(os, SMgr, N, getStateManager().getCodeDecl()); + PD.push_front(new PathDiagnosticControlFlowPiece(Start, End, + os.str())); + } + else { + SourceLocation End = + ExecutionContinues(SMgr, N, getStateManager().getCodeDecl()); + PD.push_front(new PathDiagnosticControlFlowPiece(Start, End, + "Loop condition is false. Exiting loop")); } - else - PD.push_front(new PathDiagnosticControlFlowPiece(L, - "Loop condition is false. Exiting loop.")); break; } case Stmt::WhileStmtClass: - case Stmt::ForStmtClass: { - + case Stmt::ForStmtClass: { if (*(Src->succ_begin()+1) == Dst) { std::string sbuf; llvm::raw_string_ostream os(sbuf); os << "Loop condition is false. "; - ExecutionContinues(os, SMgr, N, getStateManager().getCodeDecl()); + SourceLocation End = + ExecutionContinues(os, SMgr, N, getStateManager().getCodeDecl()); - PD.push_front(new PathDiagnosticControlFlowPiece(L, os.str())); + PD.push_front(new PathDiagnosticControlFlowPiece(Start, End, + os.str())); + } + else { + SourceLocation End = + ExecutionContinues(SMgr, N, getStateManager().getCodeDecl()); + + PD.push_front(new PathDiagnosticControlFlowPiece(Start, End, + "Loop condition is true. Entering loop body")); } - else - PD.push_front(new PathDiagnosticControlFlowPiece(L, - "Loop condition is true. Entering loop body.")); break; } - case Stmt::IfStmtClass: { + case Stmt::IfStmtClass: { + SourceLocation End = + ExecutionContinues(SMgr, N, getStateManager().getCodeDecl()); + if (*(Src->succ_begin()+1) == Dst) - PD.push_front(new PathDiagnosticControlFlowPiece(L, - "Taking false branch.")); + PD.push_front(new PathDiagnosticControlFlowPiece(Start, End, + "Taking false branch")); else - PD.push_front(new PathDiagnosticControlFlowPiece(L, - "Taking true branch.")); + PD.push_front(new PathDiagnosticControlFlowPiece(Start, End, + "Taking true branch")); break; } diff --git a/clang/lib/Analysis/PathDiagnostic.cpp b/clang/lib/Analysis/PathDiagnostic.cpp index a504e80b5961..86b113cf8733 100644 --- a/clang/lib/Analysis/PathDiagnostic.cpp +++ b/clang/lib/Analysis/PathDiagnostic.cpp @@ -65,6 +65,13 @@ PathDiagnosticPiece::PathDiagnosticPiece(FullSourceLoc pos, "PathDiagnosticPiece's must have a valid location."); } +PathDiagnosticPiece::PathDiagnosticPiece(FullSourceLoc pos, Kind k, + DisplayHint hint) + : Pos(pos), kind(k), Hint(hint) { + assert(Pos.isValid() && + "PathDiagnosticPiece's must have a valid location."); +} + PathDiagnosticPiece::~PathDiagnosticPiece() {} PathDiagnosticEventPiece::~PathDiagnosticEventPiece() {} PathDiagnosticControlFlowPiece::~PathDiagnosticControlFlowPiece() {} diff --git a/clang/lib/Frontend/PlistDiagnostics.cpp b/clang/lib/Frontend/PlistDiagnostics.cpp index afd8b26e5f21..16c5c1019d7d 100644 --- a/clang/lib/Frontend/PlistDiagnostics.cpp +++ b/clang/lib/Frontend/PlistDiagnostics.cpp @@ -17,10 +17,12 @@ #include "clang/Basic/FileManager.h" #include "llvm/Support/Compiler.h" #include "llvm/Support/raw_ostream.h" +#include "llvm/Support/Casting.h" #include "llvm/System/Path.h" #include "llvm/ADT/DenseMap.h" #include "llvm/ADT/SmallVector.h" using namespace clang; +using llvm::cast; typedef llvm::DenseMap FIDMap; @@ -67,8 +69,7 @@ static unsigned GetFID(const FIDMap& FIDs, SourceManager* SM, SourceLocation L){ } static llvm::raw_ostream& Indent(llvm::raw_ostream& o, const unsigned indent) { - for (unsigned i = 0; i < indent; ++i) - o << ' '; + for (unsigned i = 0; i < indent; ++i) o << ' '; return o; } @@ -95,22 +96,49 @@ static void EmitRange(llvm::raw_ostream& o, SourceManager* SM, SourceRange R, Indent(o, indent) << "\n"; } -static void ReportDiag(llvm::raw_ostream& o, const PathDiagnosticPiece& P, - const FIDMap& FM, SourceManager* SM) { +static void ReportControlFlow(llvm::raw_ostream& o, + const PathDiagnosticControlFlowPiece& P, + const FIDMap& FM, SourceManager *SM, + unsigned indent) { - unsigned indent = 4; Indent(o, indent) << "\n"; ++indent; + Indent(o, indent) << "kindcontrol\n"; + + // Output the start and end locations. + Indent(o, indent) << "start\n"; + EmitLocation(o, SM, P.getStartLocation(), FM, indent); + Indent(o, indent) << "end\n"; + EmitLocation(o, SM, P.getEndLocation(), FM, indent); + + // Output any helper text. + const std::string& s = P.getString(); + if (!s.empty()) { + Indent(o, indent) << "alternate" << s << "\n"; + } + + --indent; + Indent(o, indent) << "\n"; +} + +static void ReportEvent(llvm::raw_ostream& o, const PathDiagnosticPiece& P, + const FIDMap& FM, SourceManager* SM, unsigned indent) { + + Indent(o, indent) << "\n"; + ++indent; + + Indent(o, indent) << "kindevent\n"; + // Output the location. FullSourceLoc L = P.getLocation(); - + Indent(o, indent) << "location\n"; EmitLocation(o, SM, L, FM, indent); - + // Output the ranges (if any). PathDiagnosticPiece::range_iterator RI = P.ranges_begin(), - RE = P.ranges_end(); + RE = P.ranges_end(); if (RI != RE) { Indent(o, indent) << "ranges\n"; @@ -122,35 +150,55 @@ static void ReportDiag(llvm::raw_ostream& o, const PathDiagnosticPiece& P, } // Output the text. + assert(!P.getString().empty()); Indent(o, indent) << "message\n"; Indent(o, indent) << "" << P.getString() << "\n"; - - // Output the hint. -#if 0 - // Disable the display hint until we clear up its meaning. - Indent(o, indent) << "displayhint\n"; - Indent(o, indent) << "" - << (P.getDisplayHint() == PathDiagnosticPiece::Above - ? "above" : "below") - << "\n"; -#endif - // Output the PathDiagnosticPiece::Kind. - Indent(o, indent) << "kind\n"; - Indent(o, indent) << ""; - - switch (P.getKind()) { - case PathDiagnosticPiece::Event: o << "Event"; break; - case PathDiagnosticPiece::ControlFlow: o << "ControlFlow"; break; - case PathDiagnosticPiece::Macro: o << "Macro"; break; - } - o << "\n"; - - + // Finish up. --indent; Indent(o, indent); o << "\n"; } +static void ReportMacro(llvm::raw_ostream& o, + const PathDiagnosticMacroPiece& P, + const FIDMap& FM, SourceManager *SM, + unsigned indent) { + + for (PathDiagnosticMacroPiece::const_iterator I=P.begin(), E=P.end(); + I!=E; ++I) { + + switch ((*I)->getKind()) { + default: + break; + case PathDiagnosticPiece::Event: + ReportEvent(o, cast(**I), FM, SM, indent); + break; + case PathDiagnosticPiece::Macro: + ReportMacro(o, cast(**I), FM, SM, indent); + break; + } + } +} + +static void ReportDiag(llvm::raw_ostream& o, const PathDiagnosticPiece& P, + const FIDMap& FM, SourceManager* SM) { + + unsigned indent = 4; + + switch (P.getKind()) { + case PathDiagnosticPiece::ControlFlow: + ReportControlFlow(o, cast(P), FM, SM, + indent); + break; + case PathDiagnosticPiece::Event: + ReportEvent(o, cast(P), FM, SM, indent); + break; + case PathDiagnosticPiece::Macro: + ReportMacro(o, cast(P), FM, SM, indent); + break; + } +} + void PlistDiagnostics::HandlePathDiagnostic(const PathDiagnostic* D) { if (!D) return; @@ -238,11 +286,14 @@ PlistDiagnostics::~PlistDiagnostics() { o << " \n"; // Output the bug type and bug category. - o << " description\n " << D->getDescription() + o << " description" << D->getDescription() << "\n" - << " category\n " << D->getCategory() + << " category" << D->getCategory() << "\n" - << " \n"; + << " type" << D->getBugType() + << "\n" + << " \n"; + } o << " \n";