Author: Charles Zablit Date: 2026-01-26T19:56:23+01:00 New Revision: c4585b44b65d11fe00df120e344cbeddb600a3bb
URL: https://github.com/llvm/llvm-project/commit/c4585b44b65d11fe00df120e344cbeddb600a3bb DIFF: https://github.com/llvm/llvm-project/commit/c4585b44b65d11fe00df120e344cbeddb600a3bb.diff LOG: [lldb][windows] do not attach to a PseudoConsole if it is not opened (#177934) # Summary This patch ensures lldb will not try to read from a PseudoConsole if it has not been opened. # Original issue https://github.com/llvm/llvm-project/pull/168729 introduces support for the Windows ConPTY in `lldb-dap`. This caused a regression in `lldb` which was not caught by our tests: https://github.com/llvm/llvm-project/issues/175652. This patch fixes https://github.com/llvm/llvm-project/issues/175652. `lldb_private::ProcessLauncherWindows::LaunchProcess` connects the debuggee to a PseudoConsole only if: ```cpp if (hPC != INVALID_HANDLE_VALUE && launch_info.GetNumFileActions() == 0 && launch_info.GetFlags().Test(lldb::eLaunchFlagLaunchInTTY)) ``` lldb needs to check the same condition in `lldb_private::PlatformWindows::DebugProcess` to ensure that it does not read from a PseudoConsole which has not been opened. # Side effect This patch reverts what https://github.com/llvm/llvm-project/pull/168729 did by adding ConPTY support. https://github.com/llvm/llvm-project/pull/175812 is the proper fix, however it's causing tests failures which we can't reliably reproduce at desk. Given https://github.com/llvm/llvm-project/pull/168729 breaks `22.x` lldb, it's better to revert the change ahead of the release of `22.x` rather than pushing for the proper fix. Added: Modified: lldb/include/lldb/Host/ProcessLaunchInfo.h lldb/source/Host/windows/ProcessLauncherWindows.cpp lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py Removed: ################################################################################ diff --git a/lldb/include/lldb/Host/ProcessLaunchInfo.h b/lldb/include/lldb/Host/ProcessLaunchInfo.h index 1bddface440e7..023d150503da3 100644 --- a/lldb/include/lldb/Host/ProcessLaunchInfo.h +++ b/lldb/include/lldb/Host/ProcessLaunchInfo.h @@ -132,6 +132,18 @@ class ProcessLaunchInfo : public ProcessInfo { std::shared_ptr<PTY> GetPTYSP() const { return m_pty; } + /// Returns whether if lldb should read information from the PTY. This is + /// always true on non Windows. + bool ShouldUsePTY() const { +#ifdef _WIN32 + return GetPTY().GetPseudoTerminalHandle() != ((HANDLE)(long long)-1) && + GetNumFileActions() == 0 && + GetFlags().Test(lldb::eLaunchFlagLaunchInTTY); +#else + return true; +#endif + } + void SetLaunchEventData(const char *data) { m_event_data.assign(data); } const char *GetLaunchEventData() const { return m_event_data.c_str(); } diff --git a/lldb/source/Host/windows/ProcessLauncherWindows.cpp b/lldb/source/Host/windows/ProcessLauncherWindows.cpp index 76feceadf46f3..ec1a20ebb2200 100644 --- a/lldb/source/Host/windows/ProcessLauncherWindows.cpp +++ b/lldb/source/Host/windows/ProcessLauncherWindows.cpp @@ -116,9 +116,7 @@ ProcessLauncherWindows::LaunchProcess(const ProcessLaunchInfo &launch_info, startupinfoex.StartupInfo.dwFlags |= STARTF_USESTDHANDLES; HPCON hPC = launch_info.GetPTY().GetPseudoTerminalHandle(); - bool use_pty = hPC != INVALID_HANDLE_VALUE && - launch_info.GetNumFileActions() == 0 && - launch_info.GetFlags().Test(lldb::eLaunchFlagLaunchInTTY); + bool use_pty = launch_info.ShouldUsePTY(); HANDLE stdin_handle = GetStdioHandle(launch_info, STDIN_FILENO); HANDLE stdout_handle = GetStdioHandle(launch_info, STDOUT_FILENO); diff --git a/lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp b/lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp index 7d5a5a503474c..0e74cab23c99e 100644 --- a/lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp +++ b/lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp @@ -522,9 +522,10 @@ ProcessSP PlatformWindows::DebugProcess(ProcessLaunchInfo &launch_info, return nullptr; error = process_sp->Launch(launch_info); #ifdef _WIN32 - if (error.Success()) - process_sp->SetPseudoConsoleHandle(launch_info.GetPTYSP()); - else { + if (error.Success()) { + if (launch_info.ShouldUsePTY()) + process_sp->SetPseudoConsoleHandle(launch_info.GetPTYSP()); + } else { Log *log = GetLog(LLDBLog::Platform); LLDB_LOGF(log, "Platform::%s LaunchProcess() failed: %s", __FUNCTION__, error.AsCString()); 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 f7f4a8016dc19..799574c4daab5 100644 --- a/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py +++ b/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py @@ -16,7 +16,7 @@ class TestDAP_launch(lldbdap_testcase.DAPTestCaseBase): - @skipIfWindowsWithoutConPTY + @expectedFailureWindows def test_default(self): """ Tests the default launch of a simple program. No arguments, @@ -76,7 +76,6 @@ def test_failing_console(self): r"unexpected value, expected 'internalConsole\', 'integratedTerminal\' or 'externalTerminal\' at arguments.console", ) - @skipIfWindowsWithoutConPTY def test_termination(self): """ Tests the correct termination of lldb-dap upon a 'disconnect' @@ -142,6 +141,7 @@ def test_cwd(self): ) self.assertTrue(found, "verified program working directory") + @expectedFailureWindows def test_debuggerRoot(self): """ Tests the "debuggerRoot" will change the working directory of @@ -210,7 +210,7 @@ def test_disableSTDIO(self): self.assertEqual(output, "", "expect no program output") @skipIfLinux # shell argument expansion doesn't seem to work on Linux - @skipIfWindowsWithoutConPTY + @expectedFailureWindows @expectedFailureAll(oslist=["freebsd", "netbsd"], bugnumber="llvm.org/pr48349") def test_shellExpandArguments_enabled(self): """ @@ -233,7 +233,7 @@ def test_shellExpandArguments_enabled(self): quote_path, line, 'verify "%s" expanded to "%s"' % (glob, program) ) - @skipIfWindowsWithoutConPTY + @expectedFailureWindows def test_shellExpandArguments_disabled(self): """ Tests the default launch of a simple program with shell expansion @@ -255,7 +255,7 @@ def test_shellExpandArguments_disabled(self): quote_path, line, 'verify "%s" stayed to "%s"' % (glob, glob) ) - @skipIfWindowsWithoutConPTY + @expectedFailureWindows def test_args(self): """ Tests launch of a simple program with arguments @@ -280,7 +280,7 @@ def test_args(self): 'arg[%i] "%s" not in "%s"' % (i + 1, quoted_arg, lines[i]), ) - @skipIfWindowsWithoutConPTY + @expectedFailureWindows def test_environment_with_object(self): """ Tests launch of a simple program with environment variables @@ -558,7 +558,7 @@ def test_terminate_commands(self): output = self.collect_console(pattern=terminateCommands[0]) self.verify_commands("terminateCommands", output, terminateCommands) - @skipIfWindowsWithoutConPTY + @expectedFailureWindows def test_version(self): """ Tests that "initialize" response contains the "version" string the same @@ -642,7 +642,7 @@ def test_stdio_redirection(self): ) @skipIfAsan - @skipIfWindowsWithoutConPTY + @expectedFailureWindows @skipIf(oslist=["linux"], archs=no_match(["x86_64"])) @skipIfBuildType(["debug"]) def test_stdio_redirection_and_console(self): _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
