[Lldb-commits] [lldb] [lldb] When using Socket to listen on `localhost:0` on systems supporting ting ipv4 and ipv6 the second socket to initialize will not update the listening address correctly after the call to `bind`. (PR #118565)
John Harrison via lldb-commits
lldb-commits at lists.llvm.org
Tue Dec 3 16:22:39 PST 2024
https://github.com/ashgti created https://github.com/llvm/llvm-project/pull/118565
This results in the second address listed in `Socket::GetListeningConnectionURI` to have port `:0`, which is incorrect.
To fix this, correct which address is used to detect the port and update the unit tests to cover this use case.
Additionally, I updated the SocketTest's to only parameterize tests that can work on ipv4 or ipv6. This means tests like `SocketTest::DecodeHostAndPort` are only run once, instead of twice since they do not change behavior based on parameters. I also included a new unit test to cover listening on `localhost:0`, validating both sockets correctly list the updated port.
>From d38d568653aca9cb90853cf64adc6bee69289a4e Mon Sep 17 00:00:00 2001
From: John Harrison <harjohn at google.com>
Date: Tue, 3 Dec 2024 16:16:59 -0800
Subject: [PATCH] [lldb] When using Socket to listen on `localhost:0` on
systems supporting ipv4 and ipv6 the second socket to initialize will not
update the listening address correctly after the call to `bind`.
This results in the second address listed in `Socket::GetListeningConnectionURI` to have port `:0`, which is incorrect.
To fix this, correct which address is used to detect the port and update the unit tests to cover this use case.
Additionally, I updated the SocketTest's to only parameterize tests that can work on ipv4 or ipv6. This means tests like `SocketTest::DecodeHostAndPort` are only run once, instead of twice since they do not change behavior based on parameters. I also included a new unit test to cover listening on `localhost:0`, validating both sockets correctly list the updated port.
---
lldb/source/Host/common/TCPSocket.cpp | 8 ++---
lldb/unittests/Host/SocketTest.cpp | 43 +++++++++++++++++++--------
2 files changed, 35 insertions(+), 16 deletions(-)
diff --git a/lldb/source/Host/common/TCPSocket.cpp b/lldb/source/Host/common/TCPSocket.cpp
index d0055c3b6c44fb..d3282ab58b8185 100644
--- a/lldb/source/Host/common/TCPSocket.cpp
+++ b/lldb/source/Host/common/TCPSocket.cpp
@@ -216,11 +216,11 @@ Status TCPSocket::Listen(llvm::StringRef name, int backlog) {
}
if (host_port->port == 0) {
- socklen_t sa_len = address.GetLength();
- if (getsockname(fd, &address.sockaddr(), &sa_len) == 0)
- host_port->port = address.GetPort();
+ socklen_t sa_len = listen_address.GetLength();
+ if (getsockname(fd, &listen_address.sockaddr(), &sa_len) == 0)
+ host_port->port = listen_address.GetPort();
}
- m_listen_sockets[fd] = address;
+ m_listen_sockets[fd] = listen_address;
}
if (m_listen_sockets.empty()) {
diff --git a/lldb/unittests/Host/SocketTest.cpp b/lldb/unittests/Host/SocketTest.cpp
index a74352c19725d2..689ef4019c618c 100644
--- a/lldb/unittests/Host/SocketTest.cpp
+++ b/lldb/unittests/Host/SocketTest.cpp
@@ -35,7 +35,7 @@ class SocketTest : public testing::TestWithParam<SocketTestParams> {
}
};
-TEST_P(SocketTest, DecodeHostAndPort) {
+TEST_F(SocketTest, DecodeHostAndPort) {
EXPECT_THAT_EXPECTED(Socket::DecodeHostAndPort("localhost:1138"),
llvm::HasValue(Socket::HostAndPort{"localhost", 1138}));
@@ -63,9 +63,8 @@ TEST_P(SocketTest, DecodeHostAndPort) {
EXPECT_THAT_EXPECTED(Socket::DecodeHostAndPort("*:65535"),
llvm::HasValue(Socket::HostAndPort{"*", 65535}));
- EXPECT_THAT_EXPECTED(
- Socket::DecodeHostAndPort("[::1]:12345"),
- llvm::HasValue(Socket::HostAndPort{"::1", 12345}));
+ EXPECT_THAT_EXPECTED(Socket::DecodeHostAndPort("[::1]:12345"),
+ llvm::HasValue(Socket::HostAndPort{"::1", 12345}));
EXPECT_THAT_EXPECTED(
Socket::DecodeHostAndPort("[abcd:12fg:AF58::1]:12345"),
@@ -73,9 +72,10 @@ TEST_P(SocketTest, DecodeHostAndPort) {
}
#if LLDB_ENABLE_POSIX
-TEST_P(SocketTest, DomainListenConnectAccept) {
+TEST_F(SocketTest, DomainListenConnectAccept) {
llvm::SmallString<64> Path;
- std::error_code EC = llvm::sys::fs::createUniqueDirectory("DomainListenConnectAccept", Path);
+ std::error_code EC =
+ llvm::sys::fs::createUniqueDirectory("DomainListenConnectAccept", Path);
ASSERT_FALSE(EC);
llvm::sys::path::append(Path, "test");
@@ -88,7 +88,7 @@ TEST_P(SocketTest, DomainListenConnectAccept) {
CreateDomainConnectedSockets(Path, &socket_a_up, &socket_b_up);
}
-TEST_P(SocketTest, DomainListenGetListeningConnectionURI) {
+TEST_F(SocketTest, DomainListenGetListeningConnectionURI) {
llvm::SmallString<64> Path;
std::error_code EC =
llvm::sys::fs::createUniqueDirectory("DomainListenConnectAccept", Path);
@@ -110,7 +110,7 @@ TEST_P(SocketTest, DomainListenGetListeningConnectionURI) {
testing::ElementsAre(llvm::formatv("unix-connect://{0}", Path).str()));
}
-TEST_P(SocketTest, DomainMainLoopAccept) {
+TEST_F(SocketTest, DomainMainLoopAccept) {
llvm::SmallString<64> Path;
std::error_code EC =
llvm::sys::fs::createUniqueDirectory("DomainListenConnectAccept", Path);
@@ -270,6 +270,25 @@ TEST_P(SocketTest, TCPListen0GetListeningConnectionURI) {
.str()));
}
+TEST_F(SocketTest, TCPListen0MultiListenerGetListeningConnectionURI) {
+ if (!HostSupportsIPv6() || !HostSupportsIPv4())
+ return;
+
+ llvm::Expected<std::unique_ptr<TCPSocket>> sock =
+ Socket::TcpListen("localhost:0", 5);
+ ASSERT_THAT_EXPECTED(sock, llvm::Succeeded());
+ ASSERT_TRUE(sock.get()->IsValid());
+
+ EXPECT_THAT(sock.get()->GetListeningConnectionURI(),
+ testing::UnorderedElementsAre(
+ llvm::formatv("connection://[::1]:{0}",
+ sock->get()->GetLocalPortNumber())
+ .str(),
+ llvm::formatv("connection://[127.0.0.1]:{0}",
+ sock->get()->GetLocalPortNumber())
+ .str()));
+}
+
TEST_P(SocketTest, TCPGetConnectURI) {
std::unique_ptr<TCPSocket> socket_a_up;
std::unique_ptr<TCPSocket> socket_b_up;
@@ -297,10 +316,10 @@ TEST_P(SocketTest, UDPGetConnectURI) {
}
#if LLDB_ENABLE_POSIX
-TEST_P(SocketTest, DomainGetConnectURI) {
+TEST_F(SocketTest, DomainGetConnectURI) {
llvm::SmallString<64> domain_path;
- std::error_code EC =
- llvm::sys::fs::createUniqueDirectory("DomainListenConnectAccept", domain_path);
+ std::error_code EC = llvm::sys::fs::createUniqueDirectory(
+ "DomainListenConnectAccept", domain_path);
ASSERT_FALSE(EC);
llvm::sys::path::append(domain_path, "test");
@@ -325,7 +344,7 @@ INSTANTIATE_TEST_SUITE_P(
testing::Values(SocketTestParams{/*is_ipv6=*/false,
/*localhost_ip=*/"127.0.0.1"},
SocketTestParams{/*is_ipv6=*/true, /*localhost_ip=*/"::1"}),
- // Prints "SocketTests/SocketTest.DecodeHostAndPort/ipv4" etc. in test logs.
+ // Prints "SocketTests/SocketTest.TCPGetAddress/ipv4" etc. in test logs.
[](const testing::TestParamInfo<SocketTestParams> &info) {
return info.param.is_ipv6 ? "ipv6" : "ipv4";
});
More information about the lldb-commits
mailing list