diff --git a/lldb/include/lldb/Core/Communication.h b/lldb/include/lldb/Core/Communication.h --- a/lldb/include/lldb/Core/Communication.h +++ b/lldb/include/lldb/Core/Communication.h @@ -209,6 +209,22 @@ size_t Write(const void *src, size_t src_len, lldb::ConnectionStatus &status, Status *error_ptr); + /// Repeatedly attempt writing until either \a src_len bytes are written + /// or a permanent failure occurs. + /// + /// \param[in] src + /// A source buffer that must be at least \a src_len bytes + /// long. + /// + /// \param[in] src_len + /// The number of bytes to attempt to write, and also the + /// number of bytes are currently available in \a src. + /// + /// \return + /// The number of bytes actually Written. + size_t WriteAll(const void *src, size_t src_len, + lldb::ConnectionStatus &status, Status *error_ptr); + /// Sets the connection that it to be used by this class. /// /// By making a communication class that uses different connections it diff --git a/lldb/source/Core/Communication.cpp b/lldb/source/Core/Communication.cpp --- a/lldb/source/Core/Communication.cpp +++ b/lldb/source/Core/Communication.cpp @@ -189,6 +189,16 @@ return 0; } +size_t Communication::WriteAll(const void *src, size_t src_len, + ConnectionStatus &status, Status *error_ptr) { + size_t total_written = 0; + do + total_written += Write(static_cast(src) + total_written, + src_len - total_written, status, error_ptr); + while (status == eConnectionStatusSuccess && total_written < src_len); + return total_written; +} + bool Communication::StartReadThread(Status *error_ptr) { if (error_ptr) error_ptr->Clear(); diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp @@ -101,7 +101,7 @@ Log *log(ProcessGDBRemoteLog::GetLogIfAllCategoriesSet(GDBR_LOG_PACKETS)); ConnectionStatus status = eConnectionStatusSuccess; char ch = '+'; - const size_t bytes_written = Write(&ch, 1, status, nullptr); + const size_t bytes_written = WriteAll(&ch, 1, status, nullptr); LLDB_LOGF(log, "<%4" PRIu64 "> send packet: %c", (uint64_t)bytes_written, ch); m_history.AddPacket(ch, GDBRemotePacket::ePacketTypeSend, bytes_written); return bytes_written; @@ -111,7 +111,7 @@ Log *log(ProcessGDBRemoteLog::GetLogIfAllCategoriesSet(GDBR_LOG_PACKETS)); ConnectionStatus status = eConnectionStatusSuccess; char ch = '-'; - const size_t bytes_written = Write(&ch, 1, status, nullptr); + const size_t bytes_written = WriteAll(&ch, 1, status, nullptr); LLDB_LOGF(log, "<%4" PRIu64 "> send packet: %c", (uint64_t)bytes_written, ch); m_history.AddPacket(ch, GDBRemotePacket::ePacketTypeSend, bytes_written); return bytes_written; @@ -137,7 +137,7 @@ ConnectionStatus status = eConnectionStatusSuccess; const char *packet_data = packet.data(); const size_t packet_length = packet.size(); - size_t bytes_written = Write(packet_data, packet_length, status, nullptr); + size_t bytes_written = WriteAll(packet_data, packet_length, status, nullptr); if (log) { size_t binary_start_offset = 0; if (strncmp(packet_data, "$vFile:pwrite:", strlen("$vFile:pwrite:")) == diff --git a/lldb/unittests/Core/CommunicationTest.cpp b/lldb/unittests/Core/CommunicationTest.cpp --- a/lldb/unittests/Core/CommunicationTest.cpp +++ b/lldb/unittests/Core/CommunicationTest.cpp @@ -7,11 +7,18 @@ //===----------------------------------------------------------------------===// #include "lldb/Core/Communication.h" +#include "lldb/Host/Config.h" #include "lldb/Host/ConnectionFileDescriptor.h" #include "lldb/Host/Pipe.h" #include "llvm/Testing/Support/Error.h" #include "gtest/gtest.h" +#include + +#if LLDB_ENABLE_POSIX +#include +#endif + using namespace lldb_private; #ifndef _WIN32 @@ -35,3 +42,48 @@ ASSERT_TRUE(comm.StopReadThread()); } #endif + +#if LLDB_ENABLE_POSIX +TEST(CommunicationTest, WriteAll) { + Pipe pipe; + ASSERT_THAT_ERROR(pipe.CreateNew(/*child_process_inherit=*/false).ToError(), + llvm::Succeeded()); + + // Make the write end non-blocking in order to easily reproduce a partial + // write. + int write_fd = pipe.ReleaseWriteFileDescriptor(); + int flags = fcntl(write_fd, F_GETFL); + ASSERT_NE(flags, -1); + ASSERT_NE(fcntl(write_fd, F_SETFL, flags | O_NONBLOCK), -1); + + ConnectionFileDescriptor read_conn{pipe.ReleaseReadFileDescriptor(), + /*owns_fd=*/true}; + Communication write_comm("test"); + write_comm.SetConnection( + std::make_unique(write_fd, /*owns_fd=*/true)); + + std::thread read_thread{[&read_conn]() { + // Read using a smaller buffer to increase chances of partial write. + char buf[128 * 1024]; + lldb::ConnectionStatus conn_status; + + do { + read_conn.Read(buf, sizeof(buf), std::chrono::seconds(1), conn_status, + nullptr); + } while (conn_status != lldb::eConnectionStatusEndOfFile); + }}; + + // Write 1 MiB of data into the pipe. + lldb::ConnectionStatus conn_status; + Status error; + std::vector data(1024 * 1024, 0x80); + EXPECT_EQ(write_comm.WriteAll(data.data(), data.size(), conn_status, &error), + data.size()); + EXPECT_EQ(conn_status, lldb::eConnectionStatusSuccess); + EXPECT_FALSE(error.Fail()); + + // Close the write end in order to trigger EOF. + write_comm.Disconnect(); + read_thread.join(); +} +#endif