[analyzer] ExplodedGraph printing fixes

[george.karpenkov]

Summary

Fixes a number of issues:

• Global variables are not used for communication
• Trait should be defined on a graph, not on a node
• Defining the trait on a graph allows us to use a correct allocator, no longer crashing while printing trimmed graphs

[NoQ]

Yeah, these globals were weird.

clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h

 } // namespace ento
 
 } // namespace clang
 
 // GraphTraits
 
 namespace llvm {
 
-  template<> struct GraphTraits<clang::ento::ExplodedNode*> {
+  template <> struct GraphTraits<clang::ento::ExplodedGraph *> {
+    using GraphTy = clang::ento::ExplodedGraph *;
     using NodeRef = clang::ento::ExplodedNode *;
     using ChildIteratorType = clang::ento::ExplodedNode::succ_iterator;
-    using nodes_iterator = llvm::df_iterator<NodeRef>;
+    using nodes_iterator = llvm::df_iterator<GraphTy>;
 
-    static NodeRef getEntryNode(NodeRef N) { return N; }
+    static NodeRef getEntryNode(const GraphTy G) {
+      return *G->roots_begin();
+    }
 
     static ChildIteratorType child_begin(NodeRef N) {
       if (N->succ_size() == 1 && (*N->succ_begin())->isTrivial()) {
         return child_begin(*N->succ_begin());
       }
       return N->succ_begin();
     }
 
     static ChildIteratorType child_end(NodeRef N) {
       if (N->succ_size() == 1 && (*N->succ_begin())->isTrivial()) {
         return child_end(*N->succ_begin());
       }
       return N->succ_end();
     }
 
-    static nodes_iterator nodes_begin(NodeRef N) { return df_begin(N); }
-
-    static nodes_iterator nodes_end(NodeRef N) { return df_end(N); }
-  };
-
-  template<> struct GraphTraits<const clang::ento::ExplodedNode*> {
-    using NodeRef = const clang::ento::ExplodedNode *;
-    using ChildIteratorType = clang::ento::ExplodedNode::const_succ_iterator;
-    using nodes_iterator = llvm::df_iterator<NodeRef>;
-
-    static NodeRef getEntryNode(NodeRef N) { return N; }
-
-    static ChildIteratorType child_begin(NodeRef N) { return N->succ_begin(); }
-
-    static ChildIteratorType child_end(NodeRef N) { return N->succ_end(); }
-
-    static nodes_iterator nodes_begin(NodeRef N) { return df_begin(N); }
+    static nodes_iterator nodes_begin(const GraphTy G) {
+      return df_begin(G);
+    }
 
-    static nodes_iterator nodes_end(NodeRef N) { return df_end(N); }
+    static nodes_iterator nodes_end(const GraphTy G) {
+      return df_end(G);
+    }
   };
-
 } // namespace llvm
 
 #endif // LLVM_CLANG_STATICANALYZER_CORE_PATHSENSITIVE_EXPLODEDGRAPH_H

clang/lib/StaticAnalyzer/Core/ExprEngine.cpp

   Bldr.generateNode(A, Pred, Pred->getState());
 }
 
 //===----------------------------------------------------------------------===//
 // Visualization.
 //===----------------------------------------------------------------------===//
 
 #ifndef NDEBUG
-static ExprEngine* GraphPrintCheckerState;
-static SourceManager* GraphPrintSourceManager;
-
 namespace llvm {
 
 template<>
-struct DOTGraphTraits<ExplodedNode*> : public DefaultDOTGraphTraits {
+struct DOTGraphTraits<ExplodedGraph*> : public DefaultDOTGraphTraits {
   DOTGraphTraits (bool isSimple = false) : DefaultDOTGraphTraits(isSimple) {}
 
   // FIXME: Since we do not cache error nodes in ExprEngine now, this does not
   // work.
-  static std::string getNodeAttributes(const ExplodedNode *N, void*) {
+  static std::string getNodeAttributes(const ExplodedNode *N,
+                                       ExplodedGraph *G) {
     return {};
   }
 
   // De-duplicate some source location pretty-printing.
-  static void printLocation(raw_ostream &Out, SourceLocation SLoc) {
+  static void printLocation(raw_ostream &Out,
+                            SourceLocation SLoc,
+                            const SourceManager &SM) {
     if (SLoc.isFileID()) {
       Out << "\\lline="
-        << GraphPrintSourceManager->getExpansionLineNumber(SLoc)
+        << SM.getExpansionLineNumber(SLoc)
         << " col="
-        << GraphPrintSourceManager->getExpansionColumnNumber(SLoc)
+        << SM.getExpansionColumnNumber(SLoc)
         << "\\l";
     }
   }
 
   static void dumpProgramPoint(ProgramPoint Loc,
                                const ASTContext &Context,
                                llvm::raw_string_ostream &Out) {
+    const SourceManager &SM = Context.getSourceManager();
     switch (Loc.getKind()) {
     case ProgramPoint::BlockEntranceKind:
       Out << "Block Entrance: B"
           << Loc.castAs<BlockEntrance>().getBlock()->getBlockID();
       break;
 
     case ProgramPoint::BlockExitKind:
       assert(false);

       Out << "LoopExit: " << LE.getLoopStmt()->getStmtClassName();
       break;
     }
 
     case ProgramPoint::PreImplicitCallKind: {
       ImplicitCallPoint PC = Loc.castAs<ImplicitCallPoint>();
       Out << "PreCall: ";
       PC.getDecl()->print(Out, Context.getLangOpts());
-      printLocation(Out, PC.getLocation());
+      printLocation(Out, PC.getLocation(), SM);
       break;
     }
 
     case ProgramPoint::PostImplicitCallKind: {
       ImplicitCallPoint PC = Loc.castAs<ImplicitCallPoint>();
       Out << "PostCall: ";
       PC.getDecl()->print(Out, Context.getLangOpts());
-      printLocation(Out, PC.getLocation());
+      printLocation(Out, PC.getLocation(), SM);
       break;
     }
 
     case ProgramPoint::PostInitializerKind: {
       Out << "PostInitializer: ";
       const CXXCtorInitializer *Init =
           Loc.castAs<PostInitializer>().getInitializer();
       if (const FieldDecl *FD = Init->getAnyMember())

       if (const Stmt *T = E.getSrc()->getTerminator()) {
         SourceLocation SLoc = T->getBeginLoc();
 
         Out << "\\|Terminator: ";
         E.getSrc()->printTerminator(Out, Context.getLangOpts());
 
         if (SLoc.isFileID()) {
           Out << "\\lline="
-              << GraphPrintSourceManager->getExpansionLineNumber(SLoc)
+              << SM.getExpansionLineNumber(SLoc)
               << " col="
-              << GraphPrintSourceManager->getExpansionColumnNumber(SLoc);
+              << SM.getExpansionColumnNumber(SLoc);
         }
 
         if (isa<SwitchStmt>(T)) {
           const Stmt *Label = E.getDst()->getLabel();
 
           if (Label) {
             if (const auto *C = dyn_cast<CaseStmt>(Label)) {
               Out << "\\lcase ";

     default: {
       const Stmt *S = Loc.castAs<StmtPoint>().getStmt();
       assert(S != nullptr && "Expecting non-null Stmt");
 
       Out << S->getStmtClassName() << " S"
           << S->getID(Context) << " <" << (const void *)S << "> ";
       S->printPretty(Out, /*helper=*/nullptr, Context.getPrintingPolicy(),
                      /*Indentation=*/2, /*NewlineSymbol=*/"\\l");
-      printLocation(Out, S->getBeginLoc());
+      printLocation(Out, S->getBeginLoc(), SM);
 
       if (Loc.getAs<PreStmt>())
         Out << "\\lPreStmt\\l";
       else if (Loc.getAs<PostLoad>())
         Out << "\\lPostLoad\\l";
       else if (Loc.getAs<PostStore>())
         Out << "\\lPostStore\\l";
       else if (Loc.getAs<PostLValue>())
         Out << "\\lPostLValue\\l";
       else if (Loc.getAs<PostAllocatorCall>())
         Out << "\\lPostAllocatorCall\\l";
 
       break;
     }
     }
   }
 
   static bool isNodeHidden(const ExplodedNode *N) {
     return N->isTrivial();
   }
 
-  static std::string getNodeLabel(const ExplodedNode *N, void*){
+  static std::string getNodeLabel(const ExplodedNode *N, ExplodedGraph *G){
     std::string sbuf;
     llvm::raw_string_ostream Out(sbuf);
 
     // Find the first node which program point and tag has to be included in
     // the output.
     const ExplodedNode *FirstHiddenNode = N;
     while (FirstHiddenNode->pred_size() == 1 &&
            isNodeHidden(*FirstHiddenNode->pred_begin())) {

 
       OtherNode = *OtherNode->succ_begin();
 
       Out << "\\l--------\\l";
     }
 
     Out << "\\l\\|";
 
-    ExplodedGraph &Graph =
-        static_cast<ExprEngine *>(State->getStateManager().getOwningEngine())
-            ->getGraph();
-
-    Out << "StateID: ST" << State->getID() << ", NodeID: N" << N->getID(&Graph)
+    Out << "StateID: ST" << State->getID() << ", NodeID: N" << N->getID(G)
         << " <" << (const void *)N << ">\\|";
 
     bool SameAsAllPredecessors =
         std::all_of(N->pred_begin(), N->pred_end(), [&](const ExplodedNode *P) {
           return P->getState() == State;
         });
     if (!SameAsAllPredecessors)
       State->printDOT(Out, N->getLocationContext());

     // Iterate through the reports and get their nodes.
     for (BugReporter::EQClasses_iterator
            EI = BR.EQClasses_begin(), EE = BR.EQClasses_end(); EI != EE; ++EI) {
       const auto *N = const_cast<ExplodedNode *>(EI->begin()->getErrorNode());
       if (N) Src.push_back(N);
     }
 
     ViewGraph(Src);
-  }
-  else {
-    GraphPrintCheckerState = this;
-    GraphPrintSourceManager = &getContext().getSourceManager();
-
-    llvm::ViewGraph(*G.roots_begin(), "ExprEngine");
-
-    GraphPrintCheckerState = nullptr;
-    GraphPrintSourceManager = nullptr;
+  } else {
+    llvm::ViewGraph(&G, "ExprEngine");
   }
 #endif
 }
 
 void ExprEngine::ViewGraph(ArrayRef<const ExplodedNode*> Nodes) {
 #ifndef NDEBUG
-  GraphPrintCheckerState = this;
-  GraphPrintSourceManager = &getContext().getSourceManager();
-
   std::unique_ptr<ExplodedGraph> TrimmedG(G.trim(Nodes));
 
-  if (!TrimmedG.get())
+  if (!TrimmedG.get()) {
     llvm::errs() << "warning: Trimmed ExplodedGraph is empty.\n";
-  else
-    llvm::ViewGraph(*TrimmedG->roots_begin(), "TrimmedExprEngine");
-
-  GraphPrintCheckerState = nullptr;
-  GraphPrintSourceManager = nullptr;
+  } else {
+    llvm::ViewGraph(TrimmedG.get(), "TrimmedExprEngine");
+  }
 #endif
 }