emrekultursay wrote:
Thanks. Are the following patches good enough, or do we prefer a revert?
- Test fix: https://github.com/llvm/llvm-project/pull/137405
- Windows fix: https://github.com/llvm/llvm-project/pull/137414
https://github.com/llvm/llvm-project/pull/136466
__
slydiman wrote:
Note DomainSocket is not supported on Windows, so removing the following code
broke the Windows build!
```
#if LLDB_ENABLE_POSIX
socket = new DomainSocket(sockfd, /*should_close=*/true);
#else
WithColor::error() << "lldb-platform child: Unix domain sockets are not "
emrekultursay wrote:
We're getting `SetSockAddr` error which can only fail if the path is too long.
The path generated by `createUniqueDirectory` is an absolute path to a
temporary directory. I guess it can be longer than 107 bytes on the CI machines.
All the other DomainSocket related tests s
emrekultursay wrote:
Thanks folks:
1. Can you please squash-and-merge this PR? (I don't have write permissions)
2. Would you like me to create a cherry-pick into release/20.x branch, or is
there a separate process for cherry-picks (e.g., wait some time to validate,
submit a cherry-pick request
llvmbot wrote:
>> 2. Would you like me to create a cherry-pick into release/20.x branch, or is
>> there a separate process for cherry-picks (e.g., wait some time to validate,
>> submit a cherry-pick request somewhere, etc.)?
>
>Please file and issue (here's an example:
>https://github.com/llv
llvmbot wrote:
>> 2. Would you like me to create a cherry-pick into release/20.x branch, or is
>> there a separate process for cherry-picks (e.g., wait some time to validate,
>> submit a cherry-pick request somewhere, etc.)?
>
>Please file and issue (here's an example:
>https://github.com/llv
JDevlieghere wrote:
> 2. Would you like me to create a cherry-pick into release/20.x branch, or is
> there a separate process for cherry-picks (e.g., wait some time to validate,
> submit a cherry-pick request somewhere, etc.)?
Please file and issue (here's an example:
https://github.com/llvm/
https://github.com/JDevlieghere closed
https://github.com/llvm/llvm-project/pull/136466
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
https://github.com/slydiman approved this pull request.
Thanks
https://github.com/llvm/llvm-project/pull/136466
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
@@ -455,37 +455,33 @@ int main_platform(int argc, char *argv[]) {
lldb_private::Args inferior_arguments;
inferior_arguments.SetArguments(argc, const_cast(argv));
- Socket::SocketProtocol protocol = Socket::ProtocolUnixDomain;
-
+ Log *log = GetLog(LLDBLog::Platform);
@@ -182,3 +191,20 @@ std::vector
DomainSocket::GetListeningConnectionURI() const {
return {llvm::formatv("unix-connect://{0}", addr.sun_path)};
}
+
+llvm::Expected>
+DomainSocket::FromBoundNativeSocket(NativeSocket sockfd, bool should_close) {
+#ifdef __linux__
+ // Check
@@ -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::createUniq
https://github.com/emrekultursay commented:
Thanks. PTAL.
https://github.com/llvm/llvm-project/pull/136466
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
@@ -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::createUniq
@@ -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::createUniq
https://github.com/emrekultursay edited
https://github.com/llvm/llvm-project/pull/136466
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
@@ -339,6 +342,48 @@ TEST_F(SocketTest, DomainGetConnectURI) {
}
#endif
+#if LLDB_ENABLE_POSIX
emrekultursay wrote:
Done.
https://github.com/llvm/llvm-project/pull/136466
___
lldb-commits mailing list
lldb-commits@
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
Date: Sun, 20 Apr 2025 01:11:31 +
Subject: [PATCH 1/5] Fix connecting via abstract socket
Commit 82ee31f an
https://github.com/labath approved this pull request.
https://github.com/llvm/llvm-project/pull/136466
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
@@ -339,6 +342,48 @@ TEST_F(SocketTest, DomainGetConnectURI) {
}
#endif
+#if LLDB_ENABLE_POSIX
labath wrote:
```suggestion
```
https://github.com/llvm/llvm-project/pull/136466
___
lldb-commits mailing list
lldb-com
@@ -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::createUniq
@@ -182,3 +191,20 @@ std::vector
DomainSocket::GetListeningConnectionURI() const {
return {llvm::formatv("unix-connect://{0}", addr.sun_path)};
}
+
+llvm::Expected>
+DomainSocket::FromBoundNativeSocket(NativeSocket sockfd, bool should_close) {
+#ifdef __linux__
+ // Check
@@ -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::createUniq
@@ -455,37 +455,33 @@ int main_platform(int argc, char *argv[]) {
lldb_private::Args inferior_arguments;
inferior_arguments.SetArguments(argc, const_cast(argv));
- Socket::SocketProtocol protocol = Socket::ProtocolUnixDomain;
-
+ Log *log = GetLog(LLDBLog::Platform);
@@ -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::createUniq
@@ -31,8 +31,11 @@ class DomainSocket : public Socket {
std::vector GetListeningConnectionURI() const override;
+ static Socket *Create(NativeSocket sockfd, bool should_close, Status &error);
emrekultursay wrote:
Done.
https://github.com/llvm/llvm-projec
@@ -182,3 +191,22 @@ std::vector
DomainSocket::GetListeningConnectionURI() const {
return {llvm::formatv("unix-connect://{0}", addr.sun_path)};
}
+
+Socket *DomainSocket::Create(NativeSocket sockfd, bool should_close,
+ Status &error) {
+#ifdef _
https://github.com/emrekultursay commented:
PTAL. I added unit tests for the method, but I'm open to suggestions on how to
improve that test.
https://github.com/llvm/llvm-project/pull/136466
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
@@ -182,3 +191,22 @@ std::vector
DomainSocket::GetListeningConnectionURI() const {
return {llvm::formatv("unix-connect://{0}", addr.sun_path)};
}
+
+Socket *DomainSocket::Create(NativeSocket sockfd, bool should_close,
+ Status &error) {
+#ifdef _
@@ -455,35 +455,29 @@ int main_platform(int argc, char *argv[]) {
lldb_private::Args inferior_arguments;
inferior_arguments.SetArguments(argc, const_cast(argv));
- Socket::SocketProtocol protocol = Socket::ProtocolUnixDomain;
emrekultursay wrote:
Done by
@@ -455,35 +455,29 @@ int main_platform(int argc, char *argv[]) {
lldb_private::Args inferior_arguments;
inferior_arguments.SetArguments(argc, const_cast(argv));
- Socket::SocketProtocol protocol = Socket::ProtocolUnixDomain;
-
+ Log *log = GetLog(LLDBLog::Platform);
@@ -182,3 +191,22 @@ std::vector
DomainSocket::GetListeningConnectionURI() const {
return {llvm::formatv("unix-connect://{0}", addr.sun_path)};
}
+
+Socket *DomainSocket::Create(NativeSocket sockfd, bool should_close,
+ Status &error) {
+#ifdef _
https://github.com/emrekultursay edited
https://github.com/llvm/llvm-project/pull/136466
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
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
Date: Sun, 20 Apr 2025 01:11:31 +
Subject: [PATCH 1/4] Fix connecting via abstract socket
Commit 82ee31f an
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
Date: Sun, 20 Apr 2025 01:11:31 +
Subject: [PATCH 1/3] Fix connecting via abstract socket
Commit 82ee31f an
@@ -31,8 +31,11 @@ class DomainSocket : public Socket {
std::vector GetListeningConnectionURI() const override;
+ static Socket *Create(NativeSocket sockfd, bool should_close, Status &error);
labath wrote:
I have a bit of a problem with this name because,
@@ -182,3 +191,22 @@ std::vector
DomainSocket::GetListeningConnectionURI() const {
return {llvm::formatv("unix-connect://{0}", addr.sun_path)};
}
+
+Socket *DomainSocket::Create(NativeSocket sockfd, bool should_close,
+ Status &error) {
+#ifdef _
https://github.com/labath edited
https://github.com/llvm/llvm-project/pull/136466
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
@@ -182,3 +191,22 @@ std::vector
DomainSocket::GetListeningConnectionURI() const {
return {llvm::formatv("unix-connect://{0}", addr.sun_path)};
}
+
+Socket *DomainSocket::Create(NativeSocket sockfd, bool should_close,
+ Status &error) {
+#ifdef _
https://github.com/labath commented:
We should also have a test for this. A unit test for the new function should be
easy enough. I'm not going to insist on a test for the whole "lldb-server
platform" flow, but I would recommend it, as that's the only way to ensure this
does not break again.
@@ -182,3 +191,22 @@ std::vector
DomainSocket::GetListeningConnectionURI() const {
return {llvm::formatv("unix-connect://{0}", addr.sun_path)};
}
+
+Socket *DomainSocket::Create(NativeSocket sockfd, bool should_close,
+ Status &error) {
+#ifdef _
@@ -455,35 +455,29 @@ int main_platform(int argc, char *argv[]) {
lldb_private::Args inferior_arguments;
inferior_arguments.SetArguments(argc, const_cast(argv));
- Socket::SocketProtocol protocol = Socket::ProtocolUnixDomain;
-
+ Log *log = GetLog(LLDBLog::Platform);
@@ -182,3 +191,22 @@ std::vector
DomainSocket::GetListeningConnectionURI() const {
return {llvm::formatv("unix-connect://{0}", addr.sun_path)};
}
+
+Socket *DomainSocket::Create(NativeSocket sockfd, bool should_close,
+ Status &error) {
+#ifdef _
@@ -455,35 +455,29 @@ int main_platform(int argc, char *argv[]) {
lldb_private::Args inferior_arguments;
inferior_arguments.SetArguments(argc, const_cast(argv));
- Socket::SocketProtocol protocol = Socket::ProtocolUnixDomain;
slydiman wrote:
There is a p
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
Date: Sun, 20 Apr 2025 01:11:31 +
Subject: [PATCH 1/2] Fix connecting via abstract socket
Commit 82ee31f an
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
Date: Sun, 20 Apr 2025 01:11:31 +
Subject: [PATCH 1/2] Fix connecting via abstract socket
Commit 82ee31f an
slydiman wrote:
Don't forget to update the protocol passed to the
GDBRemoteCommunicationServerPlatform constructor.
https://github.com/llvm/llvm-project/pull/136466
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-b
labath wrote:
Agreed
https://github.com/llvm/llvm-project/pull/136466
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
slydiman wrote:
I like it.
I would suggest to move domain/abstract socket checking to a static helper,
something like
```
Socket* DomainSocket::Create(NativeSocket sockfd, bool should_close) {
#ifdef __linux__
// Check if fd represents domain socket or abstract socket.
if (/*abstract*/)
https://github.com/emrekultursay edited
https://github.com/llvm/llvm-project/pull/136466
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
emrekultursay wrote:
@slydiman Could you also take a look at this PR, as you are the author of the 2
commits mentioned in the description? Thanks.
https://github.com/llvm/llvm-project/pull/136466
___
lldb-commits mailing list
lldb-commits@lists.llvm.o
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
Date: Sun, 20 Apr 2025 01:11:31 +
Subject: [PATCH] Fix connecting via abstract socket
Commit 82ee31f and Co
https://github.com/emrekultursay edited
https://github.com/llvm/llvm-project/pull/136466
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
https://github.com/emrekultursay edited
https://github.com/llvm/llvm-project/pull/136466
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
https://github.com/emrekultursay edited
https://github.com/llvm/llvm-project/pull/136466
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
https://github.com/emrekultursay updated
https://github.com/llvm/llvm-project/pull/136466
>From fd514c43019e25f36e12166b7239f7882ea7eca4 Mon Sep 17 00:00:00 2001
From: Emre Kultursay
Date: Sun, 20 Apr 2025 01:11:31 +
Subject: [PATCH] Fix connecting via abstract socket
Commit 82ee31f and Co
https://github.com/emrekultursay updated
https://github.com/llvm/llvm-project/pull/136466
>From 2aedf974b8743b9291fbd333af2eaa6d4d5cadcc Mon Sep 17 00:00:00 2001
From: Emre Kultursay
Date: Sun, 20 Apr 2025 01:11:31 +
Subject: [PATCH] Fix connecting via abstract socket
Commits 82ee31f and 2
llvmbot wrote:
@llvm/pr-subscribers-lldb
Author: Emre Kultursay (emrekultursay)
Changes
Commits 82ee31f and 2e89312 added socket sharing, but only for unix domain
sockets. That broke Android, which uses unix-abstract sockets.
---
Full diff: https://github.com/llvm/llvm-project/pull/13646
https://github.com/emrekultursay created
https://github.com/llvm/llvm-project/pull/136466
Commits 82ee31f and 2e89312 added socket sharing, but only for unix domain
sockets. That broke Android, which uses unix-abstract sockets.
>From 950bafe1b6b900384b8d0f925f1cdab19a2c8e3d Mon Sep 17 00:00:00
github-actions[bot] wrote:
:warning: C/C++ code formatter, clang-format found issues in your code.
:warning:
You can test this locally with the following command:
``bash
git-clang-format --diff HEAD~1 HEAD --extensions cpp,h --
lldb/include/lldb/Host/linux/AbstractSocket.h
lldb
60 matches
Mail list logo
