[llvm] [orc-rt] Make socket unit tests portable; build them on POSIX. (PR #226662)

Lang Hames via llvm-commits llvm-commits at lists.llvm.org
Sat Sep 26 01:51:29 PDT 2026


https://github.com/lhames created https://github.com/llvm/llvm-project/pull/226662

SimpleRemoteCAOverSocketTest.cpp tests a POSIX-only implementation, so move it to the POSIX source list. Its tests are portable, so move the fixture's POSIX code into SocketTestUtils (makeStreamSocketPair, sendAll, recvAll, makeNativeNonStreamSocket) rather than moving the file under sys/posix; a Windows transport will need only Windows definitions of those helpers.

makeStreamSocketPair documents the properties the tests rely on beyond a connected stream pair: neither direction buffers StallingPayloadSize bytes, and closing an end delivers what it already sent even if it has unread data. AF_UNIX provides both; a loopback TCP pair may not.

SocketConnectorTest.cpp tests fd adoption with pipe and fcntl, so it is a test of the POSIX implementation: move it to sys/posix/.

Assisted-by: Claude

>From 4134ac3686e541d16b4bef54102c033f816b28c1 Mon Sep 17 00:00:00 2001
From: Lang Hames <lhames at gmail.com>
Date: Sat, 26 Sep 2026 17:45:19 +1000
Subject: [PATCH] [orc-rt] Make socket unit tests portable; build them on
 POSIX.

SimpleRemoteCAOverSocketTest.cpp tests a POSIX-only implementation, so
move it to the POSIX source list. Its tests are portable, so move the
fixture's POSIX code into SocketTestUtils (makeStreamSocketPair,
sendAll, recvAll, makeNativeNonStreamSocket) rather than moving the file
under sys/posix; a Windows transport will need only Windows definitions
of those helpers.

makeStreamSocketPair documents the properties the tests rely on beyond
a connected stream pair: neither direction buffers StallingPayloadSize
bytes, and closing an end delivers what it already sent even if it has
unread data. AF_UNIX provides both; a loopback TCP pair may not.

SocketConnectorTest.cpp tests fd adoption with pipe and fcntl, so it is
a test of the POSIX implementation: move it to sys/posix/.

Assisted-by: Claude
---
 orc-rt/test/unit/CMakeLists.txt               | 16 +++--
 orc-rt/test/unit/bedrock/SocketTestUtils.h    | 30 ++++++++
 .../sps/SimpleRemoteCAOverSocketTest.cpp      | 72 ++++---------------
 .../{ => sys/posix}/SocketConnectorTest.cpp   |  0
 .../bedrock/sys/posix/SocketTestUtils.cpp     | 56 +++++++++++++++
 5 files changed, 110 insertions(+), 64 deletions(-)
 rename orc-rt/test/unit/bedrock/{ => sys/posix}/SocketConnectorTest.cpp (100%)

diff --git a/orc-rt/test/unit/CMakeLists.txt b/orc-rt/test/unit/CMakeLists.txt
index a5e5f05ad2c34..c2686dff5cfe7 100644
--- a/orc-rt/test/unit/CMakeLists.txt
+++ b/orc-rt/test/unit/CMakeLists.txt
@@ -84,19 +84,24 @@ add_orc_rt_unittest(SupportTests
   LINK_LIBS orc-rt-support-objects
   )
 
-# Per-system test helpers, composed the way lib/ composes its per-system
-# implementations; see the note in lib/bedrock/CMakeLists.txt. Tests of portable
-# APIs stay in the shared list and reach the system through these.
+# Per-system test helpers and tests, composed the way lib/ composes its
+# per-system implementations; see the note in lib/bedrock/CMakeLists.txt. Tests
+# of a per-system implementation live under sys/<system>/, mirroring lib/. Tests
+# of portable APIs live outside sys/ and reach the system through the helpers,
+# so they join a system's list only once it provides the helpers, and the
+# implementation under test.
 set(ORC_RT_BEDROCK_TEST_POSIX_SOURCES
-  bedrock/SocketConnectorTest.cpp
   bedrock/SocketHandleTest.cpp
+  bedrock/sps/SimpleRemoteCAOverSocketTest.cpp
+  bedrock/sys/posix/SocketConnectorTest.cpp
   bedrock/sys/posix/SocketTestUtils.cpp
   )
 
 if (APPLE OR CMAKE_SYSTEM_NAME STREQUAL "Linux")
   set(ORC_RT_BEDROCK_TEST_SYS_SOURCES ${ORC_RT_BEDROCK_TEST_POSIX_SOURCES})
 else()
-  # No list yet, so tests needing a socket will fail to link.
+  # No socket helpers or socket transport yet, so the socket tests are not
+  # built.
   set(ORC_RT_BEDROCK_TEST_SYS_SOURCES)
 endif()
 
@@ -126,7 +131,6 @@ add_orc_rt_unittest(BedrockTests
   bedrock/sps/NativeDylibManagerSPSCITest.cpp
   bedrock/sps/SimpleNativeMemoryMapSPSCITest.cpp
   bedrock/sps/SimpleRemoteCATest.cpp
-  bedrock/sps/SimpleRemoteCAOverSocketTest.cpp
 
   bedrock/sys/CPUFeaturesTest.cpp
   bedrock/sys/TargetTripleTest.cpp
diff --git a/orc-rt/test/unit/bedrock/SocketTestUtils.h b/orc-rt/test/unit/bedrock/SocketTestUtils.h
index 75ad90dac5e07..7f695fac7841c 100644
--- a/orc-rt/test/unit/bedrock/SocketTestUtils.h
+++ b/orc-rt/test/unit/bedrock/SocketTestUtils.h
@@ -15,8 +15,11 @@
 #define ORC_RT_UNITTEST_BEDROCK_SOCKETTESTUTILS_H
 
 #include "orc-rt/bedrock/SocketHandle.h"
+#include "orc-rt/support/Error.h"
 
+#include <cstddef>
 #include <optional>
+#include <utility>
 
 namespace orc_rt::test {
 
@@ -24,6 +27,10 @@ namespace orc_rt::test {
 /// The socket is neither bound nor connected.
 std::optional<NativeSocketHandle> makeNativeSocket();
 
+/// Creates a socket that is not a stream socket, for a test to own, or nullopt
+/// if the system refuses one. The socket is neither bound nor connected.
+std::optional<NativeSocketHandle> makeNativeNonStreamSocket();
+
 /// True if H names a socket this process still has open.
 ///
 /// Only meaningful while nothing else in the process is opening sockets: a
@@ -34,6 +41,29 @@ bool isNativeSocketOpen(NativeSocketHandle H);
 /// Closes H, which must be open and owned by no SocketHandle.
 void closeNativeSocket(NativeSocketHandle H);
 
+/// A payload size that neither direction of a makeStreamSocketPair pair can
+/// buffer, so writing this much without the peer reading stalls the writer.
+inline constexpr size_t StallingPayloadSize = 1 << 20;
+
+/// Creates a connected pair of blocking stream sockets.
+///
+/// Tests of the socket transports rely on two further properties, which the
+/// definition for each system must provide:
+///
+///   - Neither direction buffers StallingPayloadSize bytes.
+///
+///   - Closing one end still delivers everything already sent from it, even
+///     if that end has unread data of its own. AF_UNIX sockets do this; a
+///     loopback TCP pair may reset the connection instead, discarding it.
+Expected<std::pair<SocketHandle, SocketHandle>> makeStreamSocketPair();
+
+/// Sends all Size bytes of Buf over H, which must be blocking.
+Error sendAll(NativeSocketHandle H, const char *Buf, size_t Size);
+
+/// Receives Size bytes into Buf from H, which must be blocking. Returns fewer
+/// only if the peer closed first.
+Expected<size_t> recvAll(NativeSocketHandle H, char *Buf, size_t Size);
+
 } // namespace orc_rt::test
 
 #endif // ORC_RT_UNITTEST_BEDROCK_SOCKETTESTUTILS_H
diff --git a/orc-rt/test/unit/bedrock/sps/SimpleRemoteCAOverSocketTest.cpp b/orc-rt/test/unit/bedrock/sps/SimpleRemoteCAOverSocketTest.cpp
index aece4eac1c647..4878c98137ed9 100644
--- a/orc-rt/test/unit/bedrock/sps/SimpleRemoteCAOverSocketTest.cpp
+++ b/orc-rt/test/unit/bedrock/sps/SimpleRemoteCAOverSocketTest.cpp
@@ -27,19 +27,17 @@
 
 #include "BedrockTestUtils.h"
 #include "CommonTestUtils.h"
+#include "bedrock/SocketTestUtils.h"
 
 #include "orc-rt-internal/support/Endian.h"
 
 #include <cassert>
-#include <cerrno>
 #include <chrono>
 #include <cstring>
 #include <future>
 #include <string>
 #include <string_view>
-#include <sys/socket.h>
 #include <thread>
-#include <unistd.h>
 #include <utility>
 #include <vector>
 
@@ -83,10 +81,10 @@ class SimpleRemoteCAOverSocketTest : public ::testing::Test {
   Session S{mockExecutorProcessInfo(), inlineDispatch, noErrors};
 
   void SetUp() override {
-    auto P = makePair();
+    auto P = makeStreamSocketPair();
     ASSERT_TRUE(!!P) << toString(P.takeError());
-    Near = SocketHandle(P->first);
-    Far = SocketHandle(P->second);
+    Near = std::move(P->first);
+    Far = std::move(P->second);
   }
 
   /// Creates a CA over Near and attaches it, the way a connector would.
@@ -115,48 +113,6 @@ class SimpleRemoteCAOverSocketTest : public ::testing::Test {
     }
   };
 
-  /// A connected pair of blocking stream sockets: one end for the CA to adopt,
-  /// one for the test to drive.
-  static Expected<std::pair<int, int>> makePair() {
-    int FDs[2];
-    if (::socketpair(AF_UNIX, SOCK_STREAM, 0, FDs) != 0)
-      return make_error<StringError>(std::string("socketpair: ") +
-                                     strerror(errno));
-    return std::make_pair(FDs[0], FDs[1]);
-  }
-
-  /// The test's end stays blocking, so these just loop until done.
-  static Error sendAll(int FDNum, const char *Buf, size_t Size) {
-    while (Size) {
-      ssize_t N = ::send(FDNum, Buf, Size, MSG_NOSIGNAL);
-      if (N < 0) {
-        if (errno == EINTR)
-          continue;
-        return make_error<StringError>(std::string("send: ") + strerror(errno));
-      }
-      Buf += N;
-      Size -= N;
-    }
-    return Error::success();
-  }
-
-  /// Returns short only if the peer closed first.
-  static Expected<size_t> recvAll(int FDNum, char *Buf, size_t Size) {
-    size_t Got = 0;
-    while (Got < Size) {
-      ssize_t N = ::recv(FDNum, Buf + Got, Size - Got, 0);
-      if (N == 0)
-        return Got;
-      if (N < 0) {
-        if (errno == EINTR)
-          continue;
-        return make_error<StringError>(std::string("recv: ") + strerror(errno));
-      }
-      Got += N;
-    }
-    return Got;
-  }
-
   /// A framed message, ready to write to the socket.
   static std::vector<char> frame(Opcode Op, uint64_t SeqNo, uint64_t Tag,
                                  std::string_view Payload) {
@@ -167,15 +123,15 @@ class SimpleRemoteCAOverSocketTest : public ::testing::Test {
     return Buf;
   }
 
-  static Error writeFrame(int FDNum, Opcode Op, uint64_t SeqNo,
+  static Error writeFrame(NativeSocketHandle Sock, Opcode Op, uint64_t SeqNo,
                           uint64_t Tag = 0, std::string_view Payload = {}) {
     auto Buf = frame(Op, SeqNo, Tag, Payload);
-    return sendAll(FDNum, Buf.data(), Buf.size());
+    return sendAll(Sock, Buf.data(), Buf.size());
   }
 
-  static Expected<Frame> readFrame(int FDNum) {
+  static Expected<Frame> readFrame(NativeSocketHandle Sock) {
     char H[HeaderSize];
-    auto N = recvAll(FDNum, H, HeaderSize);
+    auto N = recvAll(Sock, H, HeaderSize);
     if (!N)
       return N.takeError();
     if (*N != HeaderSize)
@@ -189,7 +145,7 @@ class SimpleRemoteCAOverSocketTest : public ::testing::Test {
 
     if (size_t PayloadSize = F.Fields.MsgSize - HeaderSize) {
       F.Payload.resize(PayloadSize);
-      auto M = recvAll(FDNum, F.Payload.data(), PayloadSize);
+      auto M = recvAll(Sock, F.Payload.data(), PayloadSize);
       if (!M)
         return M.takeError();
       if (*M != PayloadSize)
@@ -521,7 +477,7 @@ TEST_F(SimpleRemoteCAOverSocketTest, NothingIsQueuedBehindTheHangup) {
 
   // Echo back more than the socket buffer can hold, so the reactor stalls
   // part-way through sending the result.
-  const std::string Big(1 << 20, 'x');
+  const std::string Big(StallingPayloadSize, 'x');
   auto EchoTag = wrapperTag(reinterpret_cast<void *>(echoWrapper));
   ASSERT_FALSE(
       !!writeFrame(Far.get(), Opcode::Call, /*SeqNo=*/1, EchoTag, Big));
@@ -584,7 +540,7 @@ TEST_F(SimpleRemoteCAOverSocketTest, PeerReasonSurvivesAStalledSendQueue) {
 
   // Echo back more than the socket will hold, and never read it, so the reactor
   // is left with a part-sent message queued.
-  const std::string Big(1 << 20, 'x');
+  const std::string Big(StallingPayloadSize, 'x');
   auto Tag = wrapperTag(reinterpret_cast<void *>(echoWrapper));
   ASSERT_FALSE(!!writeFrame(Far.get(), Opcode::Call, /*SeqNo=*/1, Tag, Big));
 
@@ -593,7 +549,7 @@ TEST_F(SimpleRemoteCAOverSocketTest, PeerReasonSurvivesAStalledSendQueue) {
   ASSERT_FALSE(!!writeFrame(
       Far.get(), Opcode::Hangup, 0, 0,
       view(hangupPayload(make_error<StringError>("controller ran out of x")))));
-  ::close(Far.release());
+  Far.reset();
 
   auto Err = Disconnected.get();
   ASSERT_TRUE(!!Err) << "a hang-up carrying a reason ends with that reason";
@@ -609,7 +565,7 @@ TEST_F(SimpleRemoteCAOverSocketTest, TruncatedMessageIsReportedAsAnError) {
   // Half a header, then gone: distinguishable from a close at a boundary.
   char Half[HeaderSize / 2] = {};
   ASSERT_FALSE(!!sendAll(Far.get(), Half, sizeof(Half)));
-  ::close(Far.release());
+  Far.reset();
 
   auto Err = Disconnected.get();
   EXPECT_TRUE(!!Err) << "a truncated message must not look like a clean end";
@@ -627,7 +583,7 @@ TEST_F(SimpleRemoteCAOverSocketTest, PeerCloseWithoutAHangupIsAnError) {
   ASSERT_TRUE(!!readFrame(Far.get())) << "expected setup first";
 
   // Closed between messages, so nothing is truncated -- it is simply gone.
-  ::close(Far.release());
+  Far.reset();
 
   auto Err = Disconnected.get();
   ASSERT_TRUE(!!Err) << "a silent close is not an orderly end";
diff --git a/orc-rt/test/unit/bedrock/SocketConnectorTest.cpp b/orc-rt/test/unit/bedrock/sys/posix/SocketConnectorTest.cpp
similarity index 100%
rename from orc-rt/test/unit/bedrock/SocketConnectorTest.cpp
rename to orc-rt/test/unit/bedrock/sys/posix/SocketConnectorTest.cpp
diff --git a/orc-rt/test/unit/bedrock/sys/posix/SocketTestUtils.cpp b/orc-rt/test/unit/bedrock/sys/posix/SocketTestUtils.cpp
index 17242c5fe5163..6d16c2f2bf365 100644
--- a/orc-rt/test/unit/bedrock/sys/posix/SocketTestUtils.cpp
+++ b/orc-rt/test/unit/bedrock/sys/posix/SocketTestUtils.cpp
@@ -12,12 +12,20 @@
 
 #include "bedrock/SocketTestUtils.h"
 
+#include "orc-rt-internal/support/sys/Errno.h"
+
 #include <cerrno>
+#include <string>
 #include <sys/socket.h>
 #include <unistd.h>
 
 namespace orc_rt::test {
 
+static Error makeError(const char *Op, int ErrNum) {
+  return make_error<StringError>(std::string(Op) + ": " +
+                                 sys::strError(ErrNum));
+}
+
 std::optional<NativeSocketHandle> makeNativeSocket() {
   // Unbound, so this needs no network, peer or filesystem entry.
   NativeSocketHandle H = ::socket(AF_UNIX, SOCK_STREAM, 0);
@@ -26,6 +34,13 @@ std::optional<NativeSocketHandle> makeNativeSocket() {
   return H;
 }
 
+std::optional<NativeSocketHandle> makeNativeNonStreamSocket() {
+  NativeSocketHandle H = ::socket(AF_UNIX, SOCK_DGRAM, 0);
+  if (H == InvalidNativeSocketHandle)
+    return std::nullopt;
+  return H;
+}
+
 bool isNativeSocketOpen(NativeSocketHandle H) {
   // A zero-length send moves no data and needs no peer. An unconnected socket
   // refuses it with ENOTCONN, which still says the descriptor is there; only a
@@ -35,4 +50,45 @@ bool isNativeSocketOpen(NativeSocketHandle H) {
 
 void closeNativeSocket(NativeSocketHandle H) { ::close(H); }
 
+Expected<std::pair<SocketHandle, SocketHandle>> makeStreamSocketPair() {
+  // AF_UNIX gives the close semantics the header requires, and its default
+  // buffers are far smaller than StallingPayloadSize.
+  int FDs[2];
+  if (::socketpair(AF_UNIX, SOCK_STREAM, 0, FDs) != 0)
+    return makeError("socketpair", errno);
+  return std::make_pair(SocketHandle(FDs[0]), SocketHandle(FDs[1]));
+}
+
+Error sendAll(NativeSocketHandle H, const char *Buf, size_t Size) {
+  while (Size) {
+    // MSG_NOSIGNAL: a peer that has gone should fail the send, not kill the
+    // test with SIGPIPE.
+    ssize_t N = ::send(H, Buf, Size, MSG_NOSIGNAL);
+    if (N < 0) {
+      if (errno == EINTR)
+        continue;
+      return makeError("send", errno);
+    }
+    Buf += N;
+    Size -= N;
+  }
+  return Error::success();
+}
+
+Expected<size_t> recvAll(NativeSocketHandle H, char *Buf, size_t Size) {
+  size_t Got = 0;
+  while (Got < Size) {
+    ssize_t N = ::recv(H, Buf + Got, Size - Got, 0);
+    if (N == 0)
+      return Got;
+    if (N < 0) {
+      if (errno == EINTR)
+        continue;
+      return makeError("recv", errno);
+    }
+    Got += N;
+  }
+  return Got;
+}
+
 } // namespace orc_rt::test



More information about the llvm-commits mailing list