https://github.com/charles-zablit created https://github.com/llvm/llvm-project/pull/177610
This reverts commit 4b7cf46f6bd1bcd3f908efaedfeea27b28b6d564. >From 5c443edec3d1c9004d0bf8025cf40e9bab3cd0f8 Mon Sep 17 00:00:00 2001 From: Charles Zablit <[email protected]> Date: Fri, 23 Jan 2026 16:32:54 +0000 Subject: [PATCH] Revert "[lldb][windows] add STDIN and STDOUT forwarding support (#175812)" This reverts commit 4b7cf46f6bd1bcd3f908efaedfeea27b28b6d564. --- .../Host/windows/ProcessLauncherWindows.h | 9 +- .../include/lldb/Host/windows/PseudoConsole.h | 10 -- .../Host/windows/ProcessLauncherWindows.cpp | 24 ++-- lldb/source/Host/windows/PseudoConsole.cpp | 66 ---------- .../Process/Windows/Common/ProcessWindows.cpp | 124 +++++------------- .../Shell/Settings/TestFrameFormatColor.test | 2 +- .../Settings/TestFrameFormatNoColor.test | 2 +- 7 files changed, 43 insertions(+), 194 deletions(-) diff --git a/lldb/include/lldb/Host/windows/ProcessLauncherWindows.h b/lldb/include/lldb/Host/windows/ProcessLauncherWindows.h index 7f99a46447966..b835885dd47c6 100644 --- a/lldb/include/lldb/Host/windows/ProcessLauncherWindows.h +++ b/lldb/include/lldb/Host/windows/ProcessLauncherWindows.h @@ -11,7 +11,7 @@ #include "lldb/Host/ProcessLauncher.h" #include "lldb/Host/windows/windows.h" -#include "llvm/Support/Error.h" +#include "llvm/Support/ErrorOr.h" namespace lldb_private { @@ -38,13 +38,6 @@ class ProcThreadAttributeList { static llvm::ErrorOr<ProcThreadAttributeList> Create(STARTUPINFOEXW &startupinfoex); - /// Setup the PseudoConsole handle in the underlying - /// LPPROC_THREAD_ATTRIBUTE_LIST. - /// - /// \param hPC - /// The handle to the PseudoConsole. - llvm::Error SetupPseudoConsole(HPCON hPC); - ~ProcThreadAttributeList() { if (lpAttributeList) { DeleteProcThreadAttributeList(lpAttributeList); diff --git a/lldb/include/lldb/Host/windows/PseudoConsole.h b/lldb/include/lldb/Host/windows/PseudoConsole.h index 7a9829e23a097..039783985c025 100644 --- a/lldb/include/lldb/Host/windows/PseudoConsole.h +++ b/lldb/include/lldb/Host/windows/PseudoConsole.h @@ -61,16 +61,6 @@ class PseudoConsole { /// invalid. HANDLE GetSTDINHandle() const { return m_conpty_input; }; - /// Drains initialization sequences from the ConPTY output pipe. - /// - /// When a process first attaches to a ConPTY, Windows emits VT100/ANSI escape - /// sequences (ESC[2J for clear screen, ESC[H for cursor home and more) as - /// part of the PseudoConsole initialization. To prevent these sequences from - /// appearing in the debugger output (and flushing lldb's shell for instance) - /// we launch a short-lived dummy process that triggers the initialization, - /// then drain all output before launching the actual debuggee. - llvm::Error DrainInitSequences(); - protected: HANDLE m_conpty_handle = ((HANDLE)(long long)-1); HANDLE m_conpty_output = ((HANDLE)(long long)-1); diff --git a/lldb/source/Host/windows/ProcessLauncherWindows.cpp b/lldb/source/Host/windows/ProcessLauncherWindows.cpp index 907f0fc86bd89..76feceadf46f3 100644 --- a/lldb/source/Host/windows/ProcessLauncherWindows.cpp +++ b/lldb/source/Host/windows/ProcessLauncherWindows.cpp @@ -106,15 +106,6 @@ ProcThreadAttributeList::Create(STARTUPINFOEXW &startupinfoex) { return ProcThreadAttributeList(startupinfoex.lpAttributeList); } -llvm::Error ProcThreadAttributeList::SetupPseudoConsole(HPCON hPC) { - BOOL ok = UpdateProcThreadAttribute(lpAttributeList, 0, - PROC_THREAD_ATTRIBUTE_PSEUDOCONSOLE, hPC, - sizeof(hPC), NULL, NULL); - if (!ok) - return llvm::errorCodeToError(llvm::mapWindowsError(GetLastError())); - return llvm::Error::success(); -} - HostProcess ProcessLauncherWindows::LaunchProcess(const ProcessLaunchInfo &launch_info, Status &error) { @@ -125,8 +116,9 @@ 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; + bool use_pty = hPC != INVALID_HANDLE_VALUE && + launch_info.GetNumFileActions() == 0 && + launch_info.GetFlags().Test(lldb::eLaunchFlagLaunchInTTY); HANDLE stdin_handle = GetStdioHandle(launch_info, STDIN_FILENO); HANDLE stdout_handle = GetStdioHandle(launch_info, STDOUT_FILENO); @@ -145,15 +137,15 @@ ProcessLauncherWindows::LaunchProcess(const ProcessLaunchInfo &launch_info, error = attributelist_or_err.getError(); return HostProcess(); } - ProcThreadAttributeList attributelist = std::move(*attributelist_or_err); - llvm::scope_exit delete_attributelist( [&] { DeleteProcThreadAttributeList(startupinfoex.lpAttributeList); }); std::vector<HANDLE> inherited_handles; if (use_pty) { - if (auto err = attributelist.SetupPseudoConsole(hPC)) { - error = Status::FromError(std::move(err)); + if (!UpdateProcThreadAttribute(startupinfoex.lpAttributeList, 0, + PROC_THREAD_ATTRIBUTE_PSEUDOCONSOLE, hPC, + sizeof(hPC), NULL, NULL)) { + error = Status(::GetLastError(), eErrorTypeWin32); return HostProcess(); } } else { @@ -207,7 +199,7 @@ ProcessLauncherWindows::LaunchProcess(const ProcessLaunchInfo &launch_info, BOOL result = ::CreateProcessW( wexecutable.c_str(), pwcommandLine, NULL, NULL, - /*bInheritHandles=*/TRUE, flags, environment.data(), + /*bInheritHandles=*/!inherited_handles.empty(), flags, environment.data(), wworkingDirectory.size() == 0 ? NULL : wworkingDirectory.c_str(), reinterpret_cast<STARTUPINFOW *>(&startupinfoex), &pi); diff --git a/lldb/source/Host/windows/PseudoConsole.cpp b/lldb/source/Host/windows/PseudoConsole.cpp index 413c8a3328445..b3314fd58bbb8 100644 --- a/lldb/source/Host/windows/PseudoConsole.cpp +++ b/lldb/source/Host/windows/PseudoConsole.cpp @@ -11,7 +11,6 @@ #include <mutex> #include "lldb/Host/windows/PipeWindows.h" -#include "lldb/Host/windows/ProcessLauncherWindows.h" #include "lldb/Host/windows/windows.h" #include "lldb/Utility/LLDBLog.h" @@ -119,12 +118,6 @@ llvm::Error PseudoConsole::OpenPseudoConsole() { m_conpty_output = hOutputRead; m_conpty_input = hInputWrite; - if (auto error = DrainInitSequences()) { - Log *log = GetLog(LLDBLog::Host); - LLDB_LOG_ERROR(log, std::move(error), - "failed to finalize ConPTY's setup: {0}"); - } - return llvm::Error::success(); } @@ -140,62 +133,3 @@ void PseudoConsole::Close() { m_conpty_input = INVALID_HANDLE_VALUE; m_conpty_output = INVALID_HANDLE_VALUE; } - -llvm::Error PseudoConsole::DrainInitSequences() { - STARTUPINFOEXW startupinfoex = {}; - startupinfoex.StartupInfo.cb = sizeof(STARTUPINFOEXW); - startupinfoex.StartupInfo.dwFlags |= STARTF_USESTDHANDLES; - - auto attributelist_or_err = ProcThreadAttributeList::Create(startupinfoex); - if (!attributelist_or_err) - return llvm::errorCodeToError(attributelist_or_err.getError()); - ProcThreadAttributeList attributelist = std::move(*attributelist_or_err); - if (auto error = attributelist.SetupPseudoConsole(m_conpty_handle)) - return error; - - PROCESS_INFORMATION pi = {}; - - wchar_t comspec[MAX_PATH]; - DWORD comspecLen = GetEnvironmentVariableW(L"COMSPEC", comspec, MAX_PATH); - if (comspecLen == 0 || comspecLen >= MAX_PATH) - return llvm::createStringError( - std::error_code(GetLastError(), std::system_category()), - "Failed to get the 'COMSPEC' environment variable"); - - std::wstring cmdline_str = std::wstring(comspec) + L" /c 'echo foo && exit'"; - std::vector<wchar_t> cmdline(cmdline_str.begin(), cmdline_str.end()); - cmdline.push_back(L'\0'); - - if (!CreateProcessW(/*lpApplicationName=*/comspec, cmdline.data(), - /*lpProcessAttributes=*/NULL, /*lpThreadAttributes=*/NULL, - /*bInheritHandles=*/TRUE, - /*dwCreationFlags=*/EXTENDED_STARTUPINFO_PRESENT | - CREATE_UNICODE_ENVIRONMENT, - /*lpEnvironment=*/NULL, /*lpCurrentDirectory=*/NULL, - /*lpStartupInfo=*/ - reinterpret_cast<STARTUPINFOW *>(&startupinfoex), - /*lpProcessInformation=*/&pi)) - return llvm::errorCodeToError( - std::error_code(GetLastError(), std::system_category())); - - char buf[4096]; - OVERLAPPED ov = {}; - ov.hEvent = CreateEvent(NULL, TRUE, FALSE, NULL); - - DWORD read; - ReadFile(m_conpty_output, buf, sizeof(buf), &read, &ov); - - WaitForSingleObject(pi.hProcess, INFINITE); - - if (GetOverlappedResult(m_conpty_output, &ov, &read, FALSE) && read > 0) { - ResetEvent(ov.hEvent); - ReadFile(m_conpty_output, buf, sizeof(buf), &read, &ov); - } - - CancelIo(m_conpty_output); - CloseHandle(ov.hEvent); - CloseHandle(pi.hProcess); - CloseHandle(pi.hThread); - - return llvm::Error::success(); -} diff --git a/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp b/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp index 373729c952071..226cc147aadae 100644 --- a/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp +++ b/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp @@ -958,60 +958,21 @@ class IOHandlerProcessSTDIOWindows : public IOHandler { IOHandler::Type::ProcessIO), m_process(process), m_read_file(GetInputFD(), File::eOpenOptionReadOnly, false), - m_write_file(conpty_input), - m_interrupt_event( - CreateEvent(/*lpEventAttributes=*/NULL, /*bManualReset=*/FALSE, - /*bInitialState=*/FALSE, /*lpName=*/NULL)) {} - - ~IOHandlerProcessSTDIOWindows() override { - if (m_interrupt_event != INVALID_HANDLE_VALUE) - ::CloseHandle(m_interrupt_event); + m_write_file(conpty_input) { + m_pipe.CreateNew(); } + ~IOHandlerProcessSTDIOWindows() override = default; + void SetIsRunning(bool running) { std::lock_guard<std::mutex> guard(m_mutex); SetIsDone(!running); m_is_running = running; } - /// Peek the console for input. If it has any, drain the pipe until text input - /// is found or the pipe is empty. - /// - /// \param hStdin - /// The handle to the standard input's pipe. - /// - /// \return - /// true if the pipe has text input. - llvm::Expected<bool> ConsoleHasTextInput(const HANDLE hStdin) { - // Check if there are already characters buffered. Pressing enter counts as - // 2 characters '\r\n' and only one of them is a keyDown event. - DWORD bytesAvailable = 0; - if (PeekNamedPipe(hStdin, NULL, 0, NULL, &bytesAvailable, NULL)) { - if (bytesAvailable > 0) - return true; - } - - while (true) { - INPUT_RECORD inputRecord; - DWORD numRead = 0; - if (!PeekConsoleInput(hStdin, &inputRecord, 1, &numRead)) - return llvm::createStringError("Failed to peek standard input."); - - if (numRead == 0) - return false; - - if (inputRecord.EventType == KEY_EVENT && - inputRecord.Event.KeyEvent.bKeyDown && - inputRecord.Event.KeyEvent.uChar.AsciiChar != 0) - return true; - - if (!ReadConsoleInput(hStdin, &inputRecord, 1, &numRead)) - return llvm::createStringError("Failed to read standard input."); - } - } - void Run() override { - if (!m_read_file.IsValid() || m_write_file == INVALID_HANDLE_VALUE) { + if (!m_read_file.IsValid() || m_write_file == INVALID_HANDLE_VALUE || + !m_pipe.CanRead() || !m_pipe.CanWrite()) { SetIsDone(true); return; } @@ -1019,18 +980,9 @@ class IOHandlerProcessSTDIOWindows : public IOHandler { SetIsDone(false); SetIsRunning(true); - HANDLE hStdin = m_read_file.GetWaitableHandle(); - HANDLE waitHandles[2] = {hStdin, m_interrupt_event}; - - DWORD consoleMode; - bool isConsole = GetConsoleMode(hStdin, &consoleMode) != 0; - // With ENABLE_LINE_INPUT, ReadFile returns only when a carriage return is - // read. This will block lldb in ReadFile until the user hits enter. Save - // the previous console mode to restore it later and remove - // ENABLE_LINE_INPUT. - DWORD oldConsoleMode = consoleMode; - SetConsoleMode(hStdin, - consoleMode & ~ENABLE_LINE_INPUT & ~ENABLE_ECHO_INPUT); + HANDLE hStdin = (HANDLE)_get_osfhandle(m_read_file.GetDescriptor()); + HANDLE hInterrupt = (HANDLE)_get_osfhandle(m_pipe.GetReadFileDescriptor()); + HANDLE waitHandles[2] = {hStdin, hInterrupt}; while (true) { { @@ -1044,20 +996,6 @@ class IOHandlerProcessSTDIOWindows : public IOHandler { case WAIT_FAILED: goto exit_loop; case WAIT_OBJECT_0: { - if (isConsole) { - auto hasInputOrErr = ConsoleHasTextInput(hStdin); - if (!hasInputOrErr) { - Log *log = GetLog(WindowsLog::Process); - LLDB_LOG_ERROR(log, hasInputOrErr.takeError(), - "failed to process debuggee's IO: {0}"); - goto exit_loop; - } - - // If no text input is ready, go back to waiting. - if (!*hasInputOrErr) - continue; - } - char ch = 0; DWORD read = 0; if (!ReadFile(hStdin, &ch, 1, &read, nullptr) || read != 1) @@ -1069,10 +1007,14 @@ class IOHandlerProcessSTDIOWindows : public IOHandler { break; } case WAIT_OBJECT_0 + 1: { - ControlOp op = m_pending_op.exchange(eControlOpNone); - if (op == eControlOpQuit) + char ch = 0; + DWORD read = 0; + if (!ReadFile(hInterrupt, &ch, 1, &read, nullptr) || read != 1) + goto exit_loop; + + if (ch == eControlOpQuit) goto exit_loop; - if (op == eControlOpInterrupt && + if (ch == eControlOpInterrupt && StateIsRunningState(m_process->GetState())) m_process->SendAsyncInterrupt(); break; @@ -1084,24 +1026,24 @@ class IOHandlerProcessSTDIOWindows : public IOHandler { exit_loop:; SetIsRunning(false); - SetIsDone(true); - SetConsoleMode(hStdin, oldConsoleMode); } void Cancel() override { std::lock_guard<std::mutex> guard(m_mutex); SetIsDone(true); if (m_is_running) { - m_pending_op.store(eControlOpQuit); - ::SetEvent(m_interrupt_event); + char ch = eControlOpQuit; + if (llvm::Error err = m_pipe.Write(&ch, 1).takeError()) { + LLDB_LOG_ERROR(GetLog(LLDBLog::Process), std::move(err), + "Pipe write failed: {0}"); + } } } bool Interrupt() override { if (m_active) { - m_pending_op.store(eControlOpInterrupt); - ::SetEvent(m_interrupt_event); - return true; + char ch = eControlOpInterrupt; + return !errorToBool(m_pipe.Write(&ch, 1).takeError()); } if (StateIsRunningState(m_process->GetState())) { m_process->SendAsyncInterrupt(); @@ -1113,21 +1055,19 @@ class IOHandlerProcessSTDIOWindows : public IOHandler { void GotEOF() override {} private: + Process *m_process; + NativeFile m_read_file; // Read from this file (usually actual STDIN for LLDB + HANDLE m_write_file = + INVALID_HANDLE_VALUE; // Write to this file (usually the primary pty for + // getting io to debuggee) + Pipe m_pipe; + std::mutex m_mutex; + bool m_is_running = false; + enum ControlOp : char { eControlOpQuit = 'q', eControlOpInterrupt = 'i', - eControlOpNone = 0, }; - - Process *m_process; - /// Read from this file (usually actual STDIN for LLDB) - NativeFile m_read_file; - /// Write to this file (usually the primary pty for getting io to debuggee) - HANDLE m_write_file = INVALID_HANDLE_VALUE; - HANDLE m_interrupt_event = INVALID_HANDLE_VALUE; - std::atomic<ControlOp> m_pending_op{eControlOpNone}; - std::mutex m_mutex; - bool m_is_running = false; }; void ProcessWindows::SetPseudoConsoleHandle( diff --git a/lldb/test/Shell/Settings/TestFrameFormatColor.test b/lldb/test/Shell/Settings/TestFrameFormatColor.test index f30dafadf5919..970d7238e7512 100644 --- a/lldb/test/Shell/Settings/TestFrameFormatColor.test +++ b/lldb/test/Shell/Settings/TestFrameFormatColor.test @@ -9,4 +9,4 @@ c q # Check the ASCII escape code -# CHECK: {{\[[0-9;]+m}} +# CHECK: diff --git a/lldb/test/Shell/Settings/TestFrameFormatNoColor.test b/lldb/test/Shell/Settings/TestFrameFormatNoColor.test index 37906311c4f69..2bcdb8e82bd9d 100644 --- a/lldb/test/Shell/Settings/TestFrameFormatNoColor.test +++ b/lldb/test/Shell/Settings/TestFrameFormatNoColor.test @@ -9,4 +9,4 @@ c q # Check the ASCII escape code -# CHECK-NOT: {{\[[0-9;]+m}} +# CHECK-NOT: _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
