splhack created this revision. Herald added a subscriber: danielkiss. Herald added a project: All. splhack added a reviewer: clayborg. splhack updated this revision to Diff 475962. splhack added a comment. splhack published this revision for review. Herald added a project: LLDB. Herald added a subscriber: lldb-commits.
First, recap: what is the Android serial number. In Android ADB protocal, the serial number following the `host-serial:` is URL-safe string and it could be a `host:port` string. https://cs.android.com/android/platform/superproject/+/763ab7bb17e9c63b238222085e1e69552d978367:packages/modules/adb/sockets.cpp;l=825?q=host-serial: if (android::base::ConsumePrefix(&service, "host-serial:")) { // serial number should follow "host:" and could be a host:port string. Second, LLDB is not able to connect it to Android device when multiple devices are connected to the host. (lldb) platform select remote-android (lldb) platform connect unix-abstract-connect:///data/local/tmp/debug.sock error: Expected a single connected device, got instead 2 - try setting 'ANDROID_SERIAL' However setting ANDROID_SERIAL environment variable is not always a viable option. For instance, VS Code + lldb-server and debugging multiple Android devices at the same time. The potential solution is to use platform connect URL. (lldb) platform select remote-android (lldb) platform connect unix-abstract-connect://emulator-5554/data/local/tmp/debug.sock This works for USB-connected devices and emulators. But it does not work for TCP/IP-connected devices. (lldb) platform select remote-android (lldb) platform connect unix-abstract-connect://127.0.0.1:5556/data/local/tmp/debug.sock The reason is that the current code only uses the hostname part of the URL. https://github.com/llvm/llvm-project/blob/ed9638c44bc0c95314694fb878b21006e6c87510/lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp#L154-L155 So m_device_id will be `127.0.0.1`. But this should be `127.0.0.1:5556` Also `localhost` should not be skipped when the port number is available. m_device_id should be `localhost:5556` (lldb) platform connect unix-abstract-connect://localhost:5556/data/local/tmp/debug.sock Third, the host could be IPv6, for instance `[::1]`. (lldb) platform connect unix-abstract-connect://[::1]:5556/data/local/tmp/debug.sock This is a valid URL and serial number `[::1]:5556`. The parsed_url does not look it has the information of IPv6 brackets. Thus, `GetDeviceIDFromURL` checks the raw URL again for the bracket. Fourth, when a port number is available in the connect URL, `MakeConnectURL` function create a new TCP/IP forwarding with the port number. When `adb devices` shows a device with `host:port` style serial number, the port in the serial number is already connected or forwarded to the adb server. `host-serial:host:port` should work. https://github.com/llvm/llvm-project/blob/ed9638c44bc0c95314694fb878b21006e6c87510/lldb/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.cpp#L191-L192 https://github.com/llvm/llvm-project/blob/ed9638c44bc0c95314694fb878b21006e6c87510/lldb/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.cpp#L42-L46 Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D138164 Files: lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp lldb/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.cpp lldb/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.h lldb/unittests/Platform/Android/CMakeLists.txt lldb/unittests/Platform/Android/PlatformAndroidRemoteGDBServerTest.cpp
Index: lldb/unittests/Platform/Android/PlatformAndroidRemoteGDBServerTest.cpp =================================================================== --- /dev/null +++ lldb/unittests/Platform/Android/PlatformAndroidRemoteGDBServerTest.cpp @@ -0,0 +1,50 @@ +//===-- PlatformAndroidRemoteGDBServerTest.cpp +//-----------------------------===// +// +// 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 "Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.h" +#include "gtest/gtest.h" +#include <cstdlib> + +using namespace lldb; +using namespace lldb_private; + +namespace lldb_private { +namespace platform_android { + +class PlatformAndroidRemoteGDBServerTest : public ::testing::Test {}; + +TEST(PlatformAndroidRemoteGDBServerTest, GetDeviceIDFromURL) { + const static std::vector<std::map<std::string, std::string>> data{ + {{"url", "unix-abstract-connect:///sock"}, {"serial", ""}}, + {{"url", "unix-abstract-connect://localhost/sock"}, {"serial", ""}}, + {{"url", "unix-abstract-connect://emulator-5554/sock"}, + {"serial", "emulator-5554"}}, + {{"url", "unix-abstract-connect://localhost:5556/sock"}, + {"serial", "localhost:5556"}}, + {{"url", "unix-abstract-connect://192.168.0.1:42/sock"}, + {"serial", "192.168.0.1:42"}}, + {{"url", "unix-abstract-connect://[::1]:5557/sock"}, + {"serial", "[::1]:5557"}}, + {{"url", "unix-abstract-connect://" + "[2001:0db8:85a3:0000:0000:8a2e:0370:7334]:5556/sock"}, + {"serial", "[2001:0db8:85a3:0000:0000:8a2e:0370:7334]:5556"}}, + }; + + for (const auto &map : data) { + const auto &url = map.at("url"); + llvm::Optional<URI> parsed_url = URI::Parse(url); + EXPECT_TRUE(parsed_url); + EXPECT_EQ( + PlatformAndroidRemoteGDBServer::GetDeviceIDFromURL(url, parsed_url), + map.at("serial")); + } +} + +} // end namespace platform_android +} // end namespace lldb_private Index: lldb/unittests/Platform/Android/CMakeLists.txt =================================================================== --- lldb/unittests/Platform/Android/CMakeLists.txt +++ lldb/unittests/Platform/Android/CMakeLists.txt @@ -2,6 +2,7 @@ add_lldb_unittest(AdbClientTests AdbClientTest.cpp + PlatformAndroidRemoteGDBServerTest.cpp LINK_LIBS lldbPluginPlatformAndroid Index: lldb/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.h =================================================================== --- lldb/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.h +++ lldb/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.h @@ -28,6 +28,9 @@ ~PlatformAndroidRemoteGDBServer() override; + static std::string GetDeviceIDFromURL(llvm::StringRef url, + llvm::Optional<URI> parsed_url); + Status ConnectRemote(Args &args) override; Status DisconnectRemote() override; Index: lldb/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.cpp =================================================================== --- lldb/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.cpp +++ lldb/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.cpp @@ -79,6 +79,23 @@ DeleteForwardPortWithAdb(it.second, m_device_id); } +std::string PlatformAndroidRemoteGDBServer::GetDeviceIDFromURL( + const llvm::StringRef url, llvm::Optional<URI> parsed_url) { + const llvm::StringRef kSchemeSepWithIPv6Bracket("://["); + auto pos = url.find(kSchemeSepWithIPv6Bracket); + std::string hostname; + if (pos == std::string::npos) + hostname = parsed_url->hostname.str(); + else + hostname = "[" + parsed_url->hostname.str() + "]"; + + if (parsed_url->port.hasValue()) + return hostname + ":" + std::to_string(parsed_url->port.value()); + if (hostname != "localhost") + return hostname; + return std::string(); +} + bool PlatformAndroidRemoteGDBServer::LaunchGDBServer(lldb::pid_t &pid, std::string &connect_url) { assert(IsConnected()); @@ -122,8 +139,7 @@ llvm::Optional<URI> parsed_url = URI::Parse(url); if (!parsed_url) return Status("Invalid URL: %s", url); - if (parsed_url->hostname != "localhost") - m_device_id = parsed_url->hostname.str(); + m_device_id = GetDeviceIDFromURL(url, parsed_url); m_socket_namespace.reset(); if (parsed_url->scheme == "unix-connect") @@ -137,9 +153,8 @@ local_port = std::stoi(platform_local_port); std::string connect_url; - auto error = MakeConnectURL(g_remote_platform_pid, local_port, - parsed_url->port.value_or(0), parsed_url->path, - connect_url); + auto error = MakeConnectURL(g_remote_platform_pid, local_port, 0, + parsed_url->path, connect_url); if (error.Fail()) return error; @@ -236,8 +251,7 @@ } std::string new_connect_url; - error = MakeConnectURL(s_remote_gdbserver_fake_pid--, 0, - parsed_url->port.value_or(0), parsed_url->path, + error = MakeConnectURL(s_remote_gdbserver_fake_pid--, 0, 0, parsed_url->path, new_connect_url); if (error.Fail()) return nullptr; Index: lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp =================================================================== --- lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp +++ lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp @@ -151,8 +151,7 @@ llvm::Optional<URI> parsed_url = URI::Parse(url); if (!parsed_url) return Status("Invalid URL: %s", url); - if (parsed_url->hostname != "localhost") - m_device_id = parsed_url->hostname.str(); + m_device_id = PlatformAndroidRemoteGDBServer::GetDeviceIDFromURL(url, parsed_url); auto error = PlatformLinux::ConnectRemote(args); if (error.Success()) {
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
