Author: Da-Viper Date: 2025-03-01T18:21:21-06:00 New Revision: 8ec0d60e28f77149eef9c865515b79bc0a5e8f41
URL: https://github.com/llvm/llvm-project/commit/8ec0d60e28f77149eef9c865515b79bc0a5e8f41 DIFF: https://github.com/llvm/llvm-project/commit/8ec0d60e28f77149eef9c865515b79bc0a5e8f41.diff LOG: [lldb-dap] Add: show return value on step out (#106907) https://github.com/user-attachments/assets/cff48c6f-37ae-4f72-b881-3eff4178fb3c Added: Modified: lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py lldb/test/API/tools/lldb-dap/variables/children/TestDAP_variables_children.py lldb/test/API/tools/lldb-dap/variables/children/main.cpp lldb/test/API/tools/lldb-dap/variables/main.cpp lldb/tools/lldb-dap/Handler/VariablesRequestHandler.cpp llvm/docs/ReleaseNotes.md Removed: ################################################################################ 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 fde66a28382c7..ee5b49de14d98 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,54 @@ 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) + + return_name = "(Return Value)" + verify_locals = { + return_name: {"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": + thread_id = thread["id"] + + self.stepOut(threadId=thread_id) + + local_variables = self.dap_server.get_local_variables() + varref_dict = {} + + # `verify_variable` function only checks if the local variables + # are in the `verify_dict` passed this will cause this test to pass + # even if there is no return value. + local_variable_names = [ + variable["name"] for variable in local_variables + ] + self.assertIn( + return_name, + local_variable_names, + "return variable is not in local variables", + ) + + 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..f0e1e51365d9b 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,59 @@ def test_get_num_children(self): "`script formatter.num_children_calls", context="repl" )["body"]["result"], ) + + @skipIf(archs=["arm64"]) + 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 diff --git a/lldb/tools/lldb-dap/Handler/VariablesRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/VariablesRequestHandler.cpp index 3a62682566236..d096c4220914a 100644 --- a/lldb/tools/lldb-dap/Handler/VariablesRequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/VariablesRequestHandler.cpp @@ -164,6 +164,29 @@ void VariablesRequestHandler::operator()( variable_name_counts[GetNonNullVariableName(variable)]++; } + // Show return value if there is any ( in the local top frame ) + if (variablesReference == VARREF_LOCALS) { + 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 = dap.variables.InsertVariable(stop_return_value, + /*is_permanent=*/false); + } + variables.emplace_back( + CreateVariable(renamed_return_value, return_var_ref, hex, + dap.enable_auto_variable_summaries, + dap.enable_synthetic_child_debugging, 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); diff --git a/llvm/docs/ReleaseNotes.md b/llvm/docs/ReleaseNotes.md index 12dd09ad41135..f1f64f77ee71a 100644 --- a/llvm/docs/ReleaseNotes.md +++ b/llvm/docs/ReleaseNotes.md @@ -168,6 +168,7 @@ Changes to LLDB ### Changes to lldb-dap * Breakpoints can now be set for specific columns within a line. +* Function return value is now displayed on step-out. Changes to BOLT --------------------------------- _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits