[Lldb-commits] [lldb] Summarize std::string's when created from data. (PR #89110)

Jacob Lalonde via lldb-commits lldb-commits at lists.llvm.org
Thu May 23 07:08:04 PDT 2024


https://github.com/Jlalond updated https://github.com/llvm/llvm-project/pull/89110

>From 5955863f22d5048cad91f089e96b10ea15c05446 Mon Sep 17 00:00:00 2001
From: Jacob John Lalonde <jalalonde at fb.com>
Date: Wed, 10 Apr 2024 14:33:40 -0700
Subject: [PATCH 1/7] Fix bug where an sbvalue containing a std::string created
 from data would not use the built in summary provider, but default to the
 valueobjectprinter

std::string children are normally formatted as their summary [my_string_prop] = "hello world".
Sbvalues created from data do not follow the same summary formatter path and instead hit the value object printer.
This change fixes that and adds tests for the future.

Added tests in TestDataFormatterStdString.py and a formatter for a custom struct.
---
 .../Plugins/Language/CPlusPlus/LibStdcpp.cpp  | 31 ++++++++++--
 .../string/ConvertToDataFormatter.py          | 50 +++++++++++++++++++
 .../string/TestDataFormatterStdString.py      | 28 +++++++++++
 .../libstdcpp/string/main.cpp                 | 13 +++++
 4 files changed, 118 insertions(+), 4 deletions(-)
 create mode 100644 lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/ConvertToDataFormatter.py

diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp b/lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
index 86bb575af5ca3..0da01e9ca07fb 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
@@ -254,13 +254,13 @@ bool lldb_private::formatters::LibStdcppStringSummaryProvider(
   } else
     addr_of_string =
         valobj.GetAddressOf(scalar_is_load_addr, &addr_type);
-  if (addr_of_string != LLDB_INVALID_ADDRESS) {
+  if (addr_of_string != LLDB_INVALID_ADDRESS ||
+        addr_type == eAddressTypeHost) {
     switch (addr_type) {
     case eAddressTypeLoad: {
       ProcessSP process_sp(valobj.GetProcessSP());
       if (!process_sp)
         return false;
-
       StringPrinter::ReadStringAndDumpToStreamOptions options(valobj);
       Status error;
       lldb::addr_t addr_of_data =
@@ -287,8 +287,31 @@ bool lldb_private::formatters::LibStdcppStringSummaryProvider(
       } else
         return true;
     } break;
-    case eAddressTypeHost:
-      break;
+    case eAddressTypeHost: {
+      // We have the host address of our std::string
+      // But we need to read the pointee data from the debugged process.
+      ProcessSP process_sp(valobj.GetProcessSP());
+      DataExtractor data;
+      Status error;
+      valobj.GetData(data, error);
+      if (error.Fail())
+        return false;
+      // We want to read the address from std::string, which is the first 8 bytes.
+      lldb::offset_t offset = 0;
+      lldb::addr_t addr = data.GetAddress(&offset);
+      if (!addr)
+      {
+        stream.Printf("nullptr");
+        return true;
+      }
+
+      std::string contents;
+      process_sp->ReadCStringFromMemory(addr, contents, error);
+      if (error.Fail())
+        return false;
+      stream.Printf("%s", contents.c_str());
+      return true;
+    } break;
     case eAddressTypeInvalid:
     case eAddressTypeFile:
       break;
diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/ConvertToDataFormatter.py b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/ConvertToDataFormatter.py
new file mode 100644
index 0000000000000..57e42c920f829
--- /dev/null
+++ b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/ConvertToDataFormatter.py
@@ -0,0 +1,50 @@
+# coding=utf8
+"""
+Helper formmater to verify Std::String by created via SBData
+"""
+
+import lldb
+
+class SyntheticFormatter:
+    def __init__(self, valobj, dict):
+        self.valobj = valobj
+
+    def num_children(self):
+        return 6
+
+    def has_children(self):
+        return True
+
+    def get_child_at_index(self, index):
+        name = None
+        match index:
+            case 0:
+                name = "short_string"
+            case 1:
+                name = "long_string"
+            case 2:
+                name = "short_string_ptr"
+            case 3:
+                name = "long_string_ptr"
+            case 4:
+                name = "short_string_ref"
+            case 5:
+                name = "long_string_ref"
+            case _:
+                return None
+
+        child = self.valobj.GetChildMemberWithName(name)
+        valType = child.GetType()
+        return self.valobj.CreateValueFromData(name,
+                child.GetData(),
+                valType)
+
+
+def __lldb_init_module(debugger, dict):
+    typeName = "string_container"
+    debugger.HandleCommand(
+        'type synthetic add -x "'
+        + typeName
+        + '" --python-class '
+        + f"{__name__}.SyntheticFormatter"
+    )
diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/TestDataFormatterStdString.py b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/TestDataFormatterStdString.py
index 0b78fb7dc3911..2adfe47f9a3cf 100644
--- a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/TestDataFormatterStdString.py
+++ b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/TestDataFormatterStdString.py
@@ -37,6 +37,8 @@ def test_with_run_command(self):
             substrs=["stopped", "stop reason = breakpoint"],
         )
 
+        self.runCmd("command script import ./ConvertToDataFormatter.py", check=True)
+
         # This is the function to remove the custom formats in order to have a
         # clean slate for the next test case.
         def cleanup():
@@ -60,6 +62,7 @@ def cleanup():
         var_rQ = self.frame().FindVariable("rQ")
         var_pq = self.frame().FindVariable("pq")
         var_pQ = self.frame().FindVariable("pQ")
+        var_str_container = self.frame().FindVariable("sc")
 
         self.assertEqual(var_wempty.GetSummary(), 'L""', "wempty summary wrong")
         self.assertEqual(
@@ -89,6 +92,31 @@ def cleanup():
             "pQ summary wrong",
         )
 
+        self.assertEqual(
+            var_str_container.GetChildAtIndex(0).GetSummary(),
+            '"u22"',
+            "string container child wrong")
+        self.assertEqual(
+            var_str_container.GetChildAtIndex(1).GetSummary(),
+            '"quite a long std::string with lots of info inside it inside a struct"',
+            "string container child wrong")
+        self.assertEqual(
+            var_str_container.GetChildAtIndex(2).GetSummary(),
+            '"u22"',
+            "string container child wrong")
+        self.assertEqual(
+            var_str_container.GetChildAtIndex(3).GetSummary(),
+            '"quite a long std::string with lots of info inside it inside a struct"',
+            "string container child wrong")
+        self.assertEqual(
+            var_str_container.GetChildAtIndex(4).GetSummary(),
+            '"u22"',
+            "string container child wrong")
+        self.assertEqual(
+            var_str_container.GetChildAtIndex(5).GetSummary(),
+            '"quite a long std::string with lots of info inside it inside a struct"',
+            "string container child wrong")
+
         self.runCmd("next")
 
         self.assertEqual(var_S.GetSummary(), 'L"!!!!!"', "new S summary wrong")
diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/main.cpp b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/main.cpp
index eefb96c4573e4..b169a2e694556 100644
--- a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/main.cpp
+++ b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/main.cpp
@@ -1,5 +1,14 @@
 #include <string>
 
+struct string_container {
+    std::string short_string;
+    std::string long_string;
+    std::string *short_string_ptr = &short_string;
+    std::string *long_string_ptr = &long_string;
+    std::string &short_string_ref = short_string;
+    std::string &long_string_ref = long_string;
+};
+
 int main()
 {
     std::wstring wempty(L"");
@@ -11,6 +20,10 @@ int main()
     std::string Q("quite a long std::strin with lots of info inside it");
     auto &rq = q, &rQ = Q;
     std::string *pq = &q, *pQ = &Q;
+    string_container sc;
+    sc.short_string = "u22";
+    sc.long_string = "quite a long std::string with lots of info inside it inside a struct";
+
     S.assign(L"!!!!!"); // Set break point at this line.
     return 0;
 }

>From ec446efd5bdcaa02955bdffa04b2905b85364beb Mon Sep 17 00:00:00 2001
From: Jacob John Lalonde <jalalonde at fb.com>
Date: Wed, 17 Apr 2024 14:54:12 -0700
Subject: [PATCH 2/7] Add requested comment | Support when the value object's
 children are also in memory

---
 .../Plugins/Language/CPlusPlus/LibStdcpp.cpp  | 51 ++++++++++++++-----
 1 file changed, 38 insertions(+), 13 deletions(-)

diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp b/lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
index 0da01e9ca07fb..47bf6eb646828 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
@@ -254,6 +254,10 @@ bool lldb_private::formatters::LibStdcppStringSummaryProvider(
   } else
     addr_of_string =
         valobj.GetAddressOf(scalar_is_load_addr, &addr_type);
+
+  // We have to check for host address here
+  // because GetAddressOf returns INVALID for all non load addresses.
+  // But we can still format strings in host memory.
   if (addr_of_string != LLDB_INVALID_ADDRESS ||
         addr_type == eAddressTypeHost) {
     switch (addr_type) {
@@ -288,29 +292,50 @@ bool lldb_private::formatters::LibStdcppStringSummaryProvider(
         return true;
     } break;
     case eAddressTypeHost: {
-      // We have the host address of our std::string
-      // But we need to read the pointee data from the debugged process.
-      ProcessSP process_sp(valobj.GetProcessSP());
+
       DataExtractor data;
       Status error;
       valobj.GetData(data, error);
       if (error.Fail())
         return false;
-      // We want to read the address from std::string, which is the first 8 bytes.
+
       lldb::offset_t offset = 0;
-      lldb::addr_t addr = data.GetAddress(&offset);
-      if (!addr)
+      AddressType child_addressType = valobj.GetAddressTypeOfChildren();
+      if (child_addressType == eAddressTypeLoad)
       {
-        stream.Printf("nullptr");
+        // We have the host address of our std::string
+        // But we need to read the pointee data from the debugged process.
+        ProcessSP process_sp(valobj.GetProcessSP());
+        // We want to read the address from std::string, which is the first 8 bytes.
+        lldb::addr_t addr = data.GetAddress(&offset);
+        if (!addr)
+        {
+          stream.Printf("nullptr");
+          return true;
+        }
+        std::string contents;
+        process_sp->ReadCStringFromMemory(addr, contents, error);
+        if (error.Fail())
+          return false;
+
+        stream.Printf("%s", contents.c_str());
         return true;
       }
 
-      std::string contents;
-      process_sp->ReadCStringFromMemory(addr, contents, error);
-      if (error.Fail())
-        return false;
-      stream.Printf("%s", contents.c_str());
-      return true;
+      if (child_addressType == eAddressTypeHost)
+      {
+        lldb::offset_t size = data.GetByteSize();
+        const void* dataStart = data.GetData(&offset, size);
+        if (!dataStart)
+          return false;
+
+        const std::string* str = static_cast<const std::string*>(dataStart);
+        stream.Printf("%s", str->c_str());
+        return true;
+      }
+
+      // Fall through if the child address type is file or invalid.
+      return false;
     } break;
     case eAddressTypeInvalid:
     case eAddressTypeFile:

>From f0bf0acac6dcf1967fe30694922e34a4212063ad Mon Sep 17 00:00:00 2001
From: Jacob John Lalonde <jalalonde at fb.com>
Date: Wed, 24 Apr 2024 13:52:10 -0700
Subject: [PATCH 3/7] Split data out into it's own test case, and added minor
 refactoring to the test class to reduce code duplication

---
 .../string/TestDataFormatterStdString.py      | 57 +++++++++++++------
 1 file changed, 41 insertions(+), 16 deletions(-)

diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/TestDataFormatterStdString.py b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/TestDataFormatterStdString.py
index 2adfe47f9a3cf..8dbd4f4ae0b85 100644
--- a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/TestDataFormatterStdString.py
+++ b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/TestDataFormatterStdString.py
@@ -17,6 +17,15 @@ def setUp(self):
         # Find the line number to break at.
         self.line = line_number("main.cpp", "// Set break point at this line.")
 
+    # This is the function to remove the custom formats in order to have a
+    # clean slate for the next test case.
+    def cleanup(self):
+        self.runCmd("type format clear", check=False)
+        self.runCmd("type summary clear", check=False)
+        self.runCmd("type filter clear", check=False)
+        self.runCmd("type synth clear", check=False)
+        self.runCmd("settings set target.max-children-count 256", check=False)
+
     @add_test_categories(["libstdcxx"])
     @expectedFailureAll(bugnumber="llvm.org/pr50861", compiler="gcc")
     def test_with_run_command(self):
@@ -37,19 +46,9 @@ def test_with_run_command(self):
             substrs=["stopped", "stop reason = breakpoint"],
         )
 
-        self.runCmd("command script import ./ConvertToDataFormatter.py", check=True)
-
-        # This is the function to remove the custom formats in order to have a
-        # clean slate for the next test case.
-        def cleanup():
-            self.runCmd("type format clear", check=False)
-            self.runCmd("type summary clear", check=False)
-            self.runCmd("type filter clear", check=False)
-            self.runCmd("type synth clear", check=False)
-            self.runCmd("settings set target.max-children-count 256", check=False)
 
         # Execute the cleanup function during test case tear down.
-        self.addTearDownHook(cleanup)
+        self.addTearDownHook(self.cleanup())
 
         var_wempty = self.frame().FindVariable("wempty")
         var_s = self.frame().FindVariable("s")
@@ -62,7 +61,6 @@ def cleanup():
         var_rQ = self.frame().FindVariable("rQ")
         var_pq = self.frame().FindVariable("pq")
         var_pQ = self.frame().FindVariable("pQ")
-        var_str_container = self.frame().FindVariable("sc")
 
         self.assertEqual(var_wempty.GetSummary(), 'L""', "wempty summary wrong")
         self.assertEqual(
@@ -92,6 +90,37 @@ def cleanup():
             "pQ summary wrong",
         )
 
+        self.runCmd("next")
+
+        self.assertEqual(var_S.GetSummary(), 'L"!!!!!"', "new S summary wrong")
+
+
+    @add_test_categories(["libstdcxx"])
+    @expectedFailureAll(bugnumber="llvm.org/pr50861", compiler="gcc")
+    def test_std_string_as_data(self):
+        """Test that strings created via SBValue::CreateFromData display correctly."""
+        self.build()
+        self.runCmd("file " + self.getBuildArtifact("a.out"), CURRENT_EXECUTABLE_SET)
+
+        lldbutil.run_break_set_by_file_and_line(
+            self, "main.cpp", self.line, num_expected_locations=-1
+        )
+
+        self.runCmd("run", RUN_SUCCEEDED)
+
+        self.runCmd("command script import ./ConvertToDataFormatter.py", check=True)
+
+        self.expect(
+            "thread list",
+            STOPPED_DUE_TO_BREAKPOINT,
+            substrs=["stopped", "stop reason = breakpoint"],
+        )
+
+        # Execute the cleanup function during test case tear down.
+        self.addTearDownHook(self.cleanup())
+
+        var_str_container = self.frame().FindVariable("sc")
+
         self.assertEqual(
             var_str_container.GetChildAtIndex(0).GetSummary(),
             '"u22"',
@@ -116,7 +145,3 @@ def cleanup():
             var_str_container.GetChildAtIndex(5).GetSummary(),
             '"quite a long std::string with lots of info inside it inside a struct"',
             "string container child wrong")
-
-        self.runCmd("next")
-
-        self.assertEqual(var_S.GetSummary(), 'L"!!!!!"', "new S summary wrong")

>From c35ce31bb860b35dcd0f6ed883f7f41a57d0144a Mon Sep 17 00:00:00 2001
From: Jacob John Lalonde <jalalonde at fb.com>
Date: Tue, 30 Apr 2024 09:29:35 -0700
Subject: [PATCH 4/7] Fix formatting of curly braces

---
 lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp b/lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
index 47bf6eb646828..51afee3525673 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
@@ -281,6 +281,7 @@ bool lldb_private::formatters::LibStdcppStringSummaryProvider(
           addr_of_string + process_sp->GetAddressByteSize(), error);
       if (error.Fail())
         return false;
+
       options.SetSourceSize(size_of_data);
       options.SetHasSourceSize(true);
 
@@ -301,15 +302,13 @@ bool lldb_private::formatters::LibStdcppStringSummaryProvider(
 
       lldb::offset_t offset = 0;
       AddressType child_addressType = valobj.GetAddressTypeOfChildren();
-      if (child_addressType == eAddressTypeLoad)
-      {
+      if (child_addressType == eAddressTypeLoad) {
         // We have the host address of our std::string
         // But we need to read the pointee data from the debugged process.
         ProcessSP process_sp(valobj.GetProcessSP());
         // We want to read the address from std::string, which is the first 8 bytes.
         lldb::addr_t addr = data.GetAddress(&offset);
-        if (!addr)
-        {
+        if (!addr) {
           stream.Printf("nullptr");
           return true;
         }
@@ -322,8 +321,7 @@ bool lldb_private::formatters::LibStdcppStringSummaryProvider(
         return true;
       }
 
-      if (child_addressType == eAddressTypeHost)
-      {
+      if (child_addressType == eAddressTypeHost) {
         lldb::offset_t size = data.GetByteSize();
         const void* dataStart = data.GetData(&offset, size);
         if (!dataStart)

>From 51afd3eb68541ebe31c8e51416a0c886ca48aeb3 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Tue, 21 May 2024 10:53:25 -0700
Subject: [PATCH 5/7] Move in h host addresses to use the same code as load

---
 .../Plugins/Language/CPlusPlus/LibStdcpp.cpp  | 50 ++-----------------
 1 file changed, 4 insertions(+), 46 deletions(-)

diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp b/lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
index 51afee3525673..65ed5e7580e21 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
@@ -19,6 +19,7 @@
 #include "lldb/Utility/Endian.h"
 #include "lldb/Utility/Status.h"
 #include "lldb/Utility/Stream.h"
+#include "lldb/lldb-private-enumerations.h"
 #include <optional>
 
 using namespace lldb;
@@ -258,10 +259,10 @@ bool lldb_private::formatters::LibStdcppStringSummaryProvider(
   // We have to check for host address here
   // because GetAddressOf returns INVALID for all non load addresses.
   // But we can still format strings in host memory.
-  if (addr_of_string != LLDB_INVALID_ADDRESS ||
-        addr_type == eAddressTypeHost) {
+  if (addr_of_string != LLDB_INVALID_ADDRESS) {
     switch (addr_type) {
-    case eAddressTypeLoad: {
+    case eAddressTypeLoad:
+    case eAddressTypeHost: {
       ProcessSP process_sp(valobj.GetProcessSP());
       if (!process_sp)
         return false;
@@ -292,49 +293,6 @@ bool lldb_private::formatters::LibStdcppStringSummaryProvider(
       } else
         return true;
     } break;
-    case eAddressTypeHost: {
-
-      DataExtractor data;
-      Status error;
-      valobj.GetData(data, error);
-      if (error.Fail())
-        return false;
-
-      lldb::offset_t offset = 0;
-      AddressType child_addressType = valobj.GetAddressTypeOfChildren();
-      if (child_addressType == eAddressTypeLoad) {
-        // We have the host address of our std::string
-        // But we need to read the pointee data from the debugged process.
-        ProcessSP process_sp(valobj.GetProcessSP());
-        // We want to read the address from std::string, which is the first 8 bytes.
-        lldb::addr_t addr = data.GetAddress(&offset);
-        if (!addr) {
-          stream.Printf("nullptr");
-          return true;
-        }
-        std::string contents;
-        process_sp->ReadCStringFromMemory(addr, contents, error);
-        if (error.Fail())
-          return false;
-
-        stream.Printf("%s", contents.c_str());
-        return true;
-      }
-
-      if (child_addressType == eAddressTypeHost) {
-        lldb::offset_t size = data.GetByteSize();
-        const void* dataStart = data.GetData(&offset, size);
-        if (!dataStart)
-          return false;
-
-        const std::string* str = static_cast<const std::string*>(dataStart);
-        stream.Printf("%s", str->c_str());
-        return true;
-      }
-
-      // Fall through if the child address type is file or invalid.
-      return false;
-    } break;
     case eAddressTypeInvalid:
     case eAddressTypeFile:
       break;

>From c2fdbea407624cee4c4b504c06270d710a4cd09e Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Thu, 23 May 2024 07:03:14 -0700
Subject: [PATCH 6/7] Make value object return an address when GetAddressOf is
 called, change libstdcpp to dereference that address to get char* for
 std::string when formatting eAddressTypeHost

---
 lldb/source/Core/ValueObject.cpp              |  6 +--
 .../Plugins/Language/CPlusPlus/LibStdcpp.cpp  | 42 ++++++++++++++++---
 2 files changed, 39 insertions(+), 9 deletions(-)

diff --git a/lldb/source/Core/ValueObject.cpp b/lldb/source/Core/ValueObject.cpp
index f39bd07a25536..14db1627ff29a 100644
--- a/lldb/source/Core/ValueObject.cpp
+++ b/lldb/source/Core/ValueObject.cpp
@@ -1405,7 +1405,7 @@ addr_t ValueObject::GetAddressOf(bool scalar_is_load_address,
   case Value::ValueType::HostAddress: {
     if (address_type)
       *address_type = m_value.GetValueAddressType();
-    return LLDB_INVALID_ADDRESS;
+    return m_value.GetScalar().ULongLong(LLDB_INVALID_ADDRESS);
   } break;
   }
   if (address_type)
@@ -2745,7 +2745,7 @@ ValueObjectSP ValueObject::DoCast(const CompilerType &compiler_type) {
 ValueObjectSP ValueObject::Cast(const CompilerType &compiler_type) {
   // Only allow casts if the original type is equal or larger than the cast
   // type, unless we know this is a load address.  Getting the size wrong for
-  // a host side storage could leak lldb memory, so we absolutely want to 
+  // a host side storage could leak lldb memory, so we absolutely want to
   // prevent that.  We may not always get the right value, for instance if we
   // have an expression result value that's copied into a storage location in
   // the target may not have copied enough memory.  I'm not trying to fix that
@@ -2764,7 +2764,7 @@ ValueObjectSP ValueObject::Cast(const CompilerType &compiler_type) {
       = ExecutionContext(GetExecutionContextRef())
           .GetBestExecutionContextScope();
   if (compiler_type.GetByteSize(exe_scope)
-      <= GetCompilerType().GetByteSize(exe_scope) 
+      <= GetCompilerType().GetByteSize(exe_scope)
       || m_value.GetValueType() == Value::ValueType::LoadAddress)
         return DoCast(compiler_type);
 
diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp b/lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
index 65ed5e7580e21..d5ed5d3cacdae 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
@@ -16,10 +16,12 @@
 #include "lldb/DataFormatters/VectorIterator.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Utility/DataBufferHeap.h"
+#include "lldb/Utility/DataExtractor.h"
 #include "lldb/Utility/Endian.h"
 #include "lldb/Utility/Status.h"
 #include "lldb/Utility/Stream.h"
-#include "lldb/lldb-private-enumerations.h"
+#include "lldb/lldb-types.h"
+#include "llvm/Support/Compiler.h"
 #include <optional>
 
 using namespace lldb;
@@ -256,13 +258,9 @@ bool lldb_private::formatters::LibStdcppStringSummaryProvider(
     addr_of_string =
         valobj.GetAddressOf(scalar_is_load_addr, &addr_type);
 
-  // We have to check for host address here
-  // because GetAddressOf returns INVALID for all non load addresses.
-  // But we can still format strings in host memory.
   if (addr_of_string != LLDB_INVALID_ADDRESS) {
     switch (addr_type) {
-    case eAddressTypeLoad:
-    case eAddressTypeHost: {
+    case eAddressTypeLoad: {
       ProcessSP process_sp(valobj.GetProcessSP());
       if (!process_sp)
         return false;
@@ -293,6 +291,38 @@ bool lldb_private::formatters::LibStdcppStringSummaryProvider(
       } else
         return true;
     } break;
+    case eAddressTypeHost: {
+      // For std::strings created as data, the valueobject points to char * pointer
+      // so we can get the address of our pointer from the value object
+      // and then just read the char* as the summary value
+      ProcessSP process_sp(valobj.GetProcessSP());
+      if (!process_sp)
+        return false;
+      StringPrinter::ReadStringAndDumpToStreamOptions options(valobj);
+      DataExtractor data = DataExtractor(
+        (void*)addr_of_string,
+        process_sp->GetAddressByteSize(),
+        process_sp->GetByteOrder(),
+        8);
+      Status error;
+      lldb::offset_t offset = 0;
+      lldb::addr_t addr_of_string_from_data = data.GetAddress(&offset);
+      if (error.Fail() || addr_of_string_from_data == 0 ||
+          addr_of_string_from_data == LLDB_INVALID_ADDRESS)
+        return false;
+      options.SetLocation(addr_of_string_from_data);
+      options.SetTargetSP(valobj.GetTargetSP());
+      options.SetStream(&stream);
+      options.SetNeedsZeroTermination(true);
+      options.SetBinaryZeroIsTerminator(true);
+
+      if (!StringPrinter::ReadStringAndDumpToStream<
+              StringPrinter::StringElementType::UTF8>(options)) {
+        stream.Printf("Summary Unavailable");
+        return true;
+      } else
+        return true;
+    } break;
     case eAddressTypeInvalid:
     case eAddressTypeFile:
       break;

>From afc30d49c27bb279e25ae5b4f900a8d97cc057a5 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Thu, 23 May 2024 07:07:29 -0700
Subject: [PATCH 7/7] Run formatters

---
 lldb/source/Core/ValueObject.cpp              |  8 ++++----
 .../Plugins/Language/CPlusPlus/LibStdcpp.cpp  | 12 +++++------
 .../string/ConvertToDataFormatter.py          |  5 ++---
 .../string/TestDataFormatterStdString.py      | 20 +++++++++++--------
 .../libstdcpp/string/main.cpp                 | 15 +++++++-------
 5 files changed, 31 insertions(+), 29 deletions(-)

diff --git a/lldb/source/Core/ValueObject.cpp b/lldb/source/Core/ValueObject.cpp
index 14db1627ff29a..957863ff89a09 100644
--- a/lldb/source/Core/ValueObject.cpp
+++ b/lldb/source/Core/ValueObject.cpp
@@ -2763,10 +2763,10 @@ ValueObjectSP ValueObject::Cast(const CompilerType &compiler_type) {
   ExecutionContextScope *exe_scope
       = ExecutionContext(GetExecutionContextRef())
           .GetBestExecutionContextScope();
-  if (compiler_type.GetByteSize(exe_scope)
-      <= GetCompilerType().GetByteSize(exe_scope)
-      || m_value.GetValueType() == Value::ValueType::LoadAddress)
-        return DoCast(compiler_type);
+  if (compiler_type.GetByteSize(exe_scope) <=
+          GetCompilerType().GetByteSize(exe_scope) ||
+      m_value.GetValueType() == Value::ValueType::LoadAddress)
+    return DoCast(compiler_type);
 
   error.SetErrorString("Can only cast to a type that is equal to or smaller "
                        "than the orignal type.");
diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp b/lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
index d5ed5d3cacdae..37c70733895fd 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
@@ -292,18 +292,16 @@ bool lldb_private::formatters::LibStdcppStringSummaryProvider(
         return true;
     } break;
     case eAddressTypeHost: {
-      // For std::strings created as data, the valueobject points to char * pointer
-      // so we can get the address of our pointer from the value object
+      // For std::strings created as data, the valueobject points to char *
+      // pointer so we can get the address of our pointer from the value object
       // and then just read the char* as the summary value
       ProcessSP process_sp(valobj.GetProcessSP());
       if (!process_sp)
         return false;
       StringPrinter::ReadStringAndDumpToStreamOptions options(valobj);
-      DataExtractor data = DataExtractor(
-        (void*)addr_of_string,
-        process_sp->GetAddressByteSize(),
-        process_sp->GetByteOrder(),
-        8);
+      DataExtractor data = DataExtractor((void *)addr_of_string,
+                                         process_sp->GetAddressByteSize(),
+                                         process_sp->GetByteOrder(), 8);
       Status error;
       lldb::offset_t offset = 0;
       lldb::addr_t addr_of_string_from_data = data.GetAddress(&offset);
diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/ConvertToDataFormatter.py b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/ConvertToDataFormatter.py
index 57e42c920f829..7d354546e1d0b 100644
--- a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/ConvertToDataFormatter.py
+++ b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/ConvertToDataFormatter.py
@@ -5,6 +5,7 @@
 
 import lldb
 
+
 class SyntheticFormatter:
     def __init__(self, valobj, dict):
         self.valobj = valobj
@@ -35,9 +36,7 @@ def get_child_at_index(self, index):
 
         child = self.valobj.GetChildMemberWithName(name)
         valType = child.GetType()
-        return self.valobj.CreateValueFromData(name,
-                child.GetData(),
-                valType)
+        return self.valobj.CreateValueFromData(name, child.GetData(), valType)
 
 
 def __lldb_init_module(debugger, dict):
diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/TestDataFormatterStdString.py b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/TestDataFormatterStdString.py
index 8dbd4f4ae0b85..83a21304d04ec 100644
--- a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/TestDataFormatterStdString.py
+++ b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/TestDataFormatterStdString.py
@@ -46,7 +46,6 @@ def test_with_run_command(self):
             substrs=["stopped", "stop reason = breakpoint"],
         )
 
-
         # Execute the cleanup function during test case tear down.
         self.addTearDownHook(self.cleanup())
 
@@ -94,7 +93,6 @@ def test_with_run_command(self):
 
         self.assertEqual(var_S.GetSummary(), 'L"!!!!!"', "new S summary wrong")
 
-
     @add_test_categories(["libstdcxx"])
     @expectedFailureAll(bugnumber="llvm.org/pr50861", compiler="gcc")
     def test_std_string_as_data(self):
@@ -124,24 +122,30 @@ def test_std_string_as_data(self):
         self.assertEqual(
             var_str_container.GetChildAtIndex(0).GetSummary(),
             '"u22"',
-            "string container child wrong")
+            "string container child wrong",
+        )
         self.assertEqual(
             var_str_container.GetChildAtIndex(1).GetSummary(),
             '"quite a long std::string with lots of info inside it inside a struct"',
-            "string container child wrong")
+            "string container child wrong",
+        )
         self.assertEqual(
             var_str_container.GetChildAtIndex(2).GetSummary(),
             '"u22"',
-            "string container child wrong")
+            "string container child wrong",
+        )
         self.assertEqual(
             var_str_container.GetChildAtIndex(3).GetSummary(),
             '"quite a long std::string with lots of info inside it inside a struct"',
-            "string container child wrong")
+            "string container child wrong",
+        )
         self.assertEqual(
             var_str_container.GetChildAtIndex(4).GetSummary(),
             '"u22"',
-            "string container child wrong")
+            "string container child wrong",
+        )
         self.assertEqual(
             var_str_container.GetChildAtIndex(5).GetSummary(),
             '"quite a long std::string with lots of info inside it inside a struct"',
-            "string container child wrong")
+            "string container child wrong",
+        )
diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/main.cpp b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/main.cpp
index b169a2e694556..838781935fe98 100644
--- a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/main.cpp
+++ b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/main.cpp
@@ -1,12 +1,12 @@
 #include <string>
 
 struct string_container {
-    std::string short_string;
-    std::string long_string;
-    std::string *short_string_ptr = &short_string;
-    std::string *long_string_ptr = &long_string;
-    std::string &short_string_ref = short_string;
-    std::string &long_string_ref = long_string;
+  std::string short_string;
+  std::string long_string;
+  std::string *short_string_ptr = &short_string;
+  std::string *long_string_ptr = &long_string;
+  std::string &short_string_ref = short_string;
+  std::string &long_string_ref = long_string;
 };
 
 int main()
@@ -22,7 +22,8 @@ int main()
     std::string *pq = &q, *pQ = &Q;
     string_container sc;
     sc.short_string = "u22";
-    sc.long_string = "quite a long std::string with lots of info inside it inside a struct";
+    sc.long_string =
+        "quite a long std::string with lots of info inside it inside a struct";
 
     S.assign(L"!!!!!"); // Set break point at this line.
     return 0;



More information about the lldb-commits mailing list