https://github.com/charles-zablit updated https://github.com/llvm/llvm-project/pull/183332
>From bb1357568a98d55964cc072fe18cbbdabd30ad82 Mon Sep 17 00:00:00 2001 From: Charles Zablit <[email protected]> Date: Wed, 25 Feb 2026 16:23:47 +0000 Subject: [PATCH 1/2] [lldb][windows] refactor ConnectionGenericFile's Read and Write methods --- .../windows/ConnectionGenericFileWindows.cpp | 246 +++++++----------- 1 file changed, 94 insertions(+), 152 deletions(-) diff --git a/lldb/source/Host/windows/ConnectionGenericFileWindows.cpp b/lldb/source/Host/windows/ConnectionGenericFileWindows.cpp index dd317a6e217c2..bdc60dea10048 100644 --- a/lldb/source/Host/windows/ConnectionGenericFileWindows.cpp +++ b/lldb/source/Host/windows/ConnectionGenericFileWindows.cpp @@ -19,37 +19,6 @@ using namespace lldb; using namespace lldb_private; -namespace { -// This is a simple helper class to package up the information needed to return -// from a Read/Write operation function. Since there is a lot of code to be -// run before exit regardless of whether the operation succeeded or failed, -// combined with many possible return paths, this is the cleanest way to -// represent it. -class ReturnInfo { -public: - void Set(size_t bytes, ConnectionStatus status, DWORD error_code) { - m_error = Status(error_code, eErrorTypeWin32); - m_bytes = bytes; - m_status = status; - } - - void Set(size_t bytes, ConnectionStatus status, llvm::StringRef error_msg) { - m_error = Status::FromErrorString(error_msg.data()); - m_bytes = bytes; - m_status = status; - } - - size_t GetBytes() const { return m_bytes; } - ConnectionStatus GetStatus() const { return m_status; } - const Status &GetError() const { return m_error; } - -private: - Status m_error; - size_t m_bytes; - ConnectionStatus m_status; -}; -} - ConnectionGenericFile::ConnectionGenericFile() : m_file(INVALID_HANDLE_VALUE), m_owns_file(false) { ::ZeroMemory(&m_overlapped, sizeof(m_overlapped)); @@ -164,150 +133,123 @@ size_t ConnectionGenericFile::Read(void *dst, size_t dst_len, const Timeout<std::micro> &timeout, lldb::ConnectionStatus &status, Status *error_ptr) { - ReturnInfo return_info; - BOOL result = 0; - DWORD bytes_read = 0; - if (error_ptr) error_ptr->Clear(); - if (!IsConnected()) { - return_info.Set(0, eConnectionStatusNoConnection, ERROR_INVALID_HANDLE); - goto finish; - } + auto finish = [&](size_t bytes, ConnectionStatus s, DWORD error_code) { + m_read_pending = s == eConnectionStatusInterrupted; + status = s; + if (error_ptr) + *error_ptr = Status(error_code, eErrorTypeWin32); + + // kBytesAvailableEvent is a manual reset event. Make sure it gets reset + // here so that any subsequent operations don't immediately see bytes + // available. + ResetEvent(m_event_handles[kBytesAvailableEvent]); + IncrementFilePointer(bytes); + Log *log = GetLog(LLDBLog::Connection); + LLDB_LOGF(log, + "%p ConnectionGenericFile::Read() handle = %p, dst = %p, " + "dst_len = %zu) => %zu, error = %s", + static_cast<void *>(this), m_file, dst, dst_len, bytes, + error_code ? Status(error_code, eErrorTypeWin32).AsCString() + : ""); + return bytes; + }; - if (m_read_pending) { - if (::GetOverlappedResult(m_file, &m_overlapped, &bytes_read, FALSE)) { - m_read_pending = false; - return bytes_read; - } - } + if (!IsConnected()) + return finish(0, eConnectionStatusNoConnection, ERROR_INVALID_HANDLE); - m_overlapped.hEvent = m_event_handles[kBytesAvailableEvent]; - - result = ::ReadFile(m_file, dst, dst_len, NULL, &m_overlapped); - if (result || ::GetLastError() == ERROR_IO_PENDING) { - if (!result) { - // The expected return path. The operation is pending. Wait for the - // operation to complete or be interrupted. - DWORD milliseconds = - timeout - ? std::chrono::duration_cast<std::chrono::milliseconds>(*timeout) - .count() - : INFINITE; - DWORD wait_result = ::WaitForMultipleObjects( - std::size(m_event_handles), m_event_handles, FALSE, milliseconds); - // All of the events are manual reset events, so make sure we reset them - // to non-signalled. - switch (wait_result) { - case WAIT_OBJECT_0 + kBytesAvailableEvent: - break; - case WAIT_OBJECT_0 + kInterruptEvent: - return_info.Set(0, eConnectionStatusInterrupted, 0); - goto finish; - case WAIT_TIMEOUT: - return_info.Set(0, eConnectionStatusTimedOut, 0); - goto finish; - case WAIT_FAILED: - return_info.Set(0, eConnectionStatusError, ::GetLastError()); - goto finish; + DWORD bytes_read = 0; + if (!m_read_pending) { + m_overlapped.hEvent = m_event_handles[kBytesAvailableEvent]; + + BOOL read_result = ::ReadFile(m_file, dst, dst_len, NULL, &m_overlapped); + if (read_result || ::GetLastError() == ERROR_IO_PENDING) { + if (!read_result) { + // The expected return path. The operation is pending. Wait for the + // operation to complete or be interrupted. + DWORD milliseconds = + timeout ? std::chrono::duration_cast<std::chrono::milliseconds>( + *timeout) + .count() + : INFINITE; + DWORD wait_result = ::WaitForMultipleObjects( + std::size(m_event_handles), m_event_handles, FALSE, milliseconds); + // All of the events are manual reset events, so make sure we reset them + // to non-signalled. + switch (wait_result) { + case WAIT_OBJECT_0 + kBytesAvailableEvent: + break; + case WAIT_OBJECT_0 + kInterruptEvent: + return finish(0, eConnectionStatusInterrupted, 0); + case WAIT_TIMEOUT: + return finish(0, eConnectionStatusTimedOut, 0); + case WAIT_FAILED: + return finish(0, eConnectionStatusError, ::GetLastError()); + } } + } else if (::GetLastError() == ERROR_BROKEN_PIPE) { + // The write end of a pipe was closed. This is equivalent to EOF. + return finish(0, eConnectionStatusEndOfFile, 0); + } else { + // An unknown error occurred. Fail out. + return finish(0, eConnectionStatusError, ::GetLastError()); } - // The data is ready. Figure out how much was read and return; - if (!::GetOverlappedResult(m_file, &m_overlapped, &bytes_read, FALSE)) { - DWORD result_error = ::GetLastError(); - // ERROR_OPERATION_ABORTED occurs when someone calls Disconnect() during - // a blocking read. This triggers a call to CancelIoEx, which causes the - // operation to complete and the result to be ERROR_OPERATION_ABORTED. - if (result_error == ERROR_HANDLE_EOF || - result_error == ERROR_OPERATION_ABORTED || - result_error == ERROR_BROKEN_PIPE) - return_info.Set(bytes_read, eConnectionStatusEndOfFile, 0); - else - return_info.Set(bytes_read, eConnectionStatusError, result_error); - } else if (bytes_read == 0) - return_info.Set(bytes_read, eConnectionStatusEndOfFile, 0); - else - return_info.Set(bytes_read, eConnectionStatusSuccess, 0); - - goto finish; - } else if (::GetLastError() == ERROR_BROKEN_PIPE) { - // The write end of a pipe was closed. This is equivalent to EOF. - return_info.Set(0, eConnectionStatusEndOfFile, 0); - } else { - // An unknown error occurred. Fail out. - return_info.Set(0, eConnectionStatusError, ::GetLastError()); } - goto finish; - -finish: - m_read_pending = return_info.GetStatus() == eConnectionStatusInterrupted; - status = return_info.GetStatus(); - if (error_ptr) - *error_ptr = return_info.GetError().Clone(); - // kBytesAvailableEvent is a manual reset event. Make sure it gets reset - // here so that any subsequent operations don't immediately see bytes - // available. - ResetEvent(m_event_handles[kBytesAvailableEvent]); - - IncrementFilePointer(return_info.GetBytes()); - Log *log = GetLog(LLDBLog::Connection); - LLDB_LOGF(log, - "%p ConnectionGenericFile::Read() handle = %p, dst = %p, " - "dst_len = %zu) => %zu, error = %s", - static_cast<void *>(this), m_file, dst, dst_len, - return_info.GetBytes(), return_info.GetError().AsCString()); + // The data is ready. Figure out how much was read and return; + if (!::GetOverlappedResult(m_file, &m_overlapped, &bytes_read, FALSE)) { + DWORD result_error = ::GetLastError(); + // ERROR_OPERATION_ABORTED occurs when someone calls Disconnect() during + // a blocking read. This triggers a call to CancelIoEx, which causes the + // operation to complete and the result to be ERROR_OPERATION_ABORTED. + if (result_error == ERROR_HANDLE_EOF || + result_error == ERROR_OPERATION_ABORTED || + result_error == ERROR_BROKEN_PIPE) + return finish(bytes_read, eConnectionStatusEndOfFile, 0); + return finish(bytes_read, eConnectionStatusError, result_error); + } - return return_info.GetBytes(); + if (bytes_read == 0) + return finish(0, eConnectionStatusEndOfFile, 0); + return finish(bytes_read, eConnectionStatusSuccess, 0); } size_t ConnectionGenericFile::Write(const void *src, size_t src_len, lldb::ConnectionStatus &status, Status *error_ptr) { - ReturnInfo return_info; - DWORD bytes_written = 0; - BOOL result = 0; - if (error_ptr) error_ptr->Clear(); - if (!IsConnected()) { - return_info.Set(0, eConnectionStatusNoConnection, ERROR_INVALID_HANDLE); - goto finish; - } - - m_overlapped.hEvent = NULL; + auto finish = [&](size_t bytes, ConnectionStatus s, DWORD error_code) { + status = s; + if (error_ptr) + *error_ptr = Status(error_code, eErrorTypeWin32); + IncrementFilePointer(bytes); + Log *log = GetLog(LLDBLog::Connection); + LLDB_LOGF(log, + "%p ConnectionGenericFile::Write() handle = %p, src = %p, " + "src_len = %zu) => %zu, error = %s", + static_cast<void *>(this), m_file, src, src_len, bytes, + Status(error_code, eErrorTypeWin32).AsCString()); + return bytes; + }; - // Writes are not interruptible like reads are, so just block until it's - // done. - result = ::WriteFile(m_file, src, src_len, NULL, &m_overlapped); - if (!result && ::GetLastError() != ERROR_IO_PENDING) { - return_info.Set(0, eConnectionStatusError, ::GetLastError()); - goto finish; - } + if (!IsConnected()) + return finish(0, eConnectionStatusNoConnection, ERROR_INVALID_HANDLE); - if (!::GetOverlappedResult(m_file, &m_overlapped, &bytes_written, TRUE)) { - return_info.Set(bytes_written, eConnectionStatusError, ::GetLastError()); - goto finish; - } + m_overlapped.hEvent = NULL; - return_info.Set(bytes_written, eConnectionStatusSuccess, 0); - goto finish; + DWORD bytes_written = 0; + BOOL result = ::WriteFile(m_file, src, src_len, NULL, &m_overlapped); + if (!result && ::GetLastError() != ERROR_IO_PENDING) + return finish(0, eConnectionStatusError, ::GetLastError()); -finish: - status = return_info.GetStatus(); - if (error_ptr) - *error_ptr = return_info.GetError().Clone(); + if (!::GetOverlappedResult(m_file, &m_overlapped, &bytes_written, TRUE)) + return finish(bytes_written, eConnectionStatusError, ::GetLastError()); - IncrementFilePointer(return_info.GetBytes()); - Log *log = GetLog(LLDBLog::Connection); - LLDB_LOGF(log, - "%p ConnectionGenericFile::Write() handle = %p, src = %p, " - "src_len = %zu) => %zu, error = %s", - static_cast<void *>(this), m_file, src, src_len, - return_info.GetBytes(), return_info.GetError().AsCString()); - return return_info.GetBytes(); + return finish(bytes_written, eConnectionStatusSuccess, 0); } std::string ConnectionGenericFile::GetURI() { return m_uri; } >From 2fe560546efc907ccbee6b9aa6ac1a3798dba9b0 Mon Sep 17 00:00:00 2001 From: Charles Zablit <[email protected]> Date: Wed, 25 Feb 2026 17:54:43 +0000 Subject: [PATCH 2/2] fixup! [lldb][windows] refactor ConnectionGenericFile's Read and Write methods --- .../windows/ConnectionGenericFileWindows.cpp | 62 ++++++++++--------- 1 file changed, 33 insertions(+), 29 deletions(-) diff --git a/lldb/source/Host/windows/ConnectionGenericFileWindows.cpp b/lldb/source/Host/windows/ConnectionGenericFileWindows.cpp index bdc60dea10048..c873ce963f535 100644 --- a/lldb/source/Host/windows/ConnectionGenericFileWindows.cpp +++ b/lldb/source/Host/windows/ConnectionGenericFileWindows.cpp @@ -160,45 +160,49 @@ size_t ConnectionGenericFile::Read(void *dst, size_t dst_len, if (!IsConnected()) return finish(0, eConnectionStatusNoConnection, ERROR_INVALID_HANDLE); - DWORD bytes_read = 0; + BOOL read_result; + DWORD read_error; if (!m_read_pending) { m_overlapped.hEvent = m_event_handles[kBytesAvailableEvent]; + read_result = ::ReadFile(m_file, dst, dst_len, NULL, &m_overlapped); + read_error = ::GetLastError(); + } - BOOL read_result = ::ReadFile(m_file, dst, dst_len, NULL, &m_overlapped); - if (read_result || ::GetLastError() == ERROR_IO_PENDING) { - if (!read_result) { - // The expected return path. The operation is pending. Wait for the - // operation to complete or be interrupted. - DWORD milliseconds = - timeout ? std::chrono::duration_cast<std::chrono::milliseconds>( - *timeout) - .count() - : INFINITE; - DWORD wait_result = ::WaitForMultipleObjects( - std::size(m_event_handles), m_event_handles, FALSE, milliseconds); - // All of the events are manual reset events, so make sure we reset them - // to non-signalled. - switch (wait_result) { - case WAIT_OBJECT_0 + kBytesAvailableEvent: - break; - case WAIT_OBJECT_0 + kInterruptEvent: - return finish(0, eConnectionStatusInterrupted, 0); - case WAIT_TIMEOUT: - return finish(0, eConnectionStatusTimedOut, 0); - case WAIT_FAILED: - return finish(0, eConnectionStatusError, ::GetLastError()); - } - } - } else if (::GetLastError() == ERROR_BROKEN_PIPE) { + if (!m_read_pending && !read_result && read_error != ERROR_IO_PENDING) { + if (read_error == ERROR_BROKEN_PIPE) { // The write end of a pipe was closed. This is equivalent to EOF. return finish(0, eConnectionStatusEndOfFile, 0); - } else { - // An unknown error occurred. Fail out. + } + // An unknown error occurred. Fail out. + return finish(0, eConnectionStatusError, ::GetLastError()); + } + + if (!read_result || m_read_pending) { + // The expected return path. The operation is pending. Wait for the + // operation to complete or be interrupted. + DWORD milliseconds = + timeout + ? std::chrono::duration_cast<std::chrono::milliseconds>(*timeout) + .count() + : INFINITE; + DWORD wait_result = ::WaitForMultipleObjects( + std::size(m_event_handles), m_event_handles, FALSE, milliseconds); + // All of the events are manual reset events, so make sure we reset them + // to non-signalled. + switch (wait_result) { + case WAIT_OBJECT_0 + kBytesAvailableEvent: + break; + case WAIT_OBJECT_0 + kInterruptEvent: + return finish(0, eConnectionStatusInterrupted, 0); + case WAIT_TIMEOUT: + return finish(0, eConnectionStatusTimedOut, 0); + case WAIT_FAILED: return finish(0, eConnectionStatusError, ::GetLastError()); } } // The data is ready. Figure out how much was read and return; + DWORD bytes_read = 0; if (!::GetOverlappedResult(m_file, &m_overlapped, &bytes_read, FALSE)) { DWORD result_error = ::GetLastError(); // ERROR_OPERATION_ABORTED occurs when someone calls Disconnect() during _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
