[Lldb-commits] [lldb] r361079 - Fix IPv6 support on lldb-server platform

Davide Italiano via lldb-commits lldb-commits at lists.llvm.org
Fri May 17 19:30:39 PDT 2019


Alex, not sure if it's helpful for you as you already reverted this,
but I was able to find this failing on our lldb-sanitize bot (on Green
dragon).
Here's the stack trace.

******************** TEST 'lldb-Unit ::
Host/./HostTests/ConnectionFileDescriptorTest.TCPGetURIv6' FAILED
********************
Note: Google Test filter = ConnectionFileDescriptorTest.TCPGetURIv6
[==========] Running 1 test from 1 test case.
[----------] Global test environment set-up.
[----------] 1 test from ConnectionFileDescriptorTest
[ RUN      ] ConnectionFileDescriptorTest.TCPGetURIv6
=================================================================
==8350==ERROR: AddressSanitizer: stack-use-after-scope on address
0x7ffeea47c1dc at pc 0x000105e088d2 bp 0x7ffeea47bd90 sp
0x7ffeea47b538
READ of size 3 at 0x7ffeea47c1dc thread T0
    #0 0x105e088d1 in MemcmpInterceptorCommon(void*, int (*)(void
const*, void const*, unsigned long), void const*, void const*,
unsigned long) (libclang_rt.asan_osx_dynamic.dylib:x86_64+0x188d1)
    #1 0x105e08c95 in wrap_memcmp
(libclang_rt.asan_osx_dynamic.dylib:x86_64+0x18c95)
    #2 0x10586e475 in testing::AssertionResult
testing::internal::CmpHelperEQ<std::__1::basic_string<char,
std::__1::char_traits<char>, std::__1::allocator<char> >,
llvm::StringRef>(char const*, char const*,
std::__1::basic_string<char, std::__1::char_traits<char>,
std::__1::allocator<char> > const&, llvm::StringRef const&)
StringRef.h:67
    #3 0x10586d259 in
ConnectionFileDescriptorTest::TestGetURI(std::__1::basic_string<char,
std::__1::char_traits<char>, std::__1::allocator<char> >) gtest.h:1421
    #4 0x10586db81 in
ConnectionFileDescriptorTest_TCPGetURIv6_Test::TestBody()
ConnectionFileDescriptorTest.cpp:50
    #5 0x10590f5ea in testing::Test::Run() gtest.cc:2474
    #6 0x105911787 in testing::TestInfo::Run() gtest.cc:2656
    #7 0x105913b97 in testing::TestCase::Run() gtest.cc:2774
    #8 0x1059265d3 in testing::internal::UnitTestImpl::RunAllTests()
gtest.cc:4649
    #9 0x105925740 in testing::UnitTest::Run() gtest.cc:4257
    #10 0x1058fc61b in main gtest.h:2233
    #11 0x7fff622c7084 in start (libdyld.dylib:x86_64+0x17084)

On Fri, May 17, 2019 at 3:28 PM Alex Langford via lldb-commits
<lldb-commits at lists.llvm.org> wrote:
>
> Author: xiaobai
> Date: Fri May 17 15:30:53 2019
> New Revision: 361079
>
> URL: http://llvm.org/viewvc/llvm-project?rev=361079&view=rev
> Log:
> Fix IPv6 support on lldb-server platform
>
> This is a general fix for the ConnectionFileDescriptor class but my main
> motivation was to make lldb-server working with IPv6.
> The connect URI can use square brackets ([]) to wrap the interface part
> of the URI (e.g.: <scheme>://[<interface>]:<port>). For IPv6 addresses
> this is a must since its ip can include colons and it will overlap with
> the port colon otherwise. The URIParser class parses the square brackets
> correctly but the ConnectionFileDescriptor doesn't generate them for
> IPv6 addresses making it impossible to connect to the gdb server when
> using this protocol.
>
> How to reproduce the issue:
>
> $ lldb-server p --server --listen [::1]:8080
> ...
> $ lldb
> (lldb) platform select remote-macosx
> (lldb) platform connect connect://[::1]:8080
> (lldb) platform process -p <pid>
> error: unable to launch a GDB server on 'computer'
>
> The server was actually launched we were just not able to connect to it.
> With this fix lldb will correctly connect. I fixed this by wrapping the
> ip portion with [].
>
> Differential Revision: https://reviews.llvm.org/D61833
>
> Patch by António Afonso <antonio.afonso at gmail.com>
>
> Added:
>     lldb/trunk/unittests/Host/ConnectionFileDescriptorTest.cpp
>     lldb/trunk/unittests/Host/SocketTestUtilities.cpp
>     lldb/trunk/unittests/Host/SocketTestUtilities.h
> Modified:
>     lldb/trunk/source/Host/posix/ConnectionFileDescriptorPosix.cpp
>     lldb/trunk/unittests/Host/CMakeLists.txt
>     lldb/trunk/unittests/Host/SocketTest.cpp
>
> Modified: lldb/trunk/source/Host/posix/ConnectionFileDescriptorPosix.cpp
> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/posix/ConnectionFileDescriptorPosix.cpp?rev=361079&r1=361078&r2=361079&view=diff
> ==============================================================================
> --- lldb/trunk/source/Host/posix/ConnectionFileDescriptorPosix.cpp (original)
> +++ lldb/trunk/source/Host/posix/ConnectionFileDescriptorPosix.cpp Fri May 17 15:30:53 2019
> @@ -764,7 +764,7 @@ void ConnectionFileDescriptor::Initializ
>    m_write_sp.reset(socket);
>    m_read_sp = m_write_sp;
>    StreamString strm;
> -  strm.Printf("connect://%s:%u", tcp_socket->GetRemoteIPAddress().c_str(),
> +  strm.Printf("connect://[%s]:%u", tcp_socket->GetRemoteIPAddress().c_str(),
>                tcp_socket->GetRemotePortNumber());
>    m_uri = strm.GetString();
>  }
>
> Modified: lldb/trunk/unittests/Host/CMakeLists.txt
> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Host/CMakeLists.txt?rev=361079&r1=361078&r2=361079&view=diff
> ==============================================================================
> --- lldb/trunk/unittests/Host/CMakeLists.txt (original)
> +++ lldb/trunk/unittests/Host/CMakeLists.txt Fri May 17 15:30:53 2019
> @@ -9,6 +9,8 @@ set (FILES
>    SocketAddressTest.cpp
>    SocketTest.cpp
>    TaskPoolTest.cpp
> +  SocketTestUtilities.cpp
> +  ConnectionFileDescriptorTest.cpp
>  )
>
>  if (CMAKE_SYSTEM_NAME MATCHES "Linux|Android")
>
> Added: lldb/trunk/unittests/Host/ConnectionFileDescriptorTest.cpp
> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Host/ConnectionFileDescriptorTest.cpp?rev=361079&view=auto
> ==============================================================================
> --- lldb/trunk/unittests/Host/ConnectionFileDescriptorTest.cpp (added)
> +++ lldb/trunk/unittests/Host/ConnectionFileDescriptorTest.cpp Fri May 17 15:30:53 2019
> @@ -0,0 +1,50 @@
> +//===-- ConnectionFileDescriptorTest.cpp ------------------------*- C++ -*-===//
> +//
> +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
> +// See https://llvm.org/LICENSE.txt for license information.
> +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
> +//
> +//===----------------------------------------------------------------------===//
> +
> +#include "SocketTestUtilities.h"
> +#include "gtest/gtest.h"
> +
> +#include "lldb/Host/posix/ConnectionFileDescriptorPosix.h"
> +#include "lldb/Utility/UriParser.h"
> +
> +using namespace lldb_private;
> +
> +class ConnectionFileDescriptorTest : public testing::Test {
> +public:
> +  void SetUp() override {
> +    ASSERT_THAT_ERROR(Socket::Initialize(), llvm::Succeeded());
> +  }
> +
> +  void TearDown() override { Socket::Terminate(); }
> +
> +  void TestGetURI(std::string ip) {
> +    std::unique_ptr<TCPSocket> socket_a_up;
> +    std::unique_ptr<TCPSocket> socket_b_up;
> +    if (!IsAddressFamilySupported(ip)) {
> +      GTEST_LOG_(WARNING) << "Skipping test due to missing IPv"
> +                          << (IsIPv4(ip) ? "4" : "6") << " support.";
> +      return;
> +    }
> +    CreateTCPConnectedSockets(ip, &socket_a_up, &socket_b_up);
> +    auto socket = socket_a_up.release();
> +    ConnectionFileDescriptor connection_file_descriptor(socket);
> +
> +    llvm::StringRef scheme;
> +    llvm::StringRef hostname;
> +    int port;
> +    llvm::StringRef path;
> +    EXPECT_TRUE(UriParser::Parse(connection_file_descriptor.GetURI(), scheme,
> +                                 hostname, port, path));
> +    EXPECT_EQ(ip, hostname);
> +    EXPECT_EQ(socket->GetRemotePortNumber(), port);
> +  }
> +};
> +
> +TEST_F(ConnectionFileDescriptorTest, TCPGetURIv4) { TestGetURI("127.0.0.1"); }
> +
> +TEST_F(ConnectionFileDescriptorTest, TCPGetURIv6) { TestGetURI("::1"); }
> \ No newline at end of file
>
> Modified: lldb/trunk/unittests/Host/SocketTest.cpp
> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Host/SocketTest.cpp?rev=361079&r1=361078&r2=361079&view=diff
> ==============================================================================
> --- lldb/trunk/unittests/Host/SocketTest.cpp (original)
> +++ lldb/trunk/unittests/Host/SocketTest.cpp Fri May 17 15:30:53 2019
> @@ -6,24 +6,9 @@
>  //
>  //===----------------------------------------------------------------------===//
>
> -#include <cstdio>
> -#include <functional>
> -#include <thread>
> -
> +#include "SocketTestUtilities.h"
>  #include "gtest/gtest.h"
>
> -#include "lldb/Host/Config.h"
> -#include "lldb/Host/Socket.h"
> -#include "lldb/Host/common/TCPSocket.h"
> -#include "lldb/Host/common/UDPSocket.h"
> -#include "llvm/Support/FileSystem.h"
> -#include "llvm/Support/Path.h"
> -#include "llvm/Testing/Support/Error.h"
> -
> -#ifndef LLDB_DISABLE_POSIX
> -#include "lldb/Host/posix/DomainSocket.h"
> -#endif
> -
>  using namespace lldb_private;
>
>  class SocketTest : public testing::Test {
> @@ -33,55 +18,6 @@ public:
>    }
>
>    void TearDown() override { Socket::Terminate(); }
> -
> -protected:
> -  static void AcceptThread(Socket *listen_socket,
> -                           bool child_processes_inherit, Socket **accept_socket,
> -                           Status *error) {
> -    *error = listen_socket->Accept(*accept_socket);
> -  }
> -
> -  template <typename SocketType>
> -  void CreateConnectedSockets(
> -      llvm::StringRef listen_remote_address,
> -      const std::function<std::string(const SocketType &)> &get_connect_addr,
> -      std::unique_ptr<SocketType> *a_up, std::unique_ptr<SocketType> *b_up) {
> -    bool child_processes_inherit = false;
> -    Status error;
> -    std::unique_ptr<SocketType> listen_socket_up(
> -        new SocketType(true, child_processes_inherit));
> -    EXPECT_FALSE(error.Fail());
> -    error = listen_socket_up->Listen(listen_remote_address, 5);
> -    EXPECT_FALSE(error.Fail());
> -    EXPECT_TRUE(listen_socket_up->IsValid());
> -
> -    Status accept_error;
> -    Socket *accept_socket;
> -    std::thread accept_thread(AcceptThread, listen_socket_up.get(),
> -                              child_processes_inherit, &accept_socket,
> -                              &accept_error);
> -
> -    std::string connect_remote_address = get_connect_addr(*listen_socket_up);
> -    std::unique_ptr<SocketType> connect_socket_up(
> -        new SocketType(true, child_processes_inherit));
> -    EXPECT_FALSE(error.Fail());
> -    error = connect_socket_up->Connect(connect_remote_address);
> -    EXPECT_FALSE(error.Fail());
> -    EXPECT_TRUE(connect_socket_up->IsValid());
> -
> -    a_up->swap(connect_socket_up);
> -    EXPECT_TRUE(error.Success());
> -    EXPECT_NE(nullptr, a_up->get());
> -    EXPECT_TRUE((*a_up)->IsValid());
> -
> -    accept_thread.join();
> -    b_up->reset(static_cast<SocketType *>(accept_socket));
> -    EXPECT_TRUE(accept_error.Success());
> -    EXPECT_NE(nullptr, b_up->get());
> -    EXPECT_TRUE((*b_up)->IsValid());
> -
> -    listen_socket_up.reset();
> -  }
>  };
>
>  TEST_F(SocketTest, DecodeHostAndPort) {
> @@ -159,38 +95,24 @@ TEST_F(SocketTest, DomainListenConnectAc
>
>    std::unique_ptr<DomainSocket> socket_a_up;
>    std::unique_ptr<DomainSocket> socket_b_up;
> -  CreateConnectedSockets<DomainSocket>(
> -      Path, [=](const DomainSocket &) { return Path.str().str(); },
> -      &socket_a_up, &socket_b_up);
> +  CreateDomainConnectedSockets(Path, &socket_a_up, &socket_b_up);
>  }
>  #endif
>
>  TEST_F(SocketTest, TCPListen0ConnectAccept) {
>    std::unique_ptr<TCPSocket> socket_a_up;
>    std::unique_ptr<TCPSocket> socket_b_up;
> -  CreateConnectedSockets<TCPSocket>(
> -      "127.0.0.1:0",
> -      [=](const TCPSocket &s) {
> -        char connect_remote_address[64];
> -        snprintf(connect_remote_address, sizeof(connect_remote_address),
> -                 "127.0.0.1:%u", s.GetLocalPortNumber());
> -        return std::string(connect_remote_address);
> -      },
> -      &socket_a_up, &socket_b_up);
> +  CreateTCPConnectedSockets("127.0.0.1", &socket_a_up, &socket_b_up);
>  }
>
>  TEST_F(SocketTest, TCPGetAddress) {
>    std::unique_ptr<TCPSocket> socket_a_up;
>    std::unique_ptr<TCPSocket> socket_b_up;
> -  CreateConnectedSockets<TCPSocket>(
> -      "127.0.0.1:0",
> -      [=](const TCPSocket &s) {
> -        char connect_remote_address[64];
> -        snprintf(connect_remote_address, sizeof(connect_remote_address),
> -                 "127.0.0.1:%u", s.GetLocalPortNumber());
> -        return std::string(connect_remote_address);
> -      },
> -      &socket_a_up, &socket_b_up);
> +  if (!IsAddressFamilySupported("127.0.0.1")) {
> +    GTEST_LOG_(WARNING) << "Skipping test due to missing IPv4 support.";
> +    return;
> +  }
> +  CreateTCPConnectedSockets("127.0.0.1", &socket_a_up, &socket_b_up);
>
>    EXPECT_EQ(socket_a_up->GetLocalPortNumber(),
>              socket_b_up->GetRemotePortNumber());
>
> Added: lldb/trunk/unittests/Host/SocketTestUtilities.cpp
> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Host/SocketTestUtilities.cpp?rev=361079&view=auto
> ==============================================================================
> --- lldb/trunk/unittests/Host/SocketTestUtilities.cpp (added)
> +++ lldb/trunk/unittests/Host/SocketTestUtilities.cpp Fri May 17 15:30:53 2019
> @@ -0,0 +1,98 @@
> +//===----------------- SocketTestUtilities.cpp ------------------*- C++ -*-===//
> +//
> +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
> +// See https://llvm.org/LICENSE.txt for license information.
> +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
> +//
> +//===----------------------------------------------------------------------===//
> +
> +#include "SocketTestUtilities.h"
> +#include "lldb/Utility/StreamString.h"
> +#include <arpa/inet.h>
> +
> +using namespace lldb_private;
> +
> +const void AcceptThread(Socket *listen_socket, bool child_processes_inherit,
> +                        Socket **accept_socket, Status *error) {
> +  *error = listen_socket->Accept(*accept_socket);
> +}
> +
> +template <typename SocketType>
> +void lldb_private::CreateConnectedSockets(
> +    llvm::StringRef listen_remote_address,
> +    const std::function<std::string(const SocketType &)> &get_connect_addr,
> +    std::unique_ptr<SocketType> *a_up, std::unique_ptr<SocketType> *b_up) {
> +  bool child_processes_inherit = false;
> +  Status error;
> +  std::unique_ptr<SocketType> listen_socket_up(
> +      new SocketType(true, child_processes_inherit));
> +  EXPECT_FALSE(error.Fail());
> +  error = listen_socket_up->Listen(listen_remote_address, 5);
> +  EXPECT_FALSE(error.Fail());
> +  EXPECT_TRUE(listen_socket_up->IsValid());
> +
> +  Status accept_error;
> +  Socket *accept_socket;
> +  std::thread accept_thread(AcceptThread, listen_socket_up.get(),
> +                            child_processes_inherit, &accept_socket,
> +                            &accept_error);
> +
> +  std::string connect_remote_address = get_connect_addr(*listen_socket_up);
> +  std::unique_ptr<SocketType> connect_socket_up(
> +      new SocketType(true, child_processes_inherit));
> +  EXPECT_FALSE(error.Fail());
> +  error = connect_socket_up->Connect(connect_remote_address);
> +  EXPECT_FALSE(error.Fail());
> +  EXPECT_TRUE(connect_socket_up->IsValid());
> +
> +  a_up->swap(connect_socket_up);
> +  EXPECT_TRUE(error.Success());
> +  EXPECT_NE(nullptr, a_up->get());
> +  EXPECT_TRUE((*a_up)->IsValid());
> +
> +  accept_thread.join();
> +  b_up->reset(static_cast<SocketType *>(accept_socket));
> +  EXPECT_TRUE(accept_error.Success());
> +  EXPECT_NE(nullptr, b_up->get());
> +  EXPECT_TRUE((*b_up)->IsValid());
> +
> +  listen_socket_up.reset();
> +}
> +
> +bool lldb_private::CreateTCPConnectedSockets(
> +    std::string listen_remote_ip, std::unique_ptr<TCPSocket> *socket_a_up,
> +    std::unique_ptr<TCPSocket> *socket_b_up) {
> +  StreamString strm;
> +  strm.Printf("[%s]:0", listen_remote_ip.c_str());
> +  CreateConnectedSockets<TCPSocket>(
> +      strm.GetString(),
> +      [=](const TCPSocket &s) {
> +        char connect_remote_address[64];
> +        snprintf(connect_remote_address, sizeof(connect_remote_address),
> +                 "[%s]:%u", listen_remote_ip.c_str(), s.GetLocalPortNumber());
> +        return std::string(connect_remote_address);
> +      },
> +      socket_a_up, socket_b_up);
> +  return true;
> +}
> +
> +#ifndef LLDB_DISABLE_POSIX
> +void lldb_private::CreateDomainConnectedSockets(
> +    llvm::StringRef path, std::unique_ptr<DomainSocket> *socket_a_up,
> +    std::unique_ptr<DomainSocket> *socket_b_up) {
> +  return CreateConnectedSockets<DomainSocket>(
> +      path, [=](const DomainSocket &) { return path.str(); }, socket_a_up,
> +      socket_b_up);
> +}
> +#endif
> +
> +bool lldb_private::IsAddressFamilySupported(std::string ip) {
> +  auto addresses = lldb_private::SocketAddress::GetAddressInfo(
> +      ip.c_str(), NULL, AF_UNSPEC, SOCK_STREAM, IPPROTO_TCP);
> +  return addresses.size() > 0;
> +}
> +
> +bool lldb_private::IsIPv4(std::string ip) {
> +  struct sockaddr_in sock_addr;
> +  return inet_pton(AF_INET, ip.c_str(), &(sock_addr.sin_addr)) != 0;
> +}
>
> Added: lldb/trunk/unittests/Host/SocketTestUtilities.h
> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Host/SocketTestUtilities.h?rev=361079&view=auto
> ==============================================================================
> --- lldb/trunk/unittests/Host/SocketTestUtilities.h (added)
> +++ lldb/trunk/unittests/Host/SocketTestUtilities.h Fri May 17 15:30:53 2019
> @@ -0,0 +1,47 @@
> +//===--------------------- SocketTestUtilities.h ----------------*- C++ -*-===//
> +//
> +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
> +// See https://llvm.org/LICENSE.txt for license information.
> +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
> +//
> +//===----------------------------------------------------------------------===//
> +
> +#ifndef LLDB_UNITTESTS_HOST_SOCKETTESTUTILITIES_H
> +#define LLDB_UNITTESTS_HOST_SOCKETTESTUTILITIES_H
> +
> +#include <cstdio>
> +#include <functional>
> +#include <thread>
> +
> +#include "lldb/Host/Config.h"
> +#include "lldb/Host/Socket.h"
> +#include "lldb/Host/common/TCPSocket.h"
> +#include "lldb/Host/common/UDPSocket.h"
> +#include "llvm/Support/FileSystem.h"
> +#include "llvm/Support/Path.h"
> +#include "llvm/Testing/Support/Error.h"
> +
> +#ifndef LLDB_DISABLE_POSIX
> +#include "lldb/Host/posix/DomainSocket.h"
> +#endif
> +
> +namespace lldb_private {
> +template <typename SocketType>
> +void CreateConnectedSockets(
> +    llvm::StringRef listen_remote_address,
> +    const std::function<std::string(const SocketType &)> &get_connect_addr,
> +    std::unique_ptr<SocketType> *a_up, std::unique_ptr<SocketType> *b_up);
> +bool CreateTCPConnectedSockets(std::string listen_remote_ip,
> +                               std::unique_ptr<TCPSocket> *a_up,
> +                               std::unique_ptr<TCPSocket> *b_up);
> +#ifndef LLDB_DISABLE_POSIX
> +void CreateDomainConnectedSockets(llvm::StringRef path,
> +                                  std::unique_ptr<DomainSocket> *a_up,
> +                                  std::unique_ptr<DomainSocket> *b_up);
> +#endif
> +
> +bool IsAddressFamilySupported(std::string ip);
> +bool IsIPv4(std::string ip);
> +} // namespace lldb_private
> +
> +#endif
> \ No newline at end of file
>
>
> _______________________________________________
> lldb-commits mailing list
> lldb-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


More information about the lldb-commits mailing list