[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.

Author: ashgti

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()) {

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,19 +63,19 @@ 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"),
       llvm::HasValue(Socket::HostAndPort{"abcd:12fg:AF58::1", 12345}));
 }
 
 #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";
     });