https://github.com/jeffreytan81 updated https://github.com/llvm/llvm-project/pull/86623
>From 6cccde22723157260e7c0b19bf8372aae8d1afaa Mon Sep 17 00:00:00 2001 From: jeffreytan81 <jeffrey...@fb.com> Date: Wed, 6 Mar 2024 12:07:03 -0800 Subject: [PATCH 1/6] Fix strcmp build error on buildbot --- lldb/test/API/functionalities/fork/concurrent_vfork/main.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/lldb/test/API/functionalities/fork/concurrent_vfork/main.cpp b/lldb/test/API/functionalities/fork/concurrent_vfork/main.cpp index b0a4446ba01581..40cb63755ee8a5 100644 --- a/lldb/test/API/functionalities/fork/concurrent_vfork/main.cpp +++ b/lldb/test/API/functionalities/fork/concurrent_vfork/main.cpp @@ -1,6 +1,7 @@ #include <assert.h> #include <iostream> #include <mutex> +#include <string.h> #include <sys/wait.h> #include <thread> #include <unistd.h> >From 06ec5e2a045e6ab4d97029fa0de5cc52dc4f0769 Mon Sep 17 00:00:00 2001 From: jeffreytan81 <jeffrey...@fb.com> Date: Mon, 25 Mar 2024 20:51:56 -0700 Subject: [PATCH 2/6] Initial support for stepInTargets feature --- lldb/include/lldb/API/SBCompileUnit.h | 5 + lldb/include/lldb/API/SBLineEntry.h | 3 + lldb/include/lldb/API/SBSymbolContextList.h | 1 + .../test/tools/lldb-dap/dap_server.py | 17 +- .../test/tools/lldb-dap/lldbdap_testcase.py | 4 +- lldb/source/API/SBCompileUnit.cpp | 17 ++ lldb/source/API/SBLineEntry.cpp | 15 ++ .../API/tools/lldb-dap/stepInTargets/Makefile | 6 + .../stepInTargets/TestDAP_stepInTargets.py | 66 ++++++++ .../API/tools/lldb-dap/stepInTargets/main.cpp | 17 ++ lldb/tools/lldb-dap/DAP.h | 2 + lldb/tools/lldb-dap/lldb-dap.cpp | 149 +++++++++++++++++- 12 files changed, 293 insertions(+), 9 deletions(-) create mode 100644 lldb/test/API/tools/lldb-dap/stepInTargets/Makefile create mode 100644 lldb/test/API/tools/lldb-dap/stepInTargets/TestDAP_stepInTargets.py create mode 100644 lldb/test/API/tools/lldb-dap/stepInTargets/main.cpp diff --git a/lldb/include/lldb/API/SBCompileUnit.h b/lldb/include/lldb/API/SBCompileUnit.h index 36492d9398ce9e..4fc9ad43386f45 100644 --- a/lldb/include/lldb/API/SBCompileUnit.h +++ b/lldb/include/lldb/API/SBCompileUnit.h @@ -11,6 +11,7 @@ #include "lldb/API/SBDefines.h" #include "lldb/API/SBFileSpec.h" +#include <optional> namespace lldb { @@ -30,6 +31,10 @@ class LLDB_API SBCompileUnit { lldb::SBFileSpec GetFileSpec() const; + lldb::SBSymbolContextList + ResolveSymbolContext(const char *file, uint32_t line, + std::optional<uint16_t> column = std::nullopt) const; + uint32_t GetNumLineEntries() const; lldb::SBLineEntry GetLineEntryAtIndex(uint32_t idx) const; 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/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py index 27a76a652f4063..f2ee6bd8212415 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,30 @@ 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_stepInTargets(self, frameId): if self.exit_status is not None: - raise ValueError("request_continue called after process exited") - args_dict = {"threadId": threadId} + 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_stepIn(self, threadId, targetId): + if self.exit_status is not None: + 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_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/SBCompileUnit.cpp b/lldb/source/API/SBCompileUnit.cpp index 65fdb11032b9c0..8d99bd5cbb0d5d 100644 --- a/lldb/source/API/SBCompileUnit.cpp +++ b/lldb/source/API/SBCompileUnit.cpp @@ -49,6 +49,23 @@ SBFileSpec SBCompileUnit::GetFileSpec() const { return file_spec; } +lldb::SBSymbolContextList +SBCompileUnit::ResolveSymbolContext(const char *file, uint32_t line, + std::optional<uint16_t> column) const { + LLDB_INSTRUMENT_VA(this, file, line); + + lldb::SBSymbolContextList sc_list; + FileSpec file_spec(file); + SourceLocationSpec location_spec(file_spec, line, /*column=*/column, + /*check_inlines=*/true, + /*exact_match=*/true); + + if (m_opaque_ptr) + m_opaque_ptr->ResolveSymbolContext(location_spec, eSymbolContextLineEntry, + *sc_list); + return sc_list; +} + uint32_t SBCompileUnit::GetNumLineEntries() const { LLDB_INSTRUMENT_VA(this); 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/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..3fa955305037cf --- /dev/null +++ b/lldb/test/API/tools/lldb-dap/stepInTargets/TestDAP_stepInTargets.py @@ -0,0 +1,66 @@ +""" +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"])) # ARM flow kind 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..cd4419e8a4b047 --- /dev/null +++ b/lldb/test/API/tools/lldb-dap/stepInTargets/main.cpp @@ -0,0 +1,17 @@ + +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 55f8c920e60016..1331bf2d03d20f 100644 --- a/lldb/tools/lldb-dap/lldb-dap.cpp +++ b/lldb/tools/lldb-dap/lldb-dap.cpp @@ -1645,7 +1645,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. @@ -3180,14 +3180,158 @@ 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::addr_t line_end_addr = pc_addr.GetLineEntry() + .GetSameLineContiguousAddressRangeEnd(true) + .GetLoadAddress(g_dap.target); + + int max_inst_count = line_end_addr - pc_addr.GetLoadAddress(g_dap.target); + lldb::SBInstructionList insts = + g_dap.target.ReadInstructions(pc_addr, max_inst_count); + + 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); + // line_end_addr is exclusive of the line range. + if (inst_addr >= line_end_addr) + break; + + // 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; + + lldb::SBSymbolContext sc = g_dap.target.ResolveSymbolContextForAddress( + call_target_load_addr, lldb::eSymbolContextFunction); + + // Step in targets only supports functions 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))); } @@ -3904,6 +4048,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); >From b2dfdb546808c495779e5781c6619fcadb752b00 Mon Sep 17 00:00:00 2001 From: jeffreytan81 <jeffrey...@fb.com> Date: Mon, 25 Mar 2024 20:57:55 -0700 Subject: [PATCH 3/6] Remove unnecessary changes --- lldb/include/lldb/API/SBCompileUnit.h | 5 ----- lldb/source/API/SBCompileUnit.cpp | 17 ----------------- lldb/tools/lldb-dap/lldb-dap.cpp | 3 ++- 3 files changed, 2 insertions(+), 23 deletions(-) diff --git a/lldb/include/lldb/API/SBCompileUnit.h b/lldb/include/lldb/API/SBCompileUnit.h index 4fc9ad43386f45..36492d9398ce9e 100644 --- a/lldb/include/lldb/API/SBCompileUnit.h +++ b/lldb/include/lldb/API/SBCompileUnit.h @@ -11,7 +11,6 @@ #include "lldb/API/SBDefines.h" #include "lldb/API/SBFileSpec.h" -#include <optional> namespace lldb { @@ -31,10 +30,6 @@ class LLDB_API SBCompileUnit { lldb::SBFileSpec GetFileSpec() const; - lldb::SBSymbolContextList - ResolveSymbolContext(const char *file, uint32_t line, - std::optional<uint16_t> column = std::nullopt) const; - uint32_t GetNumLineEntries() const; lldb::SBLineEntry GetLineEntryAtIndex(uint32_t idx) const; diff --git a/lldb/source/API/SBCompileUnit.cpp b/lldb/source/API/SBCompileUnit.cpp index 8d99bd5cbb0d5d..65fdb11032b9c0 100644 --- a/lldb/source/API/SBCompileUnit.cpp +++ b/lldb/source/API/SBCompileUnit.cpp @@ -49,23 +49,6 @@ SBFileSpec SBCompileUnit::GetFileSpec() const { return file_spec; } -lldb::SBSymbolContextList -SBCompileUnit::ResolveSymbolContext(const char *file, uint32_t line, - std::optional<uint16_t> column) const { - LLDB_INSTRUMENT_VA(this, file, line); - - lldb::SBSymbolContextList sc_list; - FileSpec file_spec(file); - SourceLocationSpec location_spec(file_spec, line, /*column=*/column, - /*check_inlines=*/true, - /*exact_match=*/true); - - if (m_opaque_ptr) - m_opaque_ptr->ResolveSymbolContext(location_spec, eSymbolContextLineEntry, - *sc_list); - return sc_list; -} - uint32_t SBCompileUnit::GetNumLineEntries() const { LLDB_INSTRUMENT_VA(this); diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp index 1331bf2d03d20f..e27c078d14979d 100644 --- a/lldb/tools/lldb-dap/lldb-dap.cpp +++ b/lldb/tools/lldb-dap/lldb-dap.cpp @@ -3265,7 +3265,8 @@ void request_stepInTargets(const llvm::json::Object &request) { if (frame.IsValid()) { lldb::SBAddress pc_addr = frame.GetPCAddress(); lldb::addr_t line_end_addr = pc_addr.GetLineEntry() - .GetSameLineContiguousAddressRangeEnd(true) + .GetSameLineContiguousAddressRangeEnd( + /*include_inlined_functions=*/true) .GetLoadAddress(g_dap.target); int max_inst_count = line_end_addr - pc_addr.GetLoadAddress(g_dap.target); >From f7f8a69122506469687c42a5b9bfe020c150e34d Mon Sep 17 00:00:00 2001 From: jeffreytan81 <jeffrey...@fb.com> Date: Tue, 26 Mar 2024 09:20:27 -0700 Subject: [PATCH 4/6] Fix formatter --- .../lldbsuite/test/tools/lldb-dap/dap_server.py | 6 +++++- .../test/API/tools/lldb-dap/stepInTargets/main.cpp | 14 ++++---------- lldb/tools/lldb-dap/lldb-dap.cpp | 2 +- 3 files changed, 10 insertions(+), 12 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 f2ee6bd8212415..45e48a10c7611d 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 @@ -815,7 +815,11 @@ 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} + command_dict = { + "command": "stepInTargets", + "type": "request", + "arguments": args_dict, + } return self.send_recv(command_dict) def request_stepIn(self, threadId, targetId): diff --git a/lldb/test/API/tools/lldb-dap/stepInTargets/main.cpp b/lldb/test/API/tools/lldb-dap/stepInTargets/main.cpp index cd4419e8a4b047..d3c3dbcc139ef0 100644 --- a/lldb/test/API/tools/lldb-dap/stepInTargets/main.cpp +++ b/lldb/test/API/tools/lldb-dap/stepInTargets/main.cpp @@ -1,17 +1,11 @@ -int foo(int val, int extra) { - return val + extra; -} +int foo(int val, int extra) { return val + extra; } -int bar() { - return 22; -} +int bar() { return 22; } -int bar2() { - return 54; -} +int bar2() { return 54; } -int main(int argc, char const *argv[]) { +int main(int argc, char const *argv[]) { foo(bar(), bar2()); // set breakpoint here return 0; } diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp index e27c078d14979d..30cba6d23f542d 100644 --- a/lldb/tools/lldb-dap/lldb-dap.cpp +++ b/lldb/tools/lldb-dap/lldb-dap.cpp @@ -3186,7 +3186,7 @@ void request_stepIn(const llvm::json::Object &request) { 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; >From 55a9f706251eea2be925980ead93cb1b170000a2 Mon Sep 17 00:00:00 2001 From: jeffreytan81 <jeffrey...@fb.com> Date: Tue, 26 Mar 2024 09:29:14 -0700 Subject: [PATCH 5/6] Remove empty space --- .../packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 45e48a10c7611d..78f7c22dd600ca 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 @@ -821,7 +821,7 @@ def request_stepInTargets(self, frameId): "arguments": args_dict, } return self.send_recv(command_dict) - + def request_stepIn(self, threadId, targetId): if self.exit_status is not None: raise ValueError("request_stepIn called after process exited") >From d51bf466ea9d9b24782a36ee89fb053fb1b874a1 Mon Sep 17 00:00:00 2001 From: jeffreytan81 <jeffrey...@fb.com> Date: Tue, 26 Mar 2024 15:44:52 -0700 Subject: [PATCH 6/6] Address review feedback --- lldb/include/lldb/API/SBTarget.h | 4 ++++ lldb/source/API/SBTarget.cpp | 24 +++++++++++++++++++ .../stepInTargets/TestDAP_stepInTargets.py | 4 +++- lldb/tools/lldb-dap/lldb-dap.cpp | 16 ++++--------- 4 files changed, 36 insertions(+), 12 deletions(-) diff --git a/lldb/include/lldb/API/SBTarget.h b/lldb/include/lldb/API/SBTarget.h index 3644ac056da3dc..f707ebc658f7b0 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/source/API/SBTarget.cpp b/lldb/source/API/SBTarget.cpp index cc9f1fdd76afaa..b49ed19aeba990 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/TestDAP_stepInTargets.py b/lldb/test/API/tools/lldb-dap/stepInTargets/TestDAP_stepInTargets.py index 3fa955305037cf..6296f6554d07e5 100644 --- a/lldb/test/API/tools/lldb-dap/stepInTargets/TestDAP_stepInTargets.py +++ b/lldb/test/API/tools/lldb-dap/stepInTargets/TestDAP_stepInTargets.py @@ -10,7 +10,9 @@ class TestDAP_stepInTargets(lldbdap_testcase.DAPTestCaseBase): - @skipIf(archs=no_match(["x86_64"])) # ARM flow kind is not supported yet. + @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. diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp index 30cba6d23f542d..8fb009b5ecf3ec 100644 --- a/lldb/tools/lldb-dap/lldb-dap.cpp +++ b/lldb/tools/lldb-dap/lldb-dap.cpp @@ -3264,14 +3264,10 @@ void request_stepInTargets(const llvm::json::Object &request) { lldb::SBFrame frame = g_dap.GetLLDBFrame(*arguments); if (frame.IsValid()) { lldb::SBAddress pc_addr = frame.GetPCAddress(); - lldb::addr_t line_end_addr = pc_addr.GetLineEntry() - .GetSameLineContiguousAddressRangeEnd( - /*include_inlined_functions=*/true) - .GetLoadAddress(g_dap.target); - - int max_inst_count = line_end_addr - pc_addr.GetLoadAddress(g_dap.target); - lldb::SBInstructionList insts = - g_dap.target.ReadInstructions(pc_addr, max_inst_count); + 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; @@ -3288,9 +3284,6 @@ void request_stepInTargets(const llvm::json::Object &request) { break; lldb::addr_t inst_addr = inst.GetAddress().GetLoadAddress(g_dap.target); - // line_end_addr is exclusive of the line range. - if (inst_addr >= line_end_addr) - break; // Note: currently only x86/x64 supports flow kind. lldb::InstructionControlFlowKind flow_kind = @@ -3310,6 +3303,7 @@ void request_stepInTargets(const llvm::json::Object &request) { if (!call_target_load_addr.IsValid()) continue; + // Step in targets only supports functions with debug info. lldb::SBSymbolContext sc = g_dap.target.ResolveSymbolContextForAddress( call_target_load_addr, lldb::eSymbolContextFunction); _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits