[lldb/Utility] Add GetDescription(Stream&) to StructureData::*

medismailben · medismailben · commit bd6e0ce2041d · 2022-11-03T23:21:37.000-07:00
This patch improves the StructuredData classes to provide a GetDescription(lldb_private::Stream&) affordance. This is very convenient compared to the Dump method because this try to pretty print the structure instead of just serializing it into a JSON. This patch also updates some parts of lldb (i.e. extended crash info) to use this new affordance instead of StructuredData::Dump. Differential Revision: https://reviews.llvm.org/D135547 Signed-off-by: Med Ismail Bennani <medismail.bennani@gmail.com>
diff --git a/lldb/include/lldb/Core/StructuredDataImpl.h b/lldb/include/lldb/Core/StructuredDataImpl.h
@@ -80,7 +80,7 @@ class StructuredDataImpl {
         error.SetErrorString("No data to describe.");
         return error;
       }
-      m_data_sp->Dump(stream, true);
+      m_data_sp->GetDescription(stream);
       return error;
     }
     // Get the data's description.
diff --git a/lldb/include/lldb/Utility/StructuredData.h b/lldb/include/lldb/Utility/StructuredData.h
@@ -158,6 +158,12 @@ class StructuredData {
       Serialize(jso);
     }
 
+    virtual void GetDescription(lldb_private::Stream &s) const {
+      s.IndentMore();
+      Dump(s, false);
+      s.IndentLess();
+    }
+
   private:
     lldb::StructuredDataType m_type;
   };
@@ -277,6 +283,8 @@ class StructuredData {
 
     void Serialize(llvm::json::OStream &s) const override;
 
+    void GetDescription(lldb_private::Stream &s) const override;
+
   protected:
     typedef std::vector<ObjectSP> collection;
     collection m_items;
@@ -295,6 +303,8 @@ class StructuredData {
 
     void Serialize(llvm::json::OStream &s) const override;
 
+    void GetDescription(lldb_private::Stream &s) const override;
+
   protected:
     uint64_t m_value;
   };
@@ -312,6 +322,8 @@ class StructuredData {
 
     void Serialize(llvm::json::OStream &s) const override;
 
+    void GetDescription(lldb_private::Stream &s) const override;
+
   protected:
     double m_value;
   };
@@ -329,6 +341,8 @@ class StructuredData {
 
     void Serialize(llvm::json::OStream &s) const override;
 
+    void GetDescription(lldb_private::Stream &s) const override;
+
   protected:
     bool m_value;
   };
@@ -345,6 +359,8 @@ class StructuredData {
 
     void Serialize(llvm::json::OStream &s) const override;
 
+    void GetDescription(lldb_private::Stream &s) const override;
+
   protected:
     std::string m_value;
   };
@@ -524,6 +540,8 @@ class StructuredData {
 
     void Serialize(llvm::json::OStream &s) const override;
 
+    void GetDescription(lldb_private::Stream &s) const override;
+
   protected:
     typedef std::map<ConstString, ObjectSP> collection;
     collection m_dict;
@@ -538,6 +556,8 @@ class StructuredData {
     bool IsValid() const override { return false; }
 
     void Serialize(llvm::json::OStream &s) const override;
+
+    void GetDescription(lldb_private::Stream &s) const override;
   };
 
   class Generic : public Object {
@@ -553,12 +573,15 @@ class StructuredData {
 
     void Serialize(llvm::json::OStream &s) const override;
 
+    void GetDescription(lldb_private::Stream &s) const override;
+
   private:
     void *m_object;
   };
 
   static ObjectSP ParseJSON(const std::string &json_text);
   static ObjectSP ParseJSONFromFile(const FileSpec &file, Status &error);
+  static bool IsRecordType(const ObjectSP object_sp);
 };
 
 } // namespace lldb_private
diff --git a/lldb/source/Commands/CommandObjectProcess.cpp b/lldb/source/Commands/CommandObjectProcess.cpp
@@ -1550,8 +1550,9 @@ class CommandObjectProcessStatus : public CommandObjectParsed {
       StructuredData::DictionarySP crash_info_sp = *expected_crash_info;
 
       if (crash_info_sp) {
+        strm.EOL();
         strm.PutCString("Extended Crash Information:\n");
-        crash_info_sp->Dump(strm);
+        crash_info_sp->GetDescription(strm);
       }
     }
 
diff --git a/lldb/source/Utility/StructuredData.cpp b/lldb/source/Utility/StructuredData.cpp
@@ -50,6 +50,11 @@ StructuredData::ParseJSONFromFile(const FileSpec &input_spec, Status &error) {
   return StructuredData::ObjectSP();
 }
 
+bool StructuredData::IsRecordType(const ObjectSP object_sp) {
+  return object_sp->GetType() == lldb::eStructuredDataTypeArray ||
+         object_sp->GetType() == lldb::eStructuredDataTypeDictionary;
+}
+
 static StructuredData::ObjectSP ParseJSONValue(json::Value &value) {
   if (json::Object *O = value.getAsObject())
     return ParseJSONObject(O);
@@ -175,3 +180,98 @@ void StructuredData::Null::Serialize(json::OStream &s) const {
 void StructuredData::Generic::Serialize(json::OStream &s) const {
   s.value(llvm::formatv("{0:X}", m_object));
 }
+
+void StructuredData::Integer::GetDescription(lldb_private::Stream &s) const {
+  s.Printf("%" PRId64, static_cast<int64_t>(m_value));
+}
+
+void StructuredData::Float::GetDescription(lldb_private::Stream &s) const {
+  s.Printf("%f", m_value);
+}
+
+void StructuredData::Boolean::GetDescription(lldb_private::Stream &s) const {
+  s.Printf(m_value ? "True" : "False");
+}
+
+void StructuredData::String::GetDescription(lldb_private::Stream &s) const {
+  s.Printf("%s", m_value.empty() ? "\"\"" : m_value.c_str());
+}
+
+void StructuredData::Array::GetDescription(lldb_private::Stream &s) const {
+  size_t index = 0;
+  size_t indentation_level = s.GetIndentLevel();
+  for (const auto &item_sp : m_items) {
+    // Sanitize.
+    if (!item_sp)
+      continue;
+
+    // Reset original indentation level.
+    s.SetIndentLevel(indentation_level);
+    s.Indent();
+
+    // Print key
+    s.Printf("[%zu]:", index++);
+
+    // Return to new line and increase indentation if value is record type.
+    // Otherwise add spacing.
+    bool should_indent = IsRecordType(item_sp);
+    if (should_indent) {
+      s.EOL();
+      s.IndentMore();
+    } else {
+      s.PutChar(' ');
+    }
+
+    // Print value and new line if now last pair.
+    item_sp->GetDescription(s);
+    if (item_sp != *(--m_items.end()))
+      s.EOL();
+
+    // Reset indentation level if it was incremented previously.
+    if (should_indent)
+      s.IndentLess();
+  }
+}
+
+void StructuredData::Dictionary::GetDescription(lldb_private::Stream &s) const {
+  size_t indentation_level = s.GetIndentLevel();
+  for (const auto &pair : m_dict) {
+    // Sanitize.
+    if (pair.first.IsNull() || pair.first.IsEmpty() || !pair.second)
+      continue;
+
+    // Reset original indentation level.
+    s.SetIndentLevel(indentation_level);
+    s.Indent();
+
+    // Print key.
+    s.Printf("%s:", pair.first.AsCString());
+
+    // Return to new line and increase indentation if value is record type.
+    // Otherwise add spacing.
+    bool should_indent = IsRecordType(pair.second);
+    if (should_indent) {
+      s.EOL();
+      s.IndentMore();
+    } else {
+      s.PutChar(' ');
+    }
+
+    // Print value and new line if now last pair.
+    pair.second->GetDescription(s);
+    if (pair != *(--m_dict.end()))
+      s.EOL();
+
+    // Reset indentation level if it was incremented previously.
+    if (should_indent)
+      s.IndentLess();
+  }
+}
+
+void StructuredData::Null::GetDescription(lldb_private::Stream &s) const {
+  s.Printf("NULL");
+}
+
+void StructuredData::Generic::GetDescription(lldb_private::Stream &s) const {
+  s.Printf("%p", m_object);
+}
diff --git a/lldb/test/API/functionalities/process_crash_info/TestProcessCrashInfo.py b/lldb/test/API/functionalities/process_crash_info/TestProcessCrashInfo.py
@@ -37,7 +37,9 @@ def test_cli(self):
                     patterns=["Process .* launched: .*a.out"])
 
         self.expect('process status --verbose',
-                    patterns=["\"message\".*pointer being freed was not allocated"])
+                    patterns=["Extended Crash Information",
+                              "crash-info annotations",
+                              "pointer being freed was not allocated"])
 
 
     @skipIfAsan # The test process intentionally hits a memory bug.
diff --git a/lldb/unittests/Utility/CMakeLists.txt b/lldb/unittests/Utility/CMakeLists.txt
@@ -55,6 +55,10 @@ add_lldb_unittest(UtilityTests
     Support
   )
 
-add_unittest_inputs(UtilityTests
+set(test_inputs
   StructuredData-basic.json
+  StructuredData-nested.json
+  StructuredData-full.json
   )
+
+add_unittest_inputs(UtilityTests "${test_inputs}")
diff --git a/lldb/unittests/Utility/Inputs/StructuredData-full.json b/lldb/unittests/Utility/Inputs/StructuredData-full.json
@@ -0,0 +1,15 @@
+{
+  "Array": [
+    3.14,
+    {
+      "key": "val"
+    }
+  ],
+  "Dictionary": {
+    "FalseBool": false
+  },
+  "Integer": 1,
+  "Null": null,
+  "String": "value",
+  "TrueBool": true
+}
diff --git a/lldb/unittests/Utility/Inputs/StructuredData-nested.json b/lldb/unittests/Utility/Inputs/StructuredData-nested.json
@@ -0,0 +1,14 @@
+{
+  "my_dict": [
+    {
+      "three": 3,
+      "two": 2
+    },
+    {
+      "four": {
+        "val": 4
+      }
+    },
+    1
+  ]
+}
diff --git a/lldb/unittests/Utility/StructuredDataTest.cpp b/lldb/unittests/Utility/StructuredDataTest.cpp
@@ -31,6 +31,73 @@ TEST(StructuredDataTest, StringDump) {
   }
 }
 
+TEST(StructuredDataTest, GetDescriptionEmpty) {
+  Status status;
+  auto object_sp = StructuredData::ParseJSON("{}");
+  ASSERT_NE(nullptr, object_sp);
+
+  StreamString S;
+  object_sp->GetDescription(S);
+  EXPECT_EQ(0, S.GetSize());
+}
+
+TEST(StructuredDataTest, GetDescriptionBasic) {
+  Status status;
+  std::string input = GetInputFilePath("StructuredData-basic.json");
+  auto object_sp = StructuredData::ParseJSONFromFile(FileSpec(input), status);
+  ASSERT_NE(nullptr, object_sp);
+
+  const std::string expected = "[0]: 1\n"
+                               "[1]: 2\n"
+                               "[2]: 3";
+
+  StreamString S;
+  object_sp->GetDescription(S);
+  EXPECT_EQ(expected, S.GetString());
+}
+
+TEST(StructuredDataTest, GetDescriptionNested) {
+  Status status;
+  std::string input = GetInputFilePath("StructuredData-nested.json");
+  auto object_sp = StructuredData::ParseJSONFromFile(FileSpec(input), status);
+  ASSERT_NE(nullptr, object_sp);
+
+  const std::string expected = "my_dict:\n"
+                               "  [0]:\n"
+                               "    three: 3\n"
+                               "    two: 2\n"
+                               "  [1]:\n"
+                               "    four:\n"
+                               "      val: 4\n"
+                               "  [2]: 1";
+
+  StreamString S;
+  object_sp->GetDescription(S);
+  EXPECT_EQ(expected, S.GetString());
+}
+
+TEST(StructuredDataTest, GetDescriptionFull) {
+  Status status;
+  std::string input = GetInputFilePath("StructuredData-full.json");
+  auto object_sp = StructuredData::ParseJSONFromFile(FileSpec(input), status);
+  ASSERT_NE(nullptr, object_sp);
+
+  const std::string expected = "Array:\n"
+                               "  [0]: 3.140000\n"
+                               "  [1]:\n"
+                               "    key: val\n"
+                               "Dictionary:\n"
+                               "  FalseBool: False\n"
+                               "Integer: 1\n"
+                               "Null: NULL\n"
+                               "String: value\n"
+                               "TrueBool: True";
+
+  StreamString S;
+  object_sp->GetDescription(S);
+  EXPECT_EQ(expected, S.GetString());
+}
+
 TEST(StructuredDataTest, ParseJSONFromFile) {
   Status status;
   auto object_sp = StructuredData::ParseJSONFromFile(

Original file line number	Diff line number	Diff line change
`@@ -80,7 +80,7 @@ class StructuredDataImpl {`
`80`	`80`	`error.SetErrorString("No data to describe.");`
`81`	`81`	`return error;`
`82`	`82`	`}`
`83`		`- m_data_sp->Dump(stream, true);`
	`83`	`+ m_data_sp->GetDescription(stream);`
`84`	`84`	`return error;`
`85`	`85`	`}`
`86`	`86`	`// Get the data's description.`
Original file line number	Diff line number	Diff line change
`@@ -1550,8 +1550,9 @@ class CommandObjectProcessStatus : public CommandObjectParsed {`
`1550`	`1550`	`StructuredData::DictionarySP crash_info_sp = *expected_crash_info;`
`1551`	`1551`
`1552`	`1552`	`if (crash_info_sp) {`
	`1553`	`+ strm.EOL();`
`1553`	`1554`	`strm.PutCString("Extended Crash Information:\n");`
`1554`		`- crash_info_sp->Dump(strm);`
	`1555`	`+ crash_info_sp->GetDescription(strm);`
`1555`	`1556`	`}`
`1556`	`1557`	`}`
`1557`	`1558`
Original file line number	Diff line number	Diff line change
`@@ -55,6 +55,10 @@ add_lldb_unittest(UtilityTests`
`55`	`55`	`Support`
`56`	`56`	`)`
`57`	`57`
`58`		`-add_unittest_inputs(UtilityTests`
	`58`	`+set(test_inputs`
`59`	`59`	`StructuredData-basic.json`
	`60`	`+ StructuredData-nested.json`
	`61`	`+ StructuredData-full.json`
`60`	`62`	`)`
	`63`	`+`
	`64`	`+add_unittest_inputs(UtilityTests "${test_inputs}")`