llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: John Harrison (ashgti) <details> <summary>Changes</summary> This converts a number of json::Value's into well defined types that are used throughout lldb-dap and updates the 'launch' command to use the new well defined types. --- Patch is 68.84 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/133624.diff 22 Files Affected: - (modified) lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py (+2-1) - (modified) lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py (+1-1) - (modified) lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py (+4-2) - (modified) lldb/test/API/tools/lldb-dap/restart/TestDAP_restart_runInTerminal.py (+2-2) - (modified) lldb/test/API/tools/lldb-dap/runInTerminal/TestDAP_runInTerminal.py (+8-8) - (modified) lldb/tools/lldb-dap/DAP.cpp (+54-28) - (modified) lldb/tools/lldb-dap/DAP.h (+24-20) - (modified) lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp (+19-14) - (modified) lldb/tools/lldb-dap/Handler/CompletionsHandler.cpp (+4-3) - (modified) lldb/tools/lldb-dap/Handler/EvaluateRequestHandler.cpp (+2-1) - (modified) lldb/tools/lldb-dap/Handler/LaunchRequestHandler.cpp (+29-89) - (modified) lldb/tools/lldb-dap/Handler/RequestHandler.cpp (+51-45) - (modified) lldb/tools/lldb-dap/Handler/RequestHandler.h (+15-4) - (modified) lldb/tools/lldb-dap/Handler/RestartRequestHandler.cpp (+40-14) - (modified) lldb/tools/lldb-dap/Handler/SetVariableRequestHandler.cpp (+2-1) - (modified) lldb/tools/lldb-dap/Handler/StackTraceRequestHandler.cpp (+1-1) - (modified) lldb/tools/lldb-dap/Handler/VariablesRequestHandler.cpp (+10-10) - (modified) lldb/tools/lldb-dap/JSONUtils.cpp (+20-28) - (modified) lldb/tools/lldb-dap/JSONUtils.h (+6-5) - (modified) lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp (+169-6) - (modified) lldb/tools/lldb-dap/Protocol/ProtocolRequests.h (+150) - (modified) lldb/tools/lldb-dap/SourceBreakpoint.cpp (+3-3) ``````````diff 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 01ef4b68f2653..6e13fcddcc933 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 @@ -862,7 +862,8 @@ def request_launch( args_dict["enableAutoVariableSummaries"] = enableAutoVariableSummaries args_dict["enableSyntheticChildDebugging"] = enableSyntheticChildDebugging args_dict["displayExtendedBacktrace"] = displayExtendedBacktrace - args_dict["commandEscapePrefix"] = commandEscapePrefix + if commandEscapePrefix: + args_dict["commandEscapePrefix"] = commandEscapePrefix command_dict = {"command": "launch", "type": "request", "arguments": args_dict} response = 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 70b04b051e0ec..9ab8a905a79dd 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 @@ -443,7 +443,7 @@ def cleanup(): if not (response and response["success"]): self.assertTrue( - response["success"], "launch failed (%s)" % (response["message"]) + response["success"], "launch failed (%s)" % (response["body"]["error"]["format"]) ) return response diff --git a/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py b/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py index 64c99019a1c9b..c6a3e9cc879a4 100644 --- a/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py +++ b/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py @@ -41,7 +41,9 @@ def test_termination(self): self.dap_server.request_disconnect() # Wait until the underlying lldb-dap process dies. - self.dap_server.process.wait(timeout=lldbdap_testcase.DAPTestCaseBase.timeoutval) + self.dap_server.process.wait( + timeout=lldbdap_testcase.DAPTestCaseBase.timeoutval + ) # Check the return code self.assertEqual(self.dap_server.process.poll(), 0) @@ -459,7 +461,7 @@ def test_failing_launch_commands(self): self.assertFalse(response["success"]) self.assertRegex( - response["message"], + response["body"]["error"]["format"], r"Failed to run launch commands\. See the Debug Console for more details", ) diff --git a/lldb/test/API/tools/lldb-dap/restart/TestDAP_restart_runInTerminal.py b/lldb/test/API/tools/lldb-dap/restart/TestDAP_restart_runInTerminal.py index 5a9938c25c2c8..a94c9860c1508 100644 --- a/lldb/test/API/tools/lldb-dap/restart/TestDAP_restart_runInTerminal.py +++ b/lldb/test/API/tools/lldb-dap/restart/TestDAP_restart_runInTerminal.py @@ -21,7 +21,7 @@ def isTestSupported(self): return False @skipIfWindows - @skipIf(archs=["arm"]) # Always times out on buildbot + @skipIf(oslist=["linux"], archs=["arm"]) # Always times out on buildbot def test_basic_functionality(self): """ Test basic restarting functionality when the process is running in @@ -61,7 +61,7 @@ def test_basic_functionality(self): ) @skipIfWindows - @skipIf(archs=["arm"]) # Always times out on buildbot + @skipIf(oslist=["linux"], archs=["arm"]) # Always times out on buildbot def test_stopOnEntry(self): """ Check that stopOnEntry works correctly when using runInTerminal. diff --git a/lldb/test/API/tools/lldb-dap/runInTerminal/TestDAP_runInTerminal.py b/lldb/test/API/tools/lldb-dap/runInTerminal/TestDAP_runInTerminal.py index 9141565ac1b9b..9aab7ca3293db 100644 --- a/lldb/test/API/tools/lldb-dap/runInTerminal/TestDAP_runInTerminal.py +++ b/lldb/test/API/tools/lldb-dap/runInTerminal/TestDAP_runInTerminal.py @@ -44,7 +44,7 @@ def isTestSupported(self): return False @skipIfWindows - @skipIf(archs=no_match(["x86_64"])) + @skipIf(oslist=["linux"], archs=no_match(["x86_64"])) def test_runInTerminal(self): if not self.isTestSupported(): return @@ -90,7 +90,7 @@ def test_runInTerminal(self): env = self.dap_server.request_evaluate("foo")["body"]["result"] self.assertIn("bar", env) - @skipIf(archs=no_match(["x86_64"])) + @skipIf(oslist=["linux"], archs=no_match(["x86_64"])) def test_runInTerminalWithObjectEnv(self): if not self.isTestSupported(): return @@ -114,7 +114,7 @@ def test_runInTerminalWithObjectEnv(self): self.assertEqual("BAR", request_envs["FOO"]) @skipIfWindows - @skipIf(archs=no_match(["x86_64"])) + @skipIf(oslist=["linux"], archs=no_match(["x86_64"])) def test_runInTerminalInvalidTarget(self): if not self.isTestSupported(): return @@ -133,7 +133,7 @@ def test_runInTerminalInvalidTarget(self): ) @skipIfWindows - @skipIf(archs=no_match(["x86_64"])) + @skipIf(oslist=["linux"], archs=no_match(["x86_64"])) def test_missingArgInRunInTerminalLauncher(self): if not self.isTestSupported(): return @@ -148,7 +148,7 @@ def test_missingArgInRunInTerminalLauncher(self): ) @skipIfWindows - @skipIf(archs=no_match(["x86_64"])) + @skipIf(oslist=["linux"], archs=no_match(["x86_64"])) def test_FakeAttachedRunInTerminalLauncherWithInvalidProgram(self): if not self.isTestSupported(): return @@ -175,7 +175,7 @@ def test_FakeAttachedRunInTerminalLauncherWithInvalidProgram(self): self.assertIn("No such file or directory", stderr) @skipIfWindows - @skipIf(archs=no_match(["x86_64"])) + @skipIf(oslist=["linux"], archs=no_match(["x86_64"])) def test_FakeAttachedRunInTerminalLauncherWithValidProgram(self): if not self.isTestSupported(): return @@ -202,7 +202,7 @@ def test_FakeAttachedRunInTerminalLauncherWithValidProgram(self): self.assertIn("foo", stdout) @skipIfWindows - @skipIf(archs=no_match(["x86_64"])) + @skipIf(oslist=["linux"], archs=no_match(["x86_64"])) def test_FakeAttachedRunInTerminalLauncherAndCheckEnvironment(self): if not self.isTestSupported(): return @@ -223,7 +223,7 @@ def test_FakeAttachedRunInTerminalLauncherAndCheckEnvironment(self): self.assertIn("FOO=BAR", stdout) @skipIfWindows - @skipIf(archs=no_match(["x86_64"])) + @skipIf(oslist=["linux"], archs=no_match(["x86_64"])) def test_NonAttachedRunInTerminalLauncher(self): if not self.isTestSupported(): return diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp index 23f0400c8bd4d..52dd377bc40fb 100644 --- a/lldb/tools/lldb-dap/DAP.cpp +++ b/lldb/tools/lldb-dap/DAP.cpp @@ -14,6 +14,7 @@ #include "LLDBUtils.h" #include "OutputRedirector.h" #include "Protocol/ProtocolBase.h" +#include "Protocol/ProtocolRequests.h" #include "Protocol/ProtocolTypes.h" #include "Transport.h" #include "lldb/API/SBBreakpoint.h" @@ -73,11 +74,8 @@ DAP::DAP(llvm::StringRef path, Log *log, const ReplMode default_repl_mode, std::vector<std::string> pre_init_commands, Transport &transport) : debug_adapter_path(path), log(log), transport(transport), broadcaster("lldb-dap"), exception_breakpoints(), + focus_tid(LLDB_INVALID_THREAD_ID), is_attach(false), pre_init_commands(std::move(pre_init_commands)), - focus_tid(LLDB_INVALID_THREAD_ID), stop_at_entry(false), is_attach(false), - enable_auto_variable_summaries(false), - enable_synthetic_child_debugging(false), - display_extended_backtrace(false), restarting_process_id(LLDB_INVALID_PROCESS_ID), configuration_done_sent(false), waiting_for_run_in_terminal(false), progress_event_reporter( @@ -505,8 +503,9 @@ ReplMode DAP::DetectReplMode(lldb::SBFrame frame, std::string &expression, bool partial_expression) { // Check for the escape hatch prefix. if (!expression.empty() && - llvm::StringRef(expression).starts_with(command_escape_prefix)) { - expression = expression.substr(command_escape_prefix.size()); + llvm::StringRef(expression) + .starts_with(configuration.commandEscapePrefix)) { + expression = expression.substr(configuration.commandEscapePrefix.size()); return ReplMode::Command; } @@ -546,7 +545,7 @@ ReplMode DAP::DetectReplMode(lldb::SBFrame frame, std::string &expression, << "Warning: Expression '" << term << "' is both an LLDB command and variable. It will be evaluated as " "a variable. To evaluate the expression as an LLDB command, use '" - << command_escape_prefix << "' as a prefix.\n"; + << configuration.commandEscapePrefix << "' as a prefix.\n"; } // Variables take preference to commands in auto, since commands can always @@ -593,7 +592,7 @@ DAP::RunLaunchCommands(llvm::ArrayRef<std::string> launch_commands) { } llvm::Error DAP::RunInitCommands() { - if (!RunLLDBCommands("Running initCommands:", init_commands)) + if (!RunLLDBCommands("Running initCommands:", configuration.initCommands)) return createRunLLDBCommandsErrorMessage("initCommands"); return llvm::Error::success(); } @@ -605,29 +604,31 @@ llvm::Error DAP::RunPreInitCommands() { } llvm::Error DAP::RunPreRunCommands() { - if (!RunLLDBCommands("Running preRunCommands:", pre_run_commands)) + if (!RunLLDBCommands("Running preRunCommands:", configuration.preRunCommands)) return createRunLLDBCommandsErrorMessage("preRunCommands"); return llvm::Error::success(); } void DAP::RunPostRunCommands() { - RunLLDBCommands("Running postRunCommands:", post_run_commands); + RunLLDBCommands("Running postRunCommands:", configuration.postRunCommands); } void DAP::RunStopCommands() { - RunLLDBCommands("Running stopCommands:", stop_commands); + RunLLDBCommands("Running stopCommands:", configuration.stopCommands); } void DAP::RunExitCommands() { - RunLLDBCommands("Running exitCommands:", exit_commands); + RunLLDBCommands("Running exitCommands:", configuration.exitCommands); } void DAP::RunTerminateCommands() { - RunLLDBCommands("Running terminateCommands:", terminate_commands); + RunLLDBCommands("Running terminateCommands:", + configuration.terminateCommands); } -lldb::SBTarget -DAP::CreateTargetFromArguments(const llvm::json::Object &arguments, - lldb::SBError &error) { +lldb::SBTarget DAP::CreateTargetFromArguments(llvm::StringRef program, + llvm::StringRef targetTriple, + llvm::StringRef platformName, + lldb::SBError &error) { // Grab the name of the program we need to debug and create a target using // the given program as an argument. Executable file can be a source of target // architecture and platform, if they differ from the host. Setting exe path @@ -638,15 +639,10 @@ DAP::CreateTargetFromArguments(const llvm::json::Object &arguments, // enough information to determine correct arch and platform (or ELF can be // omitted at all), so it is good to leave the user an apportunity to specify // those. Any of those three can be left empty. - const llvm::StringRef target_triple = - GetString(arguments, "targetTriple").value_or(""); - const llvm::StringRef platform_name = - GetString(arguments, "platformName").value_or(""); - const llvm::StringRef program = GetString(arguments, "program").value_or(""); - auto target = this->debugger.CreateTarget( - program.data(), target_triple.data(), platform_name.data(), - true, // Add dependent modules. - error); + auto target = this->debugger.CreateTarget(program.data(), targetTriple.data(), + platformName.data(), + true, // Add dependent modules. + error); if (error.Fail()) { // Update message if there was an error. @@ -802,7 +798,7 @@ llvm::Error DAP::Loop() { return llvm::Error::success(); } -lldb::SBError DAP::WaitForProcessToStop(uint32_t seconds) { +lldb::SBError DAP::WaitForProcessToStop(std::chrono::seconds seconds) { lldb::SBError error; lldb::SBProcess process = target.GetProcess(); if (!process.IsValid()) { @@ -837,8 +833,8 @@ lldb::SBError DAP::WaitForProcessToStop(uint32_t seconds) { } std::this_thread::sleep_for(std::chrono::microseconds(250)); } - error.SetErrorStringWithFormat("process failed to stop within %u seconds", - seconds); + error.SetErrorStringWithFormat("process failed to stop within %lld seconds", + seconds.count()); return error; } @@ -1035,6 +1031,36 @@ bool SendEventRequestHandler::DoExecute(lldb::SBDebugger debugger, return true; } +void DAP::ConfigureSourceMaps() { + if (configuration.sourceMap.empty() && !configuration.sourcePath) + return; + + std::string sourceMapCommand; + llvm::raw_string_ostream strm(sourceMapCommand); + strm << "settings set target.source-map "; + + if (!configuration.sourceMap.empty()) { + for (const auto &kv : configuration.sourceMap) { + strm << "\"" << kv.first << "\" \"" << kv.second << "\" "; + } + } else if (configuration.sourcePath) { + strm << "\".\" \"" << *configuration.sourcePath << "\""; + } + + RunLLDBCommands("Setting source map:", {sourceMapCommand}); +} + +void DAP::SetConfiguration(const protocol::DAPConfiguration &config, + bool is_attach) { + configuration = config; + this->is_attach = is_attach; + + if (configuration.customFrameFormat) + SetFrameFormat(*configuration.customFrameFormat); + if (configuration.customThreadFormat) + SetThreadFormat(*configuration.customThreadFormat); +} + void DAP::SetFrameFormat(llvm::StringRef format) { if (format.empty()) return; diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h index 6689980806047..f3c4a7bf22798 100644 --- a/lldb/tools/lldb-dap/DAP.h +++ b/lldb/tools/lldb-dap/DAP.h @@ -35,6 +35,7 @@ #include "lldb/lldb-types.h" #include "llvm/ADT/DenseMap.h" #include "llvm/ADT/DenseSet.h" +#include "llvm/ADT/FunctionExtras.h" #include "llvm/ADT/StringMap.h" #include "llvm/ADT/StringRef.h" #include "llvm/Support/Error.h" @@ -128,21 +129,21 @@ struct Variables { struct StartDebuggingRequestHandler : public lldb::SBCommandPluginInterface { DAP &dap; - explicit StartDebuggingRequestHandler(DAP &d) : dap(d) {}; + explicit StartDebuggingRequestHandler(DAP &d) : dap(d){}; bool DoExecute(lldb::SBDebugger debugger, char **command, lldb::SBCommandReturnObject &result) override; }; struct ReplModeRequestHandler : public lldb::SBCommandPluginInterface { DAP &dap; - explicit ReplModeRequestHandler(DAP &d) : dap(d) {}; + explicit ReplModeRequestHandler(DAP &d) : dap(d){}; bool DoExecute(lldb::SBDebugger debugger, char **command, lldb::SBCommandReturnObject &result) override; }; struct SendEventRequestHandler : public lldb::SBCommandPluginInterface { DAP &dap; - explicit SendEventRequestHandler(DAP &d) : dap(d) {}; + explicit SendEventRequestHandler(DAP &d) : dap(d){}; bool DoExecute(lldb::SBDebugger debugger, char **command, lldb::SBCommandReturnObject &result) override; }; @@ -165,26 +166,21 @@ struct DAP { InstructionBreakpointMap instruction_breakpoints; std::optional<std::vector<ExceptionBreakpoint>> exception_breakpoints; llvm::once_flag init_exception_breakpoints_flag; - std::vector<std::string> pre_init_commands; - std::vector<std::string> init_commands; - std::vector<std::string> pre_run_commands; - std::vector<std::string> post_run_commands; - 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; + // A copy of the last LaunchRequest so we can reuse its arguments if we get a + // RestartRequest. Restarting an AttachRequest is not supported. + std::optional<protocol::LaunchRequestArguments> last_launch_request; lldb::tid_t focus_tid; bool disconnecting = false; llvm::once_flag terminated_event_flag; - bool stop_at_entry; bool is_attach; - bool enable_auto_variable_summaries; - bool enable_synthetic_child_debugging; - bool display_extended_backtrace; + bool stop_at_entry; + /// pre_init_commands are configured by a CLI flag and are not part of the + /// common launching/attaching definition. + std::vector<std::string> pre_init_commands; + protocol::DAPConfiguration configuration; // The process event thread normally responds to process exited events by // shutting down the entire adapter. When we're restarting, we keep the id of // the old process here so we can detect this case and keep running. @@ -201,7 +197,6 @@ struct DAP { llvm::SmallDenseMap<int64_t, std::unique_ptr<ResponseHandler>> inflight_reverse_requests; ReplMode repl_mode; - std::string command_escape_prefix = "`"; lldb::SBFormat frame_format; lldb::SBFormat thread_format; // This is used to allow request_evaluate to handle empty expressions @@ -249,6 +244,13 @@ struct DAP { /// Stop event handler threads. void StopEventHandlers(); + /// Configures the debug adapter for launching/attaching. + void SetConfiguration(const protocol::DAPConfiguration &confing, + bool is_attach); + + /// Configure source maps based on the current `DAPConfiguration`. + void ConfigureSourceMaps(); + /// Serialize the JSON value into a string and send the JSON packet to the /// "out" stream. void SendJSON(const llvm::json::Value &json); @@ -320,7 +322,9 @@ struct DAP { /// /// \return /// An SBTarget object. - lldb::SBTarget CreateTargetFromArguments(const llvm::json::Object &arguments, + lldb::SBTarget CreateTargetFromArguments(llvm::StringRef program, + llvm::StringRef targetTriple, + llvm::StringRef platformName, lldb::SBError &error); /// Set given target object as a current target for lldb-dap and start @@ -395,7 +399,7 @@ struct DAP { /// The number of seconds to poll the process to wait until it is stopped. /// /// \return Error if waiting for the process fails, no error if succeeds. - lldb::SBError WaitForProcessToStop(uint32_t seconds); + lldb::SBError WaitForProcessToStop(std::chrono::seconds seconds); void SetFrameFormat(llvm::StringRef format); diff --git a/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp index 20f7c80a1ed90..cd7fbf4f7be9f 100644 --- a/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp @@ -43,10 +43,8 @@ namespace lldb_dap { // acknowledgement, so no body field is required." // }] // } - void AttachRequestHandler::operator()(const llvm::json::Object &request) const { dap.is_attach = true; - dap.last_launch_or_attach_request = request; llvm::json::Object response; lldb::SBError error; FillResponse(request, response); @@ -63,11 +61,13 @@ void AttachRequestHandler::operator()(const llvm::json::Object &request) const { attach_info.SetProcessID(pid); const auto wait_for = GetBoolean(arguments, "waitFor").value_or(false); attach_info.SetWaitForLaunch(wait_for, false /*async*/); - dap.init_commands = GetStrings(arguments, "initCommands"); - dap.pre_run_commands = GetStrings(arguments, "preRunCommands"); - dap.stop_commands = GetStrings(argu... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/133624 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
