Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[llvm][Support] Add UNIX socket support #73603

Merged
merged 15 commits into from
Dec 13, 2023

Conversation

criis
Copy link
Contributor

@criis criis commented Nov 28, 2023

This adds support for UNIX socket communication to work similarly to raw_stream.

@llvmbot
Copy link
Collaborator

llvmbot commented Nov 28, 2023

@llvm/pr-subscribers-llvm-support

Author: None (criis)

Changes

Full diff: https://github.com/llvm/llvm-project/pull/73603.diff

4 Files Affected:

  • (modified) llvm/include/llvm/Support/raw_ostream.h (+26)
  • (modified) llvm/lib/Support/raw_ostream.cpp (+151-2)
  • (modified) llvm/unittests/Support/CMakeLists.txt (+1)
  • (added) llvm/unittests/Support/raw_socket_stream_test.cpp (+55)
diff --git a/llvm/include/llvm/Support/raw_ostream.h b/llvm/include/llvm/Support/raw_ostream.h
index 1e01eb9ea19c418..f135ee5435379fc 100644
--- a/llvm/include/llvm/Support/raw_ostream.h
+++ b/llvm/include/llvm/Support/raw_ostream.h
@@ -630,6 +630,32 @@ class raw_fd_stream : public raw_fd_ostream {
   static bool classof(const raw_ostream *OS);
 };
 
+//===----------------------------------------------------------------------===//
+// Socket Streams
+//===----------------------------------------------------------------------===//
+
+/// A raw stream for sockets reading/writing
+
+class raw_socket_stream : public raw_fd_ostream {
+  StringRef SocketPath;
+  bool ShouldUnlink;
+
+  uint64_t current_pos() const override { return 0; }
+  
+public:
+  int get_socket() {
+    return get_fd();
+  }
+  
+  static int MakeServerSocket(StringRef SocketPath, unsigned int MaxBacklog, std::error_code &EC);
+
+  raw_socket_stream(int SocketFD, StringRef SockPath, std::error_code &EC);
+  raw_socket_stream(StringRef SockPath, std::error_code &EC);
+  ~raw_socket_stream();
+
+  Expected<std::string> read_impl();
+};
+
 //===----------------------------------------------------------------------===//
 // Output Stream Adaptors
 //===----------------------------------------------------------------------===//
diff --git a/llvm/lib/Support/raw_ostream.cpp b/llvm/lib/Support/raw_ostream.cpp
index 8908e7b6a150cab..ff15b39ec69d8b1 100644
--- a/llvm/lib/Support/raw_ostream.cpp
+++ b/llvm/lib/Support/raw_ostream.cpp
@@ -1,4 +1,4 @@
-//===--- raw_ostream.cpp - Implement the raw_ostream classes --------------===//
+ //===--- raw_ostream.cpp - Implement the raw_ostream classes --------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
@@ -23,11 +23,17 @@
 #include "llvm/Support/NativeFormatting.h"
 #include "llvm/Support/Process.h"
 #include "llvm/Support/Program.h"
+#include "llvm/Support/Threading.h"
+#include "llvm/Support/Error.h"
 #include <algorithm>
 #include <cerrno>
 #include <cstdio>
 #include <sys/stat.h>
 
+#include <sys/socket.h>
+#include <sys/un.h>
+#include <iostream>
+
 // <fcntl.h> may provide O_BINARY.
 #if defined(HAVE_FCNTL_H)
 # include <fcntl.h>
@@ -58,6 +64,9 @@
 #include "llvm/Support/ConvertUTF.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/Windows/WindowsSupport.h"
+#include "raw_ostream.h"
+#include <afunix.h>
+#include <io.h>
 #endif
 
 using namespace llvm;
@@ -644,7 +653,7 @@ raw_fd_ostream::raw_fd_ostream(int fd, bool shouldClose, bool unbuffered,
   // Check if this is a console device. This is not equivalent to isatty.
   IsWindowsConsole =
       ::GetFileType((HANDLE)::_get_osfhandle(fd)) == FILE_TYPE_CHAR;
-#endif
+#endif // _WIN32
 
   // Get the starting position.
   off_t loc = ::lseek(FD, 0, SEEK_CUR);
@@ -942,6 +951,146 @@ bool raw_fd_stream::classof(const raw_ostream *OS) {
   return OS->get_kind() == OStreamKind::OK_FDStream;
 }
 
+//===----------------------------------------------------------------------===//
+//  raw_socket_stream
+//===----------------------------------------------------------------------===//
+
+int raw_socket_stream::MakeServerSocket(StringRef SocketPath, unsigned int MaxBacklog, std::error_code &EC) {
+
+#ifdef _WIN32
+  SOCKET MaybeWinsocket = socket(AF_UNIX, SOCK_STREAM, 0);
+#else
+  int MaybeWinsocket = socket(AF_UNIX, SOCK_STREAM, 0);
+#endif // defined(_WIN32)
+
+#ifdef _WIN32
+  if (MaybeWinsocket == INVALID_SOCKET) {
+#else
+  if (MaybeWinsocket == -1) {
+#endif // _WIN32
+    std::string Msg = "socket create error" + std::string(strerror(errno));
+    std::perror(Msg.c_str());
+    std::cout << Msg << std::endl;
+    EC = std::make_error_code(std::errc::connection_aborted);
+    return -1;  
+  }
+
+  struct sockaddr_un Addr;
+  memset(&Addr, 0, sizeof(Addr));
+  Addr.sun_family = AF_UNIX;
+  strncpy(Addr.sun_path, SocketPath.str().c_str(), sizeof(Addr.sun_path) - 1);
+  
+  if (bind(MaybeWinsocket, (struct sockaddr *)&Addr, sizeof(Addr)) == -1) {
+    if (errno == EADDRINUSE) {
+      ::close(MaybeWinsocket);
+      EC = std::make_error_code(std::errc::address_in_use);
+    } else {
+      EC = std::make_error_code(std::errc::inappropriate_io_control_operation);
+    }
+    return -1;
+  }
+
+  if (listen(MaybeWinsocket, MaxBacklog) == -1) {
+    EC = std::make_error_code(std::errc::address_not_available);
+    return -1;
+  }
+#ifdef _WIN32
+  return _open_osfhandle(MaybeWinsocket, 0); // flags? https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/open-osfhandle?view=msvc-170
+#else
+  return MaybeWinsocket;
+#endif // _WIN32
+}
+
+int GetSocketFD(StringRef SocketPath, std::error_code &EC) {
+#ifdef _WIN32
+  SOCKET MaybeWinsocket = socket(AF_UNIX, SOCK_STREAM, 0);
+  if (MaybeWinsocket == INVALID_SOCKET) {
+#else
+  int MaybeWinsocket = socket(AF_UNIX, SOCK_STREAM, 0);
+  if (MaybeWinsocket == -1) {
+#endif // _WIN32
+    std::string Msg = "socket create error" + std::string(strerror(errno));
+    std::perror(Msg.c_str());
+    EC = std::make_error_code(std::errc::connection_aborted);
+    return -1;
+  }
+
+  struct sockaddr_un Addr;
+  memset(&Addr, 0, sizeof(Addr));
+  Addr.sun_family = AF_UNIX;
+  strncpy(Addr.sun_path, SocketPath.str().c_str(), sizeof(Addr.sun_path) - 1);
+
+  int status = connect(MaybeWinsocket, (struct sockaddr *)&Addr, sizeof(Addr));
+  if (status == -1) {
+    std::string Msg = "socket connect error" + std::string(strerror(errno));
+    std::perror(Msg.c_str());
+    EC = std::make_error_code(std::errc::connection_aborted);
+    return -1;
+  }
+#ifdef _WIN32
+  return _open_osfhandle(MaybeWinsocket, 0);
+#else
+  return MaybeWinsocket;
+#endif // _WIN32
+}
+
+static int ServerAccept(int FD) {
+  int AcceptFD;
+#ifdef _WIN32
+  SOCKET WinServerSock = _get_osfhandle(FD);
+  SOCKET WinAcceptSock = ::accept(WinServerSock, NULL, NULL);
+  AcceptFD = _open_osfhandle(WinAcceptSock, 0); // flags?
+#else
+  AcceptFD = ::accept(FD, NULL, NULL);
+#endif //_WIN32
+  return AcceptFD;
+}
+
+// Server
+// Call raw_fd_ostream with ShouldClose=false
+raw_socket_stream::raw_socket_stream(int SocketFD, StringRef SockPath, std::error_code &EC) : raw_fd_ostream(ServerAccept(SocketFD), true) {
+  SocketPath = SockPath;
+  ShouldUnlink = true;
+}
+
+// Client
+raw_socket_stream::raw_socket_stream(StringRef SockPath, std::error_code &EC) : raw_fd_ostream(GetSocketFD(SockPath, EC), true, true, OStreamKind::OK_OStream ) {
+  SocketPath = SockPath;
+  ShouldUnlink = false;
+}
+
+raw_socket_stream::~raw_socket_stream() {
+  if (ShouldUnlink) {
+    unlink(SocketPath.str().c_str());
+  }
+}
+
+Expected<std::string> raw_socket_stream::read_impl() {
+  const size_t BUFFER_SIZE = 4096;
+  std::vector<char> Buffer(BUFFER_SIZE);
+
+  int Socket = get_socket();
+  assert(Socket >= 0 && "Socket not found.");
+
+  ssize_t n;
+#ifdef _WIN32
+  SOCKET MaybeWinsocket = _get_osfhandle(Socket);
+#else
+  int MaybeWinsocket = Socket;
+#endif // _WIN32
+  n = ::read(MaybeWinsocket, Buffer.data(), Buffer.size());
+
+  if (n < 0) {
+      std::string Msg = "Buffer read error: " + std::string(strerror(errno));
+      return llvm::make_error<StringError>(Msg, inconvertibleErrorCode());
+  }
+
+  if (n == 0) {
+      return llvm::make_error<StringError>("EOF", inconvertibleErrorCode());
+  }
+  return std::string(Buffer.data());
+}
+
 //===----------------------------------------------------------------------===//
 //  raw_string_ostream
 //===----------------------------------------------------------------------===//
diff --git a/llvm/unittests/Support/CMakeLists.txt b/llvm/unittests/Support/CMakeLists.txt
index e1bf793536b6862..df35a7b7f3626ac 100644
--- a/llvm/unittests/Support/CMakeLists.txt
+++ b/llvm/unittests/Support/CMakeLists.txt
@@ -103,6 +103,7 @@ add_llvm_unittest(SupportTests
   raw_ostream_test.cpp
   raw_pwrite_stream_test.cpp
   raw_sha1_ostream_test.cpp
+  raw_socket_stream_test.cpp
   xxhashTest.cpp
 
   DEPENDS
diff --git a/llvm/unittests/Support/raw_socket_stream_test.cpp b/llvm/unittests/Support/raw_socket_stream_test.cpp
new file mode 100644
index 000000000000000..b2f756b792289b1
--- /dev/null
+++ b/llvm/unittests/Support/raw_socket_stream_test.cpp
@@ -0,0 +1,55 @@
+#include <stdlib.h>
+#include <iostream>
+#include <future>
+#include "llvm/ADT/SmallString.h"
+#include "llvm/Config/llvm-config.h"
+#include "llvm/Support/Casting.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/FileUtilities.h"
+#include "llvm/Support/raw_ostream.h"
+#include "gtest/gtest.h"
+
+using namespace llvm;
+
+namespace {
+
+TEST(raw_socket_streamTest, CLIENT_TO_SERVER_AND_SERVER_TO_CLIENT) {
+
+  SmallString<100> SocketPath("/tmp/test_raw_socket_stream.sock");
+  std::error_code ECServer, ECClient;
+
+  int ServerFD = raw_socket_stream::MakeServerSocket(SocketPath, 3, ECServer);
+
+  raw_socket_stream Client(SocketPath, ECClient);
+  EXPECT_TRUE(!ECClient);
+
+  raw_socket_stream Client2(SocketPath, ECClient);
+
+  raw_socket_stream Server(ServerFD, SocketPath, ECServer);
+  EXPECT_TRUE(!ECServer);
+
+  Client << "01234567";
+  Client.flush();
+
+  Client2 << "abcdefgh";
+  Client2.flush();
+
+  Expected<std::string> from_client = Server.read_impl();
+
+  if (auto E = from_client.takeError()) {
+    return; // FIXME: Do something.
+  }
+  EXPECT_EQ("01234567", (*from_client));
+
+  Server << "76543210";
+  Server.flush();
+
+  Expected<std::string> from_server = Client.read_impl();
+    if (auto E = from_server.takeError()) {
+    return;
+    // YIKES! 😩
+  }
+  EXPECT_EQ("76543210", (*from_server));
+
+}
+} // namespace
\ No newline at end of file

Copy link

github-actions bot commented Nov 28, 2023

:white_check_mark: With the latest revision this PR passed the C/C++ code formatter.

@criis criis force-pushed the raw_socket_stream-support branch 2 times, most recently from fbd1c75 to e53256d Compare November 28, 2023 20:40
@ributzka ributzka self-requested a review November 29, 2023 17:53
Copy link
Contributor

@Bigcheese Bigcheese left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the patch. As a high level comment you should include a bit more information in the commit message.

Also the llvm/lib/Support/CMakeLists.txt file needs to change line 44 to:
set(system_libs ${system_libs} psapi shell32 ole32 uuid advapi32 Ws2_32)

This links against the winsock library.

llvm/lib/Support/raw_ostream.cpp Outdated Show resolved Hide resolved
llvm/lib/Support/raw_ostream.cpp Outdated Show resolved Hide resolved
llvm/lib/Support/raw_ostream.cpp Outdated Show resolved Hide resolved
llvm/lib/Support/raw_ostream.cpp Outdated Show resolved Hide resolved
llvm/include/llvm/Support/raw_ostream.h Outdated Show resolved Hide resolved
Copy link
Contributor

@Bigcheese Bigcheese left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it looks good with these last few changes.

llvm/lib/Support/raw_ostream.cpp Outdated Show resolved Hide resolved
llvm/lib/Support/raw_ostream.cpp Outdated Show resolved Hide resolved
llvm/lib/Support/raw_ostream.cpp Outdated Show resolved Hide resolved
@criis criis changed the title Add raw_socket_stream Add socket support to raw_ostream. Dec 12, 2023
@criis criis changed the title Add socket support to raw_ostream. [llvm][Support] Add UNIX socket support Dec 12, 2023
@Bigcheese Bigcheese merged commit a5ffabc into llvm:main Dec 13, 2023
4 checks passed
@yozhu
Copy link
Contributor

yozhu commented Dec 13, 2023

Hit compilation errors when building LLVM for Windows:

llvm/lib/Support/raw_ostream.cpp(1018,22): error: variable has incomplete type 'struct sockaddr_un'
[CONTEXT] 1018 | struct sockaddr_un Addr;
[CONTEXT] | ^

llvm/lib/Support/raw_ostream.cpp(1077,22): error: variable has incomplete type 'struct sockaddr_un'
[CONTEXT] 1077 | struct sockaddr_un Addr;
[CONTEXT] | ^

@Bigcheese
Copy link
Contributor

Hit compilation errors when building LLVM for Windows:

llvm/lib/Support/raw_ostream.cpp(1018,22): error: variable has incomplete type 'struct sockaddr_un' [CONTEXT] 1018 | struct sockaddr_un Addr; [CONTEXT] | ^

llvm/lib/Support/raw_ostream.cpp(1077,22): error: variable has incomplete type 'struct sockaddr_un' [CONTEXT] 1077 | struct sockaddr_un Addr; [CONTEXT] | ^

What version of MSVC do you have? This should be supported by the minimum supported version.

@shiltian
Copy link
Contributor

shiltian commented Dec 13, 2023

Am I the only one getting the following error?

/home/ac.shilei.tian/Documents/vscode/llvm-project/llvm/lib/Support/raw_ostream.cpp: In static member function ‘static llvm::Expected<llvm::ListeningSocket> llvm::ListeningSocket::createUnix(llvm::StringRef, int)’:
/home/ac.shilei.tian/Documents/vscode/llvm-project/llvm/lib/Support/raw_ostream.cpp:1040:10: error: could not convert ‘ListenSocket’ from ‘llvm::ListeningSocket’ to ‘llvm::Expected<llvm::ListeningSocket>’
   return ListenSocket;
          ^~~~~~~~~~~~

Compiler information:

➜  /usr/bin/c++ --version
c++ (SUSE Linux) 7.5.0
Copyright (C) 2017 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

@Bigcheese
Copy link
Contributor

Am I the only one getting the following error?

/home/ac.shilei.tian/Documents/vscode/llvm-project/llvm/lib/Support/raw_ostream.cpp: In static member function ‘static llvm::Expected<llvm::ListeningSocket> llvm::ListeningSocket::createUnix(llvm::StringRef, int)’:
/home/ac.shilei.tian/Documents/vscode/llvm-project/llvm/lib/Support/raw_ostream.cpp:1040:10: error: could not convert ‘ListenSocket’ from ‘llvm::ListeningSocket’ to ‘llvm::Expected<llvm::ListeningSocket>’
   return ListenSocket;
          ^~~~~~~~~~~~

Compiler information:

➜  /usr/bin/c++ --version
c++ (SUSE Linux) 7.5.0
Copyright (C) 2017 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

Assuming that's gcc 7.5, looks like it doesn't do all of P0135. That should be easy to change the code to support.

@shiltian
Copy link
Contributor

Am I the only one getting the following error?

/home/ac.shilei.tian/Documents/vscode/llvm-project/llvm/lib/Support/raw_ostream.cpp: In static member function ‘static llvm::Expected<llvm::ListeningSocket> llvm::ListeningSocket::createUnix(llvm::StringRef, int)’:
/home/ac.shilei.tian/Documents/vscode/llvm-project/llvm/lib/Support/raw_ostream.cpp:1040:10: error: could not convert ‘ListenSocket’ from ‘llvm::ListeningSocket’ to ‘llvm::Expected<llvm::ListeningSocket>’
   return ListenSocket;
          ^~~~~~~~~~~~

Compiler information:

➜  /usr/bin/c++ --version
c++ (SUSE Linux) 7.5.0
Copyright (C) 2017 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

Assuming that's gcc 7.5, looks like it doesn't do all of P0135. That should be easy to change the code to support.

Please do as it is blocking my build and GCC 7.5.0 is definitely supported by LLVM right now.

@yozhu
Copy link
Contributor

yozhu commented Dec 14, 2023

Hit compilation errors when building LLVM for Windows:
llvm/lib/Support/raw_ostream.cpp(1018,22): error: variable has incomplete type 'struct sockaddr_un' [CONTEXT] 1018 | struct sockaddr_un Addr; [CONTEXT] | ^
llvm/lib/Support/raw_ostream.cpp(1077,22): error: variable has incomplete type 'struct sockaddr_un' [CONTEXT] 1077 | struct sockaddr_un Addr; [CONTEXT] | ^

What version of MSVC do you have? This should be supported by the minimum supported version.

We are currently using WinSDK 10.0.16299.0. What version would you recommend we move to?

shiltian added a commit to shiltian/llvm-project that referenced this pull request Dec 14, 2023
This patch fixed the following compile error caused by llvm#73603.

```
llvm/lib/Support/raw_ostream.cpp: In static member function ‘static llvm::Expected<llvm::ListeningSocket> llvm::ListeningSocket::createUnix(llvm::StringRef, int)’:
llvm/lib/Support/raw_ostream.cpp:1040:10: error: could not convert ‘ListenSocket’ from ‘llvm::ListeningSocket’ to ‘llvm::Expected<llvm::ListeningSocket>’
   return ListenSocket;
          ^~~~~~~~~~~~
```
shiltian added a commit that referenced this pull request Dec 14, 2023
This patch fixed the following compile error caused by #73603.

```
llvm/lib/Support/raw_ostream.cpp: In static member function ‘static llvm::Expected<llvm::ListeningSocket> llvm::ListeningSocket::createUnix(llvm::StringRef, int)’:
llvm/lib/Support/raw_ostream.cpp:1040:10: error: could not convert ‘ListenSocket’ from ‘llvm::ListeningSocket’ to ‘llvm::Expected<llvm::ListeningSocket>’
   return ListenSocket;
          ^~~~~~~~~~~~
```
@Bigcheese
Copy link
Contributor

We are currently using WinSDK 10.0.16299.0. What version would you recommend we move to?

I think 10.0.17134.0 which is included in VS2017 ver.15.7.

However, the minimum currently supported VS is VS2019 as of https://reviews.llvm.org/D114639, which means at least Windows SDK 10.0.18362.0.

@nico
Copy link
Contributor

nico commented Dec 14, 2023

From what I understand, this makes binaries depending on Support, such as clang, now depend on winsock. That seems like an undesirable dependency in a compiler. Maybe this could live in its own library, instead of in Support? Then only tools that need unix sockets could depend on it.

@Bigcheese
Copy link
Contributor

From what I understand, this makes binaries depending on Support, such as clang, now depend on winsock. That seems like an undesirable dependency in a compiler. Maybe this could live in its own library, instead of in Support? Then only tools that need unix sockets could depend on it.

I considered that a bit, but we already depend on stuff like shell32 and COM, and just linking to winsock doesn't really do anything, you have to actually call WSAStartup before it sets up anything.

If anyone actually encounters an issue with linking against winsock then it'd be fine with moving it, but I don't really see a point in doing that right now. Also note that clang itself will depend on this once #67562 lands. It was waiting on Windows support which is what this patch was for.

@nikic
Copy link
Contributor

nikic commented Dec 14, 2023

Can you please move this out of raw_ostream.h into a separate header? This is a core header, and adding extra dependencies to it has visible effect on time to compile clang.

@nico
Copy link
Contributor

nico commented Dec 14, 2023

I considered that a bit, but we already depend on stuff like shell32 and COM, and just linking to winsock doesn't really do anything, you have to actually call WSAStartup before it sets up anything.

It does show up in dumpbin /dependents output, which is super weird for a compiler. And now, if you have a compile farm, an attacker might be able to send you source code that can just call dormant winsock functions to do network access. (Granted, previously they could LoadLibrary() it, and you should have a sandbox, but it's still bad for a compiler to have a dep on winsock.) And for the other deps, we at least delayload them.

#67562 also looks like something that should be in a separate binary, not in clang itself.

@Bigcheese
Copy link
Contributor

It does show up in dumpbin /dependents output, which is super weird for a compiler. And now, if you have a compile farm, an attacker might be able to send you source code that can just call dormant winsock functions to do network access. (Granted, previously they could LoadLibrary() it, and you should have a sandbox, but it's still bad for a compiler to have a dep on winsock.) And for the other deps, we at least delayload them.

I think it's fine to delay load it, I'll try out adding that.

As for the security concerns, it's already true that you have to lock down clang (or almost any compiler) if you're accepting arbitrary code. On most POSIX systems you already have access to the socket functions via libc anyway, and as you say on Windows you can just dynamically load them. Linking against winsock doesn't actually change anything here.

Note that GCC also uses sockets now for modules via -fmodule-mapper, including support for TCP.

Bigcheese pushed a commit that referenced this pull request Dec 19, 2023
…les (#75653)

Move the implementation of raw_socket_stream from raw_ostream.h/cpp to
raw_socket_stream.h/cpp as requested in #73603.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants