Re: [PATCH] D22622: [analyzer] Add more info to exploded graph dumps

[Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL276557: [analyzer] Pring LocationContext in ExplodedGraph 
dumps. (authored by dergachev).

Changed prior to commit:
  https://reviews.llvm.org/D22622?vs=65223=65267#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D22622

Files:
  cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp

Index: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -2521,26 +2521,10 @@
   // FIXME: Since we do not cache error nodes in ExprEngine now, this does not
   // work.
   static std::string getNodeAttributes(const ExplodedNode *N, void*) {
-
-#if 0
-  // FIXME: Replace with a general scheme to tell if the node is
-  // an error node.
-if (GraphPrintCheckerState->isImplicitNullDeref(N) ||
-GraphPrintCheckerState->isExplicitNullDeref(N) ||
-GraphPrintCheckerState->isUndefDeref(N) ||
-GraphPrintCheckerState->isUndefStore(N) ||
-GraphPrintCheckerState->isUndefControlFlow(N) ||
-GraphPrintCheckerState->isUndefResult(N) ||
-GraphPrintCheckerState->isBadCall(N) ||
-GraphPrintCheckerState->isUndefArg(N))
-  return "color=\"red\",style=\"filled\"";
-
-if (GraphPrintCheckerState->isNoReturnCall(N))
-  return "color=\"blue\",style=\"filled\"";
-#endif
 return "";
   }
 
+  // De-duplicate some source location pretty-printing.
   static void printLocation(raw_ostream , SourceLocation SLoc) {
 if (SLoc.isFileID()) {
   Out << "\\lline="
@@ -2550,6 +2534,12 @@
 << "\\l";
 }
   }
+  static void printLocation2(raw_ostream , SourceLocation SLoc) {
+if (SLoc.isFileID() && GraphPrintSourceManager->isInMainFile(SLoc))
+  Out << "line " << GraphPrintSourceManager->getExpansionLineNumber(SLoc);
+else
+  SLoc.print(Out, *GraphPrintSourceManager);
+  }
 
   static std::string getNodeLabel(const ExplodedNode *N, void*){
 
@@ -2563,12 +2553,6 @@
   case ProgramPoint::BlockEntranceKind: {
 Out << "Block Entrance: B"
 << Loc.castAs().getBlock()->getBlockID();
-if (const NamedDecl *ND =
-dyn_cast(Loc.getLocationContext()->getDecl())) {
-  Out << " (";
-  ND->printName(Out);
-  Out << ")";
-}
 break;
   }
 
@@ -2693,13 +2677,6 @@
   Out << "\\l";
 }
 
-#if 0
-  // FIXME: Replace with a general scheme to determine
-  // the name of the check.
-if (GraphPrintCheckerState->isUndefControlFlow(N)) {
-  Out << "\\|Control-flow based on\\lUndefined value.\\l";
-}
-#endif
 break;
   }
 
@@ -2721,34 +2698,47 @@
 else if (Loc.getAs())
   Out << "\\lPostLValue\\l";
 
-#if 0
-  // FIXME: Replace with a general scheme to determine
-  // the name of the check.
-if (GraphPrintCheckerState->isImplicitNullDeref(N))
-  Out << "\\|Implicit-Null Dereference.\\l";
-else if (GraphPrintCheckerState->isExplicitNullDeref(N))
-  Out << "\\|Explicit-Null Dereference.\\l";
-else if (GraphPrintCheckerState->isUndefDeref(N))
-  Out << "\\|Dereference of undefialied value.\\l";
-else if (GraphPrintCheckerState->isUndefStore(N))
-  Out << "\\|Store to Undefined Loc.";
-else if (GraphPrintCheckerState->isUndefResult(N))
-  Out << "\\|Result of operation is undefined.";
-else if (GraphPrintCheckerState->isNoReturnCall(N))
-  Out << "\\|Call to function marked \"noreturn\".";
-else if (GraphPrintCheckerState->isBadCall(N))
-  Out << "\\|Call to NULL/Undefined.";
-else if (GraphPrintCheckerState->isUndefArg(N))
-  Out << "\\|Argument in call is undefined";
-#endif
-
 break;
   }
 }
 
 ProgramStateRef state = N->getState();
 Out << "\\|StateID: " << (const void*) state.get()
 << " NodeID: " << (const void*) N << "\\|";
+
+// Analysis stack backtrace.
+Out << "Location context stack (from current to outer):\\l";
+const LocationContext *LC = Loc.getLocationContext();
+unsigned Idx = 0;
+for (; LC; LC = LC->getParent(), ++Idx) {
+  Out << Idx << ". (" << (const void *)LC << ") ";
+  switch (LC->getKind()) {
+  case LocationContext::StackFrame:
+if (const NamedDecl *D = dyn_cast(LC->getDecl()))
+  Out << "Calling " << D->getQualifiedNameAsString();
+else
+  Out << "Calling anonymous code";
+if (const Stmt *S = cast(LC)->getCallSite()) {
+  Out << " at ";
+  printLocation2(Out, S->getLocStart());
+}
+break;
+  case LocationContext::Block:
+Out << "Invoking block";
+if (const Decl *D = cast(LC)->getBlockDecl()) {
+  Out << " defined 

Re: [PATCH] D22622: [analyzer] Add more info to exploded graph dumps

[Anna Zaks via cfe-commits]

zaks.anna accepted this revision.
zaks.anna added a comment.
This revision is now accepted and ready to land.

> Yeah, just since we're all here anyway... By the way, -analyzer-config 
> prune-paths=false is also very handy.


I think you should add these to the checker writer manual in the debugging 
section.


https://reviews.llvm.org/D22622



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D22622: [analyzer] Add more info to exploded graph dumps

[Artem Dergachev via cfe-commits]

NoQ added a comment.

> We could add file names but only in cases the file name does not match the 
> main file.


Whoops, right, i guess i'd just fall back to the default source location 
printing routine for this case (or for macros).

F2198462: 3.png 

> This is not something that the patch addresses.


Yeah, just since we're all here anyway... By the way, `-analyzer-config 
prune-paths=false` is also very handy.


https://reviews.llvm.org/D22622



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D22622: [analyzer] Add more info to exploded graph dumps

[Artem Dergachev via cfe-commits]

NoQ updated this revision to Diff 65223.

https://reviews.llvm.org/D22622

Files:
  lib/StaticAnalyzer/Core/ExprEngine.cpp

Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -2521,26 +2521,10 @@
   // FIXME: Since we do not cache error nodes in ExprEngine now, this does not
   // work.
   static std::string getNodeAttributes(const ExplodedNode *N, void*) {
-
-#if 0
-  // FIXME: Replace with a general scheme to tell if the node is
-  // an error node.
-if (GraphPrintCheckerState->isImplicitNullDeref(N) ||
-GraphPrintCheckerState->isExplicitNullDeref(N) ||
-GraphPrintCheckerState->isUndefDeref(N) ||
-GraphPrintCheckerState->isUndefStore(N) ||
-GraphPrintCheckerState->isUndefControlFlow(N) ||
-GraphPrintCheckerState->isUndefResult(N) ||
-GraphPrintCheckerState->isBadCall(N) ||
-GraphPrintCheckerState->isUndefArg(N))
-  return "color=\"red\",style=\"filled\"";
-
-if (GraphPrintCheckerState->isNoReturnCall(N))
-  return "color=\"blue\",style=\"filled\"";
-#endif
 return "";
   }
 
+  // De-duplicate some source location pretty-printing.
   static void printLocation(raw_ostream , SourceLocation SLoc) {
 if (SLoc.isFileID()) {
   Out << "\\lline="
@@ -2550,6 +2534,12 @@
 << "\\l";
 }
   }
+  static void printLocation2(raw_ostream , SourceLocation SLoc) {
+if (SLoc.isFileID() && GraphPrintSourceManager->isInMainFile(SLoc))
+  Out << "line " << GraphPrintSourceManager->getExpansionLineNumber(SLoc);
+else
+  SLoc.print(Out, *GraphPrintSourceManager);
+  }
 
   static std::string getNodeLabel(const ExplodedNode *N, void*){
 
@@ -2563,12 +2553,6 @@
   case ProgramPoint::BlockEntranceKind: {
 Out << "Block Entrance: B"
 << Loc.castAs().getBlock()->getBlockID();
-if (const NamedDecl *ND =
-dyn_cast(Loc.getLocationContext()->getDecl())) {
-  Out << " (";
-  ND->printName(Out);
-  Out << ")";
-}
 break;
   }
 
@@ -2693,13 +2677,6 @@
   Out << "\\l";
 }
 
-#if 0
-  // FIXME: Replace with a general scheme to determine
-  // the name of the check.
-if (GraphPrintCheckerState->isUndefControlFlow(N)) {
-  Out << "\\|Control-flow based on\\lUndefined value.\\l";
-}
-#endif
 break;
   }
 
@@ -2721,34 +2698,47 @@
 else if (Loc.getAs())
   Out << "\\lPostLValue\\l";
 
-#if 0
-  // FIXME: Replace with a general scheme to determine
-  // the name of the check.
-if (GraphPrintCheckerState->isImplicitNullDeref(N))
-  Out << "\\|Implicit-Null Dereference.\\l";
-else if (GraphPrintCheckerState->isExplicitNullDeref(N))
-  Out << "\\|Explicit-Null Dereference.\\l";
-else if (GraphPrintCheckerState->isUndefDeref(N))
-  Out << "\\|Dereference of undefialied value.\\l";
-else if (GraphPrintCheckerState->isUndefStore(N))
-  Out << "\\|Store to Undefined Loc.";
-else if (GraphPrintCheckerState->isUndefResult(N))
-  Out << "\\|Result of operation is undefined.";
-else if (GraphPrintCheckerState->isNoReturnCall(N))
-  Out << "\\|Call to function marked \"noreturn\".";
-else if (GraphPrintCheckerState->isBadCall(N))
-  Out << "\\|Call to NULL/Undefined.";
-else if (GraphPrintCheckerState->isUndefArg(N))
-  Out << "\\|Argument in call is undefined";
-#endif
-
 break;
   }
 }
 
 ProgramStateRef state = N->getState();
 Out << "\\|StateID: " << (const void*) state.get()
 << " NodeID: " << (const void*) N << "\\|";
+
+// Analysis stack backtrace.
+Out << "Location context stack (from current to outer):\\l";
+const LocationContext *LC = Loc.getLocationContext();
+unsigned Idx = 0;
+for (; LC; LC = LC->getParent(), ++Idx) {
+  Out << Idx << ". (" << (const void *)LC << ") ";
+  switch (LC->getKind()) {
+  case LocationContext::StackFrame:
+if (const NamedDecl *D = dyn_cast(LC->getDecl()))
+  Out << "Calling " << D->getQualifiedNameAsString();
+else
+  Out << "Calling anonymous code";
+if (const Stmt *S = cast(LC)->getCallSite()) {
+  Out << " at ";
+  printLocation2(Out, S->getLocStart());
+}
+break;
+  case LocationContext::Block:
+Out << "Invoking block";
+if (const Decl *D = cast(LC)->getBlockDecl()) {
+  Out << " defined at ";
+  printLocation2(Out, D->getLocStart());
+}
+break;
+  case LocationContext::Scope:
+Out << "Entering scope";
+// FIXME: Add more info once ScopeContext is activated.
+break;
+  }
+  Out << "\\l";
+}
+Out << 

Re: [PATCH] D22622: [analyzer] Add more info to exploded graph dumps

[Anna Zaks via cfe-commits]

zaks.anna added a comment.

Great!

How does this work when a path spans multiple files, specifically, when the 
definitions from a header file are inlined? We could add file names but only in 
cases the file name does not match the main file.

> and hard to navigate (especially when using a specialized .dot-file viewer 
> like xdot - i found it 

>  much more comfortable to convert the file to .svg and view in a web browser


This is not something that the patch addresses. Feel free to add a patch to the 
checker developer manual with instructions for this.


https://reviews.llvm.org/D22622



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D22622: [analyzer] Add more info to exploded graph dumps

[Artem Dergachev via cfe-commits]

NoQ added a comment.

Whoops forgot the screenshots:

F2187578: 2.png  F2187579: 1.png 


Also not sure how to write tests for these, because we can't choose the 
directory path for the dump, can we?


https://reviews.llvm.org/D22622



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits