Skip to content

[lldb] Remove child_process_inherit from the socket classes #117699

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

Merged
merged 1 commit into from
Nov 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 6 additions & 12 deletions lldb/include/lldb/Host/Socket.h
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,6 @@ class Socket : public IOObject {
static void Terminate();

static std::unique_ptr<Socket> Create(const SocketProtocol protocol,
bool child_processes_inherit,
Status &error);

virtual Status Connect(llvm::StringRef name) = 0;
Expand All @@ -115,14 +114,13 @@ class Socket : public IOObject {
// implemented separately because the caller may wish to manipulate or query
// the socket after it is initialized, but before entering a blocking accept.
static llvm::Expected<std::unique_ptr<TCPSocket>>
TcpListen(llvm::StringRef host_and_port, bool child_processes_inherit,
int backlog = 5);
TcpListen(llvm::StringRef host_and_port, int backlog = 5);

static llvm::Expected<std::unique_ptr<Socket>>
TcpConnect(llvm::StringRef host_and_port, bool child_processes_inherit);
TcpConnect(llvm::StringRef host_and_port);

static llvm::Expected<std::unique_ptr<UDPSocket>>
UdpConnect(llvm::StringRef host_and_port, bool child_processes_inherit);
UdpConnect(llvm::StringRef host_and_port);

static int GetOption(NativeSocket sockfd, int level, int option_name,
int &option_value);
Expand Down Expand Up @@ -154,24 +152,20 @@ class Socket : public IOObject {
virtual std::string GetRemoteConnectionURI() const { return ""; };

protected:
Socket(SocketProtocol protocol, bool should_close,
bool m_child_process_inherit);
Socket(SocketProtocol protocol, bool should_close);

virtual size_t Send(const void *buf, const size_t num_bytes);

static int CloseSocket(NativeSocket sockfd);
static Status GetLastError();
static void SetLastError(Status &error);
static NativeSocket CreateSocket(const int domain, const int type,
const int protocol,
bool child_processes_inherit, Status &error);
const int protocol, Status &error);
static NativeSocket AcceptSocket(NativeSocket sockfd, struct sockaddr *addr,
socklen_t *addrlen,
bool child_processes_inherit, Status &error);
socklen_t *addrlen, Status &error);

SocketProtocol m_protocol;
NativeSocket m_socket;
bool m_child_processes_inherit;
bool m_should_close_fd;
};

Expand Down
5 changes: 2 additions & 3 deletions lldb/include/lldb/Host/common/TCPSocket.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,8 @@
namespace lldb_private {
class TCPSocket : public Socket {
public:
TCPSocket(bool should_close, bool child_processes_inherit);
TCPSocket(NativeSocket socket, bool should_close,
bool child_processes_inherit);
explicit TCPSocket(bool should_close);
TCPSocket(NativeSocket socket, bool should_close);
~TCPSocket() override;

// returns port number or 0 if error
Expand Down
4 changes: 2 additions & 2 deletions lldb/include/lldb/Host/common/UDPSocket.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@
namespace lldb_private {
class UDPSocket : public Socket {
public:
UDPSocket(bool should_close, bool child_processes_inherit);
explicit UDPSocket(bool should_close);

static llvm::Expected<std::unique_ptr<UDPSocket>>
Connect(llvm::StringRef name, bool child_processes_inherit);
CreateConnected(llvm::StringRef name);

std::string GetRemoteConnectionURI() const override;

Expand Down
2 changes: 1 addition & 1 deletion lldb/include/lldb/Host/linux/AbstractSocket.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
namespace lldb_private {
class AbstractSocket : public DomainSocket {
public:
AbstractSocket(bool child_processes_inherit);
AbstractSocket();

protected:
size_t GetNameOffset() const override;
Expand Down
7 changes: 3 additions & 4 deletions lldb/include/lldb/Host/posix/DomainSocket.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,8 @@
namespace lldb_private {
class DomainSocket : public Socket {
public:
DomainSocket(NativeSocket socket, bool should_close,
bool child_processes_inherit);
DomainSocket(bool should_close, bool child_processes_inherit);
DomainSocket(NativeSocket socket, bool should_close);
explicit DomainSocket(bool should_close);

Status Connect(llvm::StringRef name) override;
Status Listen(llvm::StringRef name, int backlog) override;
Expand All @@ -29,7 +28,7 @@ class DomainSocket : public Socket {
std::string GetRemoteConnectionURI() const override;

protected:
DomainSocket(SocketProtocol protocol, bool child_processes_inherit);
DomainSocket(SocketProtocol protocol);

virtual size_t GetNameOffset() const;
virtual void DeleteSocketFile(llvm::StringRef name);
Expand Down
53 changes: 18 additions & 35 deletions lldb/source/Host/common/Socket.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -180,12 +180,9 @@ bool Socket::FindProtocolByScheme(const char *scheme,
return false;
}

Socket::Socket(SocketProtocol protocol, bool should_close,
bool child_processes_inherit)
Socket::Socket(SocketProtocol protocol, bool should_close)
: IOObject(eFDTypeSocket), m_protocol(protocol),
m_socket(kInvalidSocketValue),
m_child_processes_inherit(child_processes_inherit),
m_should_close_fd(should_close) {}
m_socket(kInvalidSocketValue), m_should_close_fd(should_close) {}

Socket::~Socket() { Close(); }

Expand Down Expand Up @@ -214,33 +211,29 @@ void Socket::Terminate() {
}

std::unique_ptr<Socket> Socket::Create(const SocketProtocol protocol,
bool child_processes_inherit,
Status &error) {
error.Clear();

const bool should_close = true;
std::unique_ptr<Socket> socket_up;
switch (protocol) {
case ProtocolTcp:
socket_up =
std::make_unique<TCPSocket>(true, child_processes_inherit);
socket_up = std::make_unique<TCPSocket>(should_close);
break;
case ProtocolUdp:
socket_up =
std::make_unique<UDPSocket>(true, child_processes_inherit);
socket_up = std::make_unique<UDPSocket>(should_close);
break;
case ProtocolUnixDomain:
#if LLDB_ENABLE_POSIX
socket_up =
std::make_unique<DomainSocket>(true, child_processes_inherit);
socket_up = std::make_unique<DomainSocket>(should_close);
#else
error = Status::FromErrorString(
"Unix domain sockets are not supported on this platform.");
#endif
break;
case ProtocolUnixAbstract:
#ifdef __linux__
socket_up =
std::make_unique<AbstractSocket>(child_processes_inherit);
socket_up = std::make_unique<AbstractSocket>();
#else
error = Status::FromErrorString(
"Abstract domain sockets are not supported on this platform.");
Expand All @@ -255,14 +248,12 @@ std::unique_ptr<Socket> Socket::Create(const SocketProtocol protocol,
}

llvm::Expected<std::unique_ptr<Socket>>
Socket::TcpConnect(llvm::StringRef host_and_port,
bool child_processes_inherit) {
Socket::TcpConnect(llvm::StringRef host_and_port) {
Log *log = GetLog(LLDBLog::Connection);
LLDB_LOG(log, "host_and_port = {0}", host_and_port);

Status error;
std::unique_ptr<Socket> connect_socket(
Create(ProtocolTcp, child_processes_inherit, error));
std::unique_ptr<Socket> connect_socket = Create(ProtocolTcp, error);
if (error.Fail())
return error.ToError();

Expand All @@ -274,13 +265,12 @@ Socket::TcpConnect(llvm::StringRef host_and_port,
}

llvm::Expected<std::unique_ptr<TCPSocket>>
Socket::TcpListen(llvm::StringRef host_and_port, bool child_processes_inherit,
int backlog) {
Socket::TcpListen(llvm::StringRef host_and_port, int backlog) {
Log *log = GetLog(LLDBLog::Connection);
LLDB_LOG(log, "host_and_port = {0}", host_and_port);

std::unique_ptr<TCPSocket> listen_socket(
new TCPSocket(true, child_processes_inherit));
new TCPSocket(/*should_close=*/true));

Status error = listen_socket->Listen(host_and_port, backlog);
if (error.Fail())
Expand All @@ -290,9 +280,8 @@ Socket::TcpListen(llvm::StringRef host_and_port, bool child_processes_inherit,
}

llvm::Expected<std::unique_ptr<UDPSocket>>
Socket::UdpConnect(llvm::StringRef host_and_port,
bool child_processes_inherit) {
return UDPSocket::Connect(host_and_port, child_processes_inherit);
Socket::UdpConnect(llvm::StringRef host_and_port) {
return UDPSocket::CreateConnected(host_and_port);
}

llvm::Expected<Socket::HostAndPort> Socket::DecodeHostAndPort(llvm::StringRef host_and_port) {
Expand Down Expand Up @@ -445,13 +434,11 @@ int Socket::CloseSocket(NativeSocket sockfd) {
}

NativeSocket Socket::CreateSocket(const int domain, const int type,
const int protocol,
bool child_processes_inherit, Status &error) {
const int protocol, Status &error) {
error.Clear();
auto socket_type = type;
#ifdef SOCK_CLOEXEC
if (!child_processes_inherit)
socket_type |= SOCK_CLOEXEC;
socket_type |= SOCK_CLOEXEC;
#endif
auto sock = ::socket(domain, socket_type, protocol);
if (sock == kInvalidSocketValue)
Expand Down Expand Up @@ -483,8 +470,7 @@ Status Socket::Accept(const Timeout<std::micro> &timeout, Socket *&socket) {
}

NativeSocket Socket::AcceptSocket(NativeSocket sockfd, struct sockaddr *addr,
socklen_t *addrlen,
bool child_processes_inherit, Status &error) {
socklen_t *addrlen, Status &error) {
error.Clear();
#if defined(ANDROID_USE_ACCEPT_WORKAROUND)
// Hack:
Expand All @@ -494,7 +480,7 @@ NativeSocket Socket::AcceptSocket(NativeSocket sockfd, struct sockaddr *addr,
// available in older kernels. Using an older libc would fix this issue, but
// introduce other ones, as the old libraries were quite buggy.
int fd = syscall(__NR_accept, sockfd, addr, addrlen);
if (fd >= 0 && !child_processes_inherit) {
if (fd >= 0) {
int flags = ::fcntl(fd, F_GETFD);
if (flags != -1 && ::fcntl(fd, F_SETFD, flags | FD_CLOEXEC) != -1)
return fd;
Expand All @@ -503,10 +489,7 @@ NativeSocket Socket::AcceptSocket(NativeSocket sockfd, struct sockaddr *addr,
}
return fd;
#elif defined(SOCK_CLOEXEC) && defined(HAVE_ACCEPT4)
int flags = 0;
if (!child_processes_inherit) {
flags |= SOCK_CLOEXEC;
}
int flags = SOCK_CLOEXEC;
NativeSocket fd = llvm::sys::RetryAfterSignal(
static_cast<NativeSocket>(-1), ::accept4, sockfd, addr, addrlen, flags);
#else
Expand Down
25 changes: 10 additions & 15 deletions lldb/source/Host/common/TCPSocket.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,18 +38,15 @@ using namespace lldb_private;

static const int kType = SOCK_STREAM;

TCPSocket::TCPSocket(bool should_close, bool child_processes_inherit)
: Socket(ProtocolTcp, should_close, child_processes_inherit) {}
TCPSocket::TCPSocket(bool should_close) : Socket(ProtocolTcp, should_close) {}

TCPSocket::TCPSocket(NativeSocket socket, const TCPSocket &listen_socket)
: Socket(ProtocolTcp, listen_socket.m_should_close_fd,
listen_socket.m_child_processes_inherit) {
: Socket(ProtocolTcp, listen_socket.m_should_close_fd) {
m_socket = socket;
}

TCPSocket::TCPSocket(NativeSocket socket, bool should_close,
bool child_processes_inherit)
: Socket(ProtocolTcp, should_close, child_processes_inherit) {
TCPSocket::TCPSocket(NativeSocket socket, bool should_close)
: Socket(ProtocolTcp, should_close) {
m_socket = socket;
}

Expand Down Expand Up @@ -124,8 +121,7 @@ Status TCPSocket::CreateSocket(int domain) {
error = Close();
if (error.Fail())
return error;
m_socket = Socket::CreateSocket(domain, kType, IPPROTO_TCP,
m_child_processes_inherit, error);
m_socket = Socket::CreateSocket(domain, kType, IPPROTO_TCP, error);
return error;
}

Expand Down Expand Up @@ -183,8 +179,8 @@ Status TCPSocket::Listen(llvm::StringRef name, int backlog) {
std::vector<SocketAddress> addresses = SocketAddress::GetAddressInfo(
host_port->hostname.c_str(), nullptr, AF_UNSPEC, SOCK_STREAM, IPPROTO_TCP);
for (SocketAddress &address : addresses) {
int fd = Socket::CreateSocket(address.GetFamily(), kType, IPPROTO_TCP,
m_child_processes_inherit, error);
int fd =
Socket::CreateSocket(address.GetFamily(), kType, IPPROTO_TCP, error);
if (error.Fail() || fd < 0)
continue;

Expand Down Expand Up @@ -241,14 +237,13 @@ TCPSocket::Accept(MainLoopBase &loop,
std::vector<MainLoopBase::ReadHandleUP> handles;
for (auto socket : m_listen_sockets) {
auto fd = socket.first;
auto io_sp =
std::make_shared<TCPSocket>(fd, false, this->m_child_processes_inherit);
auto io_sp = std::make_shared<TCPSocket>(fd, false);
auto cb = [this, fd, sock_cb](MainLoopBase &loop) {
lldb_private::SocketAddress AcceptAddr;
socklen_t sa_len = AcceptAddr.GetMaxLength();
Status error;
NativeSocket sock = AcceptSocket(fd, &AcceptAddr.sockaddr(), &sa_len,
m_child_processes_inherit, error);
NativeSocket sock =
AcceptSocket(fd, &AcceptAddr.sockaddr(), &sa_len, error);
Log *log = GetLog(LLDBLog::Host);
if (error.Fail()) {
LLDB_LOG(log, "AcceptSocket({0}): {1}", fd, error);
Expand Down
14 changes: 7 additions & 7 deletions lldb/source/Host/common/UDPSocket.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,12 @@ static const int kType = SOCK_DGRAM;

static const char *g_not_supported_error = "Not supported";

UDPSocket::UDPSocket(NativeSocket socket) : Socket(ProtocolUdp, true, true) {
UDPSocket::UDPSocket(NativeSocket socket)
: Socket(ProtocolUdp, /*should_close=*/true) {
m_socket = socket;
}

UDPSocket::UDPSocket(bool should_close, bool child_processes_inherit)
: Socket(ProtocolUdp, should_close, child_processes_inherit) {}
UDPSocket::UDPSocket(bool should_close) : Socket(ProtocolUdp, should_close) {}

size_t UDPSocket::Send(const void *buf, const size_t num_bytes) {
return ::sendto(m_socket, static_cast<const char *>(buf), num_bytes, 0,
Expand All @@ -48,7 +48,7 @@ Status UDPSocket::Listen(llvm::StringRef name, int backlog) {
}

llvm::Expected<std::unique_ptr<UDPSocket>>
UDPSocket::Connect(llvm::StringRef name, bool child_processes_inherit) {
UDPSocket::CreateConnected(llvm::StringRef name) {
std::unique_ptr<UDPSocket> socket;

Log *log = GetLog(LLDBLog::Connection);
Expand Down Expand Up @@ -84,9 +84,9 @@ UDPSocket::Connect(llvm::StringRef name, bool child_processes_inherit) {
for (struct addrinfo *service_info_ptr = service_info_list;
service_info_ptr != nullptr;
service_info_ptr = service_info_ptr->ai_next) {
auto send_fd = CreateSocket(
service_info_ptr->ai_family, service_info_ptr->ai_socktype,
service_info_ptr->ai_protocol, child_processes_inherit, error);
auto send_fd =
CreateSocket(service_info_ptr->ai_family, service_info_ptr->ai_socktype,
service_info_ptr->ai_protocol, error);
if (error.Success()) {
socket.reset(new UDPSocket(send_fd));
socket->m_sockaddr = service_info_ptr;
Expand Down
3 changes: 1 addition & 2 deletions lldb/source/Host/linux/AbstractSocket.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,7 @@
using namespace lldb;
using namespace lldb_private;

AbstractSocket::AbstractSocket(bool child_processes_inherit)
: DomainSocket(ProtocolUnixAbstract, child_processes_inherit) {}
AbstractSocket::AbstractSocket() : DomainSocket(ProtocolUnixAbstract) {}

size_t AbstractSocket::GetNameOffset() const { return 1; }

Expand Down
Loading