[Lldb-commits] [lldb] Fix dap variable value format issue (PR #90799)

via lldb-commits lldb-commits at lists.llvm.org
Wed May 1 17:02:59 PDT 2024


https://github.com/jeffreytan81 updated https://github.com/llvm/llvm-project/pull/90799

>From 2daf4aeaee1ce9062dfa964f3baeef0d7477479c Mon Sep 17 00:00:00 2001
From: jeffreytan81 <jeffreytan at fb.com>
Date: Wed, 1 May 2024 16:35:18 -0700
Subject: [PATCH 1/3] Fix dap variable value format issue

---
 .../test/tools/lldb-dap/dap_server.py         | 24 ++++++-----
 .../lldb-dap/variables/TestDAP_variables.py   | 40 +++++++++++++++++++
 lldb/tools/lldb-dap/JSONUtils.cpp             | 14 ++++---
 3 files changed, 62 insertions(+), 16 deletions(-)

diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
index 5838281bcb1a10..54b8bb77e6ed61 100644
--- a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
+++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
@@ -448,7 +448,7 @@ def get_completions(self, text, frameId=None):
         response = self.request_completions(text, frameId)
         return response["body"]["targets"]
 
-    def get_scope_variables(self, scope_name, frameIndex=0, threadId=None):
+    def get_scope_variables(self, scope_name, frameIndex=0, threadId=None, is_hex=None):
         stackFrame = self.get_stackFrame(frameIndex=frameIndex, threadId=threadId)
         if stackFrame is None:
             return []
@@ -462,7 +462,7 @@ def get_scope_variables(self, scope_name, frameIndex=0, threadId=None):
         for scope in frame_scopes:
             if scope["name"] == scope_name:
                 varRef = scope["variablesReference"]
-                variables_response = self.request_variables(varRef)
+                variables_response = self.request_variables(varRef, is_hex=is_hex)
                 if variables_response:
                     if "body" in variables_response:
                         body = variables_response["body"]
@@ -476,9 +476,9 @@ def get_global_variables(self, frameIndex=0, threadId=None):
             "Globals", frameIndex=frameIndex, threadId=threadId
         )
 
-    def get_local_variables(self, frameIndex=0, threadId=None):
+    def get_local_variables(self, frameIndex=0, threadId=None, is_hex=None):
         return self.get_scope_variables(
-            "Locals", frameIndex=frameIndex, threadId=threadId
+            "Locals", frameIndex=frameIndex, threadId=threadId, is_hex=is_hex
         )
 
     def get_registers(self, frameIndex=0, threadId=None):
@@ -486,26 +486,26 @@ def get_registers(self, frameIndex=0, threadId=None):
             "Registers", frameIndex=frameIndex, threadId=threadId
         )
 
-    def get_local_variable(self, name, frameIndex=0, threadId=None):
-        locals = self.get_local_variables(frameIndex=frameIndex, threadId=threadId)
+    def get_local_variable(self, name, frameIndex=0, threadId=None, is_hex=None):
+        locals = self.get_local_variables(frameIndex=frameIndex, threadId=threadId, is_hex=is_hex)
         for local in locals:
             if "name" in local and local["name"] == name:
                 return local
         return None
 
-    def get_local_variable_value(self, name, frameIndex=0, threadId=None):
+    def get_local_variable_value(self, name, frameIndex=0, threadId=None, is_hex=None):
         variable = self.get_local_variable(
-            name, frameIndex=frameIndex, threadId=threadId
+            name, frameIndex=frameIndex, threadId=threadId, is_hex=is_hex
         )
         if variable and "value" in variable:
             return variable["value"]
         return None
 
-    def get_local_variable_child(self, name, child_name, frameIndex=0, threadId=None):
+    def get_local_variable_child(self, name, child_name, frameIndex=0, threadId=None, is_hex=None):
         local = self.get_local_variable(name, frameIndex, threadId)
         if local["variablesReference"] == 0:
             return None
-        children = self.request_variables(local["variablesReference"])["body"][
+        children = self.request_variables(local["variablesReference"], is_hex=is_hex)["body"][
             "variables"
         ]
         for child in children:
@@ -1035,12 +1035,14 @@ def request_threads(self):
             self.threads = None
         return response
 
-    def request_variables(self, variablesReference, start=None, count=None):
+    def request_variables(self, variablesReference, start=None, count=None, is_hex=None):
         args_dict = {"variablesReference": variablesReference}
         if start is not None:
             args_dict["start"] = start
         if count is not None:
             args_dict["count"] = count
+        if is_hex is not None:
+            args_dict["format"] = {"hex": is_hex}
         command_dict = {
             "command": "variables",
             "type": "request",
diff --git a/lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py b/lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py
index d886d0776ce58b..51e851abf8a54e 100644
--- a/lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py
+++ b/lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py
@@ -754,3 +754,43 @@ def test_darwin_dwarf_missing_obj_with_symbol_ondemand_enabled(self):
         """
         initCommands = ["settings set symbols.load-on-demand true"]
         self.darwin_dwarf_missing_obj(initCommands)
+
+    @no_debug_info_test
+    @skipIfWindows
+    @skipIfRemote
+    def test_value_format(self):
+        """
+        Test that toggle variables value format between decimal and hexical works.
+        """
+        program = self.getBuildArtifact("a.out")
+        self.build_and_launch(program)
+        source = "main.cpp"
+        breakpoint1_line = line_number(source, "// breakpoint 1")
+        lines = [breakpoint1_line]
+        
+        breakpoint_ids = self.set_source_breakpoints(source, lines)
+        self.assertEqual(
+            len(breakpoint_ids), len(lines), "expect correct number of breakpoints"
+        )
+        self.continue_to_breakpoints(breakpoint_ids)
+        
+        # Verify locals value format decimal
+        is_hex = False
+        var_pt_x = self.dap_server.get_local_variable_child("pt", "x", is_hex=is_hex)
+        self.assertEquals(var_pt_x["value"], "11")
+        var_pt_y = self.dap_server.get_local_variable_child("pt", "y", is_hex=is_hex)
+        self.assertEquals(var_pt_y["value"], "22")
+
+        # Verify locals value format hexical
+        is_hex = True
+        var_pt_x = self.dap_server.get_local_variable_child("pt", "x", is_hex=is_hex)
+        self.assertEquals(var_pt_x["value"], "0x0000000b")
+        var_pt_y = self.dap_server.get_local_variable_child("pt", "y", is_hex=is_hex)
+        self.assertEquals(var_pt_y["value"], "0x00000016")
+        
+        # Toggle and verify locals value format decimal again
+        is_hex = False
+        var_pt_x = self.dap_server.get_local_variable_child("pt", "x", is_hex=is_hex)
+        self.assertEquals(var_pt_x["value"], "11")
+        var_pt_y = self.dap_server.get_local_variable_child("pt", "y", is_hex=is_hex)
+        self.assertEquals(var_pt_y["value"], "22")
diff --git a/lldb/tools/lldb-dap/JSONUtils.cpp b/lldb/tools/lldb-dap/JSONUtils.cpp
index b4a2718bbb096e..55feab600a1972 100644
--- a/lldb/tools/lldb-dap/JSONUtils.cpp
+++ b/lldb/tools/lldb-dap/JSONUtils.cpp
@@ -748,10 +748,9 @@ llvm::json::Value CreateStackFrame(lldb::SBFrame &frame) {
     auto line = line_entry.GetLine();
     if (line && line != LLDB_INVALID_LINE_NUMBER)
       object.try_emplace("line", line);
-    else
-      object.try_emplace("line", 0);
     auto column = line_entry.GetColumn();
-    object.try_emplace("column", column);
+    if (column && column != LLDB_INVALID_COLUMN_NUMBER)
+      object.try_emplace("column", column);
   } else {
     object.try_emplace("line", 0);
     object.try_emplace("column", 0);
@@ -988,8 +987,13 @@ VariableDescription::VariableDescription(lldb::SBValue v, bool format_hex,
   display_type_name =
       !raw_display_type_name.empty() ? raw_display_type_name : NO_TYPENAME;
 
-  if (format_hex)
-    v.SetFormat(lldb::eFormatHex);
+  // Only format hex/default if there is no existing special format.
+  if (v.GetFormat() == lldb::eFormatDefault || v.GetFormat() == lldb::eFormatHex) {
+    if (format_hex)
+      v.SetFormat(lldb::eFormatHex);
+    else
+      v.SetFormat(lldb::eFormatDefault);
+  }
 
   llvm::raw_string_ostream os_display_value(display_value);
 

>From f4c19c64d44cadf6b6ad13a6703ab590c84c7928 Mon Sep 17 00:00:00 2001
From: jeffreytan81 <jeffreytan at fb.com>
Date: Wed, 1 May 2024 16:47:05 -0700
Subject: [PATCH 2/3] Fix format

---
 .../test/tools/lldb-dap/dap_server.py         | 18 ++++++---
 .../lldb-dap/variables/TestDAP_variables.py   | 38 +++++++++++--------
 2 files changed, 35 insertions(+), 21 deletions(-)

diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
index 54b8bb77e6ed61..e2126d67a5fe77 100644
--- a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
+++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
@@ -487,7 +487,9 @@ def get_registers(self, frameIndex=0, threadId=None):
         )
 
     def get_local_variable(self, name, frameIndex=0, threadId=None, is_hex=None):
-        locals = self.get_local_variables(frameIndex=frameIndex, threadId=threadId, is_hex=is_hex)
+        locals = self.get_local_variables(
+            frameIndex=frameIndex, threadId=threadId, is_hex=is_hex
+        )
         for local in locals:
             if "name" in local and local["name"] == name:
                 return local
@@ -501,13 +503,15 @@ def get_local_variable_value(self, name, frameIndex=0, threadId=None, is_hex=Non
             return variable["value"]
         return None
 
-    def get_local_variable_child(self, name, child_name, frameIndex=0, threadId=None, is_hex=None):
+    def get_local_variable_child(
+        self, name, child_name, frameIndex=0, threadId=None, is_hex=None
+    ):
         local = self.get_local_variable(name, frameIndex, threadId)
         if local["variablesReference"] == 0:
             return None
-        children = self.request_variables(local["variablesReference"], is_hex=is_hex)["body"][
-            "variables"
-        ]
+        children = self.request_variables(local["variablesReference"], is_hex=is_hex)[
+            "body"
+        ]["variables"]
         for child in children:
             if child["name"] == child_name:
                 return child
@@ -1035,7 +1039,9 @@ def request_threads(self):
             self.threads = None
         return response
 
-    def request_variables(self, variablesReference, start=None, count=None, is_hex=None):
+    def request_variables(
+        self, variablesReference, start=None, count=None, is_hex=None
+    ):
         args_dict = {"variablesReference": variablesReference}
         if start is not None:
             args_dict["start"] = start
diff --git a/lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py b/lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py
index 51e851abf8a54e..46624b82d60cbf 100644
--- a/lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py
+++ b/lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py
@@ -254,18 +254,22 @@ def do_test_scopes_variables_setVariable_evaluate(
             "pt": {
                 "equals": {"type": "PointType"},
                 "startswith": {
-                    "result": "{x:11, y:22, buffer:{...}}"
-                    if enableAutoVariableSummaries
-                    else "PointType @ 0x"
+                    "result": (
+                        "{x:11, y:22, buffer:{...}}"
+                        if enableAutoVariableSummaries
+                        else "PointType @ 0x"
+                    )
                 },
                 "hasVariablesReference": True,
             },
             "pt.buffer": {
                 "equals": {"type": "int[16]"},
                 "startswith": {
-                    "result": "{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, ...}"
-                    if enableAutoVariableSummaries
-                    else "int[16] @ 0x"
+                    "result": (
+                        "{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, ...}"
+                        if enableAutoVariableSummaries
+                        else "int[16] @ 0x"
+                    )
                 },
                 "hasVariablesReference": True,
             },
@@ -528,9 +532,11 @@ def do_test_scopes_and_evaluate_expansion(self, enableAutoVariableSummaries: boo
                 "watch": {
                     "equals": {"type": "PointType"},
                     "startswith": {
-                        "result": "{x:11, y:22, buffer:{...}}"
-                        if enableAutoVariableSummaries
-                        else "PointType @ 0x"
+                        "result": (
+                            "{x:11, y:22, buffer:{...}}"
+                            if enableAutoVariableSummaries
+                            else "PointType @ 0x"
+                        )
                     },
                     "missing": ["indexedVariables"],
                     "hasVariablesReference": True,
@@ -538,9 +544,11 @@ def do_test_scopes_and_evaluate_expansion(self, enableAutoVariableSummaries: boo
                 "variables": {
                     "equals": {"type": "PointType"},
                     "startswith": {
-                        "result": "{x:11, y:22, buffer:{...}}"
-                        if enableAutoVariableSummaries
-                        else "PointType @ 0x"
+                        "result": (
+                            "{x:11, y:22, buffer:{...}}"
+                            if enableAutoVariableSummaries
+                            else "PointType @ 0x"
+                        )
                     },
                     "missing": ["indexedVariables"],
                     "hasVariablesReference": True,
@@ -767,13 +775,13 @@ def test_value_format(self):
         source = "main.cpp"
         breakpoint1_line = line_number(source, "// breakpoint 1")
         lines = [breakpoint1_line]
-        
+
         breakpoint_ids = self.set_source_breakpoints(source, lines)
         self.assertEqual(
             len(breakpoint_ids), len(lines), "expect correct number of breakpoints"
         )
         self.continue_to_breakpoints(breakpoint_ids)
-        
+
         # Verify locals value format decimal
         is_hex = False
         var_pt_x = self.dap_server.get_local_variable_child("pt", "x", is_hex=is_hex)
@@ -787,7 +795,7 @@ def test_value_format(self):
         self.assertEquals(var_pt_x["value"], "0x0000000b")
         var_pt_y = self.dap_server.get_local_variable_child("pt", "y", is_hex=is_hex)
         self.assertEquals(var_pt_y["value"], "0x00000016")
-        
+
         # Toggle and verify locals value format decimal again
         is_hex = False
         var_pt_x = self.dap_server.get_local_variable_child("pt", "x", is_hex=is_hex)

>From 6ebea68def5e5bc6a52bc6b7428ec04cc7327712 Mon Sep 17 00:00:00 2001
From: jeffreytan81 <jeffreytan at fb.com>
Date: Wed, 1 May 2024 17:02:42 -0700
Subject: [PATCH 3/3] Fix format again

---
 .../lldb-dap/variables/TestDAP_variables.py   | 32 +++++++------------
 lldb/tools/lldb-dap/JSONUtils.cpp             |  6 ++--
 2 files changed, 15 insertions(+), 23 deletions(-)

diff --git a/lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py b/lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py
index 46624b82d60cbf..ab7dfb5216ae19 100644
--- a/lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py
+++ b/lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py
@@ -254,22 +254,18 @@ def do_test_scopes_variables_setVariable_evaluate(
             "pt": {
                 "equals": {"type": "PointType"},
                 "startswith": {
-                    "result": (
-                        "{x:11, y:22, buffer:{...}}"
-                        if enableAutoVariableSummaries
-                        else "PointType @ 0x"
-                    )
+                    "result": "{x:11, y:22, buffer:{...}}"
+                    if enableAutoVariableSummaries
+                    else "PointType @ 0x"
                 },
                 "hasVariablesReference": True,
             },
             "pt.buffer": {
                 "equals": {"type": "int[16]"},
                 "startswith": {
-                    "result": (
-                        "{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, ...}"
-                        if enableAutoVariableSummaries
-                        else "int[16] @ 0x"
-                    )
+                    "result": "{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, ...}"
+                    if enableAutoVariableSummaries
+                    else "int[16] @ 0x"
                 },
                 "hasVariablesReference": True,
             },
@@ -532,11 +528,9 @@ def do_test_scopes_and_evaluate_expansion(self, enableAutoVariableSummaries: boo
                 "watch": {
                     "equals": {"type": "PointType"},
                     "startswith": {
-                        "result": (
-                            "{x:11, y:22, buffer:{...}}"
-                            if enableAutoVariableSummaries
-                            else "PointType @ 0x"
-                        )
+                        "result": "{x:11, y:22, buffer:{...}}"
+                        if enableAutoVariableSummaries
+                        else "PointType @ 0x"
                     },
                     "missing": ["indexedVariables"],
                     "hasVariablesReference": True,
@@ -544,11 +538,9 @@ def do_test_scopes_and_evaluate_expansion(self, enableAutoVariableSummaries: boo
                 "variables": {
                     "equals": {"type": "PointType"},
                     "startswith": {
-                        "result": (
-                            "{x:11, y:22, buffer:{...}}"
-                            if enableAutoVariableSummaries
-                            else "PointType @ 0x"
-                        )
+                        "result": "{x:11, y:22, buffer:{...}}"
+                        if enableAutoVariableSummaries
+                        else "PointType @ 0x"
                     },
                     "missing": ["indexedVariables"],
                     "hasVariablesReference": True,
diff --git a/lldb/tools/lldb-dap/JSONUtils.cpp b/lldb/tools/lldb-dap/JSONUtils.cpp
index 55feab600a1972..21798078f4d7fa 100644
--- a/lldb/tools/lldb-dap/JSONUtils.cpp
+++ b/lldb/tools/lldb-dap/JSONUtils.cpp
@@ -748,13 +748,13 @@ llvm::json::Value CreateStackFrame(lldb::SBFrame &frame) {
     auto line = line_entry.GetLine();
     if (line && line != LLDB_INVALID_LINE_NUMBER)
       object.try_emplace("line", line);
+    else
+      object.try_emplace("line", 0);
     auto column = line_entry.GetColumn();
-    if (column && column != LLDB_INVALID_COLUMN_NUMBER)
-      object.try_emplace("column", column);
+    object.try_emplace("column", column);
   } else {
     object.try_emplace("line", 0);
     object.try_emplace("column", 0);
-    object.try_emplace("presentationHint", "subtle");
   }
 
   const auto pc = frame.GetPC();



More information about the lldb-commits mailing list