[lldb] Assorted improvements to the Pipe class

lldb/source/Host/common/Socket.cpp

@@ -99,7 +99,8 @@ Status SharedSocket::CompleteSending(lldb::pid_t child_pid) {
     return Status::FromError(num_bytes.takeError());
   if (*num_bytes != sizeof(protocol_info))
     return Status::FromErrorStringWithFormatv(
-        "Write(WSAPROTOCOL_INFO) failed: {0} bytes", *num_bytes);
+        "Write(WSAPROTOCOL_INFO) failed: wrote {0}/{1} bytes", *num_bytes,
+        sizeof(protocol_info));
 #endif
   return Status();
 }
@@ -117,7 +118,8 @@ Status SharedSocket::GetNativeSocket(shared_fd_t fd, NativeSocket &socket) {
       return Status::FromError(num_bytes.takeError());
     if (*num_bytes != sizeof(protocol_info)) {
       return Status::FromErrorStringWithFormatv(
-          "socket_pipe.Read(WSAPROTOCOL_INFO) failed: {0} bytes", *num_bytes);
+          "Read(WSAPROTOCOL_INFO) failed: read {0}/{1} bytes", *num_bytes,
+          sizeof(protocol_info));
     }
   }
   socket = ::WSASocket(FROM_PROTOCOL_INFO, FROM_PROTOCOL_INFO,

lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp

@@ -1162,9 +1162,9 @@ Status GDBRemoteCommunication::StartDebugserverProcess(
           char buf[10];
           if (llvm::Expected<size_t> num_bytes = socket_pipe.Read(
                   buf, std::size(buf), std::chrono::seconds(10))) {
-            port_str.append(buf, *num_bytes);
             if (*num_bytes == 0)
               break;
+            port_str.append(buf, *num_bytes);
           } else {
             error = Status::FromError(num_bytes.takeError());
           }

lldb/unittests/Host/PipeTest.cpp

@@ -10,10 +10,14 @@
 #include "TestingSupport/SubsystemRAII.h"
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Host/HostInfo.h"
+#include "llvm/Testing/Support/Error.h"
 #include "gtest/gtest.h"
+#include <chrono>
 #include <fcntl.h>
 #include <numeric>
+#include <thread>
 #include <vector>
+#include <future>
 
 using namespace lldb_private;
 
@@ -144,3 +148,52 @@ TEST_F(PipeTest, WriteWithTimeout) {
       pipe.Write(write_ptr, write_chunk_size, std::chrono::milliseconds(10)),
       llvm::Succeeded());
 }
+
+TEST_F(PipeTest, ReadWithTimeout) {
+  Pipe pipe;
+  ASSERT_THAT_ERROR(pipe.CreateNew(false).ToError(), llvm::Succeeded());
+
+  char buf[100];
+  // The pipe is initially empty. A polling read returns immediately.
+  ASSERT_THAT_EXPECTED(pipe.Read(buf, sizeof(buf), std::chrono::seconds(0)),
+                       llvm::Failed());
+
+  // With a timeout, we should wait for at least this amount of time (but not
+  // too much).
+  auto start = std::chrono::steady_clock::now();
+  ASSERT_THAT_EXPECTED(
+      pipe.Read(buf, sizeof(buf), std::chrono::milliseconds(200)),
+      llvm::Failed());
+  auto dur = std::chrono::steady_clock::now() - start;
+  EXPECT_GT(dur, std::chrono::milliseconds(200));
+  EXPECT_LT(dur, std::chrono::seconds(2));
+
+  // Write something into the pipe, and read it back. The blocking read call
+  // should return even though it hasn't filled the buffer.
+  llvm::StringRef hello_world("Hello world!");
+  ASSERT_THAT_EXPECTED(pipe.Write(hello_world.data(), hello_world.size()),
+                       llvm::HasValue(hello_world.size()));
+  ASSERT_THAT_EXPECTED(pipe.Read(buf, sizeof(buf)),
+                       llvm::HasValue(hello_world.size()));
+  EXPECT_EQ(llvm::StringRef(buf, hello_world.size()), hello_world);
+
+  // Now write something and try to read it in chunks.
+  memset(buf, 0, sizeof(buf));
+  ASSERT_THAT_EXPECTED(pipe.Write(hello_world.data(), hello_world.size()),
+                       llvm::HasValue(hello_world.size()));
+  ASSERT_THAT_EXPECTED(pipe.Read(buf, 4), llvm::HasValue(4));
+  ASSERT_THAT_EXPECTED(pipe.Read(buf + 4, sizeof(buf) - 4),
+                       llvm::HasValue(hello_world.size() - 4));
+  EXPECT_EQ(llvm::StringRef(buf, hello_world.size()), hello_world);
+
+  // A blocking read should wait until the data arrives.
+  memset(buf, 0, sizeof(buf));
+  std::future<llvm::Expected<size_t>> future_num_bytes = std::async(
+      std::launch::async, [&] { return pipe.Read(buf, sizeof(buf)); });
+  std::this_thread::sleep_for(std::chrono::seconds(1));
+  ASSERT_THAT_EXPECTED(pipe.Write(hello_world.data(), hello_world.size()),
+                       llvm::HasValue(hello_world.size()));
+  ASSERT_THAT_EXPECTED(future_num_bytes.get(),
+                       llvm::HasValue(hello_world.size()));
+  EXPECT_EQ(llvm::StringRef(buf, hello_world.size()), hello_world);
+}