[Lldb-commits] [lldb] [lldb-dap] Add: show return value on step out (PR #106907)

via lldb-commits lldb-commits at lists.llvm.org
Thu Feb 13 15:22:35 PST 2025


https://github.com/Da-Viper updated https://github.com/llvm/llvm-project/pull/106907

>From aeb8854bbe7695e576257c8403e04d4b8268b679 Mon Sep 17 00:00:00 2001
From: Ezike Ebuka <yerimyah1 at gmail.com>
Date: Sun, 1 Sep 2024 13:48:41 +0100
Subject: [PATCH 1/7] Add: show return value on step out

---
 lldb/tools/lldb-dap/lldb-dap.cpp | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp
index e323990d8b6ed..3cacc16dedb25 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -4225,6 +4225,17 @@ void request_variables(DAP &dap, const llvm::json::Object &request) {
       variable_name_counts[GetNonNullVariableName(variable)]++;
     }
 
+    // Show return value if there is any ( in the top frame )
+    auto process = g_dap.target.GetProcess();
+    auto selectedThread = process.GetSelectedThread();
+    lldb::SBValue stopReturnValue = selectedThread.GetStopReturnValue();
+    if (stopReturnValue.IsValid() &&
+        (selectedThread.GetSelectedFrame().GetFrameID() == 0)) {
+      auto renamedReturnValue = stopReturnValue.Clone("(Return Value)");
+      variables.emplace_back(
+          CreateVariable(renamedReturnValue,0, UINT64_MAX, hex, false));
+    }
+
     // Now we construct the result with unique display variable names
     for (auto i = start_idx; i < end_idx; ++i) {
       lldb::SBValue variable = top_scope->GetValueAtIndex(i);

>From 07c3c56bc987cdd947b8de126be6497f45f1da7b Mon Sep 17 00:00:00 2001
From: Ezike Ebuka <yerimyah1 at gmail.com>
Date: Sun, 1 Sep 2024 14:10:54 +0100
Subject: [PATCH 2/7] format file

---
 lldb/tools/lldb-dap/lldb-dap.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp
index 3cacc16dedb25..30b3354c184b0 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -4233,7 +4233,7 @@ void request_variables(DAP &dap, const llvm::json::Object &request) {
         (selectedThread.GetSelectedFrame().GetFrameID() == 0)) {
       auto renamedReturnValue = stopReturnValue.Clone("(Return Value)");
       variables.emplace_back(
-          CreateVariable(renamedReturnValue,0, UINT64_MAX, hex, false));
+          CreateVariable(renamedReturnValue, 0, UINT64_MAX, hex, false));
     }
 
     // Now we construct the result with unique display variable names

>From c9ef0294def8919536ade9f8097c3471b94b0355 Mon Sep 17 00:00:00 2001
From: Ezike Ebuka <yerimyah1 at gmail.com>
Date: Sun, 15 Sep 2024 22:22:09 +0100
Subject: [PATCH 3/7] [lldb-dap] Add: Show children for return values

---
 lldb/tools/lldb-dap/lldb-dap.cpp | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp
index 30b3354c184b0..00584f31848cf 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -4227,13 +4227,19 @@ void request_variables(DAP &dap, const llvm::json::Object &request) {
 
     // Show return value if there is any ( in the top frame )
     auto process = g_dap.target.GetProcess();
-    auto selectedThread = process.GetSelectedThread();
-    lldb::SBValue stopReturnValue = selectedThread.GetStopReturnValue();
-    if (stopReturnValue.IsValid() &&
-        (selectedThread.GetSelectedFrame().GetFrameID() == 0)) {
-      auto renamedReturnValue = stopReturnValue.Clone("(Return Value)");
-      variables.emplace_back(
-          CreateVariable(renamedReturnValue, 0, UINT64_MAX, hex, false));
+    auto selected_thread = process.GetSelectedThread();
+    lldb::SBValue stop_return_value = selected_thread.GetStopReturnValue();
+    if (stop_return_value.IsValid() &&
+        (selected_thread.GetSelectedFrame().GetFrameID() == 0)) {
+      auto renamed_return_value = stop_return_value.Clone("(Return Value)");
+      int64_t return_ref = 0;
+      if (stop_return_value.MightHaveChildren() ||
+          stop_return_value.IsSynthetic()) {
+        return_ref = g_dap.variables.InsertExpandableVariable(
+            stop_return_value, /*is_permanent=*/false);
+      }
+      variables.emplace_back(CreateVariable(renamed_return_value, return_ref,
+                                            UINT64_MAX, hex, false));
     }
 
     // Now we construct the result with unique display variable names

>From 91e7042f9c3438b13ff8b476d07057da52b2589f Mon Sep 17 00:00:00 2001
From: Ezike Ebuka <yerimyah1 at gmail.com>
Date: Wed, 12 Feb 2025 00:16:46 +0000
Subject: [PATCH 4/7] [lldb-dap] Add Tests: Show children for return values

---
 .../lldb-dap/variables/TestDAP_variables.py   | 57 +++++++++++++++----
 .../children/TestDAP_variables_children.py    | 55 ++++++++++++++++++
 .../lldb-dap/variables/children/main.cpp      | 12 ++++
 .../API/tools/lldb-dap/variables/main.cpp     |  9 +++
 4 files changed, 122 insertions(+), 11 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 580ad38ab51c1..1e9c6083b57a8 100644
--- a/lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py
+++ b/lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py
@@ -341,9 +341,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:19"] = {"equals": {"type": "int", "value": "89"}}
+        verify_locals["x @ main.cpp:21"] = {"equals": {"type": "int", "value": "42"}}
+        verify_locals["x @ main.cpp:23"] = {"equals": {"type": "int", "value": "72"}}
 
         self.verify_variables(verify_locals, self.dap_server.get_local_variables())
 
@@ -353,32 +353,32 @@ def do_test_scopes_variables_setVariable_evaluate(
             self.dap_server.request_setVariable(1, "x @ main.cpp:0", 9)["success"]
         )
 
-        self.assertTrue(
-            self.dap_server.request_setVariable(1, "x @ main.cpp:17", 17)["success"]
-        )
         self.assertTrue(
             self.dap_server.request_setVariable(1, "x @ main.cpp:19", 19)["success"]
         )
         self.assertTrue(
             self.dap_server.request_setVariable(1, "x @ main.cpp:21", 21)["success"]
         )
+        self.assertTrue(
+            self.dap_server.request_setVariable(1, "x @ main.cpp:23", 23)["success"]
+        )
 
         # The following should have no effect
         self.assertFalse(
-            self.dap_server.request_setVariable(1, "x @ main.cpp:21", "invalid")[
+            self.dap_server.request_setVariable(1, "x @ main.cpp:23", "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"] = "23"
 
         self.verify_variables(verify_locals, self.dap_server.get_local_variables())
 
         # The plain x variable shold refer to the innermost x
         self.assertTrue(self.dap_server.request_setVariable(1, "x", 22)["success"])
-        verify_locals["x @ main.cpp:21"]["equals"]["value"] = "22"
+        verify_locals["x @ main.cpp:23"]["equals"]["value"] = "22"
 
         self.verify_variables(verify_locals, self.dap_server.get_local_variables())
 
@@ -394,10 +394,10 @@ def do_test_scopes_variables_setVariable_evaluate(
         locals = self.dap_server.get_local_variables()
         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)
+        verify_locals["x"] = {"equals": {"type": "int", "value": "19"}}
         self.assertNotIn("x @ main.cpp:19", names)
         self.assertNotIn("x @ main.cpp:21", names)
+        self.assertNotIn("x @ main.cpp:23", names)
 
         self.verify_variables(verify_locals, locals)
 
@@ -663,6 +663,41 @@ def do_test_indexedVariables(self, enableSyntheticChildDebugging: bool):
         ]["variables"]
         self.verify_variables(verify_children, children)
 
+    def test_return_variables(self):
+        """
+        Test the stepping out of a function with return value show the variable correctly.
+        """
+        program = self.getBuildArtifact("a.out")
+        self.build_and_launch(program)
+
+        verify_locals = {
+            "(Return Value)": {"equals": {"type": "int", "value": "300"}},
+            "argc": {},
+            "argv": {},
+            "pt": {},
+            "x": {},
+            "return_result": {"equals": {"type": "int"}},
+        }
+
+        function_name = "test_return_variable"
+        breakpoint_ids = self.set_function_breakpoints([function_name])
+
+        self.assertEqual(len(breakpoint_ids), 1)
+        self.continue_to_breakpoints(breakpoint_ids)
+
+        threads = self.dap_server.get_threads()
+        for thread in threads:
+            if thread.get("reason") == "breakpoint":
+                # We have a thread that
+                thread_id = thread["id"]
+
+                self.stepOut(threadId=thread_id)
+
+                local_variables = self.dap_server.get_local_variables()
+                varref_dict = {}
+                self.verify_variables(verify_locals, local_variables, varref_dict)
+                break
+
     @skipIfWindows
     def test_indexedVariables(self):
         self.do_test_indexedVariables(enableSyntheticChildDebugging=False)
diff --git a/lldb/test/API/tools/lldb-dap/variables/children/TestDAP_variables_children.py b/lldb/test/API/tools/lldb-dap/variables/children/TestDAP_variables_children.py
index 805e88ddf8f70..f671a02c75a78 100644
--- a/lldb/test/API/tools/lldb-dap/variables/children/TestDAP_variables_children.py
+++ b/lldb/test/API/tools/lldb-dap/variables/children/TestDAP_variables_children.py
@@ -40,3 +40,58 @@ def test_get_num_children(self):
                 "`script formatter.num_children_calls", context="repl"
             )["body"]["result"],
         )
+
+    def test_return_variable_with_children(self):
+        """
+        Test the stepping out of a function with return value show the children correctly
+        """
+        program = self.getBuildArtifact("a.out")
+        self.build_and_launch(program)
+
+        function_name = "test_return_variable_with_children"
+        breakpoint_ids = self.set_function_breakpoints([function_name])
+
+        self.assertEqual(len(breakpoint_ids), 1)
+        self.continue_to_breakpoints(breakpoint_ids)
+
+        threads = self.dap_server.get_threads()
+        for thread in threads:
+            if thread.get("reason") == "breakpoint":
+                thread_id = thread.get("id")
+                self.assertIsNot(thread_id, None)
+
+                self.stepOut(threadId=thread_id)
+
+                local_variables = self.dap_server.get_local_variables()
+
+                # verify has return variable as local
+                result_variable = list(
+                    filter(
+                        lambda val: val.get("name") == "(Return Value)", local_variables
+                    )
+                )
+                self.assertEqual(len(result_variable), 1)
+                result_variable = result_variable[0]
+
+                result_var_ref = result_variable.get("variablesReference")
+                self.assertIsNot(result_var_ref, None, "There is no result value")
+
+                result_value = self.dap_server.request_variables(result_var_ref)
+                result_children = result_value["body"]["variables"]
+                self.assertNotEqual(
+                    result_children, None, "The result does not have children"
+                )
+
+                verify_children = {"buffer": '"hello world!"', "x": "10", "y": "20"}
+                for child in result_children:
+                    actual_name = child["name"]
+                    actual_value = child["value"]
+                    verify_value = verify_children.get(actual_name)
+                    self.assertNotEqual(verify_value, None)
+                    self.assertEqual(
+                        actual_value,
+                        verify_value,
+                        "Expected child value does not match",
+                    )
+
+                break
diff --git a/lldb/test/API/tools/lldb-dap/variables/children/main.cpp b/lldb/test/API/tools/lldb-dap/variables/children/main.cpp
index 5d625fe1903a3..59a86f758ab28 100644
--- a/lldb/test/API/tools/lldb-dap/variables/children/main.cpp
+++ b/lldb/test/API/tools/lldb-dap/variables/children/main.cpp
@@ -1,8 +1,20 @@
 struct Indexed {};
 struct NotIndexed {};
 
+#define BUFFER_SIZE 16
+struct NonPrimitive {
+  char buffer[BUFFER_SIZE];
+  int x;
+  long y;
+};
+
+NonPrimitive test_return_variable_with_children() {
+  return NonPrimitive{"hello world!", 10, 20};
+}
+
 int main() {
   Indexed indexed;
   NotIndexed not_indexed;
+  NonPrimitive non_primitive_result = test_return_variable_with_children();
   return 0; // break here
 }
diff --git a/lldb/test/API/tools/lldb-dap/variables/main.cpp b/lldb/test/API/tools/lldb-dap/variables/main.cpp
index 3557067ed9ce1..0e363001f2f42 100644
--- a/lldb/test/API/tools/lldb-dap/variables/main.cpp
+++ b/lldb/test/API/tools/lldb-dap/variables/main.cpp
@@ -9,6 +9,8 @@ struct PointType {
 int g_global = 123;
 static int s_global = 234;
 int test_indexedVariables();
+int test_return_variable();
+
 int main(int argc, char const *argv[]) {
   static float s_local = 2.25;
   PointType pt = {11, 22, {0}};
@@ -22,6 +24,9 @@ int main(int argc, char const *argv[]) {
       s_global = x; // breakpoint 2
     }
   }
+  {
+    int return_result = test_return_variable();
+  }
   return test_indexedVariables(); // breakpoint 3
 }
 
@@ -34,3 +39,7 @@ int test_indexedVariables() {
   large_vector.assign(200, 0);
   return 0; // breakpoint 4
 }
+
+int test_return_variable() {
+  return 300; // breakpoint 5
+}
\ No newline at end of file

>From b53a1d05a4bd8d1e8c0504769b04865bdf76fafa Mon Sep 17 00:00:00 2001
From: Ezike Ebuka <yerimyah1 at gmail.com>
Date: Wed, 12 Feb 2025 10:57:35 +0000
Subject: [PATCH 5/7] [lldb-dap] clean up: Rename to return reference variable

---
 lldb/tools/lldb-dap/lldb-dap.cpp | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp
index 00584f31848cf..b982befdbd597 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -4232,13 +4232,13 @@ void request_variables(DAP &dap, const llvm::json::Object &request) {
     if (stop_return_value.IsValid() &&
         (selected_thread.GetSelectedFrame().GetFrameID() == 0)) {
       auto renamed_return_value = stop_return_value.Clone("(Return Value)");
-      int64_t return_ref = 0;
+      int64_t return_var_ref = 0;
       if (stop_return_value.MightHaveChildren() ||
           stop_return_value.IsSynthetic()) {
-        return_ref = g_dap.variables.InsertExpandableVariable(
+        return_var_ref = g_dap.variables.InsertExpandableVariable(
             stop_return_value, /*is_permanent=*/false);
       }
-      variables.emplace_back(CreateVariable(renamed_return_value, return_ref,
+      variables.emplace_back(CreateVariable(renamed_return_value, return_var_ref,
                                             UINT64_MAX, hex, false));
     }
 

>From 42e3bd5d8e73d18d35cf8a506a345bc425ba7fe9 Mon Sep 17 00:00:00 2001
From: Ezike Ebuka <yerimyah1 at gmail.com>
Date: Thu, 13 Feb 2025 13:12:23 +0000
Subject: [PATCH 6/7] [lldb-dap] Fix: only show return values in the local
 scope.

---
 .../lldb-dap/variables/TestDAP_variables.py   |  1 -
 lldb/tools/lldb-dap/lldb-dap.cpp              | 32 ++++++++++---------
 2 files changed, 17 insertions(+), 16 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 1e9c6083b57a8..f8314308e72b5 100644
--- a/lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py
+++ b/lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py
@@ -688,7 +688,6 @@ def test_return_variables(self):
         threads = self.dap_server.get_threads()
         for thread in threads:
             if thread.get("reason") == "breakpoint":
-                # We have a thread that
                 thread_id = thread["id"]
 
                 self.stepOut(threadId=thread_id)
diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp
index b982befdbd597..179c13ae7d1f5 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -4192,7 +4192,7 @@ void request_variables(DAP &dap, const llvm::json::Object &request) {
     if (num_children == 0 && variablesReference == VARREF_LOCALS) {
       // Check for an error in the SBValueList that might explain why we don't
       // have locals. If we have an error display it as the sole value in the
-      // the locals.
+      // locals.
 
       // "error" owns the error string so we must keep it alive as long as we
       // want to use the returns "const char *"
@@ -4225,21 +4225,23 @@ void request_variables(DAP &dap, const llvm::json::Object &request) {
       variable_name_counts[GetNonNullVariableName(variable)]++;
     }
 
-    // Show return value if there is any ( in the top frame )
-    auto process = g_dap.target.GetProcess();
-    auto selected_thread = process.GetSelectedThread();
-    lldb::SBValue stop_return_value = selected_thread.GetStopReturnValue();
-    if (stop_return_value.IsValid() &&
-        (selected_thread.GetSelectedFrame().GetFrameID() == 0)) {
-      auto renamed_return_value = stop_return_value.Clone("(Return Value)");
-      int64_t return_var_ref = 0;
-      if (stop_return_value.MightHaveChildren() ||
-          stop_return_value.IsSynthetic()) {
-        return_var_ref = g_dap.variables.InsertExpandableVariable(
-            stop_return_value, /*is_permanent=*/false);
+    // Show return value if there is any ( in the local top frame )
+    if (variablesReference == VARREF_LOCALS) {
+      auto process = g_dap.target.GetProcess();
+      auto selected_thread = process.GetSelectedThread();
+      lldb::SBValue stop_return_value = selected_thread.GetStopReturnValue();
+      if (stop_return_value.IsValid() &&
+          (selected_thread.GetSelectedFrame().GetFrameID() == 0)) {
+        auto renamed_return_value = stop_return_value.Clone("(Return Value)");
+        int64_t return_var_ref = 0;
+        if (stop_return_value.MightHaveChildren() ||
+            stop_return_value.IsSynthetic()) {
+          return_var_ref = g_dap.variables.InsertExpandableVariable(
+              stop_return_value, /*is_permanent=*/false);
+        }
+        variables.emplace_back(CreateVariable(
+            renamed_return_value, return_var_ref, UINT64_MAX, hex, false));
       }
-      variables.emplace_back(CreateVariable(renamed_return_value, return_var_ref,
-                                            UINT64_MAX, hex, false));
     }
 
     // Now we construct the result with unique display variable names

>From 2047bcdded4933fa5de8b8d70d4ac692f7ae4709 Mon Sep 17 00:00:00 2001
From: Ezike Ebuka <yerimyah1 at gmail.com>
Date: Thu, 13 Feb 2025 20:49:20 +0000
Subject: [PATCH 7/7] [lldb-dap] Update to use the new function

---
 lldb/tools/lldb-dap/lldb-dap.cpp | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp
index 179c13ae7d1f5..9c2acf751865c 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -4227,20 +4227,22 @@ void request_variables(DAP &dap, const llvm::json::Object &request) {
 
     // Show return value if there is any ( in the local top frame )
     if (variablesReference == VARREF_LOCALS) {
-      auto process = g_dap.target.GetProcess();
+      auto process = dap.target.GetProcess();
       auto selected_thread = process.GetSelectedThread();
       lldb::SBValue stop_return_value = selected_thread.GetStopReturnValue();
+
       if (stop_return_value.IsValid() &&
           (selected_thread.GetSelectedFrame().GetFrameID() == 0)) {
         auto renamed_return_value = stop_return_value.Clone("(Return Value)");
         int64_t return_var_ref = 0;
+
         if (stop_return_value.MightHaveChildren() ||
             stop_return_value.IsSynthetic()) {
-          return_var_ref = g_dap.variables.InsertExpandableVariable(
-              stop_return_value, /*is_permanent=*/false);
+          return_var_ref = dap.variables.InsertVariable(stop_return_value, /*is_permanent=*/ false);
         }
         variables.emplace_back(CreateVariable(
-            renamed_return_value, return_var_ref, UINT64_MAX, hex, false));
+            renamed_return_value, return_var_ref, hex, dap.enable_auto_variable_summaries,
+            dap.enable_synthetic_child_debugging, false));
       }
     }
 



More information about the lldb-commits mailing list