diff options
Diffstat (limited to 'contrib/llvm/tools/clang/lib/StaticAnalyzer/Core/BugReporter.cpp')
| -rw-r--r-- | contrib/llvm/tools/clang/lib/StaticAnalyzer/Core/BugReporter.cpp | 517 |
1 files changed, 351 insertions, 166 deletions
diff --git a/contrib/llvm/tools/clang/lib/StaticAnalyzer/Core/BugReporter.cpp b/contrib/llvm/tools/clang/lib/StaticAnalyzer/Core/BugReporter.cpp index 571baec..c898d65 100644 --- a/contrib/llvm/tools/clang/lib/StaticAnalyzer/Core/BugReporter.cpp +++ b/contrib/llvm/tools/clang/lib/StaticAnalyzer/Core/BugReporter.cpp @@ -118,10 +118,82 @@ GetCurrentOrNextStmt(const ExplodedNode *N) { // Diagnostic cleanup. //===----------------------------------------------------------------------===// +static PathDiagnosticEventPiece * +eventsDescribeSameCondition(PathDiagnosticEventPiece *X, + PathDiagnosticEventPiece *Y) { + // Prefer diagnostics that come from ConditionBRVisitor over + // those that came from TrackConstraintBRVisitor. + const void *tagPreferred = ConditionBRVisitor::getTag(); + const void *tagLesser = TrackConstraintBRVisitor::getTag(); + + if (X->getLocation() != Y->getLocation()) + return 0; + + if (X->getTag() == tagPreferred && Y->getTag() == tagLesser) + return X; + + if (Y->getTag() == tagPreferred && X->getTag() == tagLesser) + return Y; + + return 0; +} + +/// An optimization pass over PathPieces that removes redundant diagnostics +/// generated by both ConditionBRVisitor and TrackConstraintBRVisitor. Both +/// BugReporterVisitors use different methods to generate diagnostics, with +/// one capable of emitting diagnostics in some cases but not in others. This +/// can lead to redundant diagnostic pieces at the same point in a path. +static void removeRedundantMsgs(PathPieces &path) { + unsigned N = path.size(); + if (N < 2) + return; + // NOTE: this loop intentionally is not using an iterator. Instead, we + // are streaming the path and modifying it in place. This is done by + // grabbing the front, processing it, and if we decide to keep it append + // it to the end of the path. The entire path is processed in this way. + for (unsigned i = 0; i < N; ++i) { + IntrusiveRefCntPtr<PathDiagnosticPiece> piece(path.front()); + path.pop_front(); + + switch (piece->getKind()) { + case clang::ento::PathDiagnosticPiece::Call: + removeRedundantMsgs(cast<PathDiagnosticCallPiece>(piece)->path); + break; + case clang::ento::PathDiagnosticPiece::Macro: + removeRedundantMsgs(cast<PathDiagnosticMacroPiece>(piece)->subPieces); + break; + case clang::ento::PathDiagnosticPiece::ControlFlow: + break; + case clang::ento::PathDiagnosticPiece::Event: { + if (i == N-1) + break; + + if (PathDiagnosticEventPiece *nextEvent = + dyn_cast<PathDiagnosticEventPiece>(path.front().getPtr())) { + PathDiagnosticEventPiece *event = + cast<PathDiagnosticEventPiece>(piece); + // Check to see if we should keep one of the two pieces. If we + // come up with a preference, record which piece to keep, and consume + // another piece from the path. + if (PathDiagnosticEventPiece *pieceToKeep = + eventsDescribeSameCondition(event, nextEvent)) { + piece = pieceToKeep; + path.pop_front(); + ++i; + } + } + break; + } + } + path.push_back(piece); + } +} + /// Recursively scan through a path and prune out calls and macros pieces /// that aren't needed. Return true if afterwards the path contains /// "interesting stuff" which means it should be pruned from the parent path. -static bool RemoveUneededCalls(PathPieces &pieces) { +bool BugReporter::RemoveUneededCalls(PathPieces &pieces, BugReport *R, + PathDiagnosticCallPiece *CallWithLoc) { bool containsSomethingInteresting = false; const unsigned N = pieces.size(); @@ -131,30 +203,49 @@ static bool RemoveUneededCalls(PathPieces &pieces) { IntrusiveRefCntPtr<PathDiagnosticPiece> piece(pieces.front()); pieces.pop_front(); + // Throw away pieces with invalid locations. + if (piece->getKind() != PathDiagnosticPiece::Call && + piece->getLocation().asLocation().isInvalid()) + continue; + switch (piece->getKind()) { case PathDiagnosticPiece::Call: { PathDiagnosticCallPiece *call = cast<PathDiagnosticCallPiece>(piece); + // Check if the location context is interesting. + assert(LocationContextMap.count(call)); + if (R->isInteresting(LocationContextMap[call])) { + containsSomethingInteresting = true; + break; + } // Recursively clean out the subclass. Keep this call around if // it contains any informative diagnostics. - if (!RemoveUneededCalls(call->path)) + PathDiagnosticCallPiece *NewCallWithLoc = + call->getLocation().asLocation().isValid() + ? call : CallWithLoc; + + if (!RemoveUneededCalls(call->path, R, NewCallWithLoc)) continue; + + if (NewCallWithLoc == CallWithLoc && CallWithLoc) { + call->callEnter = CallWithLoc->callEnter; + } + containsSomethingInteresting = true; break; } case PathDiagnosticPiece::Macro: { PathDiagnosticMacroPiece *macro = cast<PathDiagnosticMacroPiece>(piece); - if (!RemoveUneededCalls(macro->subPieces)) + if (!RemoveUneededCalls(macro->subPieces, R)) continue; containsSomethingInteresting = true; break; } case PathDiagnosticPiece::Event: { PathDiagnosticEventPiece *event = cast<PathDiagnosticEventPiece>(piece); + // We never throw away an event, but we do throw it away wholesale // as part of a path if we throw the entire path away. - if (event->isPrunable()) - continue; - containsSomethingInteresting = true; + containsSomethingInteresting |= !event->isPrunable(); break; } case PathDiagnosticPiece::ControlFlow: @@ -382,6 +473,35 @@ PathDiagnosticBuilder::getEnclosingStmtLocation(const Stmt *S) { } //===----------------------------------------------------------------------===// +// "Visitors only" path diagnostic generation algorithm. +//===----------------------------------------------------------------------===// +static bool GenerateVisitorsOnlyPathDiagnostic(PathDiagnostic &PD, + PathDiagnosticBuilder &PDB, + const ExplodedNode *N, + ArrayRef<BugReporterVisitor *> visitors) { + // All path generation skips the very first node (the error node). + // This is because there is special handling for the end-of-path note. + N = N->getFirstPred(); + if (!N) + return true; + + BugReport *R = PDB.getBugReport(); + while (const ExplodedNode *Pred = N->getFirstPred()) { + for (ArrayRef<BugReporterVisitor *>::iterator I = visitors.begin(), + E = visitors.end(); + I != E; ++I) { + // Visit all the node pairs, but throw the path pieces away. + PathDiagnosticPiece *Piece = (*I)->VisitNode(N, Pred, PDB, *R); + delete Piece; + } + + N = Pred; + } + + return R->isValid(); +} + +//===----------------------------------------------------------------------===// // "Minimal" path diagnostic generation algorithm. //===----------------------------------------------------------------------===// typedef std::pair<PathDiagnosticCallPiece*, const ExplodedNode*> StackDiagPair; @@ -412,7 +532,7 @@ static void updateStackPiecesWithMessage(PathDiagnosticPiece *P, static void CompactPathDiagnostic(PathPieces &path, const SourceManager& SM); -static void GenerateMinimalPathDiagnostic(PathDiagnostic& PD, +static bool GenerateMinimalPathDiagnostic(PathDiagnostic& PD, PathDiagnosticBuilder &PDB, const ExplodedNode *N, ArrayRef<BugReporterVisitor *> visitors) { @@ -430,55 +550,60 @@ static void GenerateMinimalPathDiagnostic(PathDiagnostic& PD, NextNode = GetPredecessorNode(N); ProgramPoint P = N->getLocation(); - - if (const CallExitEnd *CE = dyn_cast<CallExitEnd>(&P)) { - PathDiagnosticCallPiece *C = - PathDiagnosticCallPiece::construct(N, *CE, SMgr); - PD.getActivePath().push_front(C); - PD.pushActivePath(&C->path); - CallStack.push_back(StackDiagPair(C, N)); - continue; - } - - if (const CallEnter *CE = dyn_cast<CallEnter>(&P)) { - // Flush all locations, and pop the active path. - bool VisitedEntireCall = PD.isWithinCall(); - PD.popActivePath(); - - // Either we just added a bunch of stuff to the top-level path, or - // we have a previous CallExitEnd. If the former, it means that the - // path terminated within a function call. We must then take the - // current contents of the active path and place it within - // a new PathDiagnosticCallPiece. - PathDiagnosticCallPiece *C; - if (VisitedEntireCall) { - C = cast<PathDiagnosticCallPiece>(PD.getActivePath().front()); - } else { - const Decl *Caller = CE->getLocationContext()->getDecl(); - C = PathDiagnosticCallPiece::construct(PD.getActivePath(), Caller); + + do { + if (const CallExitEnd *CE = dyn_cast<CallExitEnd>(&P)) { + PathDiagnosticCallPiece *C = + PathDiagnosticCallPiece::construct(N, *CE, SMgr); + GRBugReporter& BR = PDB.getBugReporter(); + BR.addCallPieceLocationContextPair(C, CE->getCalleeContext()); + PD.getActivePath().push_front(C); + PD.pushActivePath(&C->path); + CallStack.push_back(StackDiagPair(C, N)); + break; } - C->setCallee(*CE, SMgr); - if (!CallStack.empty()) { - assert(CallStack.back().first == C); - CallStack.pop_back(); + if (const CallEnter *CE = dyn_cast<CallEnter>(&P)) { + // Flush all locations, and pop the active path. + bool VisitedEntireCall = PD.isWithinCall(); + PD.popActivePath(); + + // Either we just added a bunch of stuff to the top-level path, or + // we have a previous CallExitEnd. If the former, it means that the + // path terminated within a function call. We must then take the + // current contents of the active path and place it within + // a new PathDiagnosticCallPiece. + PathDiagnosticCallPiece *C; + if (VisitedEntireCall) { + C = cast<PathDiagnosticCallPiece>(PD.getActivePath().front()); + } else { + const Decl *Caller = CE->getLocationContext()->getDecl(); + C = PathDiagnosticCallPiece::construct(PD.getActivePath(), Caller); + GRBugReporter& BR = PDB.getBugReporter(); + BR.addCallPieceLocationContextPair(C, CE->getCalleeContext()); + } + + C->setCallee(*CE, SMgr); + if (!CallStack.empty()) { + assert(CallStack.back().first == C); + CallStack.pop_back(); + } + break; } - continue; - } - if (const BlockEdge *BE = dyn_cast<BlockEdge>(&P)) { - const CFGBlock *Src = BE->getSrc(); - const CFGBlock *Dst = BE->getDst(); - const Stmt *T = Src->getTerminator(); + if (const BlockEdge *BE = dyn_cast<BlockEdge>(&P)) { + const CFGBlock *Src = BE->getSrc(); + const CFGBlock *Dst = BE->getDst(); + const Stmt *T = Src->getTerminator(); - if (!T) - continue; + if (!T) + break; - PathDiagnosticLocation Start = - PathDiagnosticLocation::createBegin(T, SMgr, - N->getLocationContext()); + PathDiagnosticLocation Start = + PathDiagnosticLocation::createBegin(T, SMgr, + N->getLocationContext()); - switch (T->getStmtClass()) { + switch (T->getStmtClass()) { default: break; @@ -487,16 +612,16 @@ static void GenerateMinimalPathDiagnostic(PathDiagnostic& PD, const Stmt *S = GetNextStmt(N); if (!S) - continue; + break; std::string sbuf; llvm::raw_string_ostream os(sbuf); const PathDiagnosticLocation &End = PDB.getEnclosingStmtLocation(S); os << "Control jumps to line " - << End.asLocation().getExpansionLineNumber(); - PD.getActivePath().push_front(new PathDiagnosticControlFlowPiece(Start, End, - os.str())); + << End.asLocation().getExpansionLineNumber(); + PD.getActivePath().push_front(new PathDiagnosticControlFlowPiece( + Start, End, os.str())); break; } @@ -509,52 +634,52 @@ static void GenerateMinimalPathDiagnostic(PathDiagnostic& PD, PathDiagnosticLocation End(S, SMgr, LC); switch (S->getStmtClass()) { - default: - os << "No cases match in the switch statement. " - "Control jumps to line " - << End.asLocation().getExpansionLineNumber(); - break; - case Stmt::DefaultStmtClass: - os << "Control jumps to the 'default' case at line " - << End.asLocation().getExpansionLineNumber(); - break; - - case Stmt::CaseStmtClass: { - os << "Control jumps to 'case "; - const CaseStmt *Case = cast<CaseStmt>(S); - const Expr *LHS = Case->getLHS()->IgnoreParenCasts(); - - // Determine if it is an enum. - bool GetRawInt = true; - - if (const DeclRefExpr *DR = dyn_cast<DeclRefExpr>(LHS)) { - // FIXME: Maybe this should be an assertion. Are there cases - // were it is not an EnumConstantDecl? - const EnumConstantDecl *D = + default: + os << "No cases match in the switch statement. " + "Control jumps to line " + << End.asLocation().getExpansionLineNumber(); + break; + case Stmt::DefaultStmtClass: + os << "Control jumps to the 'default' case at line " + << End.asLocation().getExpansionLineNumber(); + break; + + case Stmt::CaseStmtClass: { + os << "Control jumps to 'case "; + const CaseStmt *Case = cast<CaseStmt>(S); + const Expr *LHS = Case->getLHS()->IgnoreParenCasts(); + + // Determine if it is an enum. + bool GetRawInt = true; + + if (const DeclRefExpr *DR = dyn_cast<DeclRefExpr>(LHS)) { + // FIXME: Maybe this should be an assertion. Are there cases + // were it is not an EnumConstantDecl? + const EnumConstantDecl *D = dyn_cast<EnumConstantDecl>(DR->getDecl()); - if (D) { - GetRawInt = false; - os << *D; - } + if (D) { + GetRawInt = false; + os << *D; } + } - if (GetRawInt) - os << LHS->EvaluateKnownConstInt(PDB.getASTContext()); + if (GetRawInt) + os << LHS->EvaluateKnownConstInt(PDB.getASTContext()); - os << ":' at line " - << End.asLocation().getExpansionLineNumber(); - break; - } + os << ":' at line " + << End.asLocation().getExpansionLineNumber(); + break; } - PD.getActivePath().push_front(new PathDiagnosticControlFlowPiece(Start, End, - os.str())); + } + PD.getActivePath().push_front(new PathDiagnosticControlFlowPiece( + Start, End, os.str())); } else { os << "'Default' branch taken. "; const PathDiagnosticLocation &End = PDB.ExecutionContinues(os, N); - PD.getActivePath().push_front(new PathDiagnosticControlFlowPiece(Start, End, - os.str())); + PD.getActivePath().push_front(new PathDiagnosticControlFlowPiece( + Start, End, os.str())); } break; @@ -565,12 +690,12 @@ static void GenerateMinimalPathDiagnostic(PathDiagnostic& PD, std::string sbuf; llvm::raw_string_ostream os(sbuf); PathDiagnosticLocation End = PDB.ExecutionContinues(os, N); - PD.getActivePath().push_front(new PathDiagnosticControlFlowPiece(Start, End, - os.str())); + PD.getActivePath().push_front(new PathDiagnosticControlFlowPiece( + Start, End, os.str())); break; } - // Determine control-flow for ternary '?'. + // Determine control-flow for ternary '?'. case Stmt::BinaryConditionalOperatorClass: case Stmt::ConditionalOperatorClass: { std::string sbuf; @@ -587,12 +712,12 @@ static void GenerateMinimalPathDiagnostic(PathDiagnostic& PD, if (const Stmt *S = End.asStmt()) End = PDB.getEnclosingStmtLocation(S); - PD.getActivePath().push_front(new PathDiagnosticControlFlowPiece(Start, End, - os.str())); + PD.getActivePath().push_front(new PathDiagnosticControlFlowPiece( + Start, End, os.str())); break; } - // Determine control-flow for short-circuited '&&' and '||'. + // Determine control-flow for short-circuited '&&' and '||'. case Stmt::BinaryOperatorClass: { if (!PDB.supportsLogicalOpControlFlow()) break; @@ -609,16 +734,16 @@ static void GenerateMinimalPathDiagnostic(PathDiagnostic& PD, os << "false"; PathDiagnosticLocation End(B->getLHS(), SMgr, LC); PathDiagnosticLocation Start = - PathDiagnosticLocation::createOperatorLoc(B, SMgr); - PD.getActivePath().push_front(new PathDiagnosticControlFlowPiece(Start, End, - os.str())); + PathDiagnosticLocation::createOperatorLoc(B, SMgr); + PD.getActivePath().push_front(new PathDiagnosticControlFlowPiece( + Start, End, os.str())); } else { os << "true"; PathDiagnosticLocation Start(B->getLHS(), SMgr, LC); PathDiagnosticLocation End = PDB.ExecutionContinues(N); - PD.getActivePath().push_front(new PathDiagnosticControlFlowPiece(Start, End, - os.str())); + PD.getActivePath().push_front(new PathDiagnosticControlFlowPiece( + Start, End, os.str())); } } else { @@ -629,16 +754,16 @@ static void GenerateMinimalPathDiagnostic(PathDiagnostic& PD, os << "false"; PathDiagnosticLocation Start(B->getLHS(), SMgr, LC); PathDiagnosticLocation End = PDB.ExecutionContinues(N); - PD.getActivePath().push_front(new PathDiagnosticControlFlowPiece(Start, End, - os.str())); + PD.getActivePath().push_front(new PathDiagnosticControlFlowPiece( + Start, End, os.str())); } else { os << "true"; PathDiagnosticLocation End(B->getLHS(), SMgr, LC); PathDiagnosticLocation Start = - PathDiagnosticLocation::createOperatorLoc(B, SMgr); - PD.getActivePath().push_front(new PathDiagnosticControlFlowPiece(Start, End, - os.str())); + PathDiagnosticLocation::createOperatorLoc(B, SMgr); + PD.getActivePath().push_front(new PathDiagnosticControlFlowPiece( + Start, End, os.str())); } } @@ -656,8 +781,8 @@ static void GenerateMinimalPathDiagnostic(PathDiagnostic& PD, if (const Stmt *S = End.asStmt()) End = PDB.getEnclosingStmtLocation(S); - PD.getActivePath().push_front(new PathDiagnosticControlFlowPiece(Start, End, - os.str())); + PD.getActivePath().push_front(new PathDiagnosticControlFlowPiece( + Start, End, os.str())); } else { PathDiagnosticLocation End = PDB.ExecutionContinues(N); @@ -665,8 +790,8 @@ static void GenerateMinimalPathDiagnostic(PathDiagnostic& PD, if (const Stmt *S = End.asStmt()) End = PDB.getEnclosingStmtLocation(S); - PD.getActivePath().push_front(new PathDiagnosticControlFlowPiece(Start, End, - "Loop condition is false. Exiting loop")); + PD.getActivePath().push_front(new PathDiagnosticControlFlowPiece( + Start, End, "Loop condition is false. Exiting loop")); } break; @@ -683,16 +808,16 @@ static void GenerateMinimalPathDiagnostic(PathDiagnostic& PD, if (const Stmt *S = End.asStmt()) End = PDB.getEnclosingStmtLocation(S); - PD.getActivePath().push_front(new PathDiagnosticControlFlowPiece(Start, End, - os.str())); + PD.getActivePath().push_front(new PathDiagnosticControlFlowPiece( + Start, End, os.str())); } else { PathDiagnosticLocation End = PDB.ExecutionContinues(N); if (const Stmt *S = End.asStmt()) End = PDB.getEnclosingStmtLocation(S); - PD.getActivePath().push_front(new PathDiagnosticControlFlowPiece(Start, End, - "Loop condition is true. Entering loop body")); + PD.getActivePath().push_front(new PathDiagnosticControlFlowPiece( + Start, End, "Loop condition is true. Entering loop body")); } break; @@ -705,16 +830,17 @@ static void GenerateMinimalPathDiagnostic(PathDiagnostic& PD, End = PDB.getEnclosingStmtLocation(S); if (*(Src->succ_begin()+1) == Dst) - PD.getActivePath().push_front(new PathDiagnosticControlFlowPiece(Start, End, - "Taking false branch")); + PD.getActivePath().push_front(new PathDiagnosticControlFlowPiece( + Start, End, "Taking false branch")); else - PD.getActivePath().push_front(new PathDiagnosticControlFlowPiece(Start, End, - "Taking true branch")); + PD.getActivePath().push_front(new PathDiagnosticControlFlowPiece( + Start, End, "Taking true branch")); break; } + } } - } + } while(0); if (NextNode) { // Add diagnostic pieces from custom visitors. @@ -730,9 +856,13 @@ static void GenerateMinimalPathDiagnostic(PathDiagnostic& PD, } } + if (!PDB.getBugReport()->isValid()) + return false; + // After constructing the full PathDiagnostic, do a pass over it to compact // PathDiagnosticPieces that occur within a macro. CompactPathDiagnostic(PD.getMutablePieces(), PDB.getSourceManager()); + return true; } //===----------------------------------------------------------------------===// @@ -944,6 +1074,11 @@ void EdgeBuilder::rawAddEdge(PathDiagnosticLocation NewLoc) { const PathDiagnosticLocation &NewLocClean = cleanUpLocation(NewLoc); const PathDiagnosticLocation &PrevLocClean = cleanUpLocation(PrevLoc); + if (PrevLocClean.asLocation().isInvalid()) { + PrevLoc = NewLoc; + return; + } + if (NewLocClean.asLocation() == PrevLocClean.asLocation()) return; @@ -1133,7 +1268,7 @@ static void reversePropagateInterestingSymbols(BugReport &R, } } -static void GenerateExtensivePathDiagnostic(PathDiagnostic& PD, +static bool GenerateExtensivePathDiagnostic(PathDiagnostic& PD, PathDiagnosticBuilder &PDB, const ExplodedNode *N, ArrayRef<BugReporterVisitor *> visitors) { @@ -1166,6 +1301,8 @@ static void GenerateExtensivePathDiagnostic(PathDiagnostic& PD, PathDiagnosticCallPiece *C = PathDiagnosticCallPiece::construct(N, *CE, SM); + GRBugReporter& BR = PDB.getBugReporter(); + BR.addCallPieceLocationContextPair(C, CE->getCalleeContext()); EB.addEdge(C->callReturn, true); EB.flushLocations(); @@ -1202,6 +1339,8 @@ static void GenerateExtensivePathDiagnostic(PathDiagnostic& PD, } else { const Decl *Caller = CE->getLocationContext()->getDecl(); C = PathDiagnosticCallPiece::construct(PD.getActivePath(), Caller); + GRBugReporter& BR = PDB.getBugReporter(); + BR.addCallPieceLocationContextPair(C, CE->getCalleeContext()); } C->setCallee(*CE, SM); @@ -1234,20 +1373,15 @@ static void GenerateExtensivePathDiagnostic(PathDiagnostic& PD, } } - const CFGBlock &Blk = *BE->getSrc(); - const Stmt *Term = Blk.getTerminator(); - // Are we jumping to the head of a loop? Add a special diagnostic. - if (const Stmt *Loop = BE->getDst()->getLoopTarget()) { + if (const Stmt *Loop = BE->getSrc()->getLoopTarget()) { PathDiagnosticLocation L(Loop, SM, PDB.LC); const CompoundStmt *CS = NULL; - if (!Term) { - if (const ForStmt *FS = dyn_cast<ForStmt>(Loop)) - CS = dyn_cast<CompoundStmt>(FS->getBody()); - else if (const WhileStmt *WS = dyn_cast<WhileStmt>(Loop)) - CS = dyn_cast<CompoundStmt>(WS->getBody()); - } + if (const ForStmt *FS = dyn_cast<ForStmt>(Loop)) + CS = dyn_cast<CompoundStmt>(FS->getBody()); + else if (const WhileStmt *WS = dyn_cast<WhileStmt>(Loop)) + CS = dyn_cast<CompoundStmt>(WS->getBody()); PathDiagnosticEventPiece *p = new PathDiagnosticEventPiece(L, @@ -1263,15 +1397,16 @@ static void GenerateExtensivePathDiagnostic(PathDiagnostic& PD, EB.addEdge(BL); } } - - if (Term) + + if (const Stmt *Term = BE->getSrc()->getTerminator()) EB.addContext(Term); break; } if (const BlockEntrance *BE = dyn_cast<BlockEntrance>(&P)) { - if (const CFGStmt *S = BE->getFirstElement().getAs<CFGStmt>()) { + CFGElement First = BE->getFirstElement(); + if (const CFGStmt *S = First.getAs<CFGStmt>()) { const Stmt *stmt = S->getStmt(); if (IsControlFlowExpr(stmt)) { // Add the proper context for '&&', '||', and '?'. @@ -1306,6 +1441,8 @@ static void GenerateExtensivePathDiagnostic(PathDiagnostic& PD, } } } + + return PDB.getBugReport()->isValid(); } //===----------------------------------------------------------------------===// @@ -1414,6 +1551,12 @@ void BugReport::markInteresting(SVal V) { markInteresting(V.getAsSymbol()); } +void BugReport::markInteresting(const LocationContext *LC) { + if (!LC) + return; + InterestingLocationContexts.insert(LC); +} + bool BugReport::isInteresting(SVal V) { return isInteresting(V.getAsRegion()) || isInteresting(V.getAsSymbol()); } @@ -1438,6 +1581,12 @@ bool BugReport::isInteresting(const MemRegion *R) { return false; } +bool BugReport::isInteresting(const LocationContext *LC) { + if (!LC) + return false; + return InterestingLocationContexts.count(LC); +} + void BugReport::lazyInitializeInterestingSets() { if (interestingSymbols.empty()) { interestingSymbols.push_back(new Symbols()); @@ -1823,17 +1972,27 @@ static void CompactPathDiagnostic(PathPieces &path, const SourceManager& SM) { path.push_back(*I); } -void GRBugReporter::GeneratePathDiagnostic(PathDiagnostic& PD, +bool GRBugReporter::generatePathDiagnostic(PathDiagnostic& PD, PathDiagnosticConsumer &PC, ArrayRef<BugReport *> &bugReports) { - assert(!bugReports.empty()); + + bool HasValid = false; SmallVector<const ExplodedNode *, 10> errorNodes; for (ArrayRef<BugReport*>::iterator I = bugReports.begin(), E = bugReports.end(); I != E; ++I) { + if ((*I)->isValid()) { + HasValid = true; errorNodes.push_back((*I)->getErrorNode()); + } else { + errorNodes.push_back(0); + } } + // If all the reports have been marked invalid, we're done. + if (!HasValid) + return false; + // Construct a new graph that contains only a single path from the error // node to a root. const std::pair<std::pair<ExplodedGraph*, NodeBackMap*>, @@ -1844,6 +2003,7 @@ void GRBugReporter::GeneratePathDiagnostic(PathDiagnostic& PD, assert(GPair.second.second < bugReports.size()); BugReport *R = bugReports[GPair.second.second]; assert(R && "No original report found for sliced graph."); + assert(R->isValid() && "Report selected from trimmed graph marked invalid."); OwningPtr<ExplodedGraph> ReportGraph(GPair.first.first); OwningPtr<NodeBackMap> BackMap(GPair.first.second); @@ -1870,36 +2030,53 @@ void GRBugReporter::GeneratePathDiagnostic(PathDiagnostic& PD, visitors.push_back((*I)->clone()); // Clear out the active path from any previous work. - PD.getActivePath().clear(); + PD.resetPath(); originalReportConfigToken = R->getConfigurationChangeToken(); // Generate the very last diagnostic piece - the piece is visible before // the trace is expanded. - PathDiagnosticPiece *LastPiece = 0; - for (BugReport::visitor_iterator I = visitors.begin(), E = visitors.end(); - I != E; ++I) { - if (PathDiagnosticPiece *Piece = (*I)->getEndPath(PDB, N, *R)) { - assert (!LastPiece && - "There can only be one final piece in a diagnostic."); - LastPiece = Piece; + if (PDB.getGenerationScheme() != PathDiagnosticConsumer::None) { + PathDiagnosticPiece *LastPiece = 0; + for (BugReport::visitor_iterator I = visitors.begin(), E = visitors.end(); + I != E; ++I) { + if (PathDiagnosticPiece *Piece = (*I)->getEndPath(PDB, N, *R)) { + assert (!LastPiece && + "There can only be one final piece in a diagnostic."); + LastPiece = Piece; + } } + if (!LastPiece) + LastPiece = BugReporterVisitor::getDefaultEndPath(PDB, N, *R); + if (LastPiece) + PD.setEndOfPath(LastPiece); + else + return false; } - if (!LastPiece) - LastPiece = BugReporterVisitor::getDefaultEndPath(PDB, N, *R); - if (LastPiece) - PD.getActivePath().push_back(LastPiece); - else - return; switch (PDB.getGenerationScheme()) { case PathDiagnosticConsumer::Extensive: - GenerateExtensivePathDiagnostic(PD, PDB, N, visitors); + if (!GenerateExtensivePathDiagnostic(PD, PDB, N, visitors)) { + assert(!R->isValid() && "Failed on valid report"); + // Try again. We'll filter out the bad report when we trim the graph. + // FIXME: It would be more efficient to use the same intermediate + // trimmed graph, and just repeat the shortest-path search. + return generatePathDiagnostic(PD, PC, bugReports); + } break; case PathDiagnosticConsumer::Minimal: - GenerateMinimalPathDiagnostic(PD, PDB, N, visitors); + if (!GenerateMinimalPathDiagnostic(PD, PDB, N, visitors)) { + assert(!R->isValid() && "Failed on valid report"); + // Try again. We'll filter out the bad report when we trim the graph. + return generatePathDiagnostic(PD, PC, bugReports); + } break; case PathDiagnosticConsumer::None: - llvm_unreachable("PathDiagnosticConsumer::None should never appear here"); + if (!GenerateVisitorsOnlyPathDiagnostic(PD, PDB, N, visitors)) { + assert(!R->isValid() && "Failed on valid report"); + // Try again. We'll filter out the bad report when we trim the graph. + return generatePathDiagnostic(PD, PC, bugReports); + } + break; } // Clean up the visitors we used. @@ -1910,18 +2087,26 @@ void GRBugReporter::GeneratePathDiagnostic(PathDiagnostic& PD, } while(finalReportConfigToken != originalReportConfigToken); // Finally, prune the diagnostic path of uninteresting stuff. - if (R->shouldPrunePath()) { - bool hasSomethingInteresting = RemoveUneededCalls(PD.getMutablePieces()); - assert(hasSomethingInteresting); - (void) hasSomethingInteresting; + if (!PD.path.empty()) { + // Remove messages that are basically the same. + removeRedundantMsgs(PD.getMutablePieces()); + + if (R->shouldPrunePath()) { + bool hasSomethingInteresting = RemoveUneededCalls(PD.getMutablePieces(), + R); + assert(hasSomethingInteresting); + (void) hasSomethingInteresting; + } } + + return true; } void BugReporter::Register(BugType *BT) { BugTypes = F.add(BugTypes, BT); } -void BugReporter::EmitReport(BugReport* R) { +void BugReporter::emitReport(BugReport* R) { // Compute the bug report's hash to determine its equivalence class. llvm::FoldingSetNodeID ID; R->Profile(ID); @@ -2078,17 +2263,17 @@ void BugReporter::FlushReport(BugReport *exampleReport, OwningPtr<PathDiagnostic> D(new PathDiagnostic(exampleReport->getDeclWithIssue(), exampleReport->getBugType().getName(), - PD.useVerboseDescription() - ? exampleReport->getDescription() - : exampleReport->getShortDescription(), + exampleReport->getDescription(), + exampleReport->getShortDescription(/*Fallback=*/false), BT.getCategory())); // Generate the full path diagnostic, using the generation scheme - // specified by the PathDiagnosticConsumer. - if (PD.getGenerationScheme() != PathDiagnosticConsumer::None) { - if (!bugReports.empty()) - GeneratePathDiagnostic(*D.get(), PD, bugReports); - } + // specified by the PathDiagnosticConsumer. Note that we have to generate + // path diagnostics even for consumers which do not support paths, because + // the BugReporterVisitors may mark this bug as a false positive. + if (!bugReports.empty()) + if (!generatePathDiagnostic(*D.get(), PD, bugReports)) + return; // If the path is empty, generate a single step path with the location // of the issue. @@ -2100,7 +2285,7 @@ void BugReporter::FlushReport(BugReport *exampleReport, llvm::tie(Beg, End) = exampleReport->getRanges(); for ( ; Beg != End; ++Beg) piece->addRange(*Beg); - D->getActivePath().push_back(piece); + D->setEndOfPath(piece); } // Get the meta data. @@ -2124,7 +2309,7 @@ void BugReporter::EmitBasicReport(const Decl *DeclWithIssue, BugReport *R = new BugReport(*BT, str, Loc); R->setDeclWithIssue(DeclWithIssue); for ( ; NumRanges > 0 ; --NumRanges, ++RBeg) R->addRange(*RBeg); - EmitReport(R); + emitReport(R); } BugType *BugReporter::getBugTypeForName(StringRef name, |
