[clang] [clang][dataflow] Fix handling of cyclical data structures in HTMLLogger. (PR #66887)

[via cfe-commits]

https://github.com/martinboehme created https://github.com/llvm/llvm-project/pull/66887

*  Include more information. We can include the "kind" field for values and
   "type" for storage locations without risking infinite recursion. Previously,
   we would display these as "undefined" (simply because this is the JavaScript
   value for a variable that isn't set), which is potentially confusing.
   Similarly, `dump(StorageLocation&)` can unconditionally call `dump(Value&)`
   without risking infinite recursion, and this ensures that the "value_id" and
   "kind" fields are populated.

*  Remove entries from `Visited` when we exit `dump()`. This ensures that we
   correctly display values and storage locations that occur multiple times in a
   dump (without being nested).

*  If we detect a cycle, add an "[in_cycle]" atttribute to make it clear why
   the display is truncated.


>From 329a7f9c1b6c0056a83d640c074468fe53bc54bb Mon Sep 17 00:00:00 2001
From: Martin Braenne <mboehme at google.com>
Date: Wed, 20 Sep 2023 10:43:39 +0000
Subject: [PATCH] [clang][dataflow] Fix handling of cyclical data structures in
 HTMLLogger.

*  Include more information. We can include the "kind" field for values and
   "type" for storage locations without risking infinite recursion. Previously,
   we would display these as "undefined" (simply because this is the JavaScript
   value for a variable that isn't set), which is potentially confusing.
   Similarly, `dump(StorageLocation&)` can unconditionally call `dump(Value&)`
   without risking infinite recursion, and this ensures that the "value_id" and
   "kind" fields are populated.

*  Remove entries from `Visited` when we exit `dump()`. This ensures that we
   correctly display values and storage locations that occur multiple times in a
   dump (without being nested).

*  If we detect a cycle, add an "[in_cycle]" atttribute to make it clear why
   the display is truncated.
---
 clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp b/clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp
index a5f64021eb6ba4b..48a48d86fce43a7 100644
--- a/clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp
+++ b/clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp
@@ -88,10 +88,12 @@ class ModelDumper {
 
   void dump(Value &V) {
     JOS.attribute("value_id", llvm::to_string(&V));
-    if (!Visited.insert(&V).second)
-      return;
-
     JOS.attribute("kind", debugString(V.getKind()));
+    if (!Visited.insert(&V).second) {
+      JOS.attribute("[in_cycle]", " ");
+      return;
+    }
+    auto EraseVisited = llvm::make_scope_exit([&] { Visited.erase(&V); });
 
     switch (V.getKind()) {
     case Value::Kind::Integer:
@@ -123,13 +125,16 @@ class ModelDumper {
   }
   void dump(const StorageLocation &L) {
     JOS.attribute("location", llvm::to_string(&L));
-    if (!Visited.insert(&L).second)
-      return;
-
     JOS.attribute("type", L.getType().getAsString());
     if (auto *V = Env.getValue(L))
       dump(*V);
 
+    if (!Visited.insert(&L).second) {
+      JOS.attribute("[in_cycle]", " ");
+      return;
+    }
+    auto EraseVisited = llvm::make_scope_exit([&] { Visited.erase(&L); });
+
     if (auto *RLoc = dyn_cast<RecordStorageLocation>(&L)) {
       for (const auto &Child : RLoc->children())
         JOS.attributeObject("f:" + Child.first->getNameAsString(), [&] {