[analyzer] print() JSONify: getNodeLabel implementation

[Charusso]

Summary

This patch also rewrites the ProgramPoint printing.

[Charusso]

This one it a little-bit heavier what it should be. I have added the JsonSupport.h because I believe we have to space it at some point because some lines are too long. It was your request @NoQ to create something identical to the original spacing. Should I touch the spacing, or it is okay?

[NoQ]

clang/lib/Analysis/ProgramPoint.cpp

65 ↗

(On Diff #201069)

If i was to change one thing about these dumps, it'd be to make the spaces consistent.

89 ↗

(On Diff #201069)

Cf.

clang/lib/StaticAnalyzer/Core/ExprEngine.cpp

3072–3094 ↗

(On Diff #201069)

The predecessors/successors part isn't very useful because we're combining multiple ExplodedGraph nodes into a single dumped node. If we print them at all, we should probably combine them with the ProgramPoint print (as each ProgramPoint corresponds to a real ExplodedGraph node). But i don't really find this info useful, as it's already displayed with arrows.

clang/test/Analysis/dump_egraph.c

13 ↗

(On Diff #201069)

Again, let's test the planned HTML generator separately.

[NoQ]

Great job tho!

[Charusso]

What do you mean by consistent spacing?

We had:

Block Entrance: B2
--------
PreStmtPurgeDeadSymbols
Tag:ExprEngine : Clean Node
--------
DeclRefExpr S2079258 <0x19d70c0> RM
line=2715 col=11
PostLValue

We have:

"program_points": [
  { "kind": "Block Entrance", "block_id": 2, "tag": null }
  { "kind": "PreStmtPurgeDeadSymbols", "tag": "ExprEngine : Clean Node" }
  { "kind": "Statement", "stmt_kind": "DeclRefExpr", "stmt_id": 2079258, "pointer": 0x3683e80, "pretty": "RM", "line": 2715, "column": 11, "stmt_point_kind": "PostLValue", "tag": null }
],

Could you show that by an example what improvements would you see? I think it is just long, but not problematic. My main problem is the order of the usefulness of the information, but it is a problem in every print() so I have not touched that yet.

[Charusso]

• 100% pure JSON.
• Removed pred/succ.
• Removed TODO.

[Charusso]

• Just rebase.

[NoQ]

What do you mean by consistent spacing?

I meant BlockEntrance vs. Block Entrance, etc.

[Charusso]

• Remove unintended spaces.

include/clang/Analysis/ProgramPoint.h

public:
   void Profile(llvm::FoldingSetNodeID& ID) const {
     ID.AddInteger((unsigned) getKind());
     ID.AddPointer(getData1());
     ID.AddPointer(getData2());
     ID.AddPointer(getLocationContext());
     ID.AddPointer(getTag());
   }
 
-  void print(StringRef CR, llvm::raw_ostream &Out) const;
+  void printJson(llvm::raw_ostream &Out, const char *NL = "\n") const;
 
   LLVM_DUMP_METHOD void dump() const;
 
   static ProgramPoint getProgramPoint(const Stmt *S, ProgramPoint::Kind K,
                                       const LocationContext *LC,
                                       const ProgramPointTag *tag);
 };
 

lib/Analysis/ProgramPoint.cpp

switch (K) {
     case ProgramPoint::PostStmtPurgeDeadSymbolsKind:
       return PostStmtPurgeDeadSymbols(S, LC, tag);
     case ProgramPoint::PreStmtPurgeDeadSymbolsKind:
       return PreStmtPurgeDeadSymbols(S, LC, tag);
   }
 }
 
 LLVM_DUMP_METHOD void ProgramPoint::dump() const {
-  return print(/*CR=*/"\n", llvm::errs());
+  return printJson(llvm::errs());
 }
 
-static void printLocation(raw_ostream &Out, SourceLocation SLoc,
-                          const SourceManager &SM,
-                          StringRef CR,
-                          StringRef Postfix) {
-  if (SLoc.isFileID()) {
-    Out << CR << "line=" << SM.getExpansionLineNumber(SLoc)
-        << " col=" << SM.getExpansionColumnNumber(SLoc) << Postfix;
+static void printLocation(raw_ostream &Out, SourceLocation Loc,
+                          const SourceManager &SM) {
+  Out << "\"location\": ";
+  if (!Loc.isFileID()) {
+    Out << "null";
+    return;
   }
+
+  Out << "{ \"line\": " << SM.getExpansionLineNumber(Loc)
+      << ", \"column\": " << SM.getExpansionColumnNumber(Loc) << " }";
 }
 
-void ProgramPoint::print(StringRef CR, llvm::raw_ostream &Out) const {
+void ProgramPoint::printJson(llvm::raw_ostream &Out, const char *NL) const {
   const ASTContext &Context =
       getLocationContext()->getAnalysisDeclContext()->getASTContext();
   const SourceManager &SM = Context.getSourceManager();
+
+  Out << "\"kind\": \"";
   switch (getKind()) {
   case ProgramPoint::BlockEntranceKind:
-    Out << "Block Entrance: B"
+    Out << "BlockEntrance\""
+        << ", \"block_id\": "
         << castAs<BlockEntrance>().getBlock()->getBlockID();
     break;
 
   case ProgramPoint::FunctionExitKind: {
     auto FEP = getAs<FunctionExitPoint>();
-    Out << "Function Exit: B" << FEP->getBlock()->getBlockID();
+    Out << "FunctionExit\""
+        << ", \"block_id\": " << FEP->getBlock()->getBlockID()
+        << ", \"stmt_id\": ";
+
     if (const ReturnStmt *RS = FEP->getStmt()) {
-      Out << CR << " Return: S" << RS->getID(Context) << CR;
-      RS->printPretty(Out, /*helper=*/nullptr, Context.getPrintingPolicy(),
-                      /*Indentation=*/2, /*NewlineSymbol=*/CR);
+      Out << RS->getID(Context) << ", \"stmt\": \"";
+      RS->printPretty(Out, /*Helper=*/nullptr, Context.getPrintingPolicy());
+      Out << '\"';
+    } else {
+      Out << "null, \"stmt\": null";
     }
     break;
   }
   case ProgramPoint::BlockExitKind:
-    assert(false);
+    llvm_unreachable("BlockExitKind");
     break;
-
   case ProgramPoint::CallEnterKind:
-    Out << "CallEnter";
+    Out << "CallEnter\"";
     break;
-
   case ProgramPoint::CallExitBeginKind:
-    Out << "CallExitBegin";
+    Out << "CallExitBegin\"";
     break;
-
   case ProgramPoint::CallExitEndKind:
-    Out << "CallExitEnd";
+    Out << "CallExitEnd\"";
     break;
-
   case ProgramPoint::PostStmtPurgeDeadSymbolsKind:
-    Out << "PostStmtPurgeDeadSymbols";
+    Out << "PostStmtPurgeDeadSymbols\"";
     break;
-
   case ProgramPoint::PreStmtPurgeDeadSymbolsKind:
-    Out << "PreStmtPurgeDeadSymbols";
+    Out << "PreStmtPurgeDeadSymbols\"";
     break;
-
   case ProgramPoint::EpsilonKind:
-    Out << "Epsilon Point";
+    Out << "EpsilonPoint\"";
     break;
 
-  case ProgramPoint::LoopExitKind: {
-    LoopExit LE = castAs<LoopExit>();
-    Out << "LoopExit: " << LE.getLoopStmt()->getStmtClassName();
+  case ProgramPoint::LoopExitKind:
+    Out << "LoopExit\", \"stmt\": \""
+        << castAs<LoopExit>().getLoopStmt()->getStmtClassName() << '\"';
     break;
-  }
 
   case ProgramPoint::PreImplicitCallKind: {
     ImplicitCallPoint PC = castAs<ImplicitCallPoint>();
-    Out << "PreCall: ";
+    Out << "PreCall\", \"stmt\": \"";
     PC.getDecl()->print(Out, Context.getLangOpts());
-    printLocation(Out, PC.getLocation(), SM, CR, /*Postfix=*/CR);
+    Out << "\", ";
+    printLocation(Out, PC.getLocation(), SM);
     break;
   }
 
   case ProgramPoint::PostImplicitCallKind: {
     ImplicitCallPoint PC = castAs<ImplicitCallPoint>();
-    Out << "PostCall: ";
+    Out << "PostCall\", \"stmt\": \"";
     PC.getDecl()->print(Out, Context.getLangOpts());
-    printLocation(Out, PC.getLocation(), SM, CR, /*Postfix=*/CR);
+    Out << "\", ";
+    printLocation(Out, PC.getLocation(), SM);
     break;
   }
 
   case ProgramPoint::PostInitializerKind: {
-    Out << "PostInitializer: ";
+    Out << "PostInitializer\", ";
     const CXXCtorInitializer *Init = castAs<PostInitializer>().getInitializer();
-    if (const FieldDecl *FD = Init->getAnyMember())
-      Out << *FD;
-    else {
+    if (const FieldDecl *FD = Init->getAnyMember()) {
+      Out << "\"field_decl\": \"" << *FD << '\"';
+    } else {
+      Out << "\"type\": \"";
       QualType Ty = Init->getTypeSourceInfo()->getType();
       Ty = Ty.getLocalUnqualifiedType();
       Ty.print(Out, Context.getLangOpts());
+      Out << '\"';
     }
     break;
   }
 
   case ProgramPoint::BlockEdgeKind: {
     const BlockEdge &E = castAs<BlockEdge>();
-    Out << "Edge: (B" << E.getSrc()->getBlockID() << ", B"
-        << E.getDst()->getBlockID() << ')';
-
-    if (const Stmt *T = E.getSrc()->getTerminatorStmt()) {
-      SourceLocation SLoc = T->getBeginLoc();
+    const Stmt *T = E.getSrc()->getTerminatorStmt();
+    Out << "Edge\", \"src_id\": " << E.getSrc()->getBlockID()
+        << ", \"dst_id\": " << E.getDst()->getBlockID()
+        << ", \"terminator\": " << (!T ? "null, \"term_kind\": null" : "\"");
+    if (!T)
+      break;
 
-      Out << "\\|Terminator: ";
-      E.getSrc()->printTerminator(Out, Context.getLangOpts());
-      printLocation(Out, SLoc, SM, CR, /*Postfix=*/"");
+    Out << "\", ";
+    printLocation(Out, T->getBeginLoc(), SM);
+    Out << ", \"term_kind\": \"";
 
-      if (isa<SwitchStmt>(T)) {
-        const Stmt *Label = E.getDst()->getLabel();
-
-        if (Label) {
-          if (const auto *C = dyn_cast<CaseStmt>(Label)) {
-            Out << CR << "case ";
-            if (C->getLHS())
-              C->getLHS()->printPretty(
-                  Out, nullptr, Context.getPrintingPolicy(),
-                  /*Indentation=*/0, /*NewlineSymbol=*/CR);
-
-            if (const Stmt *RHS = C->getRHS()) {
-              Out << " .. ";
-              RHS->printPretty(Out, nullptr, Context.getPrintingPolicy(),
-                               /*Indetation=*/0, /*NewlineSymbol=*/CR);
-            }
-
-            Out << ":";
-          } else {
-            assert(isa<DefaultStmt>(Label));
-            Out << CR << "default:";
-          }
-        } else
-          Out << CR << "(implicit) default:";
-      } else if (isa<IndirectGotoStmt>(T)) {
-        // FIXME
+      Out << "SwitchStmt\", \"case\": ";
+      if (const Stmt *Label = E.getDst()->getLabel()) {
+          Out << "{ \"lhs\": ";
+          if (const Stmt *LHS = C->getLHS())
+            LHS->printPretty(Out, nullptr, Context.getPrintingPolicy());
+          else
+            Out << "null";
+          Out << ", \"rhs\": ";
+          if (const Stmt *RHS = C->getRHS())
+            RHS->printPretty(Out, nullptr, Context.getPrintingPolicy());
+          else
+            Out << "null";
+          Out << " }";
+          Out << "\"default\"";
       } else {
-        Out << CR << "Condition: ";
-        if (*E.getSrc()->succ_begin() == E.getDst())
-          Out << "true";
-        else
-          Out << "false";
+        Out << "\"implicit default\"";
       }
-
-      Out << CR;
+    } else if (isa<IndirectGotoStmt>(T)) {
+      // FIXME: More info.
+      Out << "IndirectGotoStmt\"";
+    } else {
+      Out << "Condition\", \"value\": "
+          << (*E.getSrc()->succ_begin() == E.getDst() ? "true" : "false");
     }
-
     break;
   }
 
   default: {
     const Stmt *S = 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=*/CR);
-    printLocation(Out, S->getBeginLoc(), SM, CR, /*Postfix=*/"");
+    llvm::SmallString<256> TempBuf;
+    llvm::raw_svector_ostream TempOut(TempBuf);
 
+    Out << "Statement\", \"stmt_kind\": \"" << S->getStmtClassName()
+        << "\", \"stmt_id\": " << S->getID(Context)
+        << ", \"pointer\": \"" << (const void *)S << "\", \"pretty\": ";
+
+    // See whether the current statement is pretty-printable.
+    S->printPretty(TempOut, /*Helper=*/nullptr, Context.getPrintingPolicy());
+    if (!TempBuf.empty()) {
+      Out << '\"' << TempBuf.str().trim() << "\", ";
+      TempBuf.clear();
+    } else {
+      Out << "null, ";
+    }
+
+    printLocation(Out, S->getBeginLoc(), SM);
+
+    Out << ", \"stmt_point_kind\": ";
     if (getAs<PreStmt>())
-      Out << CR << "PreStmt" << CR;
+      Out << "\"PreStmt\"";
     else if (getAs<PostLoad>())
-      Out << CR << "PostLoad" << CR;
+      Out << "\"PostLoad\"";
     else if (getAs<PostStore>())
-      Out << CR << "PostStore" << CR;
+      Out << "\"PostStore\"";
     else if (getAs<PostLValue>())
-      Out << CR << "PostLValue" << CR;
+      Out << "\"PostLValue\"";
     else if (getAs<PostAllocatorCall>())
-      Out << CR << "PostAllocatorCall" << CR;
+      Out << "\"PostAllocatorCall\"";
+    else
+      Out << "null";
 
     break;
   }
   }
 }
 
 SimpleProgramPointTag::SimpleProgramPointTag(StringRef MsgProvider,
                                              StringRef Msg)
   : Desc((MsgProvider + " : " + Msg).str()) {}
 
 StringRef SimpleProgramPointTag::getTagDescription() const {
   return Desc;
 }

lib/StaticAnalyzer/Core/ExprEngine.cpp

void printJson(llvm::raw_ostream &Out, PrinterHelper *Helper,
     else
       Out << "\"init_id\": " << I->getID(getASTContext());
 
     // Kind
     Out << ", \"kind\": \"" << getItem().getKindAsString()
         << "\", \"argument_index\": ";
 
     if (getItem().getKind() == ConstructionContextItem::ArgumentKind)
-      Out << getItem().getIndex() << '\"';
+      Out << getItem().getIndex();
     else
       Out << "null";
 
     // Pretty-print
-    Out << ", \"pretty\": \"";
+    Out << ", \"pretty\": ";
 
     if (S) {
       llvm::SmallString<256> TempBuf;
       llvm::raw_svector_ostream TempOut(TempBuf);
 
       // See whether the current statement is pretty-printable.
       S->printPretty(TempOut, Helper, PP);
       if (!TempBuf.empty()) {
-        Out << TempBuf.str().trim() << '\"';
+        Out << '\"' << TempBuf.str().trim() << '\"';
         TempBuf.clear();
       } else {
         Out << "null";
       }
     } else {
-      Out << I->getAnyMember()->getNameAsString() << '\"';
+      Out << '\"' << I->getAnyMember()->getNameAsString() << '\"';
     }
   }
 
   void Profile(llvm::FoldingSetNodeID &ID) const {
     ID.Add(Impl.first);
     ID.AddPointer(Impl.second);
   }
 
static std::string getNodeAttributes(const ExplodedNode *N,
     return llvm::join(Out, ",");
   }
 
   static bool isNodeHidden(const ExplodedNode *N) {
     return N->isTrivial();
   }
 
   static std::string getNodeLabel(const ExplodedNode *N, ExplodedGraph *G){
-    std::string sbuf;
-    llvm::raw_string_ostream Out(sbuf);
+    std::string Buf;
+    llvm::raw_string_ostream Out(Buf);
 
+    const bool IsDot = true;
+    const unsigned int Space = 1;
     ProgramStateRef State = N->getState();
 
+    Out << "{ \"node_id\": \"" << (const void *)N
+        << "\", \"state_id\": " << State->getID()
+        << ", \"has_report\": " << (nodeHasBugReport(N) ? "true" : "false")
+        << ",\\l";
+
+    Indent(Out, Space, IsDot) << "\"program_points\": [\\l";
+
     // Dump program point for all the previously skipped nodes.
     traverseHiddenNodes(
         N,
         [&](const ExplodedNode *OtherNode) {
-          OtherNode->getLocation().print(/*CR=*/"\\l", Out);
+          Indent(Out, Space + 1, IsDot) << "{ ";
+          OtherNode->getLocation().printJson(Out, /*NL=*/"\\l");
+          Out << ", \"tag\": ";
           if (const ProgramPointTag *Tag = OtherNode->getLocation().getTag())
-            Out << "\\lTag:" << Tag->getTagDescription();
-          if (N->isSink())
-            Out << "\\lNode is sink\\l";
-          if (nodeHasBugReport(N))
-            Out << "\\lBug report attached\\l";
+            Out << '\"' << Tag->getTagDescription() << "\" }";
+          else
+            Out << "null }";
         },
-        [&](const ExplodedNode *) { Out << "\\l--------\\l"; },
+  // Adds a comma and a new-line between each program point.
+        [&](const ExplodedNode *) { Out << ",\\l"; },
         [&](const ExplodedNode *) { return false; });
 
-    Out << "\\l\\|";
-
-    Out << "StateID: ST" << State->getID() << ", NodeID: N" << N->getID(G)
-        << " <" << (const void *)N << ">\\|";
+    Out << "\\l"; // Adds a new-line to the last program point.
+    Indent(Out, Space, IsDot) << "],\\l";
 
     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());
+
+    if (!SameAsAllPredecessors) {
+      State->printDOT(Out, N->getLocationContext(), Space);
+    } else {
+      Indent(Out, Space, IsDot) << "\"program_state\": null";
+    }
+
+    Out << "\\l}";
+    if (!N->succ_empty())
+      Out << ',';
+    Out << "\\l";
+
     return Out.str();
   }
 };
 
 } // namespace llvm
 #endif
 
 void ExprEngine::ViewGraph(bool trim) {

test/Analysis/dump_egraph.c

 // RUN: %clang_analyze_cc1 -analyzer-checker=core -analyzer-dump-egraph=%t.dot %s
 // RUN: cat %t.dot | FileCheck %s
 // RUN: %clang_analyze_cc1 -analyzer-checker=core -analyzer-dump-egraph=%t.dot -trim-egraph %s
 // REQUIRES: asserts
 
 int getJ();
 
 int foo() {
   int *x = 0, *y = 0;
   return *x + *y;
 }
 
 // CHECK: digraph "Exploded Graph" {
-// CHECK: Edge: (B2, B1)
-// CHECK: Block Entrance: B1
-// CHECK: Bug report attached
+
+// CHECK: \"program_points\": [\l    \{ \"kind\": \"Edge\", \"src_id\": 2, \"dst_id\": 1, \"terminator\": null, \"term_kind\": null, \"tag\": null \}\l  ],\l  \"program_state\": null
+
+// CHECK: \"program_points\": [\l    \{ \"kind\": \"BlockEntrance\", \"block_id\": 1
+
+// CHECK: \"has_report\": true
+

test/Analysis/dump_egraph.cpp

struct T {
   T() : s() {}
 };
 
 void foo() {
   // Test that dumping symbols conjured on null statements doesn't crash.
   T t;
 }
 
-// CHECK: \"constructing_objects\": [\l    \{ \"location_context\": \"#0 Call\", \"calling\": \"foo\", \"call_line\": null, \"items\": [\l      \{ \"lctx_id\": 1, \"stmt_id\": 1155, \"kind\": \"construct into local variable\", \"argument_index\": null, \"pretty\": \"T t;\", \"value\": \"&t\"
+// CHECK: \"constructing_objects\": [\l      \{ \"location_context\": \"#0 Call\", \"calling\": \"foo\", \"call_line\": null, \"items\": [\l        \{ \"lctx_id\": 1, \"stmt_id\": 1155, \"kind\": \"construct into local variable\", \"argument_index\": null, \"pretty\": \"T t;\", \"value\": \"&t\"
 
-// CHECK: \"constructing_objects\": [\l    \{ \"location_context\": \"#0 Call\", \"calling\": \"T::T\", \"call_line\": \"16\", \"items\": [\l      \{ \"lctx_id\": 2, \"init_id\": 1092, \"kind\": \"construct into member variable\", \"argument_index\": null, \"pretty\": \"s\", \"value\": \"&t-\>s\"
+// CHECK: \"constructing_objects\": [\l      \{ \"location_context\": \"#0 Call\", \"calling\": \"T::T\", \"call_line\": \"16\", \"items\": [\l        \{ \"lctx_id\": 2, \"init_id\": 1092, \"kind\": \"construct into member variable\", \"argument_index\": null, \"pretty\": \"s\", \"value\": \"&t-\>s\"
 
-// CHECK: \"store\": [\l    \{ \"cluster\": \"t\", \"items\": [\l      \{ \"kind\": \"Default\", \"offset\": 0, \"value\": \"conj_$3\{int, LC3, no stmt, #1\}\"
+// CHECK: \"store\": [\l      \{ \"cluster\": \"t\", \"items\": [\l        \{ \"kind\": \"Default\", \"offset\": 0, \"value\": \"conj_$3\{int, LC3, no stmt, #1\}\"