From 431e4e4d0e57b62231e214fa87edb1b3de3da97d Mon Sep 17 00:00:00 2001 From: Jordan Rose Date: Tue, 19 Mar 2013 22:10:35 +0000 Subject: [PATCH] Revert "[analyzer] Do part of the work to find shortest bug paths up front." The whole reason we were doing a BFS in the first place is because an ExplodedGraph can have cycles. Unfortunately, my removeErrorNode "update" doesn't work at all if there are cycles. I'd still like to be able to avoid doing the BFS every time, but I'll come back to it later. This reverts r177353 / 481fa5071c203bc8ba4f88d929780f8d0f8837ba. llvm-svn: 177448 --- clang/lib/StaticAnalyzer/Core/BugReporter.cpp | 157 +++++++------------------- 1 file changed, 42 insertions(+), 115 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Core/BugReporter.cpp b/clang/lib/StaticAnalyzer/Core/BugReporter.cpp index 25792d8..cc67343 100644 --- a/clang/lib/StaticAnalyzer/Core/BugReporter.cpp +++ b/clang/lib/StaticAnalyzer/Core/BugReporter.cpp @@ -1888,109 +1888,86 @@ public: class TrimmedGraph { InterExplodedGraphMap ForwardMap; InterExplodedGraphMap InverseMap; - - typedef llvm::DenseMap PriorityMapTy; - PriorityMapTy PriorityMap; - OwningPtr G; - const ExplodedNode *Root; public: + /// TrimmedGraph(const ExplodedGraph *OriginalGraph, - ArrayRef Nodes); + ArrayRef Nodes) { + // The trimmed graph is created in the body of the constructor to ensure + // that the DenseMaps have been initialized already. + G.reset(OriginalGraph->trim(Nodes, &ForwardMap, &InverseMap)); + } void createBestReportGraph(ArrayRef Nodes, ReportGraph &GraphWrapper) const; - - void removeErrorNode(const ExplodedNode *Node); }; } -TrimmedGraph::TrimmedGraph(const ExplodedGraph *OriginalGraph, - ArrayRef Nodes) { - // The trimmed graph is created in the body of the constructor to ensure - // that the DenseMaps have been initialized already. - G.reset(OriginalGraph->trim(Nodes, &ForwardMap, &InverseMap)); + +void TrimmedGraph::createBestReportGraph(ArrayRef Nodes, + ReportGraph &GraphWrapper) const { + assert(!GraphWrapper.Graph && "ReportGraph is already in use"); + assert(GraphWrapper.BackMap.empty() && "ReportGraph is already in use"); // Find the (first) error node in the trimmed graph. We just need to consult // the node map which maps from nodes in the original graph to nodes // in the new graph. - std::queue > WS; + std::queue WS; typedef llvm::SmallDenseMap IndexMapTy; - IndexMapTy IndexMap(llvm::NextPowerOf2(Nodes.size() + 1)); + IndexMapTy IndexMap; for (unsigned i = 0, count = Nodes.size(); i < count; ++i) { const ExplodedNode *OriginalNode = Nodes[i]; if (const ExplodedNode *N = ForwardMap.lookup(OriginalNode)) { - WS.push(std::make_pair(N, 0)); + WS.push(N); IndexMap[OriginalNode] = i; } } assert(!WS.empty() && "No error node found in the trimmed graph."); - // Perform a reverse BFS to find all the shortest paths. - Root = 0; + // Create a new graph with a single path. This is the graph + // that will be returned to the caller. + ExplodedGraph *GNew = new ExplodedGraph(); + GraphWrapper.Graph.reset(GNew); + + // Sometimes the trimmed graph can contain a cycle. Perform a reverse BFS + // to the root node, and then construct a new graph that contains only + // a single path. + llvm::DenseMap Visited; + + unsigned cnt = 0; + const ExplodedNode *Root = 0; + while (!WS.empty()) { - const ExplodedNode *Node; - unsigned Priority; - llvm::tie(Node, Priority) = WS.front(); + const ExplodedNode *Node = WS.front(); WS.pop(); - PriorityMapTy::iterator I = PriorityMap.find(Node); - if (I != PriorityMap.end()) { - assert(I->second <= Priority); + if (Visited.find(Node) != Visited.end()) continue; - } - PriorityMap[Node] = Priority; + Visited[Node] = cnt++; - if (Node->pred_empty()) + if (Node->pred_empty()) { Root = Node; - - for (ExplodedNode::const_pred_iterator I = Node->pred_begin(), - E = Node->pred_end(); - I != E; ++I) - WS.push(std::make_pair(*I, Priority + 1)); - } - - assert(Root); -} - -void TrimmedGraph::createBestReportGraph(ArrayRef Nodes, - ReportGraph &GraphWrapper) const { - assert(!GraphWrapper.Graph && "ReportGraph is already in use"); - assert(GraphWrapper.BackMap.empty() && "ReportGraph is already in use"); - - // Find the (first) error node in the trimmed graph. We just need to consult - // the node map which maps from nodes in the original graph to nodes - // in the new graph. - std::queue > WS; - typedef llvm::SmallDenseMap IndexMapTy; - IndexMapTy IndexMap(llvm::NextPowerOf2(Nodes.size() + 1)); - - for (unsigned i = 0, count = Nodes.size(); i < count; ++i) { - const ExplodedNode *OriginalNode = Nodes[i]; - if (const ExplodedNode *N = ForwardMap.lookup(OriginalNode)) { - WS.push(std::make_pair(N, 0)); - IndexMap[OriginalNode] = i; + break; } - } - assert(!WS.empty() && "No error node found in the trimmed graph."); + for (ExplodedNode::const_pred_iterator I=Node->pred_begin(), + E=Node->pred_end(); I!=E; ++I) + WS.push(*I); + } - // Create a new graph with a single path. This is the graph - // that will be returned to the caller. - ExplodedGraph *GNew = new ExplodedGraph(); - GraphWrapper.Graph.reset(GNew); + assert(Root); // Now walk from the root down the BFS path, always taking the successor // with the lowest number. ExplodedNode *Last = 0; for ( const ExplodedNode *N = Root ;;) { // Lookup the number associated with the current node. - PriorityMapTy::const_iterator I = PriorityMap.find(N); - assert(I != PriorityMap.end()); + llvm::DenseMap::iterator I = Visited.find(N); + assert(I != Visited.end()); // Create the equivalent node in the new graph with the same state // and location. @@ -2021,9 +1998,9 @@ void TrimmedGraph::createBestReportGraph(ArrayRef Nodes, for (ExplodedNode::const_succ_iterator SI = N->succ_begin(), SE = N->succ_end(); SI != SE; ++SI) { - I = PriorityMap.find(*SI); + I = Visited.find(*SI); - if (I == PriorityMap.end()) + if (I == Visited.end()) continue; if (I->second < MinVal) { @@ -2036,54 +2013,6 @@ void TrimmedGraph::createBestReportGraph(ArrayRef Nodes, } } -void TrimmedGraph::removeErrorNode(const ExplodedNode *ErrorNode) { - ErrorNode = ForwardMap[ErrorNode]; - assert(ErrorNode && "not an error node"); - - PriorityMapTy::iterator PriorityEntry = PriorityMap.find(ErrorNode); - assert(PriorityEntry != PriorityMap.end() && "error node already removed"); - PriorityMap.erase(PriorityEntry); - - std::queue WS; - for (ExplodedNode::const_pred_iterator PI = ErrorNode->pred_begin(), - PE = ErrorNode->pred_end(); - PI != PE; ++PI) { - assert(PriorityMap.find(*PI) != PriorityMap.end()); - WS.push(*PI); - } - - // Update all nodes possibly affected by this change. - while (!WS.empty()) { - const ExplodedNode *N = WS.front(); - WS.pop(); - - unsigned MinPriority = -1U; - for (ExplodedNode::const_succ_iterator SI = N->succ_begin(), - SE = N->succ_end(); - SI != SE; ++SI) { - PriorityMapTy::iterator SuccEntry = PriorityMap.find(*SI); - if (SuccEntry == PriorityMap.end()) - continue; - MinPriority = std::min(SuccEntry->second, MinPriority); - } - - if (MinPriority == -1U) - PriorityMap.erase(N); - else if (PriorityMap[N] == MinPriority + 1) - continue; - else - PriorityMap[N] = MinPriority + 1; - - for (ExplodedNode::const_pred_iterator PI = N->pred_begin(), - PE = N->pred_end(); - PI != PE; ++PI) { - assert(PriorityMap.find(*PI) != PriorityMap.end()); - WS.push(*PI); - } - } -} - - /// CompactPathDiagnostic - This function postprocesses a PathDiagnostic object /// and collapses PathDiagosticPieces that are expanded by macros. static void CompactPathDiagnostic(PathPieces &path, const SourceManager& SM) { @@ -2286,10 +2215,8 @@ bool GRBugReporter::generatePathDiagnostic(PathDiagnostic& PD, finalReportConfigToken = R->getConfigurationChangeToken(); } while (finalReportConfigToken != origReportConfigToken); - if (!R->isValid()) { - TrimG.removeErrorNode(R->getErrorNode()); + if (!R->isValid()) continue; - } // Finally, prune the diagnostic path of uninteresting stuff. if (!PD.path.empty()) { -- 2.7.4