llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: Charles Zablit (charles-zablit) <details> <summary>Changes</summary> Add a new gdb-remote packet, `QSetSTDIOWindowSize:cols=N;rows=N`, to send the dimension of the terminal to the debuggee. On Windows, this is required to open the ConPTY with the correct dimensions. This is a follow up to: - https://github.com/llvm/llvm-project/pull/201141 --- Full diff: https://github.com/llvm/llvm-project/pull/201141.diff 11 Files Affected: - (modified) lldb/include/lldb/Host/ProcessLaunchInfo.h (+18) - (modified) lldb/include/lldb/Host/windows/PseudoConsole.h (+3-1) - (modified) lldb/include/lldb/Utility/StringExtractorGDBRemote.h (+1) - (modified) lldb/source/Host/common/ProcessLaunchInfo.cpp (+2-1) - (modified) lldb/source/Host/windows/PseudoConsole.cpp (+15-8) - (modified) lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp (+19) - (modified) lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h (+3) - (modified) lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp (+30) - (modified) lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.h (+2) - (modified) lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp (+26) - (modified) lldb/source/Utility/StringExtractorGDBRemote.cpp (+2) ``````````diff diff --git a/lldb/include/lldb/Host/ProcessLaunchInfo.h b/lldb/include/lldb/Host/ProcessLaunchInfo.h index 39f85205999de..7b22e535c3268 100644 --- a/lldb/include/lldb/Host/ProcessLaunchInfo.h +++ b/lldb/include/lldb/Host/ProcessLaunchInfo.h @@ -174,6 +174,23 @@ class ProcessLaunchInfo : public ProcessInfo { return m_flags.Test(lldb::eLaunchFlagDetachOnError); } + /// Terminal window dimensions to use when the launcher creates a + /// pseudo-terminal for the inferior's stdio. + struct STDIOWindowSize { + uint16_t cols = 0; + uint16_t rows = 0; + bool IsSet() const { return cols != 0 && rows != 0; } + }; + + void SetSTDIOWindowSize(uint16_t cols, uint16_t rows) { + m_stdio_window_size.cols = cols; + m_stdio_window_size.rows = rows; + } + + const STDIOWindowSize &GetSTDIOWindowSize() const { + return m_stdio_window_size; + } + protected: FileSpec m_working_dir; std::string m_plugin_name; @@ -186,6 +203,7 @@ class ProcessLaunchInfo : public ProcessInfo { Host::MonitorChildProcessCallback m_monitor_callback; std::string m_event_data; // A string passed to the plugin launch, having no // meaning to the upper levels of lldb. + STDIOWindowSize m_stdio_window_size; }; } diff --git a/lldb/include/lldb/Host/windows/PseudoConsole.h b/lldb/include/lldb/Host/windows/PseudoConsole.h index cf910354f1bca..b98402922bfb3 100644 --- a/lldb/include/lldb/Host/windows/PseudoConsole.h +++ b/lldb/include/lldb/Host/windows/PseudoConsole.h @@ -44,11 +44,13 @@ class PseudoConsole { /// 80x25. Also sets up the associated STDIN/STDOUT pipes and responds to /// the cursor-position query that ConPTY emits at startup. /// + /// \param req_cols, req_rows Optional terminal dimensions. + /// /// \return /// An llvm::Error if the ConPTY could not be created, or if ConPTY is /// not available on this version of Windows, llvm::Error::success() /// otherwise. - llvm::Error OpenPseudoConsole(); + llvm::Error OpenPseudoConsole(uint16_t req_cols = 0, uint16_t req_rows = 0); /// Creates a pair of anonymous pipes to use for stdio instead of a ConPTY. /// diff --git a/lldb/include/lldb/Utility/StringExtractorGDBRemote.h b/lldb/include/lldb/Utility/StringExtractorGDBRemote.h index 85ec27cac9ac0..09693a5f5e86b 100644 --- a/lldb/include/lldb/Utility/StringExtractorGDBRemote.h +++ b/lldb/include/lldb/Utility/StringExtractorGDBRemote.h @@ -75,6 +75,7 @@ class StringExtractorGDBRemote : public StringExtractor { eServerPacketType_QSetSTDIN, eServerPacketType_QSetSTDOUT, eServerPacketType_QSetSTDERR, + eServerPacketType_QSetSTDIOWindowSize, eServerPacketType_QSetWorkingDir, eServerPacketType_QStartNoAckMode, eServerPacketType_qPathComplete, diff --git a/lldb/source/Host/common/ProcessLaunchInfo.cpp b/lldb/source/Host/common/ProcessLaunchInfo.cpp index b5b82c7475822..b939904734073 100644 --- a/lldb/source/Host/common/ProcessLaunchInfo.cpp +++ b/lldb/source/Host/common/ProcessLaunchInfo.cpp @@ -244,7 +244,8 @@ llvm::Error ProcessLaunchInfo::SetUpPtyRedirection() { LLDB_LOG(log, "Generating a pty to use for stdin/out/err"); #ifdef _WIN32 - if (llvm::Error Err = m_pty->OpenPseudoConsole()) + if (llvm::Error Err = m_pty->OpenPseudoConsole(m_stdio_window_size.cols, + m_stdio_window_size.rows)) return Err; return llvm::Error::success(); #else diff --git a/lldb/source/Host/windows/PseudoConsole.cpp b/lldb/source/Host/windows/PseudoConsole.cpp index 4d98a54673795..2b8293393bfdf 100644 --- a/lldb/source/Host/windows/PseudoConsole.cpp +++ b/lldb/source/Host/windows/PseudoConsole.cpp @@ -93,7 +93,8 @@ llvm::Error PseudoConsole::CreateOverlappedPipePair(HANDLE &out_read, PseudoConsole::~PseudoConsole() { Reset(); } -llvm::Error PseudoConsole::OpenPseudoConsole() { +llvm::Error PseudoConsole::OpenPseudoConsole(uint16_t req_cols, + uint16_t req_rows) { Reset(); if (!kernel32.IsConPTYAvailable()) @@ -124,13 +125,19 @@ llvm::Error PseudoConsole::OpenPseudoConsole() { // if we can't query the real console. int cursorRow = consoleSize.Y; int cursorCol = 1; - CONSOLE_SCREEN_BUFFER_INFO csbi; - if (GetConsoleScreenBufferInfo(GetStdHandle(STD_OUTPUT_HANDLE), &csbi)) { - consoleSize = { - static_cast<SHORT>(csbi.srWindow.Right - csbi.srWindow.Left + 1), - static_cast<SHORT>(csbi.srWindow.Bottom - csbi.srWindow.Top + 1)}; - cursorRow = csbi.dwCursorPosition.Y - csbi.srWindow.Top + 1; - cursorCol = csbi.dwCursorPosition.X + 1; + if (req_cols != 0 && req_rows != 0) { + consoleSize = {static_cast<SHORT>(req_cols), static_cast<SHORT>(req_rows)}; + cursorRow = consoleSize.Y; + cursorCol = 1; + } else { + CONSOLE_SCREEN_BUFFER_INFO csbi; + if (GetConsoleScreenBufferInfo(GetStdHandle(STD_OUTPUT_HANDLE), &csbi)) { + consoleSize = { + static_cast<SHORT>(csbi.srWindow.Right - csbi.srWindow.Left + 1), + static_cast<SHORT>(csbi.srWindow.Bottom - csbi.srWindow.Top + 1)}; + cursorRow = csbi.dwCursorPosition.Y - csbi.srWindow.Top + 1; + cursorCol = csbi.dwCursorPosition.X + 1; + } } HPCON hPC = INVALID_HANDLE_VALUE; HRESULT hr = diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp index 8df7936786b04..d1f15b6ba5490 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp @@ -1957,6 +1957,25 @@ int GDBRemoteCommunicationClient::SetSTDERR(const FileSpec &file_spec) { return -1; } +int GDBRemoteCommunicationClient::SetSTDIOWindowSize(uint16_t cols, + uint16_t rows) { + if (cols == 0 || rows == 0) + return -1; + StreamString packet; + packet.Printf("QSetSTDIOWindowSize:cols=%u;rows=%u", + static_cast<unsigned>(cols), static_cast<unsigned>(rows)); + StringExtractorGDBRemote response; + if (SendPacketAndWaitForResponse(packet.GetString(), response) != + PacketResult::Success) + return -1; + if (response.IsOKResponse()) + return 0; + if (response.IsUnsupportedResponse()) + return 0; + uint8_t error = response.GetError(); + return error ? error : -1; +} + bool GDBRemoteCommunicationClient::GetWorkingDir(FileSpec &working_dir) { StringExtractorGDBRemote response; if (SendPacketAndWaitForResponse("qGetWorkingDir", response) == diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h index 79ca0bcd3ed22..5084811f8c712 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h @@ -144,6 +144,9 @@ class GDBRemoteCommunicationClient : public GDBRemoteClientBase { int SetSTDOUT(const FileSpec &file_spec); int SetSTDERR(const FileSpec &file_spec); + /// Send the dimensions of the user's stdio terminal window to the server. + int SetSTDIOWindowSize(uint16_t cols, uint16_t rows); + /// Sets the disable ASLR flag to \a enable for a process that will /// be launched with the 'A' packet. /// diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp index 16ded2c657d54..5422be805dbf2 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp @@ -117,6 +117,9 @@ GDBRemoteCommunicationServerCommon::GDBRemoteCommunicationServerCommon() RegisterMemberFunctionHandler( StringExtractorGDBRemote::eServerPacketType_QSetSTDOUT, &GDBRemoteCommunicationServerCommon::Handle_QSetSTDOUT); + RegisterMemberFunctionHandler( + StringExtractorGDBRemote::eServerPacketType_QSetSTDIOWindowSize, + &GDBRemoteCommunicationServerCommon::Handle_QSetSTDIOWindowSize); RegisterMemberFunctionHandler( StringExtractorGDBRemote::eServerPacketType_qSpeedTest, &GDBRemoteCommunicationServerCommon::Handle_qSpeedTest); @@ -963,6 +966,33 @@ GDBRemoteCommunicationServerCommon::Handle_QSetSTDERR( return SendErrorResponse(17); } +GDBRemoteCommunication::PacketResult +GDBRemoteCommunicationServerCommon::Handle_QSetSTDIOWindowSize( + StringExtractorGDBRemote &packet) { + // Format: "QSetSTDIOWindowSize:cols=N;rows=N" + packet.SetFilePos(::strlen("QSetSTDIOWindowSize:")); + llvm::StringRef body = packet.GetStringRef().substr(packet.GetFilePos()); + + uint16_t cols = 0; + uint16_t rows = 0; + llvm::SmallVector<llvm::StringRef, 4> fields; + body.split(fields, ';'); + for (llvm::StringRef field : fields) { + auto [key, value] = field.split('='); + unsigned parsed = 0; + if (value.empty() || value.getAsInteger(10, parsed) || parsed > UINT16_MAX) + continue; + if (key == "cols") + cols = static_cast<uint16_t>(parsed); + else if (key == "rows") + rows = static_cast<uint16_t>(parsed); + } + if (cols == 0 || rows == 0) + return SendErrorResponse(17); + m_process_launch_info.SetSTDIOWindowSize(cols, rows); + return SendOKResponse(); +} + GDBRemoteCommunication::PacketResult GDBRemoteCommunicationServerCommon::Handle_qLaunchSuccess( StringExtractorGDBRemote &packet) { diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.h index b4f1eb3e61c41..aa756c81a791e 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.h +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.h @@ -99,6 +99,8 @@ class GDBRemoteCommunicationServerCommon : public GDBRemoteCommunicationServer { PacketResult Handle_QSetSTDERR(StringExtractorGDBRemote &packet); + PacketResult Handle_QSetSTDIOWindowSize(StringExtractorGDBRemote &packet); + PacketResult Handle_qLaunchSuccess(StringExtractorGDBRemote &packet); PacketResult Handle_QEnvironment(StringExtractorGDBRemote &packet); diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp index f6eaf5851338b..fb3427c512121 100644 --- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp @@ -12,6 +12,7 @@ #include <cstdlib> #if LLDB_ENABLE_POSIX #include <netinet/in.h> +#include <sys/ioctl.h> #include <sys/mman.h> #include <sys/socket.h> #include <unistd.h> @@ -20,6 +21,9 @@ #if defined(__APPLE__) #include <sys/sysctl.h> #endif +#ifdef _WIN32 +#include "lldb/Host/windows/windows.h" +#endif #include <ctime> #include <sys/types.h> @@ -187,6 +191,25 @@ class PluginProperties : public Properties { std::chrono::seconds ResumeTimeout() { return std::chrono::seconds(5); } +static std::pair<uint16_t, uint16_t> GetClientTerminalSize() { +#ifdef _WIN32 + CONSOLE_SCREEN_BUFFER_INFO csbi{}; + HANDLE h = ::GetStdHandle(STD_OUTPUT_HANDLE); + if (h != INVALID_HANDLE_VALUE && ::GetConsoleScreenBufferInfo(h, &csbi)) { + int cols = csbi.srWindow.Right - csbi.srWindow.Left + 1; + int rows = csbi.srWindow.Bottom - csbi.srWindow.Top + 1; + if (cols > 0 && rows > 0) + return {static_cast<uint16_t>(cols), static_cast<uint16_t>(rows)}; + } +#elif LLDB_ENABLE_POSIX + struct winsize ws{}; + if (::ioctl(STDOUT_FILENO, TIOCGWINSZ, &ws) == 0 && ws.ws_col > 0 && + ws.ws_row > 0) + return {ws.ws_col, ws.ws_row}; +#endif + return {0, 0}; +} + } // namespace static PluginProperties &GetGlobalPluginProperties() { @@ -813,6 +836,9 @@ Status ProcessGDBRemote::DoLaunch(lldb_private::Module *exe_module, if (stderr_file_spec) m_gdb_comm.SetSTDERR(stderr_file_spec); + auto [terminal_cols, terminal_rows] = GetClientTerminalSize(); + m_gdb_comm.SetSTDIOWindowSize(terminal_cols, terminal_rows); + m_gdb_comm.SetDisableASLR(launch_flags & eLaunchFlagDisableASLR); m_gdb_comm.SetDetachOnError(launch_flags & eLaunchFlagDetachOnError); diff --git a/lldb/source/Utility/StringExtractorGDBRemote.cpp b/lldb/source/Utility/StringExtractorGDBRemote.cpp index 7b46bef933ac2..0bd20b47b42b8 100644 --- a/lldb/source/Utility/StringExtractorGDBRemote.cpp +++ b/lldb/source/Utility/StringExtractorGDBRemote.cpp @@ -120,6 +120,8 @@ StringExtractorGDBRemote::GetServerPacketType() const { return eServerPacketType_QSetSTDOUT; if (PACKET_STARTS_WITH("QSetSTDERR:")) return eServerPacketType_QSetSTDERR; + if (PACKET_STARTS_WITH("QSetSTDIOWindowSize:")) + return eServerPacketType_QSetSTDIOWindowSize; if (PACKET_STARTS_WITH("QSetWorkingDir:")) return eServerPacketType_QSetWorkingDir; if (PACKET_STARTS_WITH("QSetLogging:")) `````````` </details> https://github.com/llvm/llvm-project/pull/201141 _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
