From c1af84ceaf4fafbfa47f871436986c5c69f65a73 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Micha=C5=82=20G=C3=B3rny?= Date: Fri, 24 Sep 2021 13:33:51 +0200 Subject: [PATCH] Revert "[lldb] [Host] Refactor Socket::DecodeHostAndPort() to use LLVM API" This reverts commit a6daf99228bc16fb7f2596d67a0d00fef327ace5. It causes buildbot regressions, I'll investigate. --- lldb/include/lldb/Host/Socket.h | 6 +-- lldb/source/Host/common/Socket.cpp | 62 ++++++++++++++++++----------- lldb/source/Host/common/TCPSocket.cpp | 14 +++---- lldb/source/Host/common/UDPSocket.cpp | 7 ++-- lldb/tools/lldb-server/Acceptor.cpp | 5 +-- lldb/unittests/Host/SocketTest.cpp | 75 ++++++++++++++++++----------------- 6 files changed, 92 insertions(+), 77 deletions(-) diff --git a/lldb/include/lldb/Host/Socket.h b/lldb/include/lldb/Host/Socket.h index 12ddc11..36db0ec 100644 --- a/lldb/include/lldb/Host/Socket.h +++ b/lldb/include/lldb/Host/Socket.h @@ -103,9 +103,9 @@ public: bool IsValid() const override { return m_socket != kInvalidSocketValue; } WaitableHandle GetWaitableHandle() override; - static llvm::Error DecodeHostAndPort(llvm::StringRef host_and_port, - std::string &host_str, - std::string &port_str, uint16_t &port); + static bool DecodeHostAndPort(llvm::StringRef host_and_port, + std::string &host_str, std::string &port_str, + int32_t &port, Status *error_ptr); // If this Socket is connected then return the URI used to connect. virtual std::string GetRemoteConnectionURI() const { return ""; }; diff --git a/lldb/source/Host/common/Socket.cpp b/lldb/source/Host/common/Socket.cpp index eb2495b..d1c327d 100644 --- a/lldb/source/Host/common/Socket.cpp +++ b/lldb/source/Host/common/Socket.cpp @@ -11,9 +11,11 @@ #include "lldb/Host/Config.h" #include "lldb/Host/Host.h" #include "lldb/Host/SocketAddress.h" +#include "lldb/Host/StringConvert.h" #include "lldb/Host/common/TCPSocket.h" #include "lldb/Host/common/UDPSocket.h" #include "lldb/Utility/Log.h" +#include "lldb/Utility/RegularExpression.h" #include "llvm/ADT/STLExtras.h" #include "llvm/Support/Errno.h" @@ -170,17 +172,17 @@ Socket::TcpListen(llvm::StringRef host_and_port, bool child_processes_inherit, Log *log(lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_CONNECTION)); LLDB_LOG(log, "host_and_port = {0}", host_and_port); + Status error; std::string host_str; std::string port_str; - uint16_t port; - if (llvm::Error decode_error = - DecodeHostAndPort(host_and_port, host_str, port_str, port)) - return decode_error; + int32_t port = INT32_MIN; + if (!DecodeHostAndPort(host_and_port, host_str, port_str, port, &error)) + return error.ToError(); std::unique_ptr listen_socket( new TCPSocket(true, child_processes_inherit)); - Status error = listen_socket->Listen(host_and_port, backlog); + error = listen_socket->Listen(host_and_port, backlog); if (error.Fail()) return error.ToError(); @@ -270,33 +272,47 @@ Status Socket::UnixAbstractAccept(llvm::StringRef name, return error; } -llvm::Error Socket::DecodeHostAndPort(llvm::StringRef host_and_port, - std::string &host_str, - std::string &port_str, uint16_t &port) { - static llvm::Regex g_regex("([^:]+|\\[[0-9a-fA-F:]+.*\\]):([0-9]+)"); +bool Socket::DecodeHostAndPort(llvm::StringRef host_and_port, + std::string &host_str, std::string &port_str, + int32_t &port, Status *error_ptr) { + static RegularExpression g_regex( + llvm::StringRef("([^:]+|\\[[0-9a-fA-F:]+.*\\]):([0-9]+)")); llvm::SmallVector matches; - if (g_regex.match(host_and_port, &matches)) { + if (g_regex.Execute(host_and_port, &matches)) { host_str = matches[1].str(); port_str = matches[2].str(); // IPv6 addresses are wrapped in [] when specified with ports if (host_str.front() == '[' && host_str.back() == ']') host_str = host_str.substr(1, host_str.size() - 2); - if (to_integer(matches[2], port, 10)) - return llvm::Error::success(); - } else { - // If this was unsuccessful, then check if it's simply a signed 32-bit - // integer, representing a port with an empty host. - host_str.clear(); - port_str.clear(); - if (to_integer(host_and_port, port, 10)) { - port_str = host_and_port.str(); - return llvm::Error::success(); + bool ok = false; + port = StringConvert::ToUInt32(port_str.c_str(), UINT32_MAX, 10, &ok); + if (ok && port <= UINT16_MAX) { + if (error_ptr) + error_ptr->Clear(); + return true; } + // port is too large + if (error_ptr) + error_ptr->SetErrorStringWithFormat( + "invalid host:port specification: '%s'", host_and_port.str().c_str()); + return false; + } + + // If this was unsuccessful, then check if it's simply a signed 32-bit + // integer, representing a port with an empty host. + host_str.clear(); + port_str.clear(); + if (to_integer(host_and_port, port, 10) && port < UINT16_MAX) { + port_str = std::string(host_and_port); + if (error_ptr) + error_ptr->Clear(); + return true; } - return llvm::createStringError(llvm::inconvertibleErrorCode(), - "invalid host:port specification: '%s'", - host_and_port.str().c_str()); + if (error_ptr) + error_ptr->SetErrorStringWithFormat("invalid host:port specification: '%s'", + host_and_port.str().c_str()); + return false; } IOObject::WaitableHandle Socket::GetWaitableHandle() { diff --git a/lldb/source/Host/common/TCPSocket.cpp b/lldb/source/Host/common/TCPSocket.cpp index d705171..ea7377e 100644 --- a/lldb/source/Host/common/TCPSocket.cpp +++ b/lldb/source/Host/common/TCPSocket.cpp @@ -156,10 +156,9 @@ Status TCPSocket::Connect(llvm::StringRef name) { Status error; std::string host_str; std::string port_str; - uint16_t port; - if (llvm::Error decode_error = - DecodeHostAndPort(name, host_str, port_str, port)) - return Status(std::move(decode_error)); + int32_t port = INT32_MIN; + if (!DecodeHostAndPort(name, host_str, port_str, port, &error)) + return error; std::vector addresses = SocketAddress::GetAddressInfo( host_str.c_str(), nullptr, AF_UNSPEC, SOCK_STREAM, IPPROTO_TCP); @@ -193,10 +192,9 @@ Status TCPSocket::Listen(llvm::StringRef name, int backlog) { Status error; std::string host_str; std::string port_str; - uint16_t port; - if (llvm::Error decode_error = - DecodeHostAndPort(name, host_str, port_str, port)) - return Status(std::move(decode_error)); + int32_t port = INT32_MIN; + if (!DecodeHostAndPort(name, host_str, port_str, port, &error)) + return error; if (host_str == "*") host_str = "0.0.0.0"; diff --git a/lldb/source/Host/common/UDPSocket.cpp b/lldb/source/Host/common/UDPSocket.cpp index de4f4f2..0b537b3 100644 --- a/lldb/source/Host/common/UDPSocket.cpp +++ b/lldb/source/Host/common/UDPSocket.cpp @@ -63,10 +63,9 @@ UDPSocket::Connect(llvm::StringRef name, bool child_processes_inherit) { Status error; std::string host_str; std::string port_str; - uint16_t port; - if (llvm::Error decode_error = - DecodeHostAndPort(name, host_str, port_str, port)) - return decode_error; + int32_t port = INT32_MIN; + if (!DecodeHostAndPort(name, host_str, port_str, port, &error)) + return error.ToError(); // At this point we have setup the receive port, now we need to setup the UDP // send socket diff --git a/lldb/tools/lldb-server/Acceptor.cpp b/lldb/tools/lldb-server/Acceptor.cpp index 16ecc54..b8be9c5 100644 --- a/lldb/tools/lldb-server/Acceptor.cpp +++ b/lldb/tools/lldb-server/Acceptor.cpp @@ -96,10 +96,9 @@ std::unique_ptr Acceptor::Create(StringRef name, } else { std::string host_str; std::string port_str; - uint16_t port; + int32_t port = INT32_MIN; // Try to match socket name as $host:port - e.g., localhost:5555 - if (!llvm::errorToBool( - Socket::DecodeHostAndPort(name, host_str, port_str, port))) + if (Socket::DecodeHostAndPort(name, host_str, port_str, port, nullptr)) socket_protocol = Socket::ProtocolTcp; } diff --git a/lldb/unittests/Host/SocketTest.cpp b/lldb/unittests/Host/SocketTest.cpp index 6b9fb42..5593b77 100644 --- a/lldb/unittests/Host/SocketTest.cpp +++ b/lldb/unittests/Host/SocketTest.cpp @@ -10,7 +10,6 @@ #include "TestingSupport/SubsystemRAII.h" #include "lldb/Host/Config.h" #include "lldb/Utility/UriParser.h" -#include "llvm/Testing/Support/Error.h" #include "gtest/gtest.h" using namespace lldb_private; @@ -35,63 +34,67 @@ protected: TEST_P(SocketTest, DecodeHostAndPort) { std::string host_str; std::string port_str; - uint16_t port; - - EXPECT_THAT_ERROR( - Socket::DecodeHostAndPort("localhost:1138", host_str, port_str, port), - llvm::Succeeded()); + int32_t port; + Status error; + EXPECT_TRUE(Socket::DecodeHostAndPort("localhost:1138", host_str, port_str, + port, &error)); EXPECT_STREQ("localhost", host_str.c_str()); EXPECT_STREQ("1138", port_str.c_str()); EXPECT_EQ(1138, port); - - EXPECT_THAT_ERROR( - Socket::DecodeHostAndPort("google.com:65536", host_str, port_str, port), - llvm::FailedWithMessage( - "invalid host:port specification: 'google.com:65536'")); - - EXPECT_THAT_ERROR( - Socket::DecodeHostAndPort("google.com:-1138", host_str, port_str, port), - llvm::FailedWithMessage( - "invalid host:port specification: 'google.com:-1138'")); - - EXPECT_THAT_ERROR( - Socket::DecodeHostAndPort("google.com:65536", host_str, port_str, port), - llvm::FailedWithMessage( - "invalid host:port specification: 'google.com:65536'")); - - EXPECT_THAT_ERROR( - Socket::DecodeHostAndPort("12345", host_str, port_str, port), - llvm::Succeeded()); + EXPECT_TRUE(error.Success()); + + EXPECT_FALSE(Socket::DecodeHostAndPort("google.com:65536", host_str, port_str, + port, &error)); + EXPECT_TRUE(error.Fail()); + EXPECT_STREQ("invalid host:port specification: 'google.com:65536'", + error.AsCString()); + + EXPECT_FALSE(Socket::DecodeHostAndPort("google.com:-1138", host_str, port_str, + port, &error)); + EXPECT_TRUE(error.Fail()); + EXPECT_STREQ("invalid host:port specification: 'google.com:-1138'", + error.AsCString()); + + EXPECT_FALSE(Socket::DecodeHostAndPort("google.com:65536", host_str, port_str, + port, &error)); + EXPECT_TRUE(error.Fail()); + EXPECT_STREQ("invalid host:port specification: 'google.com:65536'", + error.AsCString()); + + EXPECT_TRUE( + Socket::DecodeHostAndPort("12345", host_str, port_str, port, &error)); EXPECT_STREQ("", host_str.c_str()); EXPECT_STREQ("12345", port_str.c_str()); EXPECT_EQ(12345, port); + EXPECT_TRUE(error.Success()); - EXPECT_THAT_ERROR(Socket::DecodeHostAndPort("*:0", host_str, port_str, port), - llvm::Succeeded()); + EXPECT_TRUE( + Socket::DecodeHostAndPort("*:0", host_str, port_str, port, &error)); EXPECT_STREQ("*", host_str.c_str()); EXPECT_STREQ("0", port_str.c_str()); EXPECT_EQ(0, port); + EXPECT_TRUE(error.Success()); - EXPECT_THAT_ERROR( - Socket::DecodeHostAndPort("*:65535", host_str, port_str, port), - llvm::Succeeded()); + EXPECT_TRUE( + Socket::DecodeHostAndPort("*:65535", host_str, port_str, port, &error)); EXPECT_STREQ("*", host_str.c_str()); EXPECT_STREQ("65535", port_str.c_str()); EXPECT_EQ(65535, port); + EXPECT_TRUE(error.Success()); - EXPECT_THAT_ERROR( - Socket::DecodeHostAndPort("[::1]:12345", host_str, port_str, port), - llvm::Succeeded()); + EXPECT_TRUE( + Socket::DecodeHostAndPort("[::1]:12345", host_str, port_str, port, &error)); EXPECT_STREQ("::1", host_str.c_str()); EXPECT_STREQ("12345", port_str.c_str()); EXPECT_EQ(12345, port); + EXPECT_TRUE(error.Success()); - EXPECT_THAT_ERROR(Socket::DecodeHostAndPort("[abcd:12fg:AF58::1]:12345", - host_str, port_str, port), - llvm::Succeeded()); + EXPECT_TRUE( + Socket::DecodeHostAndPort("[abcd:12fg:AF58::1]:12345", host_str, port_str, port, &error)); EXPECT_STREQ("abcd:12fg:AF58::1", host_str.c_str()); EXPECT_STREQ("12345", port_str.c_str()); EXPECT_EQ(12345, port); + EXPECT_TRUE(error.Success()); } #if LLDB_ENABLE_POSIX -- 2.7.4