https://github.com/vogelsgesang updated https://github.com/llvm/llvm-project/pull/105464
>From c4178df5541103388e26343f62e96f8e2a65be86 Mon Sep 17 00:00:00 2001 From: Adrian Vogelsgesang <avogelsges...@salesforce.com> Date: Mon, 12 Aug 2024 23:00:45 +0000 Subject: [PATCH 1/3] [lldb-dap] Implement `StepGranularity` for "next" and "step-in" VS Code requests a `granularity` of `instruction` if the assembly view is currently focused. By implementing `StepGranularity`, we can hence properly through assembly code single-step through assembly code. --- .../test/tools/lldb-dap/dap_server.py | 8 ++--- .../test/tools/lldb-dap/lldbdap_testcase.py | 8 ++--- .../API/tools/lldb-dap/step/TestDAP_step.py | 9 ++++++ lldb/tools/lldb-dap/lldb-dap.cpp | 31 +++++++++++++++++-- 4 files changed, 45 insertions(+), 11 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 a324af57b61df3..87ebc674f61df6 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 @@ -816,17 +816,17 @@ def request_launch( self.wait_for_event(filter=["process", "initialized"]) return response - def request_next(self, threadId): + def request_next(self, threadId, granularity="statement"): if self.exit_status is not None: raise ValueError("request_continue called after process exited") - args_dict = {"threadId": threadId} + args_dict = {"threadId": threadId, "granularity": granularity} command_dict = {"command": "next", "type": "request", "arguments": args_dict} return self.send_recv(command_dict) - def request_stepIn(self, threadId, targetId): + def request_stepIn(self, threadId, targetId, granularity="statement"): if self.exit_status is not None: raise ValueError("request_stepIn called after process exited") - args_dict = {"threadId": threadId, "targetId": targetId} + args_dict = {"threadId": threadId, "targetId": targetId, "granularity": granularity} command_dict = {"command": "stepIn", "type": "request", "arguments": args_dict} return self.send_recv(command_dict) diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py index a312a88ebd7e58..3257bd14b16fed 100644 --- a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py +++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py @@ -222,14 +222,14 @@ def set_global(self, name, value, id=None): """Set a top level global variable only.""" return self.dap_server.request_setVariable(2, name, str(value), id=id) - def stepIn(self, threadId=None, targetId=None, waitForStop=True): - self.dap_server.request_stepIn(threadId=threadId, targetId=targetId) + def stepIn(self, threadId=None, targetId=None, waitForStop=True, granularity="statement"): + self.dap_server.request_stepIn(threadId=threadId, targetId=targetId, granularity=granularity) if waitForStop: return self.dap_server.wait_for_stopped() return None - def stepOver(self, threadId=None, waitForStop=True): - self.dap_server.request_next(threadId=threadId) + def stepOver(self, threadId=None, waitForStop=True, granularity="statement"): + self.dap_server.request_next(threadId=threadId, granularity=granularity) if waitForStop: return self.dap_server.wait_for_stopped() return None diff --git a/lldb/test/API/tools/lldb-dap/step/TestDAP_step.py b/lldb/test/API/tools/lldb-dap/step/TestDAP_step.py index 8a1bb76340be73..9c8e226827611e 100644 --- a/lldb/test/API/tools/lldb-dap/step/TestDAP_step.py +++ b/lldb/test/API/tools/lldb-dap/step/TestDAP_step.py @@ -68,5 +68,14 @@ def test_step(self): self.assertEqual(x4, x3, "verify step over variable") self.assertGreater(line4, line3, "verify step over line") self.assertEqual(src1, src4, "verify step over source") + + # Step a single assembly instruction. + # Unfortunately, there is no portable way to verify the correct + # stepping behavior here, because the generated assembly code + # depends highly on the compiler, its version, the operating + # system, and many more factors. + self.stepOver(threadId=tid, waitForStop=True, granularity="instruction") + self.stepIn(threadId=tid, waitForStop=True, granularity="instruction") + # only step one thread that is at the breakpoint and stop break diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp index f50a6c17310739..a7b8716977ee0a 100644 --- a/lldb/tools/lldb-dap/lldb-dap.cpp +++ b/lldb/tools/lldb-dap/lldb-dap.cpp @@ -1677,6 +1677,9 @@ void request_initialize(const llvm::json::Object &request) { body.try_emplace("supportsCompletionsRequest", true); // The debug adapter supports the disassembly request. body.try_emplace("supportsDisassembleRequest", true); + // The debug adapter supports stepping granularities (argument `granularity`) + // for the stepping requests. + body.try_emplace("supportsSteppingGranularity", true); llvm::json::Array completion_characters; completion_characters.emplace_back("."); @@ -1985,6 +1988,13 @@ void request_launch(const llvm::json::Object &request) { g_dap.SendJSON(CreateEventObject("initialized")); } +// Check if the step-granularity is `instruction` +static bool hasInstructionGranularity(const llvm::json::Object &requestArgs) { + if (std::optional<llvm::StringRef> value = requestArgs.getString("granularity")) + return value == "instruction"; + return false; +} + // "NextRequest": { // "allOf": [ { "$ref": "#/definitions/Request" }, { // "type": "object", @@ -2012,6 +2022,10 @@ void request_launch(const llvm::json::Object &request) { // "threadId": { // "type": "integer", // "description": "Execute 'next' for this thread." +// }, +// "granularity": { +// "$ref": "#/definitions/SteppingGranularity", +// "description": "Stepping granularity. If no granularity is specified, a granularity of `statement` is assumed." // } // }, // "required": [ "threadId" ] @@ -2032,7 +2046,11 @@ void request_next(const llvm::json::Object &request) { // Remember the thread ID that caused the resume so we can set the // "threadCausedFocus" boolean value in the "stopped" events. g_dap.focus_tid = thread.GetThreadID(); - thread.StepOver(); + if (hasInstructionGranularity(*arguments)) { + thread.StepInstruction(/*step_over=*/ true); + } else { + thread.StepOver(); + } } else { response["success"] = llvm::json::Value(false); } @@ -3193,7 +3211,10 @@ void request_stackTrace(const llvm::json::Object &request) { // "targetId": { // "type": "integer", // "description": "Optional id of the target to step into." -// } +// }, +// "granularity": { +// "$ref": "#/definitions/SteppingGranularity", +// "description": "Stepping granularity. If no granularity is specified, a granularity of `statement` is assumed." // }, // "required": [ "threadId" ] // }, @@ -3223,7 +3244,11 @@ void request_stepIn(const llvm::json::Object &request) { // Remember the thread ID that caused the resume so we can set the // "threadCausedFocus" boolean value in the "stopped" events. g_dap.focus_tid = thread.GetThreadID(); - thread.StepInto(step_in_target.c_str(), run_mode); + if (hasInstructionGranularity(*arguments)) { + thread.StepInstruction(/*step_over=*/ false); + } else { + thread.StepInto(step_in_target.c_str(), run_mode); + } } else { response["success"] = llvm::json::Value(false); } >From c6ae03665952b0baf287a83df327821cfa19f48f Mon Sep 17 00:00:00 2001 From: Adrian Vogelsgesang <avogelsges...@salesforce.com> Date: Wed, 21 Aug 2024 02:31:02 +0000 Subject: [PATCH 2/3] Fix code formatting --- .../lldbsuite/test/tools/lldb-dap/dap_server.py | 6 +++++- .../test/tools/lldb-dap/lldbdap_testcase.py | 8 ++++++-- lldb/test/API/tools/lldb-dap/step/TestDAP_step.py | 8 ++++++-- lldb/tools/lldb-dap/lldb-dap.cpp | 13 ++++++++----- 4 files changed, 25 insertions(+), 10 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 87ebc674f61df6..874383a13e2bb6 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 @@ -826,7 +826,11 @@ def request_next(self, threadId, granularity="statement"): def request_stepIn(self, threadId, targetId, granularity="statement"): if self.exit_status is not None: raise ValueError("request_stepIn called after process exited") - args_dict = {"threadId": threadId, "targetId": targetId, "granularity": granularity} + args_dict = { + "threadId": threadId, + "targetId": targetId, + "granularity": granularity, + } command_dict = {"command": "stepIn", "type": "request", "arguments": args_dict} return self.send_recv(command_dict) diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py index 3257bd14b16fed..27545816f20707 100644 --- a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py +++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py @@ -222,8 +222,12 @@ def set_global(self, name, value, id=None): """Set a top level global variable only.""" return self.dap_server.request_setVariable(2, name, str(value), id=id) - def stepIn(self, threadId=None, targetId=None, waitForStop=True, granularity="statement"): - self.dap_server.request_stepIn(threadId=threadId, targetId=targetId, granularity=granularity) + def stepIn( + self, threadId=None, targetId=None, waitForStop=True, granularity="statement" + ): + self.dap_server.request_stepIn( + threadId=threadId, targetId=targetId, granularity=granularity + ) if waitForStop: return self.dap_server.wait_for_stopped() return None diff --git a/lldb/test/API/tools/lldb-dap/step/TestDAP_step.py b/lldb/test/API/tools/lldb-dap/step/TestDAP_step.py index 9c8e226827611e..42a39e3c8c080b 100644 --- a/lldb/test/API/tools/lldb-dap/step/TestDAP_step.py +++ b/lldb/test/API/tools/lldb-dap/step/TestDAP_step.py @@ -74,8 +74,12 @@ def test_step(self): # stepping behavior here, because the generated assembly code # depends highly on the compiler, its version, the operating # system, and many more factors. - self.stepOver(threadId=tid, waitForStop=True, granularity="instruction") - self.stepIn(threadId=tid, waitForStop=True, granularity="instruction") + self.stepOver( + threadId=tid, waitForStop=True, granularity="instruction" + ) + self.stepIn( + threadId=tid, waitForStop=True, granularity="instruction" + ) # only step one thread that is at the breakpoint and stop break diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp index a7b8716977ee0a..469b4df544b316 100644 --- a/lldb/tools/lldb-dap/lldb-dap.cpp +++ b/lldb/tools/lldb-dap/lldb-dap.cpp @@ -1990,7 +1990,8 @@ void request_launch(const llvm::json::Object &request) { // Check if the step-granularity is `instruction` static bool hasInstructionGranularity(const llvm::json::Object &requestArgs) { - if (std::optional<llvm::StringRef> value = requestArgs.getString("granularity")) + if (std::optional<llvm::StringRef> value = + requestArgs.getString("granularity")) return value == "instruction"; return false; } @@ -2025,7 +2026,8 @@ static bool hasInstructionGranularity(const llvm::json::Object &requestArgs) { // }, // "granularity": { // "$ref": "#/definitions/SteppingGranularity", -// "description": "Stepping granularity. If no granularity is specified, a granularity of `statement` is assumed." +// "description": "Stepping granularity. If no granularity is specified, a +// granularity of `statement` is assumed." // } // }, // "required": [ "threadId" ] @@ -2047,7 +2049,7 @@ void request_next(const llvm::json::Object &request) { // "threadCausedFocus" boolean value in the "stopped" events. g_dap.focus_tid = thread.GetThreadID(); if (hasInstructionGranularity(*arguments)) { - thread.StepInstruction(/*step_over=*/ true); + thread.StepInstruction(/*step_over=*/true); } else { thread.StepOver(); } @@ -3214,7 +3216,8 @@ void request_stackTrace(const llvm::json::Object &request) { // }, // "granularity": { // "$ref": "#/definitions/SteppingGranularity", -// "description": "Stepping granularity. If no granularity is specified, a granularity of `statement` is assumed." +// "description": "Stepping granularity. If no granularity is specified, a +// granularity of `statement` is assumed." // }, // "required": [ "threadId" ] // }, @@ -3245,7 +3248,7 @@ void request_stepIn(const llvm::json::Object &request) { // "threadCausedFocus" boolean value in the "stopped" events. g_dap.focus_tid = thread.GetThreadID(); if (hasInstructionGranularity(*arguments)) { - thread.StepInstruction(/*step_over=*/ false); + thread.StepInstruction(/*step_over=*/false); } else { thread.StepInto(step_in_target.c_str(), run_mode); } >From 8c0634d8fe2317da7455edd9f5263c1257c2c12f Mon Sep 17 00:00:00 2001 From: Adrian Vogelsgesang <avogelsges...@salesforce.com> Date: Wed, 21 Aug 2024 17:57:04 +0000 Subject: [PATCH 3/3] Fix comment --- lldb/tools/lldb-dap/lldb-dap.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp index 469b4df544b316..b534a48660a5f8 100644 --- a/lldb/tools/lldb-dap/lldb-dap.cpp +++ b/lldb/tools/lldb-dap/lldb-dap.cpp @@ -3218,6 +3218,7 @@ void request_stackTrace(const llvm::json::Object &request) { // "$ref": "#/definitions/SteppingGranularity", // "description": "Stepping granularity. If no granularity is specified, a // granularity of `statement` is assumed." +// } // }, // "required": [ "threadId" ] // }, _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits