Author: jeffreytan81 Date: 2024-04-25T11:49:10-07:00 New Revision: 2f2e31c3c980407b2660b4f5d10e7cdb3fa79138
URL: https://github.com/llvm/llvm-project/commit/2f2e31c3c980407b2660b4f5d10e7cdb3fa79138 DIFF: https://github.com/llvm/llvm-project/commit/2f2e31c3c980407b2660b4f5d10e7cdb3fa79138.diff LOG: Initial step in targets DAP support (#86623) This patch provides the initial implementation for the "Step Into Specific/Step In Targets" feature in VSCode DAP. The implementation disassembles all the call instructions in step range and try to resolve operand name (assuming one operand) using debug info. Later, the call target function name is chosen by end user and specified in the StepInto() API call. It is v1 because of using the existing step in target function name API. This implementation has several limitations: * Won't for indirect/virtual function call -- in most cases, our disassembler won't be able to solve the indirect call target address/name. * Won't work for target function without debug info -- if the target function has symbol but not debug info, the existing ThreadPlanStepInRange won't stop. * Relying on function names can be fragile -- if there is some middle glue/thunk code, our disassembler can only resolve the glue/thunk code's name not the real target function name. It can be fragile to depend compiler/linker emits the same names for both. * Does not support step into raw address call sites -- it is a valid scenario that in Visual Studio debugger, user can explicitly choose a raw address to step into which land in the function without debug info/symbol, then choose UI to load the debug info on-demand for that module/frame to continue exploring. A more reliable design could be extending the ThreadPlanStepInRange to support step in based on call-site instruction offset/PC which I will propose in next iteration. --------- Co-authored-by: jeffreytan81 <jeffrey...@fb.com> Added: lldb/test/API/tools/lldb-dap/stepInTargets/Makefile lldb/test/API/tools/lldb-dap/stepInTargets/TestDAP_stepInTargets.py lldb/test/API/tools/lldb-dap/stepInTargets/main.cpp Modified: lldb/include/lldb/API/SBLineEntry.h lldb/include/lldb/API/SBSymbolContextList.h lldb/include/lldb/API/SBTarget.h lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py lldb/source/API/SBLineEntry.cpp lldb/source/API/SBTarget.cpp lldb/tools/lldb-dap/DAP.h lldb/tools/lldb-dap/lldb-dap.cpp Removed: ################################################################################ diff --git a/lldb/include/lldb/API/SBLineEntry.h b/lldb/include/lldb/API/SBLineEntry.h index 7c2431ba3c8a51..d70c4fac6ec717 100644 --- a/lldb/include/lldb/API/SBLineEntry.h +++ b/lldb/include/lldb/API/SBLineEntry.h @@ -29,6 +29,9 @@ class LLDB_API SBLineEntry { lldb::SBAddress GetEndAddress() const; + lldb::SBAddress + GetSameLineContiguousAddressRangeEnd(bool include_inlined_functions) const; + explicit operator bool() const; bool IsValid() const; diff --git a/lldb/include/lldb/API/SBSymbolContextList.h b/lldb/include/lldb/API/SBSymbolContextList.h index 4026afc213571c..95100d219df20f 100644 --- a/lldb/include/lldb/API/SBSymbolContextList.h +++ b/lldb/include/lldb/API/SBSymbolContextList.h @@ -44,6 +44,7 @@ class LLDB_API SBSymbolContextList { protected: friend class SBModule; friend class SBTarget; + friend class SBCompileUnit; lldb_private::SymbolContextList *operator->() const; diff --git a/lldb/include/lldb/API/SBTarget.h b/lldb/include/lldb/API/SBTarget.h index 823615e6a36df5..feeaa1cb71132b 100644 --- a/lldb/include/lldb/API/SBTarget.h +++ b/lldb/include/lldb/API/SBTarget.h @@ -879,6 +879,10 @@ class LLDB_API SBTarget { uint32_t count, const char *flavor_string); + lldb::SBInstructionList ReadInstructions(lldb::SBAddress start_addr, + lldb::SBAddress end_addr, + const char *flavor_string); + lldb::SBInstructionList GetInstructions(lldb::SBAddress base_addr, const void *buf, size_t size); 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 27a76a652f4063..5838281bcb1a10 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 @@ -811,23 +811,34 @@ def request_next(self, threadId): command_dict = {"command": "next", "type": "request", "arguments": args_dict} return self.send_recv(command_dict) - def request_stepIn(self, threadId): + def request_stepIn(self, threadId, targetId): if self.exit_status is not None: - raise ValueError("request_continue called after process exited") - args_dict = {"threadId": threadId} + raise ValueError("request_stepIn called after process exited") + args_dict = {"threadId": threadId, "targetId": targetId} command_dict = {"command": "stepIn", "type": "request", "arguments": args_dict} return self.send_recv(command_dict) + def request_stepInTargets(self, frameId): + if self.exit_status is not None: + raise ValueError("request_stepInTargets called after process exited") + args_dict = {"frameId": frameId} + command_dict = { + "command": "stepInTargets", + "type": "request", + "arguments": args_dict, + } + return self.send_recv(command_dict) + def request_stepOut(self, threadId): if self.exit_status is not None: - raise ValueError("request_continue called after process exited") + raise ValueError("request_stepOut called after process exited") args_dict = {"threadId": threadId} command_dict = {"command": "stepOut", "type": "request", "arguments": args_dict} return self.send_recv(command_dict) def request_pause(self, threadId=None): if self.exit_status is not None: - raise ValueError("request_continue called after process exited") + raise ValueError("request_pause called after process exited") if threadId is None: threadId = self.get_thread_id() args_dict = {"threadId": threadId} 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 23f650d2d36fdd..d56ea5dca14beb 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 @@ -218,8 +218,8 @@ 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, waitForStop=True): - self.dap_server.request_stepIn(threadId=threadId) + def stepIn(self, threadId=None, targetId=None, waitForStop=True): + self.dap_server.request_stepIn(threadId=threadId, targetId=targetId) if waitForStop: return self.dap_server.wait_for_stopped() return None diff --git a/lldb/source/API/SBLineEntry.cpp b/lldb/source/API/SBLineEntry.cpp index 99a7b8fe644cb5..216ea6d18eab89 100644 --- a/lldb/source/API/SBLineEntry.cpp +++ b/lldb/source/API/SBLineEntry.cpp @@ -67,6 +67,21 @@ SBAddress SBLineEntry::GetEndAddress() const { return sb_address; } +SBAddress SBLineEntry::GetSameLineContiguousAddressRangeEnd( + bool include_inlined_functions) const { + LLDB_INSTRUMENT_VA(this); + + SBAddress sb_address; + if (m_opaque_up) { + AddressRange line_range = m_opaque_up->GetSameLineContiguousAddressRange( + include_inlined_functions); + + sb_address.SetAddress(line_range.GetBaseAddress()); + sb_address.OffsetAddress(line_range.GetByteSize()); + } + return sb_address; +} + bool SBLineEntry::IsValid() const { LLDB_INSTRUMENT_VA(this); return this->operator bool(); diff --git a/lldb/source/API/SBTarget.cpp b/lldb/source/API/SBTarget.cpp index 75f0444f629114..962ce9ba83cc77 100644 --- a/lldb/source/API/SBTarget.cpp +++ b/lldb/source/API/SBTarget.cpp @@ -2011,6 +2011,30 @@ lldb::SBInstructionList SBTarget::ReadInstructions(lldb::SBAddress base_addr, return sb_instructions; } +lldb::SBInstructionList SBTarget::ReadInstructions(lldb::SBAddress start_addr, + lldb::SBAddress end_addr, + const char *flavor_string) { + LLDB_INSTRUMENT_VA(this, start_addr, end_addr, flavor_string); + + SBInstructionList sb_instructions; + + TargetSP target_sp(GetSP()); + if (target_sp) { + lldb::addr_t start_load_addr = start_addr.GetLoadAddress(*this); + lldb::addr_t end_load_addr = end_addr.GetLoadAddress(*this); + if (end_load_addr > start_load_addr) { + lldb::addr_t size = end_load_addr - start_load_addr; + + AddressRange range(start_load_addr, size); + const bool force_live_memory = true; + sb_instructions.SetDisassembler(Disassembler::DisassembleRange( + target_sp->GetArchitecture(), nullptr, flavor_string, *target_sp, + range, force_live_memory)); + } + } + return sb_instructions; +} + lldb::SBInstructionList SBTarget::GetInstructions(lldb::SBAddress base_addr, const void *buf, size_t size) { diff --git a/lldb/test/API/tools/lldb-dap/stepInTargets/Makefile b/lldb/test/API/tools/lldb-dap/stepInTargets/Makefile new file mode 100644 index 00000000000000..f772575cd5613b --- /dev/null +++ b/lldb/test/API/tools/lldb-dap/stepInTargets/Makefile @@ -0,0 +1,6 @@ + +ENABLE_THREADS := YES + +CXX_SOURCES := main.cpp + +include Makefile.rules diff --git a/lldb/test/API/tools/lldb-dap/stepInTargets/TestDAP_stepInTargets.py b/lldb/test/API/tools/lldb-dap/stepInTargets/TestDAP_stepInTargets.py new file mode 100644 index 00000000000000..6296f6554d07e5 --- /dev/null +++ b/lldb/test/API/tools/lldb-dap/stepInTargets/TestDAP_stepInTargets.py @@ -0,0 +1,68 @@ +""" +Test lldb-dap stepInTargets request +""" + +import dap_server +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import * +import lldbdap_testcase +from lldbsuite.test import lldbutil + + +class TestDAP_stepInTargets(lldbdap_testcase.DAPTestCaseBase): + @skipIf( + archs=no_match(["x86_64"]) + ) # InstructionControlFlowKind for ARM is not supported yet. + def test_basic(self): + """ + Tests the basic stepping in targets with directly calls. + """ + program = self.getBuildArtifact("a.out") + self.build_and_launch(program) + source = "main.cpp" + + breakpoint_line = line_number(source, "// set breakpoint here") + lines = [breakpoint_line] + # Set breakpoint in the thread function so we can step the threads + 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) + + threads = self.dap_server.get_threads() + self.assertEqual(len(threads), 1, "expect one thread") + tid = threads[0]["id"] + + leaf_frame = self.dap_server.get_stackFrame() + self.assertIsNotNone(leaf_frame, "expect a leaf frame") + + # Request all step in targets list and verify the response. + step_in_targets_response = self.dap_server.request_stepInTargets( + leaf_frame["id"] + ) + self.assertEqual(step_in_targets_response["success"], True, "expect success") + self.assertIn( + "body", step_in_targets_response, "expect body field in response body" + ) + self.assertIn( + "targets", + step_in_targets_response["body"], + "expect targets field in response body", + ) + + step_in_targets = step_in_targets_response["body"]["targets"] + self.assertEqual(len(step_in_targets), 3, "expect 3 step in targets") + + # Verify the target names are correct. + self.assertEqual(step_in_targets[0]["label"], "bar()", "expect bar()") + self.assertEqual(step_in_targets[1]["label"], "bar2()", "expect bar2()") + self.assertEqual( + step_in_targets[2]["label"], "foo(int, int)", "expect foo(int, int)" + ) + + # Choose to step into second target and verify that we are in bar2() + self.stepIn(threadId=tid, targetId=step_in_targets[1]["id"], waitForStop=True) + leaf_frame = self.dap_server.get_stackFrame() + self.assertIsNotNone(leaf_frame, "expect a leaf frame") + self.assertEqual(leaf_frame["name"], "bar2()") diff --git a/lldb/test/API/tools/lldb-dap/stepInTargets/main.cpp b/lldb/test/API/tools/lldb-dap/stepInTargets/main.cpp new file mode 100644 index 00000000000000..d3c3dbcc139ef0 --- /dev/null +++ b/lldb/test/API/tools/lldb-dap/stepInTargets/main.cpp @@ -0,0 +1,11 @@ + +int foo(int val, int extra) { return val + extra; } + +int bar() { return 22; } + +int bar2() { return 54; } + +int main(int argc, char const *argv[]) { + foo(bar(), bar2()); // set breakpoint here + return 0; +} diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h index 8015dec9ba8fe6..5c70a056fea4bf 100644 --- a/lldb/tools/lldb-dap/DAP.h +++ b/lldb/tools/lldb-dap/DAP.h @@ -162,6 +162,8 @@ struct DAP { std::vector<std::string> exit_commands; std::vector<std::string> stop_commands; std::vector<std::string> terminate_commands; + // Map step in target id to list of function targets that user can choose. + llvm::DenseMap<lldb::addr_t, std::string> step_in_targets; // A copy of the last LaunchRequest or AttachRequest so we can reuse its // arguments if we get a RestartRequest. std::optional<llvm::json::Object> last_launch_or_attach_request; diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp index 16c50ed5791b0a..d0fbb9155715b1 100644 --- a/lldb/tools/lldb-dap/lldb-dap.cpp +++ b/lldb/tools/lldb-dap/lldb-dap.cpp @@ -1650,7 +1650,7 @@ void request_initialize(const llvm::json::Object &request) { // The debug adapter supports the gotoTargetsRequest. body.try_emplace("supportsGotoTargetsRequest", false); // The debug adapter supports the stepInTargetsRequest. - body.try_emplace("supportsStepInTargetsRequest", false); + body.try_emplace("supportsStepInTargetsRequest", true); // The debug adapter supports the completions request. body.try_emplace("supportsCompletionsRequest", true); // The debug adapter supports the disassembly request. @@ -3185,14 +3185,155 @@ void request_stepIn(const llvm::json::Object &request) { llvm::json::Object response; FillResponse(request, response); auto arguments = request.getObject("arguments"); + + std::string step_in_target; + uint64_t target_id = GetUnsigned(arguments, "targetId", 0); + auto it = g_dap.step_in_targets.find(target_id); + if (it != g_dap.step_in_targets.end()) + step_in_target = it->second; + + const bool single_thread = GetBoolean(arguments, "singleThread", false); + lldb::RunMode run_mode = + single_thread ? lldb::eOnlyThisThread : lldb::eOnlyDuringStepping; lldb::SBThread thread = g_dap.GetLLDBThread(*arguments); if (thread.IsValid()) { // 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(); + thread.StepInto(step_in_target.c_str(), run_mode); + } else { + response["success"] = llvm::json::Value(false); + } + g_dap.SendJSON(llvm::json::Value(std::move(response))); +} + +// "StepInTargetsRequest": { +// "allOf": [ { "$ref": "#/definitions/Request" }, { +// "type": "object", +// "description": "This request retrieves the possible step-in targets for +// the specified stack frame.\nThese targets can be used in the `stepIn` +// request.\nClients should only call this request if the corresponding +// capability `supportsStepInTargetsRequest` is true.", "properties": { +// "command": { +// "type": "string", +// "enum": [ "stepInTargets" ] +// }, +// "arguments": { +// "$ref": "#/definitions/StepInTargetsArguments" +// } +// }, +// "required": [ "command", "arguments" ] +// }] +// }, +// "StepInTargetsArguments": { +// "type": "object", +// "description": "Arguments for `stepInTargets` request.", +// "properties": { +// "frameId": { +// "type": "integer", +// "description": "The stack frame for which to retrieve the possible +// step-in targets." +// } +// }, +// "required": [ "frameId" ] +// }, +// "StepInTargetsResponse": { +// "allOf": [ { "$ref": "#/definitions/Response" }, { +// "type": "object", +// "description": "Response to `stepInTargets` request.", +// "properties": { +// "body": { +// "type": "object", +// "properties": { +// "targets": { +// "type": "array", +// "items": { +// "$ref": "#/definitions/StepInTarget" +// }, +// "description": "The possible step-in targets of the specified +// source location." +// } +// }, +// "required": [ "targets" ] +// } +// }, +// "required": [ "body" ] +// }] +// } +void request_stepInTargets(const llvm::json::Object &request) { + llvm::json::Object response; + FillResponse(request, response); + auto arguments = request.getObject("arguments"); + + g_dap.step_in_targets.clear(); + lldb::SBFrame frame = g_dap.GetLLDBFrame(*arguments); + if (frame.IsValid()) { + lldb::SBAddress pc_addr = frame.GetPCAddress(); + lldb::SBAddress line_end_addr = + pc_addr.GetLineEntry().GetSameLineContiguousAddressRangeEnd(true); + lldb::SBInstructionList insts = g_dap.target.ReadInstructions( + pc_addr, line_end_addr, /*flavor_string=*/nullptr); + + if (!insts.IsValid()) { + response["success"] = false; + response["message"] = "Failed to get instructions for frame."; + g_dap.SendJSON(llvm::json::Value(std::move(response))); + return; + } + + llvm::json::Array step_in_targets; + const auto num_insts = insts.GetSize(); + for (size_t i = 0; i < num_insts; ++i) { + lldb::SBInstruction inst = insts.GetInstructionAtIndex(i); + if (!inst.IsValid()) + break; + + lldb::addr_t inst_addr = inst.GetAddress().GetLoadAddress(g_dap.target); + + // Note: currently only x86/x64 supports flow kind. + lldb::InstructionControlFlowKind flow_kind = + inst.GetControlFlowKind(g_dap.target); + if (flow_kind == lldb::eInstructionControlFlowKindCall) { + // Use call site instruction address as id which is easy to debug. + llvm::json::Object step_in_target; + step_in_target["id"] = inst_addr; + + llvm::StringRef call_operand_name = inst.GetOperands(g_dap.target); + lldb::addr_t call_target_addr; + if (call_operand_name.getAsInteger(0, call_target_addr)) + continue; + + lldb::SBAddress call_target_load_addr = + g_dap.target.ResolveLoadAddress(call_target_addr); + if (!call_target_load_addr.IsValid()) + continue; + + // The existing ThreadPlanStepInRange only accept step in target + // function with debug info. + lldb::SBSymbolContext sc = g_dap.target.ResolveSymbolContextForAddress( + call_target_load_addr, lldb::eSymbolContextFunction); + + // The existing ThreadPlanStepInRange only accept step in target + // function with debug info. + std::string step_in_target_name; + if (sc.IsValid() && sc.GetFunction().IsValid()) + step_in_target_name = sc.GetFunction().GetDisplayName(); + + // Skip call sites if we fail to resolve its symbol name. + if (step_in_target_name.empty()) + continue; + + g_dap.step_in_targets.try_emplace(inst_addr, step_in_target_name); + step_in_target.try_emplace("label", step_in_target_name); + step_in_targets.emplace_back(std::move(step_in_target)); + } + } + llvm::json::Object body; + body.try_emplace("targets", std::move(step_in_targets)); + response.try_emplace("body", std::move(body)); } else { response["success"] = llvm::json::Value(false); + response["message"] = "Failed to get frame for input frameId."; } g_dap.SendJSON(llvm::json::Value(std::move(response))); } @@ -3909,6 +4050,7 @@ void RegisterRequestCallbacks() { g_dap.RegisterRequestCallback("source", request_source); g_dap.RegisterRequestCallback("stackTrace", request_stackTrace); g_dap.RegisterRequestCallback("stepIn", request_stepIn); + g_dap.RegisterRequestCallback("stepInTargets", request_stepInTargets); g_dap.RegisterRequestCallback("stepOut", request_stepOut); g_dap.RegisterRequestCallback("threads", request_threads); g_dap.RegisterRequestCallback("variables", request_variables); _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
