llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: Piyush Jaiswal (piyushjaiswal98) <details> <summary>Changes</summary> TLDR ---------- This PR adds `--no-lldbinit` as a new CLI flag to the `lldb-dap` Motivation ----------- Rcently Users reported being unable to control `.lldbinit` file sourcing when debugging through VS Code. https://github.com/llvm/llvm-project/issues/155802. VS Code extensions cannot easily inject custom parameters into the DAP initialize request. Adding `--no-lldbinit` as a CLI flag solves this problem by allowing the decision to skip `.lldbinit` files to be made at debugger startup, before any initialization requests are processed. VS Code extensions can control this behavior by specifying the flag through `debugAdapterArgs` or similar mechanisms in launch configurations. ``` { "type": <extension-type>, "request": "launch", "name": "Debug with --no-lldbinit", "program": "${workspaceFolder}/your-program", "debugAdapterArgs": ["--no-lldbinit"] } ``` Summary ---------- This PR introduces a new command-line flag `--no-lldbinit` (with alias `-x`) to `lldb-dap`. The flag prevents automatic parsing of `.lldbinit` files during debugger initialization, giving users control over whether their LLDB initialization scripts are loaded. ### Key Changes: 1. **CLI Option Definition** (`Options.td`): Added the `--no-lldbinit` flag with `-x` alias 2. **Core Implementation** (`DAP.cpp`): Added support for storing and using the no-lldbinit flag 3. **Initialization Handler** (`InitializeRequestHandler.cpp`): Modified to respect the flag during debugger initialization 4. **Main Tool** (`lldb-dap.cpp`): Added argument parsing for the new flag 5. **Test Infrastructure** (`dap_server.py & lldbdap_testcase.py`): Enhanced test framework to support additional arguments Test Plan --------- ### New Test Coverage (`TestDAP_launch.py`) **Test Method:** `test_no_lldbinit_flag()` **Test Strategy:** 1. **Setup**: Creates a temporary `.lldbinit` file with specific settings that would normally be loaded 2. **Execution**: Launches lldb-dap with the `--no-lldbinit` flag 3. **Verification**: Confirms that the settings from `.lldbinit` are NOT applied, proving the flag works correctly **Test Environment:** * Uses a temporary home directory with a custom `.lldbinit` file * Sets specific LLDB settings (`stop-disassembly-display never`, `target.x86-disassembly-flavor intel`) * Launches debug adapter with `--no-lldbinit` flag via `additional_args` parameter **Validation Approach:** * Executes `settings show stop-disassembly-display` command during initialization * Verifies the output does NOT contain "never" (which would indicate `.lldbinit` was sourced) * Confirms that initialization commands are still executed properly ### Testing Infrastructure Enhancements **File Modifications:** * `dap_server.py`: Enhanced to accept `additional_args` parameter for passing extra CLI flags * `lldbdap_testcase.py`: Updated `build_and_create_debug_adapter()` method to support additional arguments and environment variables ### Unit Test Integration **Unit Test Updates** (`DAPTest.cpp`): * Added initialization of the new flag in test setup to ensure consistent test behavior **Test Run** <img width="1759" height="1373" alt="Screenshot 2025-08-29 at 5 56 18 PM" src="https://github.com/user-attachments/assets/769b319a-5009-4ade-aff8-c5f548b38123"; /> --- Full diff: https://github.com/llvm/llvm-project/pull/156131.diff 10 Files Affected: - (modified) lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py (+7-1) - (modified) lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py (+4-1) - (modified) lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py (+44) - (modified) lldb/tools/lldb-dap/DAP.cpp (+3-2) - (modified) lldb/tools/lldb-dap/DAP.h (+8-2) - (modified) lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp (+5-2) - (modified) lldb/tools/lldb-dap/Options.td (+6) - (modified) lldb/tools/lldb-dap/tool/lldb-dap.cpp (+9-6) - (modified) lldb/unittests/DAP/DAPTest.cpp (+1) - (modified) lldb/unittests/DAP/TestBase.cpp (+1) ``````````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 0608ac3fd83be..3e83b8d5bdad7 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 @@ -1490,12 +1490,14 @@ def __init__( init_commands: list[str] = [], log_file: Optional[TextIO] = None, env: Optional[dict[str, str]] = None, + additional_args: list[str] = [], ): self.process = None self.connection = None if executable is not None: process, connection = DebugAdapterServer.launch( - executable=executable, connection=connection, env=env, log_file=log_file + executable=executable, connection=connection, env=env, log_file=log_file, + additional_args=additional_args ) self.process = process self.connection = connection @@ -1528,6 +1530,7 @@ def launch( env: Optional[dict[str, str]] = None, log_file: Optional[TextIO] = None, connection: Optional[str] = None, + additional_args: list[str] = [], ) -> tuple[subprocess.Popen, Optional[str]]: adapter_env = os.environ.copy() if env is not None: @@ -1537,6 +1540,9 @@ def launch( adapter_env["LLDBDAP_LOG"] = log_file args = [executable] + # Add additional arguments first (like --no-lldbinit) + args.extend(additional_args) + if connection is not None: args.append("--connection") args.append(connection) 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 b28a78792c70f..fffd4c23d6fcd 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 @@ -21,6 +21,7 @@ def create_debug_adapter( self, lldbDAPEnv: Optional[dict[str, str]] = None, connection: Optional[str] = None, + additional_args: Optional[list[str]] = None, ): """Create the Visual Studio Code debug adapter""" self.assertTrue( @@ -33,15 +34,17 @@ def create_debug_adapter( init_commands=self.setUpCommands(), log_file=log_file_path, env=lldbDAPEnv, + additional_args=additional_args or [], ) def build_and_create_debug_adapter( self, lldbDAPEnv: Optional[dict[str, str]] = None, dictionary: Optional[dict] = None, + additional_args: Optional[list[str]] = None, ): self.build(dictionary=dictionary) - self.create_debug_adapter(lldbDAPEnv) + self.create_debug_adapter(lldbDAPEnv, additional_args=additional_args) def build_and_create_debug_adapter_for_attach(self): """Variant of build_and_create_debug_adapter that builds a uniquely 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 41112a4f5b9e3..4b384e260ad24 100644 --- a/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py +++ b/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py @@ -7,6 +7,7 @@ import lldbdap_testcase import os import re +import tempfile # Many tests are skipped on Windows because get_stdout() returns None there. # Despite the test program printing correctly. See @@ -582,3 +583,46 @@ def test_version(self): version_string.splitlines(), "version string does not match", ) + + def test_no_lldbinit_flag(self): + """ + Test that the --no-lldbinit flag prevents sourcing .lldbinit files. + """ + # Create a temporary .lldbinit file in the home directory + with tempfile.TemporaryDirectory() as temp_home: + lldbinit_path = os.path.join(temp_home, ".lldbinit") + + # Write a command to the .lldbinit file that would set a unique setting + with open(lldbinit_path, "w") as f: + f.write("settings set stop-disassembly-display never\n") + f.write("settings set target.x86-disassembly-flavor intel\n") + + # Test with --no-lldbinit flag (should NOT source .lldbinit) + self.build_and_create_debug_adapter( + lldbDAPEnv={"HOME": temp_home}, + additional_args=["--no-lldbinit"] + ) + program = self.getBuildArtifact("a.out") + + # Use initCommands to check if .lldbinit was sourced + initCommands = ["settings show stop-disassembly-display"] + + # Initialize the DAP session (should NOT source .lldbinit due to --no-lldbinit) + self.dap_server.request_initialize() + + # Launch with initCommands to check the setting + self.launch(program, initCommands=initCommands, stopOnEntry=True) + + # Get console output to verify the setting was NOT set from .lldbinit + output = self.get_console() + self.assertTrue(output and len(output) > 0, "expect console output") + + # Verify the setting has default value, not "never" from .lldbinit + self.assertNotIn( + "never", + output, + "Setting should have default value when --no-lldbinit is used", + ) + + # Verify the initCommands were executed + self.verify_commands("initCommands", output, initCommands) diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp index b1ad38d983893..9c1966ea4238a 100644 --- a/lldb/tools/lldb-dap/DAP.cpp +++ b/lldb/tools/lldb-dap/DAP.cpp @@ -121,12 +121,13 @@ static std::string capitalize(llvm::StringRef str) { llvm::StringRef DAP::debug_adapter_path = ""; DAP::DAP(Log *log, const ReplMode default_repl_mode, - std::vector<std::string> pre_init_commands, + std::vector<std::string> pre_init_commands, bool no_lldbinit, llvm::StringRef client_name, DAPTransport &transport, MainLoop &loop) : log(log), transport(transport), broadcaster("lldb-dap"), progress_event_reporter( [&](const ProgressEvent &event) { SendJSON(event.ToJSON()); }), - repl_mode(default_repl_mode), m_client_name(client_name), m_loop(loop) { + repl_mode(default_repl_mode), no_lldbinit(no_lldbinit), + m_client_name(client_name), m_loop(loop) { configuration.preInitCommands = std::move(pre_init_commands); RegisterRequests(); } diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h index 04f70f76a09cd..71681fd4b51ed 100644 --- a/lldb/tools/lldb-dap/DAP.h +++ b/lldb/tools/lldb-dap/DAP.h @@ -156,6 +156,9 @@ struct DAP final : private DAPTransport::MessageHandler { /// The set of features supported by the connected client. llvm::DenseSet<ClientFeature> clientFeatures; + /// Whether to disable sourcing .lldbinit files. + bool no_lldbinit; + /// The initial thread list upon attaching. std::vector<protocol::Thread> initial_thread_list; @@ -178,13 +181,16 @@ struct DAP final : private DAPTransport::MessageHandler { /// \param[in] pre_init_commands /// LLDB commands to execute as soon as the debugger instance is /// allocated. + /// \param[in] no_lldbinit + /// Whether to disable sourcing .lldbinit files. /// \param[in] transport /// Transport for this debug session. /// \param[in] loop /// Main loop associated with this instance. DAP(Log *log, const ReplMode default_repl_mode, - std::vector<std::string> pre_init_commands, llvm::StringRef client_name, - DAPTransport &transport, lldb_private::MainLoop &loop); + std::vector<std::string> pre_init_commands, bool no_lldbinit, + llvm::StringRef client_name, DAPTransport &transport, + lldb_private::MainLoop &loop); ~DAP(); diff --git a/lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp index b499a69876e2c..9069de4a3a690 100644 --- a/lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp @@ -42,8 +42,11 @@ llvm::Expected<InitializeResponse> InitializeRequestHandler::Run( // The sourceInitFile option is not part of the DAP specification. It is an // extension used by the test suite to prevent sourcing `.lldbinit` and - // changing its behavior. - if (arguments.lldbExtSourceInitFile.value_or(true)) { + // changing its behavior. The CLI flag --no-lldbinit takes precedence over + // the DAP parameter. + bool should_source_init_files = + !dap.no_lldbinit && arguments.lldbExtSourceInitFile.value_or(true); + if (should_source_init_files) { dap.debugger.SkipLLDBInitFiles(false); dap.debugger.SkipAppInitFiles(false); lldb::SBCommandReturnObject init; diff --git a/lldb/tools/lldb-dap/Options.td b/lldb/tools/lldb-dap/Options.td index 867753e9294a6..b3fb592c84a8e 100644 --- a/lldb/tools/lldb-dap/Options.td +++ b/lldb/tools/lldb-dap/Options.td @@ -61,3 +61,9 @@ def pre_init_command: S<"pre-init-command">, def: Separate<["-"], "c">, Alias<pre_init_command>, HelpText<"Alias for --pre-init-command">; + +def no_lldbinit: F<"no-lldbinit">, + HelpText<"Do not automatically parse any '.lldbinit' files.">; +def: Flag<["-"], "x">, + Alias<no_lldbinit>, + HelpText<"Alias for --no-lldbinit">; diff --git a/lldb/tools/lldb-dap/tool/lldb-dap.cpp b/lldb/tools/lldb-dap/tool/lldb-dap.cpp index b74085f25f4e2..2431cf8fb2f24 100644 --- a/lldb/tools/lldb-dap/tool/lldb-dap.cpp +++ b/lldb/tools/lldb-dap/tool/lldb-dap.cpp @@ -258,7 +258,8 @@ validateConnection(llvm::StringRef conn) { static llvm::Error serveConnection(const Socket::SocketProtocol &protocol, const std::string &name, Log *log, const ReplMode default_repl_mode, - const std::vector<std::string> &pre_init_commands) { + const std::vector<std::string> &pre_init_commands, + bool no_lldbinit) { Status status; static std::unique_ptr<Socket> listener = Socket::Create(protocol, status); if (status.Fail()) { @@ -303,8 +304,8 @@ serveConnection(const Socket::SocketProtocol &protocol, const std::string &name, llvm::set_thread_name(client_name + ".runloop"); MainLoop loop; Transport transport(client_name, log, io, io); - DAP dap(log, default_repl_mode, pre_init_commands, client_name, transport, - loop); + DAP dap(log, default_repl_mode, pre_init_commands, no_lldbinit, + client_name, transport, loop); if (auto Err = dap.ConfigureIO()) { llvm::logAllUnhandledErrors(std::move(Err), llvm::errs(), @@ -508,6 +509,8 @@ int main(int argc, char *argv[]) { pre_init_commands.push_back(arg); } + bool no_lldbinit = input_args.hasArg(OPT_no_lldbinit); + if (!connection.empty()) { auto maybeProtoclAndName = validateConnection(connection); if (auto Err = maybeProtoclAndName.takeError()) { @@ -520,7 +523,7 @@ int main(int argc, char *argv[]) { std::string name; std::tie(protocol, name) = *maybeProtoclAndName; if (auto Err = serveConnection(protocol, name, log.get(), default_repl_mode, - pre_init_commands)) { + pre_init_commands, no_lldbinit)) { llvm::logAllUnhandledErrors(std::move(Err), llvm::errs(), "Connection failed: "); return EXIT_FAILURE; @@ -556,8 +559,8 @@ int main(int argc, char *argv[]) { constexpr llvm::StringLiteral client_name = "stdio"; MainLoop loop; Transport transport(client_name, log.get(), input, output); - DAP dap(log.get(), default_repl_mode, pre_init_commands, client_name, - transport, loop); + DAP dap(log.get(), default_repl_mode, pre_init_commands, no_lldbinit, + client_name, transport, loop); // stdout/stderr redirection to the IDE's console if (auto Err = dap.ConfigureIO(stdout, stderr)) { diff --git a/lldb/unittests/DAP/DAPTest.cpp b/lldb/unittests/DAP/DAPTest.cpp index d5a9591ad0a43..2090fe6896d6b 100644 --- a/lldb/unittests/DAP/DAPTest.cpp +++ b/lldb/unittests/DAP/DAPTest.cpp @@ -28,6 +28,7 @@ TEST_F(DAPTest, SendProtocolMessages) { /*log=*/nullptr, /*default_repl_mode=*/ReplMode::Auto, /*pre_init_commands=*/{}, + /*no_lldbinit=*/false, /*client_name=*/"test_client", /*transport=*/*transport, /*loop=*/loop, diff --git a/lldb/unittests/DAP/TestBase.cpp b/lldb/unittests/DAP/TestBase.cpp index 54ac27da694e6..ba7baf2103799 100644 --- a/lldb/unittests/DAP/TestBase.cpp +++ b/lldb/unittests/DAP/TestBase.cpp @@ -55,6 +55,7 @@ void DAPTestBase::SetUp() { /*log=*/log.get(), /*default_repl_mode=*/ReplMode::Auto, /*pre_init_commands=*/std::vector<std::string>(), + /*no_lldbinit=*/false, /*client_name=*/"test_client", /*transport=*/*transport, /*loop=*/loop); } `````````` </details> https://github.com/llvm/llvm-project/pull/156131 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
