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/2] 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 1f4192763397b362efd03637f645c8746c7d3681 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/2] Move socket checking to static helper --- lldb/include/lldb/Host/posix/DomainSocket.h | 2 + lldb/source/Host/posix/DomainSocket.cpp | 24 +++++++++ lldb/tools/lldb-server/lldb-platform.cpp | 55 ++++----------------- 3 files changed, 35 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..94881c5e1d6f4 100644 --- a/lldb/source/Host/posix/DomainSocket.cpp +++ b/lldb/source/Host/posix/DomainSocket.cpp @@ -8,14 +8,19 @@ #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" #include <cstddef> #include <memory> +#ifdef __linux__ #include <sys/socket.h> #include <sys/un.h> +#endif using namespace lldb; using namespace lldb_private; @@ -188,3 +193,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; _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits