[Lldb-commits] [lldb] [lldb-vscode] Show value addresses in a short format (PR #66534)

Walter Erquinigo via lldb-commits lldb-commits at lists.llvm.org
Fri Sep 15 11:07:56 PDT 2023


https://github.com/walter-erquinigo created https://github.com/llvm/llvm-project/pull/66534

The variables pane on VSCode is very narrow by default, and lldb-vscode has been using the default formatter for addresses, which uses 18 characters for each address. That's a bit too much because it prints too many leading zeroes.
As a way to improve readability of variables, I'm adding some logic to format addresses manually using as few chars as possible. I don't want to mess with the default LLDB formatter because, if the user uses the debug console, they should see addresses formatted in the regular way.


>From 82a50d63e896bcce943370e12e14f442c2a140b7 Mon Sep 17 00:00:00 2001
From: walter erquinigo <walter at modular.com>
Date: Fri, 15 Sep 2023 12:40:33 -0400
Subject: [PATCH] [lldb-vscode] Show value addresses in a short format

The variables pane on VSCode is very narrow by default, and lldb-vscode has been using the default formatter for addresses, which uses 18 characters for each address. That's a bit too much because it prints too many leading zeroes.
As a way to improve readability of variables, I'm adding some logic to format addresses manually using as few chars as possible. I don't want to mess with the default LLDB formatter because, if the user uses the debug console, they should see addresses formatted in the regular way.
---
 lldb/include/lldb/API/SBType.h                |   4 +
 lldb/source/API/SBType.cpp                    |   4 +
 .../API/tools/lldb-vscode/evaluate/main.cpp   |   2 +-
 .../variables/TestVSCode_variables.py         | 107 +++++++++++++++---
 .../API/tools/lldb-vscode/variables/main.cpp  |   6 +
 lldb/tools/lldb-vscode/JSONUtils.cpp          |  21 +++-
 6 files changed, 127 insertions(+), 17 deletions(-)

diff --git a/lldb/include/lldb/API/SBType.h b/lldb/include/lldb/API/SBType.h
index 5962f0c50dee14f..c8fcb759dede6b2 100644
--- a/lldb/include/lldb/API/SBType.h
+++ b/lldb/include/lldb/API/SBType.h
@@ -121,6 +121,10 @@ class SBType {
 
   uint64_t GetByteSize();
 
+  /// \return
+  ///    Whether the type is a pointer or a reference.
+  bool IsPointerOrReferenceType();
+
   bool IsPointerType();
 
   bool IsReferenceType();
diff --git a/lldb/source/API/SBType.cpp b/lldb/source/API/SBType.cpp
index ee5b6447428098e..38fcb54f5ea98dc 100644
--- a/lldb/source/API/SBType.cpp
+++ b/lldb/source/API/SBType.cpp
@@ -127,6 +127,10 @@ uint64_t SBType::GetByteSize() {
   return 0;
 }
 
+bool SBType::IsPointerOrReferenceType() {
+  return IsPointerType() || IsReferenceType();
+}
+
 bool SBType::IsPointerType() {
   LLDB_INSTRUMENT_VA(this);
 
diff --git a/lldb/test/API/tools/lldb-vscode/evaluate/main.cpp b/lldb/test/API/tools/lldb-vscode/evaluate/main.cpp
index 3a541b21b220828..bed853ba6e1433e 100644
--- a/lldb/test/API/tools/lldb-vscode/evaluate/main.cpp
+++ b/lldb/test/API/tools/lldb-vscode/evaluate/main.cpp
@@ -43,6 +43,6 @@ int main(int argc, char const *argv[]) {
   my_bool_vec.push_back(true);
   my_bool_vec.push_back(false); // breakpoint 6
   my_bool_vec.push_back(true); // breakpoint 7
-  
+
   return 0;
 }
diff --git a/lldb/test/API/tools/lldb-vscode/variables/TestVSCode_variables.py b/lldb/test/API/tools/lldb-vscode/variables/TestVSCode_variables.py
index fc24b3b34e70283..efb5f3ec284589f 100644
--- a/lldb/test/API/tools/lldb-vscode/variables/TestVSCode_variables.py
+++ b/lldb/test/API/tools/lldb-vscode/variables/TestVSCode_variables.py
@@ -42,6 +42,17 @@ def verify_values(self, verify_dict, actual, varref_dict=None, expression=None):
                     ('"%s" value "%s" doesn\'t start with' ' "%s")')
                     % (key, actual_value, verify_value),
                 )
+        if "notstartswith" in verify_dict:
+            verify = verify_dict["notstartswith"]
+            for key in verify:
+                verify_value = verify[key]
+                actual_value = actual[key]
+                startswith = actual_value.startswith(verify_value)
+                self.assertFalse(
+                    startswith,
+                    ('"%s" value "%s" starts with' ' "%s")')
+                    % (key, actual_value, verify_value),
+                )
         if "contains" in verify_dict:
             verify = verify_dict["contains"]
             for key in verify:
@@ -155,6 +166,7 @@ def do_test_scopes_variables_setVariable_evaluate(
             "argv": {
                 "equals": {"type": "const char **"},
                 "startswith": {"value": "0x"},
+                "notstartswith": {"value": "0x0"},
                 "hasVariablesReference": True,
             },
             "pt": {
@@ -166,8 +178,53 @@ def do_test_scopes_variables_setVariable_evaluate(
                     "buffer": {"children": buffer_children},
                 },
             },
+            "pt_ptr": {
+                "equals": {"type": "PointType *"},
+                "startswith": {"value": "0x"},
+                "notstartswith": {"value": "0x0"},
+                "hasVariablesReference": True,
+            },
+            "another_pt_ptr": {
+                "equals": {"type": "PointType *"},
+                "startswith": {"value": "<null>"},
+                "hasVariablesReference": True,
+            },
             "x": {"equals": {"type": "int"}},
+            "some_int": {
+                "equals": {
+                    "type": "int",
+                    "value": "10",
+                },
+            },
+            "some_int_ptr": {
+                "equals": {"type": "int *"},
+                "startswith": {"value": "0x"},
+                "notstartswith": {"value": "0x0"},
+            },
+            "another_int_ptr": {
+                "equals": {
+                    "type": "int *",
+                    "value": "<null>",
+                },
+            },
         }
+        if enableAutoVariableSummaries:
+            verify_locals["pt_ptr"] = {
+                "equals": {"type": "PointType *"},
+                "hasVariablesReference": True,
+                "children": {
+                    "x": {"equals": {"type": "int", "value": "11"}},
+                    "y": {"equals": {"type": "int", "value": "22"}},
+                    "buffer": {"children": buffer_children},
+                },
+            }
+            verify_locals["some_int_ptr"] ={
+                "equals": {
+                    "type": "int *",
+                    "value": "20",
+                },
+            }
+
         verify_globals = {
             "s_local": {"equals": {"type": "float", "value": "2.25"}},
             "::g_global": {"equals": {"type": "int", "value": "123"}},
@@ -297,9 +354,9 @@ def do_test_scopes_variables_setVariable_evaluate(
 
         verify_locals["argc"]["equals"]["value"] = "123"
         verify_locals["pt"]["children"]["x"]["equals"]["value"] = "111"
-        verify_locals["x @ main.cpp:17"] = {"equals": {"type": "int", "value": "89"}}
-        verify_locals["x @ main.cpp:19"] = {"equals": {"type": "int", "value": "42"}}
-        verify_locals["x @ main.cpp:21"] = {"equals": {"type": "int", "value": "72"}}
+        verify_locals["x @ main.cpp:23"] = {"equals": {"type": "int", "value": "89"}}
+        verify_locals["x @ main.cpp:25"] = {"equals": {"type": "int", "value": "42"}}
+        verify_locals["x @ main.cpp:27"] = {"equals": {"type": "int", "value": "72"}}
 
         self.verify_variables(verify_locals, self.vscode.get_local_variables())
 
@@ -310,29 +367,29 @@ def do_test_scopes_variables_setVariable_evaluate(
         )
 
         self.assertTrue(
-            self.vscode.request_setVariable(1, "x @ main.cpp:17", 17)["success"]
+            self.vscode.request_setVariable(1, "x @ main.cpp:23", 17)["success"]
         )
         self.assertTrue(
-            self.vscode.request_setVariable(1, "x @ main.cpp:19", 19)["success"]
+            self.vscode.request_setVariable(1, "x @ main.cpp:25", 19)["success"]
         )
         self.assertTrue(
-            self.vscode.request_setVariable(1, "x @ main.cpp:21", 21)["success"]
+            self.vscode.request_setVariable(1, "x @ main.cpp:27", 21)["success"]
         )
 
         # The following should have no effect
         self.assertFalse(
-            self.vscode.request_setVariable(1, "x @ main.cpp:21", "invalid")["success"]
+            self.vscode.request_setVariable(1, "x @ main.cpp:27", "invalid")["success"]
         )
 
-        verify_locals["x @ main.cpp:17"]["equals"]["value"] = "17"
-        verify_locals["x @ main.cpp:19"]["equals"]["value"] = "19"
-        verify_locals["x @ main.cpp:21"]["equals"]["value"] = "21"
+        verify_locals["x @ main.cpp:23"]["equals"]["value"] = "17"
+        verify_locals["x @ main.cpp:25"]["equals"]["value"] = "19"
+        verify_locals["x @ main.cpp:27"]["equals"]["value"] = "21"
 
         self.verify_variables(verify_locals, self.vscode.get_local_variables())
 
         # The plain x variable shold refer to the innermost x
         self.assertTrue(self.vscode.request_setVariable(1, "x", 22)["success"])
-        verify_locals["x @ main.cpp:21"]["equals"]["value"] = "22"
+        verify_locals["x @ main.cpp:27"]["equals"]["value"] = "22"
 
         self.verify_variables(verify_locals, self.vscode.get_local_variables())
 
@@ -349,9 +406,9 @@ def do_test_scopes_variables_setVariable_evaluate(
         names = [var["name"] for var in locals]
         # The first shadowed x shouldn't have a suffix anymore
         verify_locals["x"] = {"equals": {"type": "int", "value": "17"}}
-        self.assertNotIn("x @ main.cpp:17", names)
-        self.assertNotIn("x @ main.cpp:19", names)
-        self.assertNotIn("x @ main.cpp:21", names)
+        self.assertNotIn("x @ main.cpp:23", names)
+        self.assertNotIn("x @ main.cpp:25", names)
+        self.assertNotIn("x @ main.cpp:27", names)
 
         self.verify_variables(verify_locals, locals)
 
@@ -421,10 +478,32 @@ def do_test_scopes_and_evaluate_expansion(self, enableAutoVariableSummaries: boo
                     },
                 },
             },
+            "pt_ptr": {
+                "equals": {"type": "PointType *"},
+                "hasVariablesReference": True,
+                "missing": ["indexedVariables"],
+            },
+            "another_pt_ptr": {
+                "equals": {"type": "PointType *"},
+                "hasVariablesReference": True,
+                "missing": ["indexedVariables"],
+            },
             "x": {
                 "equals": {"type": "int"},
                 "missing": ["indexedVariables"],
             },
+            "some_int": {
+                "equals": {"type": "int"},
+                "missing": ["indexedVariables"],
+            },
+            "some_int_ptr": {
+                "equals": {"type": "int *"},
+                "missing": ["indexedVariables"],
+            },
+            "another_int_ptr": {
+                "equals": {"type": "int *"},
+                "missing": ["indexedVariables"],
+            },
         }
         self.verify_variables(verify_locals, locals)
 
diff --git a/lldb/test/API/tools/lldb-vscode/variables/main.cpp b/lldb/test/API/tools/lldb-vscode/variables/main.cpp
index d81a9a20544a856..10e3459e1c1fb25 100644
--- a/lldb/test/API/tools/lldb-vscode/variables/main.cpp
+++ b/lldb/test/API/tools/lldb-vscode/variables/main.cpp
@@ -12,8 +12,14 @@ int test_indexedVariables();
 int main(int argc, char const *argv[]) {
   static float s_local = 2.25;
   PointType pt = { 11,22, {0}};
+  PointType *pt_ptr = new PointType{11, 22, {0}};
+  PointType *another_pt_ptr = nullptr;
   for (int i=0; i<BUFFER_SIZE; ++i)
     pt.buffer[i] = i;
+
+  int some_int = 10;
+  int *some_int_ptr = new int{20};
+  int *another_int_ptr = nullptr;
   int x = s_global - g_global - pt.y; // breakpoint 1
   {
     int x = 42;
diff --git a/lldb/tools/lldb-vscode/JSONUtils.cpp b/lldb/tools/lldb-vscode/JSONUtils.cpp
index c6b422e4d7a02e6..30c35b9e52c8404 100644
--- a/lldb/tools/lldb-vscode/JSONUtils.cpp
+++ b/lldb/tools/lldb-vscode/JSONUtils.cpp
@@ -200,7 +200,7 @@ static bool ShouldBeDereferencedForSummary(lldb::SBValue &v) {
   if (!g_vsc.enable_auto_variable_summaries)
     return false;
 
-  if (!v.GetType().IsPointerType() && !v.GetType().IsReferenceType())
+  if (!v.GetType().IsPointerOrReferenceType())
     return false;
 
   // If we are referencing a pointer, we don't dereference to avoid confusing
@@ -228,7 +228,24 @@ void SetValueForKey(lldb::SBValue &v, llvm::json::Object &object,
     strm << "<error: " << error.GetCString() << ">";
   } else {
     auto tryDumpSummaryAndValue = [&strm](lldb::SBValue value) {
-      llvm::StringRef val = value.GetValue();
+      std::string val;
+      // Whenever the value is a non-synthetic address, we format it ourselves
+      // to use as few chars as possible because the variables pane on VS Code
+      // is by default narrow.
+      if (!value.IsSynthetic() && value.GetType().IsPointerOrReferenceType()) {
+        lldb::addr_t address = value.GetValueAsUnsigned(LLDB_INVALID_ADDRESS);
+        if (address == LLDB_INVALID_ADDRESS) {
+          val = "<invalid address>";
+        } else if (address == 0) {
+          val = "<null>";
+        } else {
+          llvm::raw_string_ostream os(val);
+          os << llvm::format_hex(address, 0);
+        }
+      } else {
+        val = llvm::StringRef(value.GetValue()).str();
+      }
+
       llvm::StringRef summary = value.GetSummary();
       if (!val.empty()) {
         strm << val;



More information about the lldb-commits mailing list