[analyzer] Fix hidden node traversal in exploded graph dumps.

[NoQ]

Summary

The joined nodes now actually have the same state. That was intended from the start but the original implementation turned out to be buggy.

Before:

After:

[NoQ]

clang/test/Analysis/dump_egraph.c

32

The actual filename is gracefully omitted so that not to confuse buildbots because it's going to be an absolute path.

[Charusso]

When you see

static bool traverseHiddenNodes(
    const ExplodedNode *N,
    llvm::function_ref<void(const ExplodedNode *)> PreCallback,
    llvm::function_ref<void(const ExplodedNode *)> PostCallback,
    llvm::function_ref<bool(const ExplodedNode *)> Stop) {

that is 100% will produce errors, as it wants to be so smart. I think it is a very bad design from the beginning, so it will be still bugprone.

Other than that now every node makes sense, thanks!

clang/test/Analysis/dump_egraph.c

46

\"file\": \"{{(.+)}}dump_egraph.c\" \} is a cool workaround, but otherwise if you do not want to end line 32, could you remove this line's ending please?

[NoQ]

clang/test/Analysis/dump_egraph.c

46

Interestingly, this doesn't work in my case but does work in this older case, due to greediness: the first {(.+)}}dump_egraph.c matches everything until the second dump_egraph.c and then it can't find the PreStmtPurgeDeadSymbols line after it because it was already skipped.

I tried to make it non-greedy: {{(.+?)}}dump_egraph.c but it says it's not allowed.

I'm pretty bad with regexps so i decided to screw it :) But let's keep at least this test in order to test that the filename is actually printed.

clang/lib/StaticAnalyzer/Core/ExprEngine.cpp

struct DOTGraphTraits<ExplodedGraph*> : public DefaultDOTGraphTraits {
   /// \p PostCallback: callback after break.
   /// \p Stop: stop iteration if returns {@code true}
   /// \return Whether {@code Stop} ever returned {@code true}.
   static bool traverseHiddenNodes(
       const ExplodedNode *N,
       llvm::function_ref<void(const ExplodedNode *)> PreCallback,
       llvm::function_ref<void(const ExplodedNode *)> PostCallback,
       llvm::function_ref<bool(const ExplodedNode *)> Stop) {
-    const ExplodedNode *FirstHiddenNode = N;
-    while (FirstHiddenNode->pred_size() == 1 &&
-           isNodeHidden(*FirstHiddenNode->pred_begin())) {
-      FirstHiddenNode = *FirstHiddenNode->pred_begin();
-    }
-    const ExplodedNode *OtherNode = FirstHiddenNode;
     while (true) {
-      PreCallback(OtherNode);
-      if (Stop(OtherNode))
+      PreCallback(N);
+      if (Stop(N))
         return true;
 
-      if (OtherNode == N)
+      if (N->succ_size() != 1 || !isNodeHidden(N->getFirstSucc()))
         break;
-      PostCallback(OtherNode);
+      PostCallback(N);
 
-      OtherNode = *OtherNode->succ_begin();
+      N = N->getFirstSucc();
     }
     return false;
   }
 
   static bool isNodeHidden(const ExplodedNode *N) {
     return N->isTrivial();
   }
 

clang/test/Analysis/dump_egraph.c

 
 int foo() {
   int *x = 0, *y = 0;
   char c = '\x13';
 
   return *x + *y;
 }
 
-// CHECK: \"program_points\": [\l    \{ \"kind\": \"Edge\", \"src_id\": 2, \"dst_id\": 1, \"terminator\": null, \"term_kind\": null, \"tag\": null, \"node_id\": 1, \"is_sink\": 0, \"has_report\": 0 \}\l  ],\l  \"program_state\": null
-
-// CHECK: \"program_points\": [\l    \{ \"kind\": \"BlockEntrance\", \"block_id\": 1
-
+// CHECK: \"program_points\": [\l
+// CHECK-SAME: \{ \"kind\": \"Edge\", \"src_id\": 2, \"dst_id\": 1,
+// CHECK-SAME:    \"terminator\": null, \"term_kind\": null, \"tag\": null,
+// CHECK-SAME:    \"node_id\": 1, \"is_sink\": 0, \"has_report\": 0
+// CHECK-SAME: \},
+// CHECK-SAME: \{ \"kind\": \"BlockEntrance\", \"block_id\": 1, \"tag\": null,
+// CHECK-SAME:    \"node_id\": 2, \"is_sink\": 0, \"has_report\": 0
+// CHECK-SAME: \},
+// CHECK-SAME: \{ \"kind\": \"Statement\", \"stmt_kind\": \"IntegerLiteral\",
+// CHECK-SAME:    \"stmt_id\": 597, \"pointer\": \"0x{{[0-9a-f]*}}\",
+// CHECK-SAME:    \"pretty\": \"0\", \"location\": \{
+// CHECK-SAME:        \"line\": 15, \"column\": 12, \"file\": \"
+// CHECK-SAME:    \}, \"stmt_point_kind\": \"PreStmtPurgeDeadSymbols\",
+// CHECK-SAME:    \"tag\": \"ExprEngine : Clean Node\", \"node_id\": 3,
+// CHECK-SAME:    \"is_sink\": 0, \"has_report\": 0
+// CHECK-SAME: \},
+// CHECK-SAME: \{ \"kind\": \"Statement\", \"stmt_kind\": \"IntegerLiteral\",
+// CHECK-SAME:    \"stmt_id\": 597, \"pointer\": \"0x{{[0-9a-f]*}}\",
+// CHECK-SAME:    \"pretty\": \"0\", \"location\": \{
+// CHECK-SAME:        \"line\": 15, \"column\": 12, \"file\":
+// CHECK-SAME:    \}, \"stmt_point_kind\": \"PostStmt\", \"tag\": null,
+// CHECK-SAME:    \"node_id\": 4, \"is_sink\": 0, \"has_report\": 0
+// CHECK-SAME: \}
+// CHECK-SAME: ]
 
 // CHECK: \"pretty\": \"*x\", \"location\": \{ \"line\": 18, \"column\": 10, \"file\": \"{{(.+)}}dump_egraph.c\" \}
 
 // CHECK: \"pretty\": \"'\\\\x13'\"
 
 // CHECK: \"has_report\": 1