https://github.com/emrekultursay updated
https://github.com/llvm/llvm-project/pull/136466
>From b6c46b7a28a807b507f06d748ed93f20d26fdfcc Mon Sep 17 00:00:00 2001
From: Emre Kultursay <emrekultur...@google.com>
Date: Sun, 20 Apr 2025 01:11:31 +0000
Subject: [PATCH 1/4] Fix connecting via abstract socket
Commit 82ee31f and Commit 2e893124 added socket sharing, but only for
unix domain sockets. That broke Android, which uses unix-abstract
sockets.
---
lldb/include/lldb/Host/linux/AbstractSocket.h | 1 +
lldb/include/lldb/Host/posix/DomainSocket.h | 1 +
lldb/source/Host/linux/AbstractSocket.cpp | 3 ++
lldb/source/Host/posix/DomainSocket.cpp | 6 +++
lldb/tools/lldb-server/lldb-platform.cpp | 40 +++++++++++++++++--
5 files changed, 47 insertions(+), 4 deletions(-)
diff --git a/lldb/include/lldb/Host/linux/AbstractSocket.h
b/lldb/include/lldb/Host/linux/AbstractSocket.h
index accfd01457a5e..c6a0e2f8af63b 100644
--- a/lldb/include/lldb/Host/linux/AbstractSocket.h
+++ b/lldb/include/lldb/Host/linux/AbstractSocket.h
@@ -15,6 +15,7 @@ namespace lldb_private {
class AbstractSocket : public DomainSocket {
public:
AbstractSocket();
+ AbstractSocket(NativeSocket socket, bool should_close);
protected:
size_t GetNameOffset() const override;
diff --git a/lldb/include/lldb/Host/posix/DomainSocket.h
b/lldb/include/lldb/Host/posix/DomainSocket.h
index 3dbe6206da2c5..562c011ee73e6 100644
--- a/lldb/include/lldb/Host/posix/DomainSocket.h
+++ b/lldb/include/lldb/Host/posix/DomainSocket.h
@@ -33,6 +33,7 @@ class DomainSocket : public Socket {
protected:
DomainSocket(SocketProtocol protocol);
+ DomainSocket(SocketProtocol protocol, NativeSocket socket, bool
should_close);
virtual size_t GetNameOffset() const;
virtual void DeleteSocketFile(llvm::StringRef name);
diff --git a/lldb/source/Host/linux/AbstractSocket.cpp
b/lldb/source/Host/linux/AbstractSocket.cpp
index 8393a80e86e72..681aa2d1ebc72 100644
--- a/lldb/source/Host/linux/AbstractSocket.cpp
+++ b/lldb/source/Host/linux/AbstractSocket.cpp
@@ -15,6 +15,9 @@ using namespace lldb_private;
AbstractSocket::AbstractSocket() : DomainSocket(ProtocolUnixAbstract) {}
+AbstractSocket::AbstractSocket(NativeSocket socket, bool should_close)
+ : DomainSocket(ProtocolUnixAbstract, socket, should_close) {}
+
size_t AbstractSocket::GetNameOffset() const { return 1; }
void AbstractSocket::DeleteSocketFile(llvm::StringRef name) {}
diff --git a/lldb/source/Host/posix/DomainSocket.cpp
b/lldb/source/Host/posix/DomainSocket.cpp
index 6c490cdda47ed..9b1e4f71bba2a 100644
--- a/lldb/source/Host/posix/DomainSocket.cpp
+++ b/lldb/source/Host/posix/DomainSocket.cpp
@@ -67,6 +67,12 @@ DomainSocket::DomainSocket(NativeSocket socket,
m_socket = socket;
}
+DomainSocket::DomainSocket(SocketProtocol protocol, NativeSocket socket,
+ bool should_close)
+ : Socket(protocol, should_close) {
+ m_socket = socket;
+}
+
Status DomainSocket::Connect(llvm::StringRef name) {
sockaddr_un saddr_un;
socklen_t saddr_un_len;
diff --git a/lldb/tools/lldb-server/lldb-platform.cpp
b/lldb/tools/lldb-server/lldb-platform.cpp
index 23d36ffb4cb66..59a83c28b0212 100644
--- a/lldb/tools/lldb-server/lldb-platform.cpp
+++ b/lldb/tools/lldb-server/lldb-platform.cpp
@@ -39,11 +39,19 @@
#if LLDB_ENABLE_POSIX
#include "lldb/Host/posix/DomainSocket.h"
#endif
+#ifdef __linux__
+#include "lldb/Host/linux/AbstractSocket.h"
+#endif
#include "lldb/Utility/FileSpec.h"
#include "lldb/Utility/LLDBLog.h"
#include "lldb/Utility/Status.h"
#include "lldb/Utility/UriParser.h"
+#if LLDB_ENABLE_POSIX
+#include <sys/socket.h>
+#include <sys/un.h>
+#endif
+
using namespace lldb;
using namespace lldb_private;
using namespace lldb_private::lldb_server;
@@ -455,14 +463,28 @@ int main_platform(int argc, char *argv[]) {
lldb_private::Args inferior_arguments;
inferior_arguments.SetArguments(argc, const_cast<const char **>(argv));
+ Log *log = GetLog(LLDBLog::Platform);
Socket::SocketProtocol protocol = Socket::ProtocolUnixDomain;
if (fd != SharedSocket::kInvalidFD) {
// Child process will handle the connection and exit.
- if (gdbserver_port)
+ if (gdbserver_port) {
protocol = Socket::ProtocolTcp;
+ } else {
+#ifdef LLDB_ENABLE_POSIX
+ // Check if fd represents domain socket or abstract socket.
+ struct sockaddr_un addr;
+ socklen_t addr_len = sizeof(addr);
+ if (getsockname(fd, (struct sockaddr *)&addr, &addr_len) == -1) {
+ LLDB_LOGF(log, "lldb-platform child: not a socket or error occurred");
+ return socket_error;
+ }
- Log *log = GetLog(LLDBLog::Platform);
+ if (addr.sun_family == AF_UNIX && addr.sun_path[0] == '\0') {
+ protocol = Socket::ProtocolUnixAbstract;
+ }
+#endif
+ }
NativeSocket sockfd;
error = SharedSocket::GetNativeSocket(fd, sockfd);
@@ -473,17 +495,27 @@ int main_platform(int argc, char *argv[]) {
GDBRemoteCommunicationServerPlatform platform(protocol, gdbserver_port);
Socket *socket;
- if (protocol == Socket::ProtocolTcp)
+ if (protocol == Socket::ProtocolTcp) {
socket = new TCPSocket(sockfd, /*should_close=*/true);
- else {
+ } else if (protocol == Socket::ProtocolUnixDomain) {
#if LLDB_ENABLE_POSIX
socket = new DomainSocket(sockfd, /*should_close=*/true);
#else
WithColor::error() << "lldb-platform child: Unix domain sockets are not "
"supported on this platform.";
return socket_error;
+#endif
+ } else {
+#ifdef __linux__
+ socket = new AbstractSocket(sockfd, /*should_close=*/true);
+#else
+ WithColor::error()
+ << "lldb-platform child: Abstract domain sockets are not "
+ "supported on this platform.";
+ return socket_error;
#endif
}
+
platform.SetConnection(
std::unique_ptr<Connection>(new ConnectionFileDescriptor(socket)));
client_handle(platform, inferior_arguments);
>From e7ddcc994def7599b95c028feb16be19b5cfe4a0 Mon Sep 17 00:00:00 2001
From: Emre Kultursay <emrekultur...@google.com>
Date: Tue, 22 Apr 2025 23:22:37 +0000
Subject: [PATCH 2/4] Move socket checking to static helper
---
lldb/include/lldb/Host/posix/DomainSocket.h | 2 +
lldb/source/Host/posix/DomainSocket.cpp | 22 +++++++++
lldb/tools/lldb-server/lldb-platform.cpp | 55 ++++-----------------
3 files changed, 33 insertions(+), 46 deletions(-)
diff --git a/lldb/include/lldb/Host/posix/DomainSocket.h
b/lldb/include/lldb/Host/posix/DomainSocket.h
index 562c011ee73e6..70a64869f8eb7 100644
--- a/lldb/include/lldb/Host/posix/DomainSocket.h
+++ b/lldb/include/lldb/Host/posix/DomainSocket.h
@@ -31,6 +31,8 @@ class DomainSocket : public Socket {
std::vector<std::string> GetListeningConnectionURI() const override;
+ static Socket *Create(NativeSocket sockfd, bool should_close, Status &error);
+
protected:
DomainSocket(SocketProtocol protocol);
DomainSocket(SocketProtocol protocol, NativeSocket socket, bool
should_close);
diff --git a/lldb/source/Host/posix/DomainSocket.cpp
b/lldb/source/Host/posix/DomainSocket.cpp
index 9b1e4f71bba2a..d2132c093b7ba 100644
--- a/lldb/source/Host/posix/DomainSocket.cpp
+++ b/lldb/source/Host/posix/DomainSocket.cpp
@@ -8,6 +8,9 @@
#include "lldb/Host/posix/DomainSocket.h"
#include "lldb/Utility/LLDBLog.h"
+#ifdef __linux__
+#include <lldb/Host/linux/AbstractSocket.h>
+#endif
#include "llvm/Support/Errno.h"
#include "llvm/Support/FileSystem.h"
@@ -188,3 +191,22 @@ std::vector<std::string>
DomainSocket::GetListeningConnectionURI() const {
return {llvm::formatv("unix-connect://{0}", addr.sun_path)};
}
+
+Socket *DomainSocket::Create(NativeSocket sockfd, bool should_close,
+ Status &error) {
+#ifdef __linux__
+ // Check if fd represents domain socket or abstract socket.
+ struct sockaddr_un addr;
+ socklen_t addr_len = sizeof(addr);
+ if (getsockname(sockfd, (struct sockaddr *)&addr, &addr_len) == -1) {
+ error = Status::FromErrorString(
+ "lldb-platform child: not a socket or error occurred");
+ return nullptr;
+ }
+
+ if (addr.sun_family == AF_UNIX && addr.sun_path[0] == '\0') {
+ return new AbstractSocket(sockfd, should_close);
+ }
+#endif
+ return new DomainSocket(sockfd, should_close);
+}
\ No newline at end of file
diff --git a/lldb/tools/lldb-server/lldb-platform.cpp
b/lldb/tools/lldb-server/lldb-platform.cpp
index 59a83c28b0212..dec4a7dc5b160 100644
--- a/lldb/tools/lldb-server/lldb-platform.cpp
+++ b/lldb/tools/lldb-server/lldb-platform.cpp
@@ -39,19 +39,11 @@
#if LLDB_ENABLE_POSIX
#include "lldb/Host/posix/DomainSocket.h"
#endif
-#ifdef __linux__
-#include "lldb/Host/linux/AbstractSocket.h"
-#endif
#include "lldb/Utility/FileSpec.h"
#include "lldb/Utility/LLDBLog.h"
#include "lldb/Utility/Status.h"
#include "lldb/Utility/UriParser.h"
-#if LLDB_ENABLE_POSIX
-#include <sys/socket.h>
-#include <sys/un.h>
-#endif
-
using namespace lldb;
using namespace lldb_private;
using namespace lldb_private::lldb_server;
@@ -464,28 +456,8 @@ int main_platform(int argc, char *argv[]) {
inferior_arguments.SetArguments(argc, const_cast<const char **>(argv));
Log *log = GetLog(LLDBLog::Platform);
- Socket::SocketProtocol protocol = Socket::ProtocolUnixDomain;
-
if (fd != SharedSocket::kInvalidFD) {
// Child process will handle the connection and exit.
- if (gdbserver_port) {
- protocol = Socket::ProtocolTcp;
- } else {
-#ifdef LLDB_ENABLE_POSIX
- // Check if fd represents domain socket or abstract socket.
- struct sockaddr_un addr;
- socklen_t addr_len = sizeof(addr);
- if (getsockname(fd, (struct sockaddr *)&addr, &addr_len) == -1) {
- LLDB_LOGF(log, "lldb-platform child: not a socket or error occurred");
- return socket_error;
- }
-
- if (addr.sun_family == AF_UNIX && addr.sun_path[0] == '\0') {
- protocol = Socket::ProtocolUnixAbstract;
- }
-#endif
- }
-
NativeSocket sockfd;
error = SharedSocket::GetNativeSocket(fd, sockfd);
if (error.Fail()) {
@@ -493,29 +465,19 @@ int main_platform(int argc, char *argv[]) {
return socket_error;
}
- GDBRemoteCommunicationServerPlatform platform(protocol, gdbserver_port);
Socket *socket;
- if (protocol == Socket::ProtocolTcp) {
+ if (gdbserver_port) {
socket = new TCPSocket(sockfd, /*should_close=*/true);
- } else if (protocol == Socket::ProtocolUnixDomain) {
-#if LLDB_ENABLE_POSIX
- socket = new DomainSocket(sockfd, /*should_close=*/true);
-#else
- WithColor::error() << "lldb-platform child: Unix domain sockets are not "
- "supported on this platform.";
- return socket_error;
-#endif
} else {
-#ifdef __linux__
- socket = new AbstractSocket(sockfd, /*should_close=*/true);
-#else
- WithColor::error()
- << "lldb-platform child: Abstract domain sockets are not "
- "supported on this platform.";
- return socket_error;
-#endif
+ socket = DomainSocket::Create(sockfd, /*should_close=*/true, error);
+ if (error.Fail()) {
+ LLDB_LOGF(log, "Failed to create socket: %s\n", error.AsCString());
+ return socket_error;
+ }
}
+ Socket::SocketProtocol protocol = socket->GetSocketProtocol();
+ GDBRemoteCommunicationServerPlatform platform(protocol, gdbserver_port);
platform.SetConnection(
std::unique_ptr<Connection>(new ConnectionFileDescriptor(socket)));
client_handle(platform, inferior_arguments);
@@ -530,6 +492,7 @@ int main_platform(int argc, char *argv[]) {
return 1;
}
+ Socket::SocketProtocol protocol;
std::string address;
std::string gdb_address;
uint16_t platform_port = 0;
>From 710bf40998661c4d9c21664017a889f202176e50 Mon Sep 17 00:00:00 2001
From: Emre Kultursay <emrekultur...@google.com>
Date: Wed, 23 Apr 2025 21:25:09 +0000
Subject: [PATCH 3/4] Address code review comments
---
lldb/include/lldb/Host/posix/DomainSocket.h | 3 ++-
lldb/source/Host/posix/DomainSocket.cpp | 24 ++++++++++-----------
lldb/tools/lldb-server/lldb-platform.cpp | 20 +++++++++--------
3 files changed, 24 insertions(+), 23 deletions(-)
diff --git a/lldb/include/lldb/Host/posix/DomainSocket.h
b/lldb/include/lldb/Host/posix/DomainSocket.h
index 70a64869f8eb7..a840d474429ec 100644
--- a/lldb/include/lldb/Host/posix/DomainSocket.h
+++ b/lldb/include/lldb/Host/posix/DomainSocket.h
@@ -31,7 +31,8 @@ class DomainSocket : public Socket {
std::vector<std::string> GetListeningConnectionURI() const override;
- static Socket *Create(NativeSocket sockfd, bool should_close, Status &error);
+ static llvm::Expected<std::unique_ptr<DomainSocket>>
+ FromBoundNativeSocket(NativeSocket sockfd, bool should_close);
protected:
DomainSocket(SocketProtocol protocol);
diff --git a/lldb/source/Host/posix/DomainSocket.cpp
b/lldb/source/Host/posix/DomainSocket.cpp
index d2132c093b7ba..8831c91aa8167 100644
--- a/lldb/source/Host/posix/DomainSocket.cpp
+++ b/lldb/source/Host/posix/DomainSocket.cpp
@@ -192,21 +192,19 @@ std::vector<std::string>
DomainSocket::GetListeningConnectionURI() const {
return {llvm::formatv("unix-connect://{0}", addr.sun_path)};
}
-Socket *DomainSocket::Create(NativeSocket sockfd, bool should_close,
- Status &error) {
+llvm::Expected<std::unique_ptr<DomainSocket>>
+DomainSocket::FromBoundNativeSocket(NativeSocket sockfd, bool should_close) {
#ifdef __linux__
// Check if fd represents domain socket or abstract socket.
struct sockaddr_un addr;
socklen_t addr_len = sizeof(addr);
- if (getsockname(sockfd, (struct sockaddr *)&addr, &addr_len) == -1) {
- error = Status::FromErrorString(
- "lldb-platform child: not a socket or error occurred");
- return nullptr;
- }
-
- if (addr.sun_family == AF_UNIX && addr.sun_path[0] == '\0') {
- return new AbstractSocket(sockfd, should_close);
- }
+ if (getsockname(sockfd, (struct sockaddr *)&addr, &addr_len) == -1)
+ return llvm::createStringError("not a socket or error occurred");
+ if (addr.sun_family != AF_UNIX)
+ return llvm::createStringError("Bad socket type");
+ if (addr_len > offsetof(struct sockaddr_un, sun_path) &&
+ addr.sun_path[0] == '\0')
+ return std::make_unique<AbstractSocket>(sockfd, should_close);
#endif
- return new DomainSocket(sockfd, should_close);
-}
\ No newline at end of file
+ return std::make_unique<DomainSocket>(sockfd, should_close);
+}
diff --git a/lldb/tools/lldb-server/lldb-platform.cpp
b/lldb/tools/lldb-server/lldb-platform.cpp
index dec4a7dc5b160..aa35ced6667f6 100644
--- a/lldb/tools/lldb-server/lldb-platform.cpp
+++ b/lldb/tools/lldb-server/lldb-platform.cpp
@@ -465,21 +465,23 @@ int main_platform(int argc, char *argv[]) {
return socket_error;
}
- Socket *socket;
+ std::unique_ptr<Socket> socket;
if (gdbserver_port) {
- socket = new TCPSocket(sockfd, /*should_close=*/true);
+ socket = std::make_unique<TCPSocket>(sockfd, /*should_close=*/true);
} else {
- socket = DomainSocket::Create(sockfd, /*should_close=*/true, error);
- if (error.Fail()) {
+ llvm::Expected<std::unique_ptr<DomainSocket>> domain_socket =
+ DomainSocket::FromBoundNativeSocket(sockfd, /*should_close=*/true);
+ if (!domain_socket) {
LLDB_LOGF(log, "Failed to create socket: %s\n", error.AsCString());
return socket_error;
}
+ socket = std::move(domain_socket.get());
}
- Socket::SocketProtocol protocol = socket->GetSocketProtocol();
- GDBRemoteCommunicationServerPlatform platform(protocol, gdbserver_port);
- platform.SetConnection(
- std::unique_ptr<Connection>(new ConnectionFileDescriptor(socket)));
+ GDBRemoteCommunicationServerPlatform platform(socket->GetSocketProtocol(),
+ gdbserver_port);
+ platform.SetConnection(std::unique_ptr<Connection>(
+ new ConnectionFileDescriptor(socket.release())));
client_handle(platform, inferior_arguments);
return 0;
}
@@ -492,7 +494,7 @@ int main_platform(int argc, char *argv[]) {
return 1;
}
- Socket::SocketProtocol protocol;
+ Socket::SocketProtocol protocol = Socket::ProtocolUnixDomain;
std::string address;
std::string gdb_address;
uint16_t platform_port = 0;
>From f55869ce06120abfeb3c2669ffc53ece16732ffc Mon Sep 17 00:00:00 2001
From: Emre Kultursay <emrekultur...@google.com>
Date: Thu, 24 Apr 2025 00:54:15 +0000
Subject: [PATCH 4/4] Add test for FromBoundNativeSocket
---
lldb/unittests/Host/SocketTest.cpp | 45 ++++++++++++++++++++++++++++++
1 file changed, 45 insertions(+)
diff --git a/lldb/unittests/Host/SocketTest.cpp
b/lldb/unittests/Host/SocketTest.cpp
index 9cbf3e26b0883..884832440cecd 100644
--- a/lldb/unittests/Host/SocketTest.cpp
+++ b/lldb/unittests/Host/SocketTest.cpp
@@ -15,6 +15,9 @@
#include "gmock/gmock.h"
#include "gtest/gtest.h"
#include <chrono>
+#if __linux__
+#include <lldb/Host/linux/AbstractSocket.h>
+#endif
using namespace lldb_private;
@@ -339,6 +342,48 @@ TEST_F(SocketTest, DomainGetConnectURI) {
}
#endif
+#if LLDB_ENABLE_POSIX
+TEST_F(SocketTest, DomainSocketFromBoundNativeSocket) {
+ // Generate a name for the domain socket.
+ llvm::SmallString<64> name;
+ std::error_code EC = llvm::sys::fs::createUniqueDirectory(
+ "DomainSocketFromBoundNativeSocket", name);
+ ASSERT_FALSE(EC);
+ llvm::sys::path::append(name, "test");
+
+ DomainSocket socket(true);
+ Status error = socket.Listen(name, /*backlog=*/10);
+ ASSERT_FALSE(error.ToError());
+ NativeSocket native_socket = socket.GetNativeSocket();
+
+ llvm::Expected<std::unique_ptr<DomainSocket>> sock =
+ DomainSocket::FromBoundNativeSocket(native_socket,
/*should_close=*/true);
+ ASSERT_THAT_EXPECTED(sock, llvm::Succeeded());
+ ASSERT_EQ(Socket::ProtocolUnixDomain, sock->get()->GetSocketProtocol());
+}
+#endif
+
+#if __linux__
+TEST_F(SocketTest, AbstractSocketFromBoundNativeSocket) {
+ // Generate a name for the abstract socket.
+ llvm::SmallString<64> name;
+ std::error_code EC = llvm::sys::fs::createUniqueDirectory(
+ "AbstractSocketFromBoundNativeSocket", name);
+ ASSERT_FALSE(EC);
+ llvm::sys::path::append(name, "test");
+
+ AbstractSocket socket;
+ Status error = socket.Listen(name, /*backlog=*/10);
+ ASSERT_FALSE(error.ToError());
+ NativeSocket native_socket = socket.GetNativeSocket();
+
+ llvm::Expected<std::unique_ptr<DomainSocket>> sock =
+ DomainSocket::FromBoundNativeSocket(native_socket,
/*should_close=*/true);
+ ASSERT_THAT_EXPECTED(sock, llvm::Succeeded());
+ ASSERT_EQ(Socket::ProtocolUnixAbstract, sock->get()->GetSocketProtocol());
+}
+#endif
+
INSTANTIATE_TEST_SUITE_P(
SocketTests, SocketTest,
testing::Values(SocketTestParams{/*is_ipv6=*/false,
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
