llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: John Harrison (ashgti) <details> <summary>Changes</summary> 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. --- Full diff: https://github.com/llvm/llvm-project/pull/118565.diff 2 Files Affected: - (modified) lldb/source/Host/common/TCPSocket.cpp (+4-4) - (modified) lldb/unittests/Host/SocketTest.cpp (+31-12) ``````````diff 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"; }); `````````` </details> https://github.com/llvm/llvm-project/pull/118565 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
