[Lldb-commits] [lldb] [lldb] make PlatformAndroid/AdbClient::GetSyncService threadsafe (PR #145382)
Chad Smith via lldb-commits
lldb-commits at lists.llvm.org
Thu Jul 10 11:06:05 PDT 2025
https://github.com/cs01 updated https://github.com/llvm/llvm-project/pull/145382
>From d585051b103acc1973671679bd1fa04b92fb0bf9 Mon Sep 17 00:00:00 2001
From: Chad Smith <cs01 at users.noreply.github.com>
Date: Mon, 23 Jun 2025 11:07:00 -0700
Subject: [PATCH 1/8] [lldb] make PlatformAndroid/AdbClient::GetSyncService
threadsafe
---
.../Plugins/Platform/Android/AdbClient.cpp | 29 ++++-
.../Plugins/Platform/Android/AdbClient.h | 2 +
.../Platform/Android/PlatformAndroid.cpp | 10 +-
.../Platform/Android/PlatformAndroid.h | 4 +-
.../Platform/Android/AdbClientTest.cpp | 121 +++++++++++++++++-
5 files changed, 149 insertions(+), 17 deletions(-)
diff --git a/lldb/source/Plugins/Platform/Android/AdbClient.cpp b/lldb/source/Plugins/Platform/Android/AdbClient.cpp
index a179260ca15f6..4c5342f7af60b 100644
--- a/lldb/source/Plugins/Platform/Android/AdbClient.cpp
+++ b/lldb/source/Plugins/Platform/Android/AdbClient.cpp
@@ -417,13 +417,30 @@ Status AdbClient::ShellToFile(const char *command, milliseconds timeout,
return Status();
}
+// Returns a sync service for file operations.
+// This operation is thread-safe - each call creates an isolated sync service
+// with its own connection to avoid race conditions.
std::unique_ptr<AdbClient::SyncService>
AdbClient::GetSyncService(Status &error) {
- std::unique_ptr<SyncService> sync_service;
- error = StartSync();
- if (error.Success())
- sync_service.reset(new SyncService(std::move(m_conn)));
+ std::lock_guard<std::mutex> lock(m_sync_mutex);
+
+ // Create a temporary AdbClient with its own connection for this sync service
+ // This avoids the race condition of multiple threads accessing the same
+ // connection
+ AdbClient temp_client(m_device_id);
+
+ // Connect and start sync on the temporary client
+ error = temp_client.Connect();
+ if (error.Fail())
+ return nullptr;
+ error = temp_client.StartSync();
+ if (error.Fail())
+ return nullptr;
+
+ // Move the connection from the temporary client to the sync service
+ std::unique_ptr<SyncService> sync_service;
+ sync_service.reset(new SyncService(std::move(temp_client.m_conn)));
return sync_service;
}
@@ -487,7 +504,9 @@ Status AdbClient::SyncService::internalPushFile(const FileSpec &local_file,
error.AsCString());
}
error = SendSyncRequest(
- kDONE, llvm::sys::toTimeT(FileSystem::Instance().GetModificationTime(local_file)),
+ kDONE,
+ llvm::sys::toTimeT(
+ FileSystem::Instance().GetModificationTime(local_file)),
nullptr);
if (error.Fail())
return error;
diff --git a/lldb/source/Plugins/Platform/Android/AdbClient.h b/lldb/source/Plugins/Platform/Android/AdbClient.h
index 851c09957bd4a..9ef780bc6202b 100644
--- a/lldb/source/Plugins/Platform/Android/AdbClient.h
+++ b/lldb/source/Plugins/Platform/Android/AdbClient.h
@@ -14,6 +14,7 @@
#include <functional>
#include <list>
#include <memory>
+#include <mutex>
#include <string>
#include <vector>
@@ -135,6 +136,7 @@ class AdbClient {
std::string m_device_id;
std::unique_ptr<Connection> m_conn;
+ mutable std::mutex m_sync_mutex;
};
} // namespace platform_android
diff --git a/lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp b/lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp
index 5bc9cc133fbd3..68e4886cb1c7a 100644
--- a/lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp
+++ b/lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp
@@ -474,13 +474,11 @@ std::string PlatformAndroid::GetRunAs() {
return run_as.str();
}
-AdbClient::SyncService *PlatformAndroid::GetSyncService(Status &error) {
- if (m_adb_sync_svc && m_adb_sync_svc->IsConnected())
- return m_adb_sync_svc.get();
-
+std::unique_ptr<AdbClient::SyncService>
+PlatformAndroid::GetSyncService(Status &error) {
AdbClientUP adb(GetAdbClient(error));
if (error.Fail())
return nullptr;
- m_adb_sync_svc = adb->GetSyncService(error);
- return (error.Success()) ? m_adb_sync_svc.get() : nullptr;
+
+ return adb->GetSyncService(error);
}
diff --git a/lldb/source/Plugins/Platform/Android/PlatformAndroid.h b/lldb/source/Plugins/Platform/Android/PlatformAndroid.h
index 5602edf73c1d3..3140acb573416 100644
--- a/lldb/source/Plugins/Platform/Android/PlatformAndroid.h
+++ b/lldb/source/Plugins/Platform/Android/PlatformAndroid.h
@@ -80,9 +80,7 @@ class PlatformAndroid : public platform_linux::PlatformLinux {
std::string GetRunAs();
private:
- AdbClient::SyncService *GetSyncService(Status &error);
-
- std::unique_ptr<AdbClient::SyncService> m_adb_sync_svc;
+ std::unique_ptr<AdbClient::SyncService> GetSyncService(Status &error);
std::string m_device_id;
uint32_t m_sdk_version;
};
diff --git a/lldb/unittests/Platform/Android/AdbClientTest.cpp b/lldb/unittests/Platform/Android/AdbClientTest.cpp
index 0808b96f69fc8..c2f658b9d1bc1 100644
--- a/lldb/unittests/Platform/Android/AdbClientTest.cpp
+++ b/lldb/unittests/Platform/Android/AdbClientTest.cpp
@@ -6,9 +6,13 @@
//
//===----------------------------------------------------------------------===//
-#include "gtest/gtest.h"
#include "Plugins/Platform/Android/AdbClient.h"
+#include "gtest/gtest.h"
+#include <atomic>
#include <cstdlib>
+#include <future>
+#include <thread>
+#include <vector>
static void set_env(const char *var, const char *value) {
#ifdef _WIN32
@@ -31,14 +35,14 @@ class AdbClientTest : public ::testing::Test {
void TearDown() override { set_env("ANDROID_SERIAL", ""); }
};
-TEST(AdbClientTest, CreateByDeviceId) {
+TEST_F(AdbClientTest, CreateByDeviceId) {
AdbClient adb;
Status error = AdbClient::CreateByDeviceID("device1", adb);
EXPECT_TRUE(error.Success());
EXPECT_EQ("device1", adb.GetDeviceID());
}
-TEST(AdbClientTest, CreateByDeviceId_ByEnvVar) {
+TEST_F(AdbClientTest, CreateByDeviceId_ByEnvVar) {
set_env("ANDROID_SERIAL", "device2");
AdbClient adb;
@@ -47,5 +51,116 @@ TEST(AdbClientTest, CreateByDeviceId_ByEnvVar) {
EXPECT_EQ("device2", adb.GetDeviceID());
}
+TEST_F(AdbClientTest, GetSyncServiceThreadSafe) {
+ // Test high-volume concurrent access to GetSyncService
+ // This test verifies thread safety under sustained load with many rapid calls
+ // Catches race conditions that emerge when multiple threads make repeated
+ // calls to GetSyncService on the same AdbClient instance
+
+ AdbClient shared_adb_client("test_device");
+
+ const int num_threads = 8;
+ std::vector<std::future<bool>> futures;
+ std::atomic<int> success_count{0};
+ std::atomic<int> null_count{0};
+
+ // Launch multiple threads that all call GetSyncService on the SAME AdbClient
+ for (int i = 0; i < num_threads; ++i) {
+ futures.push_back(std::async(std::launch::async, [&]() {
+ // Multiple rapid calls to trigger the race condition
+ for (int j = 0; j < 20; ++j) {
+ Status error;
+
+ auto sync_service = shared_adb_client.GetSyncService(error);
+
+ if (sync_service != nullptr) {
+ success_count++;
+ } else {
+ null_count++;
+ }
+
+ // Small delay to increase chance of hitting the race condition
+ std::this_thread::sleep_for(std::chrono::microseconds(1));
+ }
+ return true;
+ }));
+ }
+
+ // Wait for all threads to complete
+ bool all_completed = true;
+ for (auto &future : futures) {
+ bool thread_result = future.get();
+ if (!thread_result) {
+ all_completed = false;
+ }
+ }
+
+ // This should pass (though sync services may fail
+ // to connect)
+ EXPECT_TRUE(all_completed) << "Parallel GetSyncService calls should not "
+ "crash due to race conditions. "
+ << "Successes: " << success_count.load()
+ << ", Nulls: " << null_count.load();
+
+ // The key test: we should complete all operations without crashing
+ int total_operations = num_threads * 20;
+ int completed_operations = success_count.load() + null_count.load();
+ EXPECT_EQ(total_operations, completed_operations)
+ << "All operations should complete without crashing";
+}
+
+TEST_F(AdbClientTest, ConnectionMoveRaceCondition) {
+ // Test simultaneous access timing to GetSyncService
+ // This test verifies thread safety when multiple threads start at exactly
+ // the same time, maximizing the chance of hitting precise timing conflicts
+ // Catches race conditions that occur with synchronized simultaneous access
+
+ AdbClient adb_client("test_device");
+
+ // Try to trigger the exact race condition by having multiple threads
+ // simultaneously call GetSyncService
+
+ std::atomic<bool> start_flag{false};
+ std::vector<std::thread> threads;
+ std::atomic<int> null_service_count{0};
+ std::atomic<int> valid_service_count{0};
+
+ const int num_threads = 10;
+
+ // Create threads that will all start simultaneously
+ for (int i = 0; i < num_threads; ++i) {
+ threads.emplace_back([&]() {
+ // Wait for all threads to be ready
+ while (!start_flag.load()) {
+ std::this_thread::yield();
+ }
+
+ Status error;
+ auto sync_service = adb_client.GetSyncService(error);
+
+ if (sync_service == nullptr) {
+ null_service_count++;
+ } else {
+ valid_service_count++;
+ }
+ });
+ }
+
+ // Start all threads simultaneously to maximize chance of race condition
+ start_flag.store(true);
+
+ // Wait for all threads to complete
+ for (auto &thread : threads) {
+ thread.join();
+ }
+
+ // The test passes if we don't crash
+ int total_results = null_service_count.load() + valid_service_count.load();
+ EXPECT_EQ(num_threads, total_results)
+ << "All threads should complete without crashing. "
+ << "Null services: " << null_service_count.load()
+ << ", Valid services: " << valid_service_count.load();
+}
+
} // end namespace platform_android
} // end namespace lldb_private
>From 6d96b50d868760e880739f7cc6ac242abf4e081d Mon Sep 17 00:00:00 2001
From: Chad Smith <cs01 at users.noreply.github.com>
Date: Fri, 27 Jun 2025 11:52:30 -0700
Subject: [PATCH 2/8] clean up android apis
---
.../posix/ConnectionFileDescriptorPosix.cpp | 6 +
.../Plugins/Platform/Android/AdbClient.cpp | 120 +++---
.../Plugins/Platform/Android/AdbClient.h | 39 +-
.../Platform/Android/PlatformAndroid.cpp | 63 ++-
.../Platform/Android/PlatformAndroid.h | 10 +-
.../Platform/Android/AdbClientTest.cpp | 122 +-----
.../Platform/Android/PlatformAndroidTest.cpp | 391 ++++++++++++------
7 files changed, 410 insertions(+), 341 deletions(-)
diff --git a/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp b/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
index ae935dda5af4e..e7f46d9576165 100644
--- a/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
+++ b/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
@@ -220,6 +220,12 @@ ConnectionStatus ConnectionFileDescriptor::Disconnect(Status *error_ptr) {
// Prevents reads and writes during shutdown.
m_shutting_down = true;
+ if (!m_io_sp) {
+ if (error_ptr)
+ *error_ptr = Status::FromErrorString("not connected");
+ return eConnectionStatusNoConnection;
+ }
+
Status error = m_io_sp->Close();
if (error.Fail())
status = eConnectionStatusError;
diff --git a/lldb/source/Plugins/Platform/Android/AdbClient.cpp b/lldb/source/Plugins/Platform/Android/AdbClient.cpp
index 4c5342f7af60b..b55d99c7c2538 100644
--- a/lldb/source/Plugins/Platform/Android/AdbClient.cpp
+++ b/lldb/source/Plugins/Platform/Android/AdbClient.cpp
@@ -130,14 +130,7 @@ const std::string &AdbClient::GetDeviceID() const { return m_device_id; }
Status AdbClient::Connect() {
Status error;
m_conn = std::make_unique<ConnectionFileDescriptor>();
- std::string port = "5037";
- if (const char *env_port = std::getenv("ANDROID_ADB_SERVER_PORT")) {
- port = env_port;
- }
- std::string uri = "connect://127.0.0.1:" + port;
- m_conn->Connect(uri.c_str(), &error);
-
- return error;
+ return ConnectToAdb(*m_conn);
}
Status AdbClient::GetDevices(DeviceIDList &device_list) {
@@ -241,6 +234,10 @@ Status AdbClient::SendDeviceMessage(const std::string &packet) {
Status AdbClient::ReadMessage(std::vector<char> &message) {
message.clear();
+ if (!m_conn) {
+ return Status::FromErrorString("No connection available");
+ }
+
char buffer[5];
buffer[4] = 0;
@@ -264,6 +261,10 @@ Status AdbClient::ReadMessageStream(std::vector<char> &message,
auto start = steady_clock::now();
message.clear();
+ if (!m_conn) {
+ return Status::FromErrorString("No connection available");
+ }
+
Status error;
lldb::ConnectionStatus status = lldb::eConnectionStatusSuccess;
char buffer[1024];
@@ -310,7 +311,7 @@ Status AdbClient::GetResponseError(const char *response_id) {
return Status(std::string(&error_message[0], error_message.size()));
}
-Status AdbClient::SwitchDeviceTransport() {
+Status AdbClient::SelectTargetDevice() {
std::ostringstream msg;
msg << "host:transport:" << m_device_id;
@@ -321,21 +322,8 @@ Status AdbClient::SwitchDeviceTransport() {
return ReadResponseStatus();
}
-Status AdbClient::StartSync() {
- auto error = SwitchDeviceTransport();
- if (error.Fail())
- return Status::FromErrorStringWithFormat(
- "Failed to switch to device transport: %s", error.AsCString());
-
- error = Sync();
- if (error.Fail())
- return Status::FromErrorStringWithFormat("Sync failed: %s",
- error.AsCString());
- return error;
-}
-
-Status AdbClient::Sync() {
+Status AdbClient::EnterSyncMode() {
auto error = SendMessage("sync:", false);
if (error.Fail())
return error;
@@ -344,6 +332,9 @@ Status AdbClient::Sync() {
}
Status AdbClient::ReadAllBytes(void *buffer, size_t size) {
+ if (!m_conn) {
+ return Status::FromErrorString("No connection available");
+ }
return ::ReadAllBytes(*m_conn, buffer, size);
}
@@ -351,10 +342,10 @@ Status AdbClient::internalShell(const char *command, milliseconds timeout,
std::vector<char> &output_buf) {
output_buf.clear();
- auto error = SwitchDeviceTransport();
+ auto error = SelectTargetDevice();
if (error.Fail())
return Status::FromErrorStringWithFormat(
- "Failed to switch to device transport: %s", error.AsCString());
+ "Failed to select target device: %s", error.AsCString());
StreamString adb_command;
adb_command.Printf("shell:%s", command);
@@ -417,32 +408,6 @@ Status AdbClient::ShellToFile(const char *command, milliseconds timeout,
return Status();
}
-// Returns a sync service for file operations.
-// This operation is thread-safe - each call creates an isolated sync service
-// with its own connection to avoid race conditions.
-std::unique_ptr<AdbClient::SyncService>
-AdbClient::GetSyncService(Status &error) {
- std::lock_guard<std::mutex> lock(m_sync_mutex);
-
- // Create a temporary AdbClient with its own connection for this sync service
- // This avoids the race condition of multiple threads accessing the same
- // connection
- AdbClient temp_client(m_device_id);
-
- // Connect and start sync on the temporary client
- error = temp_client.Connect();
- if (error.Fail())
- return nullptr;
-
- error = temp_client.StartSync();
- if (error.Fail())
- return nullptr;
-
- // Move the connection from the temporary client to the sync service
- std::unique_ptr<SyncService> sync_service;
- sync_service.reset(new SyncService(std::move(temp_client.m_conn)));
- return sync_service;
-}
Status AdbClient::SyncService::internalPullFile(const FileSpec &remote_file,
const FileSpec &local_file) {
@@ -599,17 +564,23 @@ bool AdbClient::SyncService::IsConnected() const {
return m_conn && m_conn->IsConnected();
}
-AdbClient::SyncService::SyncService(std::unique_ptr<Connection> &&conn)
- : m_conn(std::move(conn)) {}
+AdbClient::SyncService::SyncService(std::unique_ptr<Connection> conn, const std::string &device_id)
+ : m_conn(std::move(conn)), m_device_id(device_id) {}
Status
AdbClient::SyncService::executeCommand(const std::function<Status()> &cmd) {
- if (!m_conn)
- return Status::FromErrorString("SyncService is disconnected");
+ if (!m_conn || !m_conn->IsConnected()) {
+ Status reconnect_error = SetupSyncConnection(m_device_id);
+ if (reconnect_error.Fail()) {
+ return Status::FromErrorStringWithFormat(
+ "SyncService connection failed: %s", reconnect_error.AsCString());
+ }
+ }
Status error = cmd();
- if (error.Fail())
+ if (error.Fail()) {
m_conn.reset();
+ }
return error;
}
@@ -684,3 +655,40 @@ Status AdbClient::SyncService::PullFileChunk(std::vector<char> &buffer,
Status AdbClient::SyncService::ReadAllBytes(void *buffer, size_t size) {
return ::ReadAllBytes(*m_conn, buffer, size);
}
+
+Status AdbClient::SyncService::SetupSyncConnection(const std::string &device_id) {
+ if (!m_conn) {
+ m_conn = std::make_unique<ConnectionFileDescriptor>();
+ }
+
+ AdbClient temp_client(device_id);
+ Status error = temp_client.ConnectToAdb(*m_conn);
+ if (error.Fail())
+ return error;
+
+ temp_client.m_conn = std::move(m_conn);
+ error = temp_client.SelectTargetDevice();
+ if (error.Fail()) {
+ m_conn = std::move(temp_client.m_conn);
+ return error;
+ }
+
+ error = temp_client.EnterSyncMode();
+ if (error.Fail()) {
+ return error;
+ }
+ m_conn = std::move(temp_client.m_conn);
+ return error;
+}
+
+Status AdbClient::ConnectToAdb(Connection &conn) {
+ std::string port = "5037";
+ if (const char *env_port = std::getenv("ANDROID_ADB_SERVER_PORT")) {
+ port = env_port;
+ }
+ std::string uri = "connect://127.0.0.1:" + port;
+
+ Status error;
+ conn.Connect(uri.c_str(), &error);
+ return error;
+}
diff --git a/lldb/source/Plugins/Platform/Android/AdbClient.h b/lldb/source/Plugins/Platform/Android/AdbClient.h
index 9ef780bc6202b..ae1be37186900 100644
--- a/lldb/source/Plugins/Platform/Android/AdbClient.h
+++ b/lldb/source/Plugins/Platform/Android/AdbClient.h
@@ -14,7 +14,6 @@
#include <functional>
#include <list>
#include <memory>
-#include <mutex>
#include <string>
#include <vector>
@@ -37,31 +36,32 @@ class AdbClient {
friend class AdbClient;
public:
+ explicit SyncService(std::unique_ptr<Connection> conn, const std::string &device_id);
+
virtual ~SyncService();
virtual Status PullFile(const FileSpec &remote_file,
const FileSpec &local_file);
- Status PushFile(const FileSpec &local_file, const FileSpec &remote_file);
+ virtual Status PushFile(const FileSpec &local_file, const FileSpec &remote_file);
virtual Status Stat(const FileSpec &remote_file, uint32_t &mode,
uint32_t &size, uint32_t &mtime);
- bool IsConnected() const;
+ virtual bool IsConnected() const;
+
+ const std::string &GetDeviceId() const { return m_device_id; }
protected:
- explicit SyncService(std::unique_ptr<Connection> &&conn);
+ virtual Status SendSyncRequest(const char *request_id, const uint32_t data_len,
+ const void *data);
+ virtual Status ReadSyncHeader(std::string &response_id, uint32_t &data_len);
+ virtual Status ReadAllBytes(void *buffer, size_t size);
private:
- Status SendSyncRequest(const char *request_id, const uint32_t data_len,
- const void *data);
-
- Status ReadSyncHeader(std::string &response_id, uint32_t &data_len);
Status PullFileChunk(std::vector<char> &buffer, bool &eof);
- Status ReadAllBytes(void *buffer, size_t size);
-
Status internalPullFile(const FileSpec &remote_file,
const FileSpec &local_file);
@@ -73,7 +73,11 @@ class AdbClient {
Status executeCommand(const std::function<Status()> &cmd);
+ // Internal connection setup methods
+ Status SetupSyncConnection(const std::string &device_id);
+
std::unique_ptr<Connection> m_conn;
+ std::string m_device_id;
};
static Status CreateByDeviceID(const std::string &device_id, AdbClient &adb);
@@ -103,15 +107,15 @@ class AdbClient {
std::chrono::milliseconds timeout,
const FileSpec &output_file_spec);
- virtual std::unique_ptr<SyncService> GetSyncService(Status &error);
+ Status SelectTargetDevice();
- Status SwitchDeviceTransport();
+ Status EnterSyncMode();
private:
- Status Connect();
-
void SetDeviceID(const std::string &device_id);
+ Status Connect();
+
Status SendMessage(const std::string &packet, const bool reconnect = true);
Status SendDeviceMessage(const std::string &packet);
@@ -125,18 +129,15 @@ class AdbClient {
Status ReadResponseStatus();
- Status Sync();
-
- Status StartSync();
-
Status internalShell(const char *command, std::chrono::milliseconds timeout,
std::vector<char> &output_buf);
Status ReadAllBytes(void *buffer, size_t size);
+ Status ConnectToAdb(Connection &conn);
+
std::string m_device_id;
std::unique_ptr<Connection> m_conn;
- mutable std::mutex m_sync_mutex;
};
} // namespace platform_android
diff --git a/lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp b/lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp
index 68e4886cb1c7a..c471ad742f0dd 100644
--- a/lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp
+++ b/lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp
@@ -9,6 +9,7 @@
#include "lldb/Core/Module.h"
#include "lldb/Core/PluginManager.h"
#include "lldb/Core/Section.h"
+#include "lldb/Host/ConnectionFileDescriptor.h"
#include "lldb/Host/HostInfo.h"
#include "lldb/Utility/LLDBLog.h"
#include "lldb/Utility/Log.h"
@@ -209,36 +210,48 @@ Status PlatformAndroid::GetFile(const FileSpec &source,
if (IsHost() || !m_remote_platform_sp)
return PlatformLinux::GetFile(source, destination);
- FileSpec source_spec(source.GetPath(false), FileSpec::Style::posix);
+ std::string source_path = source.GetPath(false);
+ if (source_path.empty())
+ return Status::FromErrorString("Source file path cannot be empty");
+
+ std::string destination_path = destination.GetPath(false);
+ if (destination_path.empty())
+ return Status::FromErrorString("Destination file path cannot be empty");
+
+ FileSpec source_spec(source_path, FileSpec::Style::posix);
if (source_spec.IsRelative())
source_spec = GetRemoteWorkingDirectory().CopyByAppendingPathComponent(
source_spec.GetPathAsConstString(false).GetStringRef());
Status error;
auto sync_service = GetSyncService(error);
- if (error.Fail())
- return error;
-
- uint32_t mode = 0, size = 0, mtime = 0;
- error = sync_service->Stat(source_spec, mode, size, mtime);
- if (error.Fail())
- return error;
-
- if (mode != 0)
- return sync_service->PullFile(source_spec, destination);
+
+ // If sync service is available, try to use it
+ if (error.Success() && sync_service) {
+ uint32_t mode = 0, size = 0, mtime = 0;
+ error = sync_service->Stat(source_spec, mode, size, mtime);
+ if (error.Success()) {
+ if (mode != 0)
+ return sync_service->PullFile(source_spec, destination);
+
+ // mode == 0 can signify that adbd cannot access the file due security
+ // constraints - fall through to try "cat ..." as a fallback.
+ Log *log = GetLog(LLDBLog::Platform);
+ LLDB_LOGF(log, "Got mode == 0 on '%s': try to get file via 'shell cat'",
+ source_spec.GetPath(false).c_str());
+ }
+ }
+ // Fallback to shell cat command if sync service failed or returned mode == 0
std::string source_file = source_spec.GetPath(false);
Log *log = GetLog(LLDBLog::Platform);
- LLDB_LOGF(log, "Got mode == 0 on '%s': try to get file via 'shell cat'",
- source_file.c_str());
+ LLDB_LOGF(log, "Using shell cat fallback for '%s'", source_file.c_str());
if (strchr(source_file.c_str(), '\'') != nullptr)
return Status::FromErrorString(
"Doesn't support single-quotes in filenames");
- // mode == 0 can signify that adbd cannot access the file due security
- // constraints - try "cat ..." as a fallback.
AdbClientUP adb(GetAdbClient(error));
if (error.Fail())
return error;
@@ -275,12 +288,19 @@ Status PlatformAndroid::DownloadModuleSlice(const FileSpec &src_file_spec,
const uint64_t src_offset,
const uint64_t src_size,
const FileSpec &dst_file_spec) {
+ std::string source_file = src_file_spec.GetPath(false);
+ if (source_file.empty())
+ return Status::FromErrorString("Source file path cannot be empty");
+
+ std::string destination_file = dst_file_spec.GetPath(false);
+ if (destination_file.empty())
+ return Status::FromErrorString("Destination file path cannot be empty");
+
// In Android API level 23 and above, dynamic loader is able to load .so
// file directly from APK. In that case, src_offset will be an non-zero.
if (src_offset == 0) // Use GetFile for a normal file.
return GetFile(src_file_spec, dst_file_spec);
- std::string source_file = src_file_spec.GetPath(false);
if (source_file.find('\'') != std::string::npos)
return Status::FromErrorString(
"Doesn't support single-quotes in filenames");
@@ -476,9 +496,10 @@ std::string PlatformAndroid::GetRunAs() {
std::unique_ptr<AdbClient::SyncService>
PlatformAndroid::GetSyncService(Status &error) {
- AdbClientUP adb(GetAdbClient(error));
- if (error.Fail())
- return nullptr;
-
- return adb->GetSyncService(error);
+ auto conn = std::make_unique<ConnectionFileDescriptor>();
+ auto sync_service = std::make_unique<AdbClient::SyncService>(
+ std::move(conn), m_device_id);
+
+ error.Clear();
+ return sync_service;
}
diff --git a/lldb/source/Plugins/Platform/Android/PlatformAndroid.h b/lldb/source/Plugins/Platform/Android/PlatformAndroid.h
index 3140acb573416..92738f6fa79f1 100644
--- a/lldb/source/Plugins/Platform/Android/PlatformAndroid.h
+++ b/lldb/source/Plugins/Platform/Android/PlatformAndroid.h
@@ -73,14 +73,18 @@ class PlatformAndroid : public platform_linux::PlatformLinux {
GetLibdlFunctionDeclarations(lldb_private::Process *process) override;
typedef std::unique_ptr<AdbClient> AdbClientUP;
- virtual AdbClientUP GetAdbClient(Status &error);
+ std::string GetRunAs();
+
+public:
+ // Exposed for testing
+ virtual AdbClientUP GetAdbClient(Status &error);
virtual llvm::StringRef GetPropertyPackageName();
- std::string GetRunAs();
+protected:
+ virtual std::unique_ptr<AdbClient::SyncService> GetSyncService(Status &error);
private:
- std::unique_ptr<AdbClient::SyncService> GetSyncService(Status &error);
std::string m_device_id;
uint32_t m_sdk_version;
};
diff --git a/lldb/unittests/Platform/Android/AdbClientTest.cpp b/lldb/unittests/Platform/Android/AdbClientTest.cpp
index c2f658b9d1bc1..06822968124ec 100644
--- a/lldb/unittests/Platform/Android/AdbClientTest.cpp
+++ b/lldb/unittests/Platform/Android/AdbClientTest.cpp
@@ -8,11 +8,7 @@
#include "Plugins/Platform/Android/AdbClient.h"
#include "gtest/gtest.h"
-#include <atomic>
#include <cstdlib>
-#include <future>
-#include <thread>
-#include <vector>
static void set_env(const char *var, const char *value) {
#ifdef _WIN32
@@ -51,115 +47,17 @@ TEST_F(AdbClientTest, CreateByDeviceId_ByEnvVar) {
EXPECT_EQ("device2", adb.GetDeviceID());
}
-TEST_F(AdbClientTest, GetSyncServiceThreadSafe) {
- // Test high-volume concurrent access to GetSyncService
- // This test verifies thread safety under sustained load with many rapid calls
- // Catches race conditions that emerge when multiple threads make repeated
- // calls to GetSyncService on the same AdbClient instance
-
- AdbClient shared_adb_client("test_device");
-
- const int num_threads = 8;
- std::vector<std::future<bool>> futures;
- std::atomic<int> success_count{0};
- std::atomic<int> null_count{0};
-
- // Launch multiple threads that all call GetSyncService on the SAME AdbClient
- for (int i = 0; i < num_threads; ++i) {
- futures.push_back(std::async(std::launch::async, [&]() {
- // Multiple rapid calls to trigger the race condition
- for (int j = 0; j < 20; ++j) {
- Status error;
-
- auto sync_service = shared_adb_client.GetSyncService(error);
-
- if (sync_service != nullptr) {
- success_count++;
- } else {
- null_count++;
- }
-
- // Small delay to increase chance of hitting the race condition
- std::this_thread::sleep_for(std::chrono::microseconds(1));
- }
- return true;
- }));
- }
-
- // Wait for all threads to complete
- bool all_completed = true;
- for (auto &future : futures) {
- bool thread_result = future.get();
- if (!thread_result) {
- all_completed = false;
- }
- }
-
- // This should pass (though sync services may fail
- // to connect)
- EXPECT_TRUE(all_completed) << "Parallel GetSyncService calls should not "
- "crash due to race conditions. "
- << "Successes: " << success_count.load()
- << ", Nulls: " << null_count.load();
-
- // The key test: we should complete all operations without crashing
- int total_operations = num_threads * 20;
- int completed_operations = success_count.load() + null_count.load();
- EXPECT_EQ(total_operations, completed_operations)
- << "All operations should complete without crashing";
-}
-
-TEST_F(AdbClientTest, ConnectionMoveRaceCondition) {
- // Test simultaneous access timing to GetSyncService
- // This test verifies thread safety when multiple threads start at exactly
- // the same time, maximizing the chance of hitting precise timing conflicts
- // Catches race conditions that occur with synchronized simultaneous access
-
+TEST_F(AdbClientTest, SyncServiceCreation) {
AdbClient adb_client("test_device");
-
- // Try to trigger the exact race condition by having multiple threads
- // simultaneously call GetSyncService
-
- std::atomic<bool> start_flag{false};
- std::vector<std::thread> threads;
- std::atomic<int> null_service_count{0};
- std::atomic<int> valid_service_count{0};
-
- const int num_threads = 10;
-
- // Create threads that will all start simultaneously
- for (int i = 0; i < num_threads; ++i) {
- threads.emplace_back([&]() {
- // Wait for all threads to be ready
- while (!start_flag.load()) {
- std::this_thread::yield();
- }
-
- Status error;
- auto sync_service = adb_client.GetSyncService(error);
-
- if (sync_service == nullptr) {
- null_service_count++;
- } else {
- valid_service_count++;
- }
- });
- }
-
- // Start all threads simultaneously to maximize chance of race condition
- start_flag.store(true);
-
- // Wait for all threads to complete
- for (auto &thread : threads) {
- thread.join();
- }
-
- // The test passes if we don't crash
- int total_results = null_service_count.load() + valid_service_count.load();
- EXPECT_EQ(num_threads, total_results)
- << "All threads should complete without crashing. "
- << "Null services: " << null_service_count.load()
- << ", Valid services: " << valid_service_count.load();
+
+ std::unique_ptr<Connection> conn = std::make_unique<ConnectionFileDescriptor>();
+ auto sync_service = std::make_unique<AdbClient::SyncService>(
+ std::move(conn), adb_client.GetDeviceID());
+
+ EXPECT_NE(sync_service, nullptr);
+ EXPECT_EQ(sync_service->GetDeviceId(), "test_device");
+
+ EXPECT_FALSE(sync_service->IsConnected());
}
} // end namespace platform_android
diff --git a/lldb/unittests/Platform/Android/PlatformAndroidTest.cpp b/lldb/unittests/Platform/Android/PlatformAndroidTest.cpp
index d021562d94d28..cadaa5491c60f 100644
--- a/lldb/unittests/Platform/Android/PlatformAndroidTest.cpp
+++ b/lldb/unittests/Platform/Android/PlatformAndroidTest.cpp
@@ -8,8 +8,6 @@
#include "Plugins/Platform/Android/PlatformAndroid.h"
#include "Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.h"
-#include "TestingSupport/SubsystemRAII.h"
-#include "TestingSupport/TestUtilities.h"
#include "lldb/Utility/Connection.h"
#include "gmock/gmock.h"
@@ -20,17 +18,48 @@ using namespace testing;
namespace {
-class MockSyncService : public AdbClient::SyncService {
+class MockConnection : public Connection {
public:
- MockSyncService() : SyncService(std::unique_ptr<Connection>()) {}
+ MockConnection(bool should_fail = false) : should_fail_(should_fail) {}
+
+ ConnectionStatus Connect(llvm::StringRef url, Status *error_ptr) override {
+ if (should_fail_ && error_ptr) {
+ *error_ptr = Status::FromErrorString("Mock connection failed");
+ return eConnectionStatusError;
+ }
+ return eConnectionStatusSuccess;
+ }
- MOCK_METHOD2(PullFile,
- Status(const FileSpec &remote_file, const FileSpec &local_file));
- MOCK_METHOD4(Stat, Status(const FileSpec &remote_file, uint32_t &mode,
- uint32_t &size, uint32_t &mtime));
-};
+ ConnectionStatus Disconnect(Status *error_ptr) override {
+ return eConnectionStatusSuccess;
+ }
+
+ bool IsConnected() const override { return !should_fail_; }
+
+ size_t Read(void *dst, size_t dst_len, const Timeout<std::micro> &timeout,
+ ConnectionStatus &status, Status *error_ptr) override {
+ status = should_fail_ ? eConnectionStatusError : eConnectionStatusSuccess;
+ if (should_fail_ && error_ptr) {
+ *error_ptr = Status::FromErrorString("Mock read failed");
+ }
+ return should_fail_ ? 0 : dst_len;
+ }
+
+ size_t Write(const void *src, size_t src_len, ConnectionStatus &status,
+ Status *error_ptr) override {
+ status = should_fail_ ? eConnectionStatusError : eConnectionStatusSuccess;
+ if (should_fail_ && error_ptr) {
+ *error_ptr = Status::FromErrorString("Mock write failed");
+ }
+ return should_fail_ ? 0 : src_len;
+ }
-typedef std::unique_ptr<AdbClient::SyncService> SyncServiceUP;
+ std::string GetURI() override { return "mock://connection"; }
+ bool InterruptRead() override { return true; }
+
+private:
+ bool should_fail_;
+};
class MockAdbClient : public AdbClient {
public:
@@ -39,193 +68,295 @@ class MockAdbClient : public AdbClient {
MOCK_METHOD3(ShellToFile,
Status(const char *command, std::chrono::milliseconds timeout,
const FileSpec &output_file_spec));
- MOCK_METHOD1(GetSyncService, SyncServiceUP(Status &error));
};
class PlatformAndroidTest : public PlatformAndroid, public ::testing::Test {
public:
PlatformAndroidTest() : PlatformAndroid(false) {
m_remote_platform_sp = PlatformSP(new PlatformAndroidRemoteGDBServer());
+
+ // Set up default mock behavior to avoid uninteresting call warnings
+ ON_CALL(*this, GetSyncService(_))
+ .WillByDefault([](Status &error) -> std::unique_ptr<AdbClient::SyncService> {
+ error = Status::FromErrorString("Sync service unavailable");
+ return nullptr;
+ });
}
MOCK_METHOD1(GetAdbClient, AdbClientUP(Status &error));
MOCK_METHOD0(GetPropertyPackageName, llvm::StringRef());
+ MOCK_METHOD1(GetSyncService, std::unique_ptr<AdbClient::SyncService>(Status &error));
+
+ // Make GetSyncService public for testing
+ using PlatformAndroid::GetSyncService;
};
} // namespace
-TEST_F(PlatformAndroidTest, DownloadModuleSliceWithAdbClientError) {
+TEST_F(PlatformAndroidTest, DownloadModuleSlice_AdbClientError_FailsGracefully) {
EXPECT_CALL(*this, GetAdbClient(_))
- .Times(1)
.WillOnce(DoAll(WithArg<0>([](auto &arg) {
- arg = Status::FromErrorString(
- "Failed to create AdbClient");
+ arg = Status::FromErrorString("Failed to create AdbClient");
}),
Return(ByMove(AdbClientUP()))));
- EXPECT_TRUE(
- DownloadModuleSlice(
- FileSpec("/system/app/Test/Test.apk!/lib/arm64-v8a/libtest.so"), 4096,
- 3600, FileSpec())
- .Fail());
+ Status result = DownloadModuleSlice(
+ FileSpec("/system/app/Test/Test.apk!/lib/arm64-v8a/libtest.so"),
+ 4096, 3600, FileSpec("/tmp/libtest.so"));
+
+ EXPECT_TRUE(result.Fail());
+ EXPECT_THAT(result.AsCString(), HasSubstr("Failed to create AdbClient"));
}
-TEST_F(PlatformAndroidTest, DownloadModuleSliceWithNormalFile) {
- auto sync_service = new MockSyncService();
- EXPECT_CALL(*sync_service, Stat(FileSpec("/system/lib64/libc.so"), _, _, _))
- .Times(1)
- .WillOnce(DoAll(SetArgReferee<1>(1), Return(Status())));
- EXPECT_CALL(*sync_service, PullFile(FileSpec("/system/lib64/libc.so"), _))
- .Times(1)
+TEST_F(PlatformAndroidTest, DownloadModuleSlice_ZipFile_UsesCorrectDdCommand) {
+ auto *adb_client = new MockAdbClient();
+ EXPECT_CALL(*adb_client,
+ ShellToFile(StrEq("dd if='/system/app/Test/Test.apk' "
+ "iflag=skip_bytes,count_bytes "
+ "skip=4096 count=3600 status=none"),
+ _, _))
.WillOnce(Return(Status()));
- auto adb_client = new MockAdbClient();
- EXPECT_CALL(*adb_client, GetSyncService(_))
- .Times(1)
- .WillOnce(Return(ByMove(SyncServiceUP(sync_service))));
+ EXPECT_CALL(*this, GetPropertyPackageName())
+ .WillOnce(Return(llvm::StringRef("")));
EXPECT_CALL(*this, GetAdbClient(_))
- .Times(1)
.WillOnce(Return(ByMove(AdbClientUP(adb_client))));
- EXPECT_TRUE(
- DownloadModuleSlice(FileSpec("/system/lib64/libc.so"), 0, 0, FileSpec())
- .Success());
+ Status result = DownloadModuleSlice(
+ FileSpec("/system/app/Test/Test.apk!/lib/arm64-v8a/libtest.so"),
+ 4096, 3600, FileSpec("/tmp/libtest.so"));
+
+ EXPECT_TRUE(result.Success());
}
-TEST_F(PlatformAndroidTest, DownloadModuleSliceWithZipFile) {
- auto adb_client = new MockAdbClient();
+TEST_F(PlatformAndroidTest, DownloadModuleSlice_ZipFileWithRunAs_UsesRunAsCommand) {
+ auto *adb_client = new MockAdbClient();
EXPECT_CALL(*adb_client,
- ShellToFile(StrEq("dd if='/system/app/Test/Test.apk' "
+ ShellToFile(StrEq("run-as 'com.example.test' "
+ "dd if='/system/app/Test/Test.apk' "
"iflag=skip_bytes,count_bytes "
"skip=4096 count=3600 status=none"),
_, _))
- .Times(1)
.WillOnce(Return(Status()));
+ EXPECT_CALL(*this, GetPropertyPackageName())
+ .WillOnce(Return(llvm::StringRef("com.example.test")));
+
EXPECT_CALL(*this, GetAdbClient(_))
- .Times(1)
.WillOnce(Return(ByMove(AdbClientUP(adb_client))));
- EXPECT_TRUE(
- DownloadModuleSlice(
- FileSpec("/system/app/Test/Test.apk!/lib/arm64-v8a/libtest.so"), 4096,
- 3600, FileSpec())
- .Success());
+ Status result = DownloadModuleSlice(
+ FileSpec("/system/app/Test/Test.apk!/lib/arm64-v8a/libtest.so"),
+ 4096, 3600, FileSpec("/tmp/libtest.so"));
+
+ EXPECT_TRUE(result.Success());
}
-TEST_F(PlatformAndroidTest, DownloadModuleSliceWithZipFileAndRunAs) {
- auto adb_client = new MockAdbClient();
+TEST_F(PlatformAndroidTest, DownloadModuleSlice_LargeFile_CalculatesParametersCorrectly) {
+ const uint64_t large_offset = 100 * 1024 * 1024 ; // 100MB offset
+ const uint64_t large_size = 50 * 1024 * 1024; // 50MB size
+
+ auto *adb_client = new MockAdbClient();
EXPECT_CALL(*adb_client,
- ShellToFile(StrEq("run-as 'com.example.test' "
- "dd if='/system/app/Test/Test.apk' "
+ ShellToFile(StrEq("dd if='/system/app/Large.apk' "
"iflag=skip_bytes,count_bytes "
- "skip=4096 count=3600 status=none"),
+ "skip=104857600 count=52428800 status=none"),
_, _))
- .Times(1)
.WillOnce(Return(Status()));
EXPECT_CALL(*this, GetPropertyPackageName())
- .Times(1)
- .WillOnce(Return(llvm::StringRef("com.example.test")));
+ .WillOnce(Return(llvm::StringRef("")));
EXPECT_CALL(*this, GetAdbClient(_))
- .Times(1)
.WillOnce(Return(ByMove(AdbClientUP(adb_client))));
- EXPECT_TRUE(
- DownloadModuleSlice(
- FileSpec("/system/app/Test/Test.apk!/lib/arm64-v8a/libtest.so"), 4096,
- 3600, FileSpec())
- .Success());
+ Status result = DownloadModuleSlice(
+ FileSpec("/system/app/Large.apk!/lib/arm64-v8a/large.so"),
+ large_offset, large_size, FileSpec("/tmp/large.so"));
+
+ EXPECT_TRUE(result.Success());
}
-TEST_F(PlatformAndroidTest, GetFileWithNormalFile) {
- auto sync_service = new MockSyncService();
- EXPECT_CALL(*sync_service, Stat(FileSpec("/data/local/tmp/test"), _, _, _))
- .Times(1)
- .WillOnce(DoAll(SetArgReferee<1>(1), Return(Status())));
- EXPECT_CALL(*sync_service, PullFile(FileSpec("/data/local/tmp/test"), _))
- .Times(1)
+TEST_F(PlatformAndroidTest, GetFile_SyncServiceUnavailable_FallsBackToShellCat) {
+ auto *adb_client = new MockAdbClient();
+ EXPECT_CALL(*adb_client, ShellToFile(StrEq("cat '/data/local/tmp/test'"), _, _))
.WillOnce(Return(Status()));
- auto adb_client = new MockAdbClient();
- EXPECT_CALL(*adb_client, GetSyncService(_))
- .Times(1)
- .WillOnce(Return(ByMove(SyncServiceUP(sync_service))));
+ EXPECT_CALL(*this, GetPropertyPackageName())
+ .WillOnce(Return(llvm::StringRef("")));
EXPECT_CALL(*this, GetAdbClient(_))
- .Times(1)
- .WillOnce(Return(ByMove(AdbClientUP(adb_client))));
+ .WillOnce(DoAll(WithArg<0>([](auto &arg) { arg.Clear(); }),
+ Return(ByMove(AdbClientUP(adb_client)))));
+
+ EXPECT_CALL(*this, GetSyncService(_))
+ .WillOnce([](Status &error) -> std::unique_ptr<AdbClient::SyncService> {
+ error = Status::FromErrorString("Sync service unavailable");
+ return nullptr;
+ });
- EXPECT_TRUE(GetFile(FileSpec("/data/local/tmp/test"), FileSpec()).Success());
+ Status result = GetFile(FileSpec("/data/local/tmp/test"), FileSpec("/tmp/test"));
+ EXPECT_TRUE(result.Success());
}
-TEST_F(PlatformAndroidTest, GetFileWithCatFallback) {
- auto sync_service = new MockSyncService();
- EXPECT_CALL(
- *sync_service,
- Stat(FileSpec("/data/data/com.example.app/lib-main/libtest.so"), _, _, _))
- .Times(1)
- .WillOnce(DoAll(SetArgReferee<1>(0), Return(Status())));
-
- auto adb_client0 = new MockAdbClient();
- EXPECT_CALL(*adb_client0, GetSyncService(_))
- .Times(1)
- .WillOnce(Return(ByMove(SyncServiceUP(sync_service))));
-
- auto adb_client1 = new MockAdbClient();
- EXPECT_CALL(
- *adb_client1,
- ShellToFile(StrEq("cat '/data/data/com.example.app/lib-main/libtest.so'"),
- _, _))
- .Times(1)
+TEST_F(PlatformAndroidTest, GetFile_WithRunAs_UsesRunAsInShellCommand) {
+ auto *adb_client = new MockAdbClient();
+ EXPECT_CALL(*adb_client,
+ ShellToFile(StrEq("run-as 'com.example.app' "
+ "cat '/data/data/com.example.app/lib-main/libtest.so'"),
+ _, _))
.WillOnce(Return(Status()));
+ EXPECT_CALL(*this, GetPropertyPackageName())
+ .WillOnce(Return(llvm::StringRef("com.example.app")));
+
EXPECT_CALL(*this, GetAdbClient(_))
- .Times(2)
- .WillOnce(Return(ByMove(AdbClientUP(adb_client0))))
- .WillOnce(Return(ByMove(AdbClientUP(adb_client1))));
-
- EXPECT_TRUE(
- GetFile(FileSpec("/data/data/com.example.app/lib-main/libtest.so"),
- FileSpec())
- .Success());
+ .WillOnce(DoAll(WithArg<0>([](auto &arg) { arg.Clear(); }),
+ Return(ByMove(AdbClientUP(adb_client)))));
+
+ EXPECT_CALL(*this, GetSyncService(_))
+ .WillOnce([](Status &error) -> std::unique_ptr<AdbClient::SyncService> {
+ error = Status::FromErrorString("Sync service unavailable");
+ return nullptr;
+ });
+
+ Status result = GetFile(FileSpec("/data/data/com.example.app/lib-main/libtest.so"),
+ FileSpec("/tmp/libtest.so"));
+ EXPECT_TRUE(result.Success());
+}
+
+TEST_F(PlatformAndroidTest, GetFile_FilenameWithSingleQuotes_Rejected) {
+ EXPECT_CALL(*this, GetSyncService(_))
+ .WillOnce([](Status &error) -> std::unique_ptr<AdbClient::SyncService> {
+ error = Status::FromErrorString("Sync service unavailable");
+ return nullptr;
+ });
+
+ Status result = GetFile(FileSpec("/test/file'with'quotes"), FileSpec("/tmp/output"));
+
+ EXPECT_TRUE(result.Fail());
+ EXPECT_THAT(result.AsCString(), HasSubstr("single-quotes"));
+}
+
+TEST_F(PlatformAndroidTest, DownloadModuleSlice_FilenameWithSingleQuotes_Rejected) {
+ Status result = DownloadModuleSlice(FileSpec("/test/file'with'quotes"), 100, 200, FileSpec("/tmp/output"));
+
+ EXPECT_TRUE(result.Fail());
+ EXPECT_THAT(result.AsCString(), HasSubstr("single-quotes"));
}
-TEST_F(PlatformAndroidTest, GetFileWithCatFallbackAndRunAs) {
- auto sync_service = new MockSyncService();
- EXPECT_CALL(
- *sync_service,
- Stat(FileSpec("/data/data/com.example.app/lib-main/libtest.so"), _, _, _))
- .Times(1)
- .WillOnce(DoAll(SetArgReferee<1>(0), Return(Status())));
-
- auto adb_client0 = new MockAdbClient();
- EXPECT_CALL(*adb_client0, GetSyncService(_))
- .Times(1)
- .WillOnce(Return(ByMove(SyncServiceUP(sync_service))));
-
- auto adb_client1 = new MockAdbClient();
- EXPECT_CALL(
- *adb_client1,
- ShellToFile(StrEq("run-as 'com.example.app' "
- "cat '/data/data/com.example.app/lib-main/libtest.so'"),
- _, _))
- .Times(1)
+TEST_F(PlatformAndroidTest, GetFile_EmptyFilenames_FailWithMeaningfulErrors) {
+ // Empty source
+ Status result1 = GetFile(FileSpec(""), FileSpec("/tmp/output"));
+ EXPECT_TRUE(result1.Fail());
+ EXPECT_THAT(result1.AsCString(), HasSubstr("Source file path cannot be empty"));
+
+ // Empty destination
+ Status result2 = GetFile(FileSpec("/data/test.txt"), FileSpec(""));
+ EXPECT_TRUE(result2.Fail());
+ EXPECT_THAT(result2.AsCString(), HasSubstr("Destination file path cannot be empty"));
+}
+
+TEST_F(PlatformAndroidTest, DownloadModuleSlice_EmptyFilenames_FailWithMeaningfulErrors) {
+ // Empty source
+ Status result1 = DownloadModuleSlice(FileSpec(""), 0, 100, FileSpec("/tmp/output"));
+ EXPECT_TRUE(result1.Fail());
+ EXPECT_THAT(result1.AsCString(), HasSubstr("Source file path cannot be empty"));
+
+ // Empty destination
+ Status result2 = DownloadModuleSlice(FileSpec("/data/test.apk"), 100, 200, FileSpec(""));
+ EXPECT_TRUE(result2.Fail());
+ EXPECT_THAT(result2.AsCString(), HasSubstr("Destination file path cannot be empty"));
+}
+
+TEST_F(PlatformAndroidTest, GetFile_NetworkTimeout_PropagatesErrorCorrectly) {
+ auto *adb_client = new MockAdbClient();
+ EXPECT_CALL(*adb_client, ShellToFile(_, _, _))
+ .WillOnce(Return(Status::FromErrorString("Network timeout")));
+
+ EXPECT_CALL(*this, GetPropertyPackageName())
+ .WillOnce(Return(llvm::StringRef("")));
+
+ EXPECT_CALL(*this, GetAdbClient(_))
+ .WillOnce(DoAll(WithArg<0>([](auto &arg) { arg.Clear(); }),
+ Return(ByMove(AdbClientUP(adb_client)))));
+
+ EXPECT_CALL(*this, GetSyncService(_))
+ .WillOnce([](Status &error) -> std::unique_ptr<AdbClient::SyncService> {
+ error = Status::FromErrorString("Sync service unavailable");
+ return nullptr;
+ });
+
+ Status result = GetFile(FileSpec("/data/large/file.so"), FileSpec("/tmp/large.so"));
+ EXPECT_TRUE(result.Fail());
+ EXPECT_THAT(result.AsCString(), HasSubstr("Network timeout"));
+}
+
+TEST_F(PlatformAndroidTest, SyncService_ConnectionFailsGracefully) {
+ auto mock_conn = std::make_unique<MockConnection>(true); // Configure to fail
+
+ // Constructor should succeed even with a failing connection
+ AdbClient::SyncService sync_service(std::move(mock_conn), "test-device");
+
+ // The service should report as not connected
+ EXPECT_FALSE(sync_service.IsConnected());
+ EXPECT_EQ(sync_service.GetDeviceId(), "test-device");
+
+ // Operations should fail gracefully when connection setup fails
+ FileSpec remote_file("/data/test.txt");
+ FileSpec local_file("/tmp/test.txt");
+ uint32_t mode, size, mtime;
+
+ Status result = sync_service.Stat(remote_file, mode, size, mtime);
+ EXPECT_TRUE(result.Fail());
+ EXPECT_THAT(result.AsCString(), HasSubstr("connection failed"));
+}
+
+TEST_F(PlatformAndroidTest, GetRunAs_FormatsPackageNameCorrectly) {
+ // Empty package name
+ EXPECT_CALL(*this, GetPropertyPackageName())
+ .WillOnce(Return(llvm::StringRef("")));
+ EXPECT_EQ(GetRunAs(), "");
+
+ // Valid package name
+ EXPECT_CALL(*this, GetPropertyPackageName())
+ .WillOnce(Return(llvm::StringRef("com.example.test")));
+ EXPECT_EQ(GetRunAs(), "run-as 'com.example.test' ");
+}
+
+TEST_F(PlatformAndroidTest, GetAdbClient_CreatesValidClient) {
+ Status error;
+
+ PlatformAndroid real_platform(false);
+ auto adb_client = real_platform.GetAdbClient(error);
+
+ EXPECT_TRUE(error.Success());
+ EXPECT_NE(adb_client, nullptr);
+ EXPECT_EQ(adb_client->GetDeviceID(), "");
+}
+
+TEST_F(PlatformAndroidTest, DownloadModuleSlice_ZeroOffset_CallsGetFileInsteadOfDd) {
+ // When offset=0, DownloadModuleSlice calls GetFile which uses 'cat', not 'dd'
+ // We need to ensure the sync service fails so GetFile falls back to shell cat
+ auto *adb_client = new MockAdbClient();
+ EXPECT_CALL(*adb_client, ShellToFile(StrEq("cat '/system/lib64/libc.so'"), _, _))
.WillOnce(Return(Status()));
EXPECT_CALL(*this, GetPropertyPackageName())
- .Times(1)
- .WillOnce(Return(llvm::StringRef("com.example.app")));
+ .WillOnce(Return(llvm::StringRef("")));
EXPECT_CALL(*this, GetAdbClient(_))
- .Times(2)
- .WillOnce(Return(ByMove(AdbClientUP(adb_client0))))
- .WillOnce(Return(ByMove(AdbClientUP(adb_client1))));
-
- EXPECT_TRUE(
- GetFile(FileSpec("/data/data/com.example.app/lib-main/libtest.so"),
- FileSpec())
- .Success());
+ .WillOnce(DoAll(WithArg<0>([](auto &arg) { arg.Clear(); }),
+ Return(ByMove(AdbClientUP(adb_client)))));
+
+ // Mock GetSyncService to fail, forcing GetFile to use shell cat fallback
+ EXPECT_CALL(*this, GetSyncService(_))
+ .WillOnce(DoAll(WithArg<0>([](auto &arg) {
+ arg = Status::FromErrorString("Sync service unavailable");
+ }),
+ Return(ByMove(std::unique_ptr<AdbClient::SyncService>()))));
+
+ Status result = DownloadModuleSlice(FileSpec("/system/lib64/libc.so"), 0, 0, FileSpec("/tmp/libc.so"));
+ EXPECT_TRUE(result.Success());
}
>From 06df0a17733f03840f535e94aafe961865c81044 Mon Sep 17 00:00:00 2001
From: Chad Smith <cs01 at users.noreply.github.com>
Date: Wed, 9 Jul 2025 17:34:16 -0700
Subject: [PATCH 3/8] refactor adb code
---
.../Plugins/Platform/Android/AdbClient.cpp | 498 +-----------------
.../Plugins/Platform/Android/AdbClient.h | 20 +-
.../Platform/Android/AdbClientUtils.cpp | 142 +++++
.../Plugins/Platform/Android/AdbClientUtils.h | 46 ++
.../Platform/Android/AdbSyncService.cpp | 285 ++++++++++
.../Plugins/Platform/Android/AdbSyncService.h | 50 ++
.../Plugins/Platform/Android/CMakeLists.txt | 6 +-
.../Platform/Android/PlatformAndroid.cpp | 15 +-
.../Platform/Android/PlatformAndroid.h | 3 +-
9 files changed, 568 insertions(+), 497 deletions(-)
create mode 100644 lldb/source/Plugins/Platform/Android/AdbClientUtils.cpp
create mode 100644 lldb/source/Plugins/Platform/Android/AdbClientUtils.h
create mode 100644 lldb/source/Plugins/Platform/Android/AdbSyncService.cpp
create mode 100644 lldb/source/Plugins/Platform/Android/AdbSyncService.h
diff --git a/lldb/source/Plugins/Platform/Android/AdbClient.cpp b/lldb/source/Plugins/Platform/Android/AdbClient.cpp
index b55d99c7c2538..67b0b570e2cdc 100644
--- a/lldb/source/Plugins/Platform/Android/AdbClient.cpp
+++ b/lldb/source/Plugins/Platform/Android/AdbClient.cpp
@@ -8,85 +8,24 @@
#include "AdbClient.h"
-#include "llvm/ADT/STLExtras.h"
-#include "llvm/ADT/SmallVector.h"
-#include "llvm/ADT/StringRef.h"
-#include "llvm/Support/FileUtilities.h"
-
-#include "lldb/Host/ConnectionFileDescriptor.h"
-#include "lldb/Host/FileSystem.h"
-#include "lldb/Host/PosixApi.h"
-#include "lldb/Utility/DataBuffer.h"
-#include "lldb/Utility/DataBufferHeap.h"
-#include "lldb/Utility/DataEncoder.h"
-#include "lldb/Utility/DataExtractor.h"
+#include "lldb/Host/posix/ConnectionFileDescriptorPosix.h"
#include "lldb/Utility/FileSpec.h"
#include "lldb/Utility/StreamString.h"
-#include "lldb/Utility/Timeout.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringRef.h"
#include <climits>
-
-#include <algorithm>
#include <cstdlib>
-#include <fstream>
#include <sstream>
-// On Windows, transitive dependencies pull in <Windows.h>, which defines a
-// macro that clashes with a method name.
-#ifdef SendMessage
-#undef SendMessage
-#endif
-
using namespace lldb;
using namespace lldb_private;
using namespace lldb_private::platform_android;
using namespace std::chrono;
+using namespace adb_client_utils;
-static const seconds kReadTimeout(20);
-static const char *kOKAY = "OKAY";
-static const char *kFAIL = "FAIL";
-static const char *kDATA = "DATA";
-static const char *kDONE = "DONE";
-
-static const char *kSEND = "SEND";
-static const char *kRECV = "RECV";
-static const char *kSTAT = "STAT";
-
-static const size_t kSyncPacketLen = 8;
-// Maximum size of a filesync DATA packet.
-static const size_t kMaxPushData = 2 * 1024;
-// Default mode for pushed files.
-static const uint32_t kDefaultMode = 0100770; // S_IFREG | S_IRWXU | S_IRWXG
-
-static const char *kSocketNamespaceAbstract = "localabstract";
-static const char *kSocketNamespaceFileSystem = "localfilesystem";
-
-static Status ReadAllBytes(Connection &conn, void *buffer, size_t size) {
-
- Status error;
- ConnectionStatus status;
- char *read_buffer = static_cast<char *>(buffer);
-
- auto now = steady_clock::now();
- const auto deadline = now + kReadTimeout;
- size_t total_read_bytes = 0;
- while (total_read_bytes < size && now < deadline) {
- auto read_bytes =
- conn.Read(read_buffer + total_read_bytes, size - total_read_bytes,
- duration_cast<microseconds>(deadline - now), status, &error);
- if (error.Fail())
- return error;
- total_read_bytes += read_bytes;
- if (status != eConnectionStatusSuccess)
- break;
- now = steady_clock::now();
- }
- if (total_read_bytes < size)
- error = Status::FromErrorStringWithFormat(
- "Unable to read requested number of bytes. Connection status: %d.",
- status);
- return error;
-}
+const static char *kSocketNamespaceAbstract = "localabstract";
+const static char *kSocketNamespaceFileSystem = "localfilesystem";
Status AdbClient::CreateByDeviceID(const std::string &device_id,
AdbClient &adb) {
@@ -115,9 +54,16 @@ Status AdbClient::CreateByDeviceID(const std::string &device_id,
return error;
}
-AdbClient::AdbClient() = default;
-AdbClient::AdbClient(const std::string &device_id) : m_device_id(device_id) {}
+AdbClient::AdbClient(const std::string &device_id) : m_device_id(device_id) {
+ m_conn = std::make_unique<ConnectionFileDescriptor>();
+ Connect();
+}
+
+AdbClient::AdbClient() {
+ m_conn = std::make_unique<ConnectionFileDescriptor>();
+ Connect();
+}
AdbClient::~AdbClient() = default;
@@ -129,23 +75,22 @@ const std::string &AdbClient::GetDeviceID() const { return m_device_id; }
Status AdbClient::Connect() {
Status error;
- m_conn = std::make_unique<ConnectionFileDescriptor>();
return ConnectToAdb(*m_conn);
}
Status AdbClient::GetDevices(DeviceIDList &device_list) {
device_list.clear();
- auto error = SendMessage("host:devices");
+ auto error = SendAdbMessage(*m_conn, "host:devices", true);
if (error.Fail())
return error;
- error = ReadResponseStatus();
+ error = ReadResponseStatus(*m_conn);
if (error.Fail())
return error;
std::vector<char> in_buffer;
- error = ReadMessage(in_buffer);
+ error = ReadAdbMessage(*m_conn, in_buffer);
llvm::StringRef response(&in_buffer[0], in_buffer.size());
llvm::SmallVector<llvm::StringRef, 4> devices;
@@ -156,7 +101,8 @@ Status AdbClient::GetDevices(DeviceIDList &device_list) {
// Force disconnect since ADB closes connection after host:devices response
// is sent.
- m_conn.reset();
+ m_conn = std::make_unique<ConnectionFileDescriptor>();
+ error = Connect();
return error;
}
@@ -170,7 +116,7 @@ Status AdbClient::SetPortForwarding(const uint16_t local_port,
if (error.Fail())
return error;
- return ReadResponseStatus();
+ return ReadResponseStatus(*m_conn);
}
Status
@@ -189,7 +135,7 @@ AdbClient::SetPortForwarding(const uint16_t local_port,
if (error.Fail())
return error;
- return ReadResponseStatus();
+ return ReadResponseStatus(*m_conn);
}
Status AdbClient::DeletePortForwarding(const uint16_t local_port) {
@@ -200,60 +146,13 @@ Status AdbClient::DeletePortForwarding(const uint16_t local_port) {
if (error.Fail())
return error;
- return ReadResponseStatus();
-}
-
-Status AdbClient::SendMessage(const std::string &packet, const bool reconnect) {
- Status error;
- if (!m_conn || reconnect) {
- error = Connect();
- if (error.Fail())
- return error;
- }
-
- char length_buffer[5];
- snprintf(length_buffer, sizeof(length_buffer), "%04x",
- static_cast<int>(packet.size()));
-
- ConnectionStatus status;
-
- m_conn->Write(length_buffer, 4, status, &error);
- if (error.Fail())
- return error;
-
- m_conn->Write(packet.c_str(), packet.size(), status, &error);
- return error;
+ return ReadResponseStatus(*m_conn);
}
Status AdbClient::SendDeviceMessage(const std::string &packet) {
std::ostringstream msg;
msg << "host-serial:" << m_device_id << ":" << packet;
- return SendMessage(msg.str());
-}
-
-Status AdbClient::ReadMessage(std::vector<char> &message) {
- message.clear();
-
- if (!m_conn) {
- return Status::FromErrorString("No connection available");
- }
-
- char buffer[5];
- buffer[4] = 0;
-
- auto error = ReadAllBytes(buffer, 4);
- if (error.Fail())
- return error;
-
- unsigned int packet_len = 0;
- sscanf(buffer, "%x", &packet_len);
-
- message.resize(packet_len, 0);
- error = ReadAllBytes(&message[0], packet_len);
- if (error.Fail())
- message.clear();
-
- return error;
+ return SendAdbMessage(*m_conn, msg.str(), true);
}
Status AdbClient::ReadMessageStream(std::vector<char> &message,
@@ -261,9 +160,8 @@ Status AdbClient::ReadMessageStream(std::vector<char> &message,
auto start = steady_clock::now();
message.clear();
- if (!m_conn) {
+ if (!m_conn)
return Status::FromErrorString("No connection available");
- }
Status error;
lldb::ConnectionStatus status = lldb::eConnectionStatusSuccess;
@@ -283,77 +181,22 @@ Status AdbClient::ReadMessageStream(std::vector<char> &message,
return error;
}
-Status AdbClient::ReadResponseStatus() {
- char response_id[5];
-
- static const size_t packet_len = 4;
- response_id[packet_len] = 0;
-
- auto error = ReadAllBytes(response_id, packet_len);
- if (error.Fail())
- return error;
-
- if (strncmp(response_id, kOKAY, packet_len) != 0)
- return GetResponseError(response_id);
-
- return error;
-}
-
-Status AdbClient::GetResponseError(const char *response_id) {
- if (strcmp(response_id, kFAIL) != 0)
- return Status::FromErrorStringWithFormat(
- "Got unexpected response id from adb: \"%s\"", response_id);
-
- std::vector<char> error_message;
- auto error = ReadMessage(error_message);
- if (!error.Success())
- return error;
- return Status(std::string(&error_message[0], error_message.size()));
-}
-
-Status AdbClient::SelectTargetDevice() {
- std::ostringstream msg;
- msg << "host:transport:" << m_device_id;
-
- auto error = SendMessage(msg.str());
- if (error.Fail())
- return error;
-
- return ReadResponseStatus();
-}
-
-
-Status AdbClient::EnterSyncMode() {
- auto error = SendMessage("sync:", false);
- if (error.Fail())
- return error;
-
- return ReadResponseStatus();
-}
-
-Status AdbClient::ReadAllBytes(void *buffer, size_t size) {
- if (!m_conn) {
- return Status::FromErrorString("No connection available");
- }
- return ::ReadAllBytes(*m_conn, buffer, size);
-}
-
Status AdbClient::internalShell(const char *command, milliseconds timeout,
std::vector<char> &output_buf) {
output_buf.clear();
- auto error = SelectTargetDevice();
+ auto error = SelectTargetDevice(*m_conn, m_device_id);
if (error.Fail())
return Status::FromErrorStringWithFormat(
"Failed to select target device: %s", error.AsCString());
StreamString adb_command;
adb_command.Printf("shell:%s", command);
- error = SendMessage(std::string(adb_command.GetString()), false);
+ error = SendAdbMessage(*m_conn, std::string(adb_command.GetString()), false);
if (error.Fail())
return error;
- error = ReadResponseStatus();
+ error = ReadResponseStatus(*m_conn);
if (error.Fail())
return error;
@@ -407,288 +250,3 @@ Status AdbClient::ShellToFile(const char *command, milliseconds timeout,
output_filename.c_str());
return Status();
}
-
-
-Status AdbClient::SyncService::internalPullFile(const FileSpec &remote_file,
- const FileSpec &local_file) {
- const auto local_file_path = local_file.GetPath();
- llvm::FileRemover local_file_remover(local_file_path);
-
- std::error_code EC;
- llvm::raw_fd_ostream dst(local_file_path, EC, llvm::sys::fs::OF_None);
- if (EC)
- return Status::FromErrorStringWithFormat("Unable to open local file %s",
- local_file_path.c_str());
-
- const auto remote_file_path = remote_file.GetPath(false);
- auto error = SendSyncRequest(kRECV, remote_file_path.length(),
- remote_file_path.c_str());
- if (error.Fail())
- return error;
-
- std::vector<char> chunk;
- bool eof = false;
- while (!eof) {
- error = PullFileChunk(chunk, eof);
- if (error.Fail())
- return error;
- if (!eof)
- dst.write(&chunk[0], chunk.size());
- }
- dst.close();
- if (dst.has_error())
- return Status::FromErrorStringWithFormat("Failed to write file %s",
- local_file_path.c_str());
-
- local_file_remover.releaseFile();
- return error;
-}
-
-Status AdbClient::SyncService::internalPushFile(const FileSpec &local_file,
- const FileSpec &remote_file) {
- const auto local_file_path(local_file.GetPath());
- std::ifstream src(local_file_path.c_str(), std::ios::in | std::ios::binary);
- if (!src.is_open())
- return Status::FromErrorStringWithFormat("Unable to open local file %s",
- local_file_path.c_str());
-
- std::stringstream file_description;
- file_description << remote_file.GetPath(false).c_str() << "," << kDefaultMode;
- std::string file_description_str = file_description.str();
- auto error = SendSyncRequest(kSEND, file_description_str.length(),
- file_description_str.c_str());
- if (error.Fail())
- return error;
-
- char chunk[kMaxPushData];
- while (!src.eof() && !src.read(chunk, kMaxPushData).bad()) {
- size_t chunk_size = src.gcount();
- error = SendSyncRequest(kDATA, chunk_size, chunk);
- if (error.Fail())
- return Status::FromErrorStringWithFormat("Failed to send file chunk: %s",
- error.AsCString());
- }
- error = SendSyncRequest(
- kDONE,
- llvm::sys::toTimeT(
- FileSystem::Instance().GetModificationTime(local_file)),
- nullptr);
- if (error.Fail())
- return error;
-
- std::string response_id;
- uint32_t data_len;
- error = ReadSyncHeader(response_id, data_len);
- if (error.Fail())
- return Status::FromErrorStringWithFormat("Failed to read DONE response: %s",
- error.AsCString());
- if (response_id == kFAIL) {
- std::string error_message(data_len, 0);
- error = ReadAllBytes(&error_message[0], data_len);
- if (error.Fail())
- return Status::FromErrorStringWithFormat(
- "Failed to read DONE error message: %s", error.AsCString());
- return Status::FromErrorStringWithFormat("Failed to push file: %s",
- error_message.c_str());
- } else if (response_id != kOKAY)
- return Status::FromErrorStringWithFormat("Got unexpected DONE response: %s",
- response_id.c_str());
-
- // If there was an error reading the source file, finish the adb file
- // transfer first so that adb isn't expecting any more data.
- if (src.bad())
- return Status::FromErrorStringWithFormat("Failed read on %s",
- local_file_path.c_str());
- return error;
-}
-
-Status AdbClient::SyncService::internalStat(const FileSpec &remote_file,
- uint32_t &mode, uint32_t &size,
- uint32_t &mtime) {
- const std::string remote_file_path(remote_file.GetPath(false));
- auto error = SendSyncRequest(kSTAT, remote_file_path.length(),
- remote_file_path.c_str());
- if (error.Fail())
- return Status::FromErrorStringWithFormat("Failed to send request: %s",
- error.AsCString());
-
- static const size_t stat_len = strlen(kSTAT);
- static const size_t response_len = stat_len + (sizeof(uint32_t) * 3);
-
- std::vector<char> buffer(response_len);
- error = ReadAllBytes(&buffer[0], buffer.size());
- if (error.Fail())
- return Status::FromErrorStringWithFormat("Failed to read response: %s",
- error.AsCString());
-
- DataExtractor extractor(&buffer[0], buffer.size(), eByteOrderLittle,
- sizeof(void *));
- offset_t offset = 0;
-
- const void *command = extractor.GetData(&offset, stat_len);
- if (!command)
- return Status::FromErrorStringWithFormat("Failed to get response command");
- const char *command_str = static_cast<const char *>(command);
- if (strncmp(command_str, kSTAT, stat_len))
- return Status::FromErrorStringWithFormat("Got invalid stat command: %s",
- command_str);
-
- mode = extractor.GetU32(&offset);
- size = extractor.GetU32(&offset);
- mtime = extractor.GetU32(&offset);
- return Status();
-}
-
-Status AdbClient::SyncService::PullFile(const FileSpec &remote_file,
- const FileSpec &local_file) {
- return executeCommand([this, &remote_file, &local_file]() {
- return internalPullFile(remote_file, local_file);
- });
-}
-
-Status AdbClient::SyncService::PushFile(const FileSpec &local_file,
- const FileSpec &remote_file) {
- return executeCommand([this, &local_file, &remote_file]() {
- return internalPushFile(local_file, remote_file);
- });
-}
-
-Status AdbClient::SyncService::Stat(const FileSpec &remote_file, uint32_t &mode,
- uint32_t &size, uint32_t &mtime) {
- return executeCommand([this, &remote_file, &mode, &size, &mtime]() {
- return internalStat(remote_file, mode, size, mtime);
- });
-}
-
-bool AdbClient::SyncService::IsConnected() const {
- return m_conn && m_conn->IsConnected();
-}
-
-AdbClient::SyncService::SyncService(std::unique_ptr<Connection> conn, const std::string &device_id)
- : m_conn(std::move(conn)), m_device_id(device_id) {}
-
-Status
-AdbClient::SyncService::executeCommand(const std::function<Status()> &cmd) {
- if (!m_conn || !m_conn->IsConnected()) {
- Status reconnect_error = SetupSyncConnection(m_device_id);
- if (reconnect_error.Fail()) {
- return Status::FromErrorStringWithFormat(
- "SyncService connection failed: %s", reconnect_error.AsCString());
- }
- }
-
- Status error = cmd();
- if (error.Fail()) {
- m_conn.reset();
- }
-
- return error;
-}
-
-AdbClient::SyncService::~SyncService() = default;
-
-Status AdbClient::SyncService::SendSyncRequest(const char *request_id,
- const uint32_t data_len,
- const void *data) {
- DataEncoder encoder(eByteOrderLittle, sizeof(void *));
- encoder.AppendData(llvm::StringRef(request_id));
- encoder.AppendU32(data_len);
- llvm::ArrayRef<uint8_t> bytes = encoder.GetData();
- Status error;
- ConnectionStatus status;
- m_conn->Write(bytes.data(), kSyncPacketLen, status, &error);
- if (error.Fail())
- return error;
-
- if (data)
- m_conn->Write(data, data_len, status, &error);
- return error;
-}
-
-Status AdbClient::SyncService::ReadSyncHeader(std::string &response_id,
- uint32_t &data_len) {
- char buffer[kSyncPacketLen];
-
- auto error = ReadAllBytes(buffer, kSyncPacketLen);
- if (error.Success()) {
- response_id.assign(&buffer[0], 4);
- DataExtractor extractor(&buffer[4], 4, eByteOrderLittle, sizeof(void *));
- offset_t offset = 0;
- data_len = extractor.GetU32(&offset);
- }
-
- return error;
-}
-
-Status AdbClient::SyncService::PullFileChunk(std::vector<char> &buffer,
- bool &eof) {
- buffer.clear();
-
- std::string response_id;
- uint32_t data_len;
- auto error = ReadSyncHeader(response_id, data_len);
- if (error.Fail())
- return error;
-
- if (response_id == kDATA) {
- buffer.resize(data_len, 0);
- error = ReadAllBytes(&buffer[0], data_len);
- if (error.Fail())
- buffer.clear();
- } else if (response_id == kDONE) {
- eof = true;
- } else if (response_id == kFAIL) {
- std::string error_message(data_len, 0);
- error = ReadAllBytes(&error_message[0], data_len);
- if (error.Fail())
- return Status::FromErrorStringWithFormat(
- "Failed to read pull error message: %s", error.AsCString());
- return Status::FromErrorStringWithFormat("Failed to pull file: %s",
- error_message.c_str());
- } else
- return Status::FromErrorStringWithFormat(
- "Pull failed with unknown response: %s", response_id.c_str());
-
- return Status();
-}
-
-Status AdbClient::SyncService::ReadAllBytes(void *buffer, size_t size) {
- return ::ReadAllBytes(*m_conn, buffer, size);
-}
-
-Status AdbClient::SyncService::SetupSyncConnection(const std::string &device_id) {
- if (!m_conn) {
- m_conn = std::make_unique<ConnectionFileDescriptor>();
- }
-
- AdbClient temp_client(device_id);
- Status error = temp_client.ConnectToAdb(*m_conn);
- if (error.Fail())
- return error;
-
- temp_client.m_conn = std::move(m_conn);
- error = temp_client.SelectTargetDevice();
- if (error.Fail()) {
- m_conn = std::move(temp_client.m_conn);
- return error;
- }
-
- error = temp_client.EnterSyncMode();
- if (error.Fail()) {
- return error;
- }
- m_conn = std::move(temp_client.m_conn);
- return error;
-}
-
-Status AdbClient::ConnectToAdb(Connection &conn) {
- std::string port = "5037";
- if (const char *env_port = std::getenv("ANDROID_ADB_SERVER_PORT")) {
- port = env_port;
- }
- std::string uri = "connect://127.0.0.1:" + port;
-
- Status error;
- conn.Connect(uri.c_str(), &error);
- return error;
-}
diff --git a/lldb/source/Plugins/Platform/Android/AdbClient.h b/lldb/source/Plugins/Platform/Android/AdbClient.h
index ae1be37186900..e86641a6f38d9 100644
--- a/lldb/source/Plugins/Platform/Android/AdbClient.h
+++ b/lldb/source/Plugins/Platform/Android/AdbClient.h
@@ -10,6 +10,8 @@
#define LLDB_SOURCE_PLUGINS_PLATFORM_ANDROID_ADBCLIENT_H
#include "lldb/Utility/Status.h"
+#include "AdbClientUtils.h"
+#include "AdbSyncService.h"
#include <chrono>
#include <functional>
#include <list>
@@ -36,7 +38,7 @@ class AdbClient {
friend class AdbClient;
public:
- explicit SyncService(std::unique_ptr<Connection> conn, const std::string &device_id);
+ explicit SyncService(std::unique_ptr<Connection> conn, std::string device_id);
virtual ~SyncService();
@@ -56,7 +58,6 @@ class AdbClient {
virtual Status SendSyncRequest(const char *request_id, const uint32_t data_len,
const void *data);
virtual Status ReadSyncHeader(std::string &response_id, uint32_t &data_len);
- virtual Status ReadAllBytes(void *buffer, size_t size);
private:
@@ -107,35 +108,22 @@ class AdbClient {
std::chrono::milliseconds timeout,
const FileSpec &output_file_spec);
- Status SelectTargetDevice();
-
Status EnterSyncMode();
private:
void SetDeviceID(const std::string &device_id);
Status Connect();
-
- Status SendMessage(const std::string &packet, const bool reconnect = true);
+ Status EnsureConnection();
Status SendDeviceMessage(const std::string &packet);
- Status ReadMessage(std::vector<char> &message);
-
Status ReadMessageStream(std::vector<char> &message,
std::chrono::milliseconds timeout);
- Status GetResponseError(const char *response_id);
-
- Status ReadResponseStatus();
-
Status internalShell(const char *command, std::chrono::milliseconds timeout,
std::vector<char> &output_buf);
- Status ReadAllBytes(void *buffer, size_t size);
-
- Status ConnectToAdb(Connection &conn);
-
std::string m_device_id;
std::unique_ptr<Connection> m_conn;
};
diff --git a/lldb/source/Plugins/Platform/Android/AdbClientUtils.cpp b/lldb/source/Plugins/Platform/Android/AdbClientUtils.cpp
new file mode 100644
index 0000000000000..42cf494836578
--- /dev/null
+++ b/lldb/source/Plugins/Platform/Android/AdbClientUtils.cpp
@@ -0,0 +1,142 @@
+#include "lldb/Utility/Connection.h"
+#include "AdbClientUtils.h"
+#include "lldb/Utility/Status.h"
+#include "lldb/Utility/Timeout.h"
+#include <chrono>
+#include <cstdlib>
+#include <sstream>
+
+using namespace lldb;
+using namespace lldb_private;
+using namespace lldb_private::platform_android;
+using namespace std::chrono;
+
+namespace lldb_private {
+namespace platform_android {
+namespace adb_client_utils {
+
+Status ReadAllBytes(Connection &conn, void *buffer, size_t size) {
+ Status error;
+ ConnectionStatus status;
+ char *read_buffer = static_cast<char *>(buffer);
+
+ auto now = steady_clock::now();
+ const auto deadline = now + kReadTimeout;
+ size_t total_read_bytes = 0;
+ while (total_read_bytes < size && now < deadline) {
+ auto read_bytes =
+ conn.Read(read_buffer + total_read_bytes, size - total_read_bytes,
+ duration_cast<microseconds>(deadline - now), status, &error);
+ if (error.Fail())
+ return error;
+ total_read_bytes += read_bytes;
+ if (status != eConnectionStatusSuccess)
+ break;
+ now = steady_clock::now();
+ }
+ if (total_read_bytes < size)
+ error = Status::FromErrorStringWithFormat(
+ "Unable to read requested number of bytes. Connection status: %d.",
+ status);
+ return error;
+}
+
+Status SendAdbMessage(Connection &conn, const std::string &packet, const bool reconnect) {
+ Status error;
+
+ char length_buffer[5];
+ snprintf(length_buffer, sizeof(length_buffer), "%04x",
+ static_cast<int>(packet.size()));
+
+ ConnectionStatus status;
+
+ conn.Write(length_buffer, 4, status, &error);
+ if (error.Fail())
+ return error;
+
+ conn.Write(packet.c_str(), packet.size(), status, &error);
+ return error;
+}
+
+Status GetResponseError(Connection &conn, const char *response_id) {
+ if (strcmp(response_id, kFAIL) != 0)
+ return Status::FromErrorStringWithFormat(
+ "Got unexpected response id from adb: \"%s\"", response_id);
+
+ std::vector<char> error_message;
+ auto error = ReadAdbMessage(conn, error_message);
+ if (!error.Success())
+ return error;
+ return Status(std::string(&error_message[0], error_message.size()));
+}
+
+Status ConnectToAdb(Connection &conn) {
+ std::string port = "5037";
+ if (const char *env_port = std::getenv("ANDROID_ADB_SERVER_PORT"))
+ port = env_port;
+ std::string uri = "connect://127.0.0.1:" + port;
+
+ Status error;
+ conn.Connect(uri.c_str(), &error);
+ return error;
+}
+
+Status EnterSyncMode(Connection &conn) {
+ auto error = SendAdbMessage(conn, "sync:", false);
+ if (error.Fail())
+ return error;
+
+ return ReadResponseStatus(conn);
+}
+
+Status SelectTargetDevice(Connection &conn, const std::string &device_id) {
+ std::ostringstream msg;
+ msg << "host:transport:" << device_id;
+
+ auto error = SendAdbMessage(conn, msg.str(), true);
+ if (error.Fail())
+ return error;
+
+ return ReadResponseStatus(conn);
+}
+
+Status ReadAdbMessage(Connection &conn, std::vector<char> &message) {
+ message.clear();
+
+ char buffer[5];
+ buffer[4] = 0;
+
+ auto error = ReadAllBytes(conn, buffer, 4);
+ if (error.Fail())
+ return error;
+
+ unsigned int packet_len = 0;
+ sscanf(buffer, "%x", &packet_len);
+
+ message.resize(packet_len, 0);
+ error = ReadAllBytes(conn, &message[0], packet_len);
+ if (error.Fail())
+ message.clear();
+
+ return error;
+}
+
+Status ReadResponseStatus(Connection &conn) {
+ char response_id[5];
+
+ const size_t packet_len = 4;
+ response_id[packet_len] = 0;
+
+ auto error = ReadAllBytes(conn, response_id, packet_len);
+ if (error.Fail())
+ return error;
+
+ if (strncmp(response_id, kOKAY, packet_len) != 0)
+ return GetResponseError(conn, response_id);
+
+ return error;
+}
+
+} // namespace adb_client_utils
+} // namespace platform_android
+} // namespace lldb_private
diff --git a/lldb/source/Plugins/Platform/Android/AdbClientUtils.h b/lldb/source/Plugins/Platform/Android/AdbClientUtils.h
new file mode 100644
index 0000000000000..a5d3cbc2dde1b
--- /dev/null
+++ b/lldb/source/Plugins/Platform/Android/AdbClientUtils.h
@@ -0,0 +1,46 @@
+//===-- AdbClientUtils.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_SOURCE_PLUGINS_PLATFORM_ANDROID_ADBCLIENTUTILS_H
+#define LLDB_SOURCE_PLUGINS_PLATFORM_ANDROID_ADBCLIENTUTILS_H
+
+#include "lldb/Utility/Status.h"
+#include <string>
+
+namespace lldb_private {
+class Connection;
+
+namespace platform_android {
+
+const std::chrono::seconds kReadTimeout(20);
+const static char *kOKAY = "OKAY";
+const static char *kFAIL = "FAIL";
+const static char *kDATA = "DATA";
+const static char *kDONE = "DONE";
+const static char *kSEND = "SEND";
+const static char *kRECV = "RECV";
+const static char *kSTAT = "STAT";
+const static size_t kSyncPacketLen = 8;
+const static size_t kMaxPushData = 2 * 1024;
+const static uint32_t kDefaultMode = 0100770;
+
+namespace adb_client_utils {
+
+Status ReadAllBytes(Connection &conn, void *buffer, size_t size);
+Status SendAdbMessage(Connection &conn, const std::string &packet, const bool reconnect);
+Status GetResponseError(Connection &conn, const char *response_id);
+Status ConnectToAdb(Connection &conn);
+Status EnterSyncMode(Connection &conn);
+Status SelectTargetDevice(Connection &conn, const std::string &device_id);
+Status ReadAdbMessage(Connection &conn, std::vector<char> &message);
+Status ReadResponseStatus(Connection &conn);
+
+} // namespace adb_client_utils
+} // namespace platform_android
+} // namespace lldb_private
+
+#endif // LLDB_SOURCE_PLUGINS_PLATFORM_ANDROID_ADBCLIENTUTILS_H
diff --git a/lldb/source/Plugins/Platform/Android/AdbSyncService.cpp b/lldb/source/Plugins/Platform/Android/AdbSyncService.cpp
new file mode 100644
index 0000000000000..2c5a3b5436f14
--- /dev/null
+++ b/lldb/source/Plugins/Platform/Android/AdbSyncService.cpp
@@ -0,0 +1,285 @@
+//===-- AdbClient.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 "AdbClientUtils.h"
+#include "AdbSyncService.h"
+
+#include "lldb/Host/posix/ConnectionFileDescriptorPosix.h"
+#include "lldb/Utility/Connection.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/FileUtilities.h"
+
+#include "lldb/Host/FileSystem.h"
+#include "lldb/Utility/DataEncoder.h"
+#include "lldb/Utility/DataExtractor.h"
+#include "lldb/Utility/FileSpec.h"
+
+#include <climits>
+
+#include <cstdlib>
+#include <fstream>
+#include <sstream>
+
+using namespace lldb;
+using namespace lldb_private;
+using namespace lldb_private::platform_android;
+using namespace std::chrono;
+using namespace adb_client_utils;
+
+
+Status AdbSyncService::internalPullFile(const FileSpec &remote_file,
+ const FileSpec &local_file) {
+ const auto local_file_path = local_file.GetPath();
+ llvm::FileRemover local_file_remover(local_file_path);
+
+ std::error_code EC;
+ llvm::raw_fd_ostream dst(local_file_path, EC, llvm::sys::fs::OF_None);
+ if (EC)
+ return Status::FromErrorStringWithFormat("Unable to open local file %s",
+ local_file_path.c_str());
+
+ const auto remote_file_path = remote_file.GetPath(false);
+ auto error = SendSyncRequest(kRECV, remote_file_path.length(),
+ remote_file_path.c_str());
+ if (error.Fail())
+ return error;
+
+ std::vector<char> chunk;
+ bool eof = false;
+ while (!eof) {
+ error = PullFileChunk(chunk, eof);
+ if (error.Fail())
+ return error;
+ if (!eof)
+ dst.write(&chunk[0], chunk.size());
+ }
+ dst.close();
+ if (dst.has_error())
+ return Status::FromErrorStringWithFormat("Failed to write file %s",
+ local_file_path.c_str());
+
+ local_file_remover.releaseFile();
+ return error;
+}
+
+Status AdbSyncService::internalPushFile(const FileSpec &local_file,
+ const FileSpec &remote_file) {
+ const auto local_file_path(local_file.GetPath());
+ std::ifstream src(local_file_path.c_str(), std::ios::in | std::ios::binary);
+ if (!src.is_open())
+ return Status::FromErrorStringWithFormat("Unable to open local file %s",
+ local_file_path.c_str());
+
+ std::stringstream file_description;
+ file_description << remote_file.GetPath(false).c_str() << "," << kDefaultMode;
+ std::string file_description_str = file_description.str();
+ auto error = SendSyncRequest(kSEND, file_description_str.length(),
+ file_description_str.c_str());
+ if (error.Fail())
+ return error;
+
+ char chunk[kMaxPushData];
+ while (!src.eof() && !src.read(chunk, kMaxPushData).bad()) {
+ size_t chunk_size = src.gcount();
+ error = SendSyncRequest(kDATA, chunk_size, chunk);
+ if (error.Fail())
+ return Status::FromErrorStringWithFormat("Failed to send file chunk: %s",
+ error.AsCString());
+ }
+ error = SendSyncRequest(
+ kDONE,
+ llvm::sys::toTimeT(
+ FileSystem::Instance().GetModificationTime(local_file)),
+ nullptr);
+ if (error.Fail())
+ return error;
+
+ std::string response_id;
+ uint32_t data_len;
+ error = ReadSyncHeader(response_id, data_len);
+ if (error.Fail())
+ return Status::FromErrorStringWithFormat("Failed to read DONE response: %s",
+ error.AsCString());
+ if (response_id == kFAIL) {
+ std::string error_message(data_len, 0);
+ error = ReadAllBytes(*m_conn, &error_message[0], data_len);
+ if (error.Fail())
+ return Status::FromErrorStringWithFormat(
+ "Failed to read DONE error message: %s", error.AsCString());
+ return Status::FromErrorStringWithFormat("Failed to push file: %s",
+ error_message.c_str());
+ } else if (response_id != kOKAY)
+ return Status::FromErrorStringWithFormat("Got unexpected DONE response: %s",
+ response_id.c_str());
+
+ // If there was an error reading the source file, finish the adb file
+ // transfer first so that adb isn't expecting any more data.
+ if (src.bad())
+ return Status::FromErrorStringWithFormat("Failed read on %s",
+ local_file_path.c_str());
+ return error;
+}
+
+Status AdbSyncService::internalStat(const FileSpec &remote_file,
+ uint32_t &mode, uint32_t &size,
+ uint32_t &mtime) {
+ const std::string remote_file_path(remote_file.GetPath(false));
+ auto error = SendSyncRequest(kSTAT, remote_file_path.length(),
+ remote_file_path.c_str());
+ if (error.Fail())
+ return Status::FromErrorStringWithFormat("Failed to send request: %s",
+ error.AsCString());
+
+ static const size_t stat_len = strlen(kSTAT);
+ static const size_t response_len = stat_len + (sizeof(uint32_t) * 3);
+
+ std::vector<char> buffer(response_len);
+ error = ReadAllBytes(*m_conn, &buffer[0], buffer.size());
+ if (error.Fail())
+ return Status::FromErrorStringWithFormat("Failed to read response: %s",
+ error.AsCString());
+
+ DataExtractor extractor(&buffer[0], buffer.size(), eByteOrderLittle,
+ sizeof(void *));
+ offset_t offset = 0;
+
+ const void *command = extractor.GetData(&offset, stat_len);
+ if (!command)
+ return Status::FromErrorStringWithFormat("Failed to get response command");
+ const char *command_str = static_cast<const char *>(command);
+ if (strncmp(command_str, kSTAT, stat_len))
+ return Status::FromErrorStringWithFormat("Got invalid stat command: %s",
+ command_str);
+
+ mode = extractor.GetU32(&offset);
+ size = extractor.GetU32(&offset);
+ mtime = extractor.GetU32(&offset);
+ return Status();
+}
+
+Status AdbSyncService::PullFile(const FileSpec &remote_file,
+ const FileSpec &local_file) {
+ return executeCommand([this, &remote_file, &local_file]() {
+ return internalPullFile(remote_file, local_file);
+ });
+}
+
+Status AdbSyncService::PushFile(const FileSpec &local_file,
+ const FileSpec &remote_file) {
+ return executeCommand([this, &local_file, &remote_file]() {
+ return internalPushFile(local_file, remote_file);
+ });
+}
+
+Status AdbSyncService::Stat(const FileSpec &remote_file, uint32_t &mode,
+ uint32_t &size, uint32_t &mtime) {
+ return executeCommand([this, &remote_file, &mode, &size, &mtime]() {
+ return internalStat(remote_file, mode, size, mtime);
+ });
+}
+
+bool AdbSyncService::IsConnected() const {
+ return m_conn && m_conn->IsConnected();
+}
+
+AdbSyncService::AdbSyncService(std::string device_id)
+ : m_device_id(std::move(device_id)) {
+ m_conn = std::make_unique<ConnectionFileDescriptor>();
+}
+
+Status
+AdbSyncService::executeCommand(const std::function<Status()> &cmd) {
+ if (!m_conn)
+ return Status::FromErrorString("AdbSyncService is disconnected");
+
+ Status error = cmd();
+
+ return error;
+}
+
+AdbSyncService::~AdbSyncService() = default;
+
+Status AdbSyncService::SendSyncRequest(const char *request_id,
+ const uint32_t data_len,
+ const void *data) {
+ DataEncoder encoder(eByteOrderLittle, sizeof(void *));
+ encoder.AppendData(llvm::StringRef(request_id));
+ encoder.AppendU32(data_len);
+ llvm::ArrayRef<uint8_t> bytes = encoder.GetData();
+ Status error;
+ ConnectionStatus status;
+ m_conn->Write(bytes.data(), kSyncPacketLen, status, &error);
+ if (error.Fail())
+ return error;
+
+ if (data)
+ m_conn->Write(data, data_len, status, &error);
+ return error;
+}
+
+Status AdbSyncService::ReadSyncHeader(std::string &response_id,
+ uint32_t &data_len) {
+ char buffer[kSyncPacketLen];
+
+ auto error = ReadAllBytes(*m_conn, buffer, kSyncPacketLen);
+ if (error.Success()) {
+ response_id.assign(&buffer[0], 4);
+ DataExtractor extractor(&buffer[4], 4, eByteOrderLittle, sizeof(void *));
+ offset_t offset = 0;
+ data_len = extractor.GetU32(&offset);
+ }
+
+ return error;
+}
+
+Status AdbSyncService::PullFileChunk(std::vector<char> &buffer,
+ bool &eof) {
+ buffer.clear();
+
+ std::string response_id;
+ uint32_t data_len;
+ auto error = ReadSyncHeader(response_id, data_len);
+ if (error.Fail())
+ return error;
+
+ if (response_id == kDATA) {
+ buffer.resize(data_len, 0);
+ error = ReadAllBytes(*m_conn, &buffer[0], data_len);
+ if (error.Fail())
+ buffer.clear();
+ } else if (response_id == kDONE) {
+ eof = true;
+ } else if (response_id == kFAIL) {
+ std::string error_message(data_len, 0);
+ error = ReadAllBytes(*m_conn, &error_message[0], data_len);
+ if (error.Fail())
+ return Status::FromErrorStringWithFormat(
+ "Failed to read pull error message: %s", error.AsCString());
+ return Status::FromErrorStringWithFormat("Failed to pull file: %s",
+ error_message.c_str());
+ } else
+ return Status::FromErrorStringWithFormat(
+ "Pull failed with unknown response: %s", response_id.c_str());
+
+ return Status();
+}
+
+Status AdbSyncService::SetupSyncConnection() {
+ Status error = ConnectToAdb(*m_conn);
+ if (error.Fail())
+ return error;
+
+ error = SelectTargetDevice(*m_conn, m_device_id);
+ if (error.Fail()) {
+ return error;
+ }
+
+ error = EnterSyncMode(*m_conn);
+ return error;
+}
diff --git a/lldb/source/Plugins/Platform/Android/AdbSyncService.h b/lldb/source/Plugins/Platform/Android/AdbSyncService.h
new file mode 100644
index 0000000000000..25c0ae14a5b90
--- /dev/null
+++ b/lldb/source/Plugins/Platform/Android/AdbSyncService.h
@@ -0,0 +1,50 @@
+//===-- AdbClient.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_SOURCE_PLUGINS_PLATFORM_ANDROID_ADBSYNCSERVICE_H
+#define LLDB_SOURCE_PLUGINS_PLATFORM_ANDROID_ADBSYNCSERVICE_H
+
+#include "lldb/Utility/Status.h"
+#include <memory>
+#include <string>
+
+namespace lldb_private {
+class FileSpec;
+class Connection;
+
+namespace platform_android {
+
+class AdbSyncService {
+public:
+ explicit AdbSyncService(const std::string device_id);
+ virtual ~AdbSyncService();
+ Status SetupSyncConnection();
+
+ virtual Status PullFile(const FileSpec &remote_file, const FileSpec &local_file);
+ virtual Status PushFile(const FileSpec &local_file, const FileSpec &remote_file);
+ virtual Status Stat(const FileSpec &remote_file, uint32_t &mode, uint32_t &size, uint32_t &mtime);
+ virtual bool IsConnected() const;
+
+ const std::string &GetDeviceId() const { return m_device_id; }
+ Connection &GetConnection() const { return *m_conn; }
+private:
+ Status SendSyncRequest(const char *request_id, const uint32_t data_len, const void *data);
+ Status ReadSyncHeader(std::string &response_id, uint32_t &data_len);
+ Status PullFileChunk(std::vector<char> &buffer, bool &eof);
+ Status internalPullFile(const FileSpec &remote_file, const FileSpec &local_file);
+ Status internalPushFile(const FileSpec &local_file, const FileSpec &remote_file);
+ Status internalStat(const FileSpec &remote_file, uint32_t &mode, uint32_t &size, uint32_t &mtime);
+ Status executeCommand(const std::function<Status()> &cmd);
+
+ std::unique_ptr<Connection> m_conn;
+ std::string m_device_id;
+};
+
+} // namespace platform_android
+} // namespace lldb_private
+
+#endif
diff --git a/lldb/source/Plugins/Platform/Android/CMakeLists.txt b/lldb/source/Plugins/Platform/Android/CMakeLists.txt
index 71108371daf09..f08c5a88cfeb7 100644
--- a/lldb/source/Plugins/Platform/Android/CMakeLists.txt
+++ b/lldb/source/Plugins/Platform/Android/CMakeLists.txt
@@ -8,17 +8,19 @@ lldb_tablegen(PlatformAndroidPropertiesEnum.inc -gen-lldb-property-enum-defs
add_lldb_library(lldbPluginPlatformAndroid PLUGIN
AdbClient.cpp
+ AdbSyncService.cpp
+ AdbClientUtils.cpp
PlatformAndroid.cpp
PlatformAndroidRemoteGDBServer.cpp
- LINK_COMPONENTS
- Support
LINK_LIBS
lldbCore
lldbHost
lldbValueObject
lldbPluginPlatformLinux
lldbPluginPlatformGDB
+ LINK_COMPONENTS
+ Support
)
add_dependencies(lldbPluginPlatformAndroid
diff --git a/lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp b/lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp
index c471ad742f0dd..12999d5cf9422 100644
--- a/lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp
+++ b/lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp
@@ -18,6 +18,7 @@
#include "lldb/ValueObject/ValueObject.h"
#include "AdbClient.h"
+#include "AdbSyncService.h"
#include "PlatformAndroid.h"
#include "PlatformAndroidRemoteGDBServer.h"
#include "lldb/Target/Target.h"
@@ -465,7 +466,7 @@ PlatformAndroid::GetLibdlFunctionDeclarations(lldb_private::Process *process) {
}
PlatformAndroid::AdbClientUP PlatformAndroid::GetAdbClient(Status &error) {
- AdbClientUP adb(std::make_unique<AdbClient>(m_device_id));
+ AdbClientUP adb = std::make_unique<AdbClient>();
if (adb)
error.Clear();
else
@@ -493,13 +494,11 @@ std::string PlatformAndroid::GetRunAs() {
}
return run_as.str();
}
-
-std::unique_ptr<AdbClient::SyncService>
+std::unique_ptr<AdbSyncService>
PlatformAndroid::GetSyncService(Status &error) {
- auto conn = std::make_unique<ConnectionFileDescriptor>();
- auto sync_service = std::make_unique<AdbClient::SyncService>(
- std::move(conn), m_device_id);
-
- error.Clear();
+ auto sync_service = std::make_unique<AdbSyncService>(m_device_id);
+ error = sync_service->SetupSyncConnection();
+ if (error.Fail())
+ return nullptr;
return sync_service;
}
diff --git a/lldb/source/Plugins/Platform/Android/PlatformAndroid.h b/lldb/source/Plugins/Platform/Android/PlatformAndroid.h
index 92738f6fa79f1..4905e877eeea0 100644
--- a/lldb/source/Plugins/Platform/Android/PlatformAndroid.h
+++ b/lldb/source/Plugins/Platform/Android/PlatformAndroid.h
@@ -14,6 +14,7 @@
#include "Plugins/Platform/Linux/PlatformLinux.h"
+#include "AdbSyncService.h"
#include "AdbClient.h"
namespace lldb_private {
@@ -82,7 +83,7 @@ class PlatformAndroid : public platform_linux::PlatformLinux {
virtual llvm::StringRef GetPropertyPackageName();
protected:
- virtual std::unique_ptr<AdbClient::SyncService> GetSyncService(Status &error);
+ virtual std::unique_ptr<AdbSyncService> GetSyncService(Status &error);
private:
std::string m_device_id;
>From a05403223cb1f28cb139dccd7481ba8ed61e7181 Mon Sep 17 00:00:00 2001
From: Chad Smith <cs01 at users.noreply.github.com>
Date: Wed, 9 Jul 2025 21:56:32 -0700
Subject: [PATCH 4/8] add logging and PlatformAndroid::FindProcesses
---
.../Plugins/Platform/Android/AdbClient.cpp | 27 ++++++--
.../Plugins/Platform/Android/AdbClient.h | 52 +--------------
.../Platform/Android/AdbClientUtils.cpp | 17 ++++-
.../Platform/Android/AdbSyncService.cpp | 17 +++--
.../Platform/Android/PlatformAndroid.cpp | 65 +++++++++++++++++--
.../Platform/Android/PlatformAndroid.h | 3 +
6 files changed, 109 insertions(+), 72 deletions(-)
diff --git a/lldb/source/Plugins/Platform/Android/AdbClient.cpp b/lldb/source/Plugins/Platform/Android/AdbClient.cpp
index 67b0b570e2cdc..36a72d3bddbfa 100644
--- a/lldb/source/Plugins/Platform/Android/AdbClient.cpp
+++ b/lldb/source/Plugins/Platform/Android/AdbClient.cpp
@@ -10,6 +10,8 @@
#include "lldb/Host/posix/ConnectionFileDescriptorPosix.h"
#include "lldb/Utility/FileSpec.h"
+#include "lldb/Utility/LLDBLog.h"
+#include "lldb/Utility/Log.h"
#include "lldb/Utility/StreamString.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/StringRef.h"
@@ -30,13 +32,13 @@ const static char *kSocketNamespaceFileSystem = "localfilesystem";
Status AdbClient::CreateByDeviceID(const std::string &device_id,
AdbClient &adb) {
Status error;
- std::string android_serial;
+ std::string preferred_android_serial;
if (!device_id.empty())
- android_serial = device_id;
+ preferred_android_serial = device_id;
else if (const char *env_serial = std::getenv("ANDROID_SERIAL"))
- android_serial = env_serial;
+ preferred_android_serial = env_serial;
- if (android_serial.empty()) {
+ if (preferred_android_serial.empty()) {
DeviceIDList connected_devices;
error = adb.GetDevices(connected_devices);
if (error.Fail())
@@ -49,23 +51,32 @@ Status AdbClient::CreateByDeviceID(const std::string &device_id,
connected_devices.size());
adb.SetDeviceID(connected_devices.front());
} else {
- adb.SetDeviceID(android_serial);
+ adb.SetDeviceID(preferred_android_serial);
}
return error;
}
AdbClient::AdbClient(const std::string &device_id) : m_device_id(device_id) {
+ Log *log = GetLog(LLDBLog::Platform);
+ LLDB_LOGF(log, "AdbClient::AdbClient(device_id='%s') - Creating AdbClient with device ID",
+ device_id.c_str());
m_conn = std::make_unique<ConnectionFileDescriptor>();
Connect();
}
AdbClient::AdbClient() {
+ Log *log = GetLog(LLDBLog::Platform);
+ LLDB_LOGF(log, "AdbClient::AdbClient() - Creating AdbClient with default constructor");
m_conn = std::make_unique<ConnectionFileDescriptor>();
Connect();
}
-AdbClient::~AdbClient() = default;
+AdbClient::~AdbClient() {
+ Log *log = GetLog(LLDBLog::Platform);
+ LLDB_LOGF(log, "AdbClient::~AdbClient() - Destroying AdbClient for device: %s",
+ m_device_id.c_str());
+}
void AdbClient::SetDeviceID(const std::string &device_id) {
m_device_id = device_id;
@@ -74,7 +85,9 @@ void AdbClient::SetDeviceID(const std::string &device_id) {
const std::string &AdbClient::GetDeviceID() const { return m_device_id; }
Status AdbClient::Connect() {
- Status error;
+ if (m_conn->IsConnected())
+ return Status();
+
return ConnectToAdb(*m_conn);
}
diff --git a/lldb/source/Plugins/Platform/Android/AdbClient.h b/lldb/source/Plugins/Platform/Android/AdbClient.h
index e86641a6f38d9..d62d3abe9b156 100644
--- a/lldb/source/Plugins/Platform/Android/AdbClient.h
+++ b/lldb/source/Plugins/Platform/Android/AdbClient.h
@@ -34,53 +34,6 @@ class AdbClient {
using DeviceIDList = std::list<std::string>;
- class SyncService {
- friend class AdbClient;
-
- public:
- explicit SyncService(std::unique_ptr<Connection> conn, std::string device_id);
-
- virtual ~SyncService();
-
- virtual Status PullFile(const FileSpec &remote_file,
- const FileSpec &local_file);
-
- virtual Status PushFile(const FileSpec &local_file, const FileSpec &remote_file);
-
- virtual Status Stat(const FileSpec &remote_file, uint32_t &mode,
- uint32_t &size, uint32_t &mtime);
-
- virtual bool IsConnected() const;
-
- const std::string &GetDeviceId() const { return m_device_id; }
-
- protected:
- virtual Status SendSyncRequest(const char *request_id, const uint32_t data_len,
- const void *data);
- virtual Status ReadSyncHeader(std::string &response_id, uint32_t &data_len);
-
- private:
-
- Status PullFileChunk(std::vector<char> &buffer, bool &eof);
-
- Status internalPullFile(const FileSpec &remote_file,
- const FileSpec &local_file);
-
- Status internalPushFile(const FileSpec &local_file,
- const FileSpec &remote_file);
-
- Status internalStat(const FileSpec &remote_file, uint32_t &mode,
- uint32_t &size, uint32_t &mtime);
-
- Status executeCommand(const std::function<Status()> &cmd);
-
- // Internal connection setup methods
- Status SetupSyncConnection(const std::string &device_id);
-
- std::unique_ptr<Connection> m_conn;
- std::string m_device_id;
- };
-
static Status CreateByDeviceID(const std::string &device_id, AdbClient &adb);
AdbClient();
@@ -108,14 +61,11 @@ class AdbClient {
std::chrono::milliseconds timeout,
const FileSpec &output_file_spec);
- Status EnterSyncMode();
+ Status Connect();
private:
void SetDeviceID(const std::string &device_id);
- Status Connect();
- Status EnsureConnection();
-
Status SendDeviceMessage(const std::string &packet);
Status ReadMessageStream(std::vector<char> &message,
diff --git a/lldb/source/Plugins/Platform/Android/AdbClientUtils.cpp b/lldb/source/Plugins/Platform/Android/AdbClientUtils.cpp
index 42cf494836578..90c9aeaf60ba4 100644
--- a/lldb/source/Plugins/Platform/Android/AdbClientUtils.cpp
+++ b/lldb/source/Plugins/Platform/Android/AdbClientUtils.cpp
@@ -1,5 +1,7 @@
#include "lldb/Utility/Connection.h"
#include "AdbClientUtils.h"
+#include "lldb/Utility/LLDBLog.h"
+#include "lldb/Utility/Log.h"
#include "lldb/Utility/Status.h"
#include "lldb/Utility/Timeout.h"
#include <chrono>
@@ -34,10 +36,11 @@ Status ReadAllBytes(Connection &conn, void *buffer, size_t size) {
break;
now = steady_clock::now();
}
- if (total_read_bytes < size)
+ if (total_read_bytes < size) {
error = Status::FromErrorStringWithFormat(
"Unable to read requested number of bytes. Connection status: %d.",
status);
+ }
return error;
}
@@ -67,7 +70,11 @@ Status GetResponseError(Connection &conn, const char *response_id) {
auto error = ReadAdbMessage(conn, error_message);
if (!error.Success())
return error;
- return Status(std::string(&error_message[0], error_message.size()));
+
+ std::string error_str(&error_message[0], error_message.size());
+ Log *log = GetLog(LLDBLog::Platform);
+ LLDB_LOGF(log, "ADB error: %s", error_str.c_str());
+ return Status(error_str);
}
Status ConnectToAdb(Connection &conn) {
@@ -76,6 +83,9 @@ Status ConnectToAdb(Connection &conn) {
port = env_port;
std::string uri = "connect://127.0.0.1:" + port;
+ Log *log = GetLog(LLDBLog::Platform);
+ LLDB_LOGF(log, "Connecting to ADB server at %s", uri.c_str());
+
Status error;
conn.Connect(uri.c_str(), &error);
return error;
@@ -90,6 +100,9 @@ Status EnterSyncMode(Connection &conn) {
}
Status SelectTargetDevice(Connection &conn, const std::string &device_id) {
+ Log *log = GetLog(LLDBLog::Platform);
+ LLDB_LOGF(log, "Selecting device: %s", device_id.c_str());
+
std::ostringstream msg;
msg << "host:transport:" << device_id;
diff --git a/lldb/source/Plugins/Platform/Android/AdbSyncService.cpp b/lldb/source/Plugins/Platform/Android/AdbSyncService.cpp
index 2c5a3b5436f14..65fbde8cbe4ca 100644
--- a/lldb/source/Plugins/Platform/Android/AdbSyncService.cpp
+++ b/lldb/source/Plugins/Platform/Android/AdbSyncService.cpp
@@ -1,4 +1,4 @@
-//===-- AdbClient.cpp -----------------------------------------------------===//
+//===-- AdbSyncService.cpp ------------------------------------------------===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
@@ -19,6 +19,8 @@
#include "lldb/Utility/DataEncoder.h"
#include "lldb/Utility/DataExtractor.h"
#include "lldb/Utility/FileSpec.h"
+#include "lldb/Utility/LLDBLog.h"
+#include "lldb/Utility/Log.h"
#include <climits>
@@ -191,19 +193,22 @@ bool AdbSyncService::IsConnected() const {
AdbSyncService::AdbSyncService(std::string device_id)
: m_device_id(std::move(device_id)) {
m_conn = std::make_unique<ConnectionFileDescriptor>();
+ Log *log = GetLog(LLDBLog::Platform);
+ LLDB_LOGF(log, "AdbSyncService::AdbSyncService() - Creating AdbSyncService for device: %s",
+ m_device_id.c_str());
}
Status
AdbSyncService::executeCommand(const std::function<Status()> &cmd) {
- if (!m_conn)
- return Status::FromErrorString("AdbSyncService is disconnected");
-
Status error = cmd();
-
return error;
}
-AdbSyncService::~AdbSyncService() = default;
+AdbSyncService::~AdbSyncService() {
+ Log *log = GetLog(LLDBLog::Platform);
+ LLDB_LOGF(log, "AdbSyncService::~AdbSyncService() - Destroying AdbSyncService for device: %s",
+ m_device_id.c_str());
+}
Status AdbSyncService::SendSyncRequest(const char *request_id,
const uint32_t data_len,
diff --git a/lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp b/lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp
index 12999d5cf9422..4cd199a7e3d0a 100644
--- a/lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp
+++ b/lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp
@@ -439,13 +439,68 @@ bool PlatformAndroid::GetRemoteOSVersion() {
return !m_os_version.empty();
}
+uint32_t PlatformAndroid::FindProcesses(
+ const ProcessInstanceInfoMatch &match_info,
+ ProcessInstanceInfoList &process_infos) {
+ Status error;
+ AdbClientUP adb(GetAdbClient(error));
+ if (error.Fail()) {
+ return 0;
+ }
+
+ std::string ps_output;
+ error = adb->Shell("ps -A -o PID,NAME", seconds(10), &ps_output);
+ // output looks like
+ // PID NAME
+ // 1 init
+ // 2 kthreadd
+ // 3 com.android.settings
+
+ if (error.Fail())
+ return 0;
+
+ llvm::StringRef ps_lines(ps_output);
+
+ while (!ps_lines.empty()) {
+ llvm::StringRef line;
+ std::tie(line, ps_lines) = ps_lines.split('\n');
+ line = line.trim();
+
+ if (line.empty() || line.starts_with("PID") || line.starts_with("USER"))
+ continue;
+
+ // Parse PID and process name from ps output
+ llvm::StringRef pid_str, name_str;
+ std::tie(pid_str, name_str) = line.split(' ');
+ pid_str = pid_str.trim();
+ name_str = name_str.trim();
+
+ // Skip if we can't parse PID
+ lldb::pid_t pid;
+ if (pid_str.getAsInteger(10, pid))
+ continue;
+
+ // Create ProcessInstanceInfo for this process
+ ProcessInstanceInfo process_info;
+ process_info.SetProcessID(pid);
+ process_info.GetExecutableFile().SetFile(name_str, FileSpec::Style::posix);
+
+ // Check if this process matches our search criteria
+ if (match_info.Matches(process_info)) {
+ process_infos.push_back(process_info);
+ }
+ }
+
+ return process_infos.size();
+}
+
llvm::StringRef
PlatformAndroid::GetLibdlFunctionDeclarations(lldb_private::Process *process) {
SymbolContextList matching_symbols;
std::vector<const char *> dl_open_names = {"__dl_dlopen", "dlopen"};
const char *dl_open_name = nullptr;
Target &target = process->GetTarget();
- for (auto name : dl_open_names) {
+ for (const auto *name : dl_open_names) {
target.GetImages().FindFunctionSymbols(
ConstString(name), eFunctionNameTypeFull, matching_symbols);
if (matching_symbols.GetSize()) {
@@ -466,11 +521,9 @@ PlatformAndroid::GetLibdlFunctionDeclarations(lldb_private::Process *process) {
}
PlatformAndroid::AdbClientUP PlatformAndroid::GetAdbClient(Status &error) {
- AdbClientUP adb = std::make_unique<AdbClient>();
- if (adb)
- error.Clear();
- else
- error = Status::FromErrorString("Failed to create AdbClient");
+ AdbClientUP adb = std::make_unique<AdbClient>(m_device_id);
+ error.Clear();
+ error = adb->Connect();
return adb;
}
diff --git a/lldb/source/Plugins/Platform/Android/PlatformAndroid.h b/lldb/source/Plugins/Platform/Android/PlatformAndroid.h
index 4905e877eeea0..c9a8240d9ebb8 100644
--- a/lldb/source/Plugins/Platform/Android/PlatformAndroid.h
+++ b/lldb/source/Plugins/Platform/Android/PlatformAndroid.h
@@ -60,6 +60,9 @@ class PlatformAndroid : public platform_linux::PlatformLinux {
uint32_t GetDefaultMemoryCacheLineSize() override;
+ uint32_t FindProcesses(const ProcessInstanceInfoMatch &match_info,
+ ProcessInstanceInfoList &process_infos) override;
+
protected:
const char *GetCacheHostname() override;
>From 29451011dbef38dc4ce2089f34003a1cb0530202 Mon Sep 17 00:00:00 2001
From: Chad Smith <cs01 at users.noreply.github.com>
Date: Wed, 9 Jul 2025 22:14:34 -0700
Subject: [PATCH 5/8] remove reconnect argument
---
lldb/source/Plugins/Platform/Android/AdbClient.cpp | 6 +++---
lldb/source/Plugins/Platform/Android/AdbClientUtils.cpp | 6 +++---
lldb/source/Plugins/Platform/Android/AdbClientUtils.h | 2 +-
3 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/lldb/source/Plugins/Platform/Android/AdbClient.cpp b/lldb/source/Plugins/Platform/Android/AdbClient.cpp
index 36a72d3bddbfa..54ab165921965 100644
--- a/lldb/source/Plugins/Platform/Android/AdbClient.cpp
+++ b/lldb/source/Plugins/Platform/Android/AdbClient.cpp
@@ -94,7 +94,7 @@ Status AdbClient::Connect() {
Status AdbClient::GetDevices(DeviceIDList &device_list) {
device_list.clear();
- auto error = SendAdbMessage(*m_conn, "host:devices", true);
+ auto error = SendAdbMessage(*m_conn, "host:devices");
if (error.Fail())
return error;
@@ -165,7 +165,7 @@ Status AdbClient::DeletePortForwarding(const uint16_t local_port) {
Status AdbClient::SendDeviceMessage(const std::string &packet) {
std::ostringstream msg;
msg << "host-serial:" << m_device_id << ":" << packet;
- return SendAdbMessage(*m_conn, msg.str(), true);
+ return SendAdbMessage(*m_conn, msg.str());
}
Status AdbClient::ReadMessageStream(std::vector<char> &message,
@@ -205,7 +205,7 @@ Status AdbClient::internalShell(const char *command, milliseconds timeout,
StreamString adb_command;
adb_command.Printf("shell:%s", command);
- error = SendAdbMessage(*m_conn, std::string(adb_command.GetString()), false);
+ error = SendAdbMessage(*m_conn, std::string(adb_command.GetString()));
if (error.Fail())
return error;
diff --git a/lldb/source/Plugins/Platform/Android/AdbClientUtils.cpp b/lldb/source/Plugins/Platform/Android/AdbClientUtils.cpp
index 90c9aeaf60ba4..88144337b57c4 100644
--- a/lldb/source/Plugins/Platform/Android/AdbClientUtils.cpp
+++ b/lldb/source/Plugins/Platform/Android/AdbClientUtils.cpp
@@ -44,7 +44,7 @@ Status ReadAllBytes(Connection &conn, void *buffer, size_t size) {
return error;
}
-Status SendAdbMessage(Connection &conn, const std::string &packet, const bool reconnect) {
+Status SendAdbMessage(Connection &conn, const std::string &packet) {
Status error;
char length_buffer[5];
@@ -92,7 +92,7 @@ Status ConnectToAdb(Connection &conn) {
}
Status EnterSyncMode(Connection &conn) {
- auto error = SendAdbMessage(conn, "sync:", false);
+ auto error = SendAdbMessage(conn, "sync:");
if (error.Fail())
return error;
@@ -106,7 +106,7 @@ Status SelectTargetDevice(Connection &conn, const std::string &device_id) {
std::ostringstream msg;
msg << "host:transport:" << device_id;
- auto error = SendAdbMessage(conn, msg.str(), true);
+ auto error = SendAdbMessage(conn, msg.str());
if (error.Fail())
return error;
diff --git a/lldb/source/Plugins/Platform/Android/AdbClientUtils.h b/lldb/source/Plugins/Platform/Android/AdbClientUtils.h
index a5d3cbc2dde1b..ec3c035345a72 100644
--- a/lldb/source/Plugins/Platform/Android/AdbClientUtils.h
+++ b/lldb/source/Plugins/Platform/Android/AdbClientUtils.h
@@ -31,7 +31,7 @@ const static uint32_t kDefaultMode = 0100770;
namespace adb_client_utils {
Status ReadAllBytes(Connection &conn, void *buffer, size_t size);
-Status SendAdbMessage(Connection &conn, const std::string &packet, const bool reconnect);
+Status SendAdbMessage(Connection &conn, const std::string &packet);
Status GetResponseError(Connection &conn, const char *response_id);
Status ConnectToAdb(Connection &conn);
Status EnterSyncMode(Connection &conn);
>From 2e441d03285171b0d3198864c76c82b70fb59cff Mon Sep 17 00:00:00 2001
From: Chad Smith <cs01 at users.noreply.github.com>
Date: Wed, 9 Jul 2025 22:19:40 -0700
Subject: [PATCH 6/8] fix unit tests
---
.../Platform/Android/AdbClientTest.cpp | 12 ++++------
.../Platform/Android/PlatformAndroidTest.cpp | 23 ++++++++-----------
2 files changed, 14 insertions(+), 21 deletions(-)
diff --git a/lldb/unittests/Platform/Android/AdbClientTest.cpp b/lldb/unittests/Platform/Android/AdbClientTest.cpp
index 06822968124ec..da8238d5ab2be 100644
--- a/lldb/unittests/Platform/Android/AdbClientTest.cpp
+++ b/lldb/unittests/Platform/Android/AdbClientTest.cpp
@@ -7,6 +7,7 @@
//===----------------------------------------------------------------------===//
#include "Plugins/Platform/Android/AdbClient.h"
+#include "Plugins/Platform/Android/AdbSyncService.h"
#include "gtest/gtest.h"
#include <cstdlib>
@@ -48,16 +49,11 @@ TEST_F(AdbClientTest, CreateByDeviceId_ByEnvVar) {
}
TEST_F(AdbClientTest, SyncServiceCreation) {
- AdbClient adb_client("test_device");
+ AdbSyncService sync_service("test_device");
- std::unique_ptr<Connection> conn = std::make_unique<ConnectionFileDescriptor>();
- auto sync_service = std::make_unique<AdbClient::SyncService>(
- std::move(conn), adb_client.GetDeviceID());
+ EXPECT_EQ(sync_service.GetDeviceId(), "test_device");
- EXPECT_NE(sync_service, nullptr);
- EXPECT_EQ(sync_service->GetDeviceId(), "test_device");
-
- EXPECT_FALSE(sync_service->IsConnected());
+ EXPECT_FALSE(sync_service.IsConnected());
}
} // end namespace platform_android
diff --git a/lldb/unittests/Platform/Android/PlatformAndroidTest.cpp b/lldb/unittests/Platform/Android/PlatformAndroidTest.cpp
index cadaa5491c60f..8406157e2a788 100644
--- a/lldb/unittests/Platform/Android/PlatformAndroidTest.cpp
+++ b/lldb/unittests/Platform/Android/PlatformAndroidTest.cpp
@@ -63,7 +63,7 @@ class MockConnection : public Connection {
class MockAdbClient : public AdbClient {
public:
- explicit MockAdbClient() : AdbClient("mock") {}
+ explicit MockAdbClient() : AdbClient() {}
MOCK_METHOD3(ShellToFile,
Status(const char *command, std::chrono::milliseconds timeout,
@@ -77,7 +77,7 @@ class PlatformAndroidTest : public PlatformAndroid, public ::testing::Test {
// Set up default mock behavior to avoid uninteresting call warnings
ON_CALL(*this, GetSyncService(_))
- .WillByDefault([](Status &error) -> std::unique_ptr<AdbClient::SyncService> {
+ .WillByDefault([](Status &error) -> std::unique_ptr<AdbSyncService> {
error = Status::FromErrorString("Sync service unavailable");
return nullptr;
});
@@ -85,7 +85,7 @@ class PlatformAndroidTest : public PlatformAndroid, public ::testing::Test {
MOCK_METHOD1(GetAdbClient, AdbClientUP(Status &error));
MOCK_METHOD0(GetPropertyPackageName, llvm::StringRef());
- MOCK_METHOD1(GetSyncService, std::unique_ptr<AdbClient::SyncService>(Status &error));
+ MOCK_METHOD1(GetSyncService, std::unique_ptr<AdbSyncService>(Status &error));
// Make GetSyncService public for testing
using PlatformAndroid::GetSyncService;
@@ -191,7 +191,7 @@ TEST_F(PlatformAndroidTest, GetFile_SyncServiceUnavailable_FallsBackToShellCat)
Return(ByMove(AdbClientUP(adb_client)))));
EXPECT_CALL(*this, GetSyncService(_))
- .WillOnce([](Status &error) -> std::unique_ptr<AdbClient::SyncService> {
+ .WillOnce([](Status &error) -> std::unique_ptr<AdbSyncService> {
error = Status::FromErrorString("Sync service unavailable");
return nullptr;
});
@@ -216,7 +216,7 @@ TEST_F(PlatformAndroidTest, GetFile_WithRunAs_UsesRunAsInShellCommand) {
Return(ByMove(AdbClientUP(adb_client)))));
EXPECT_CALL(*this, GetSyncService(_))
- .WillOnce([](Status &error) -> std::unique_ptr<AdbClient::SyncService> {
+ .WillOnce([](Status &error) -> std::unique_ptr<AdbSyncService> {
error = Status::FromErrorString("Sync service unavailable");
return nullptr;
});
@@ -228,7 +228,7 @@ TEST_F(PlatformAndroidTest, GetFile_WithRunAs_UsesRunAsInShellCommand) {
TEST_F(PlatformAndroidTest, GetFile_FilenameWithSingleQuotes_Rejected) {
EXPECT_CALL(*this, GetSyncService(_))
- .WillOnce([](Status &error) -> std::unique_ptr<AdbClient::SyncService> {
+ .WillOnce([](Status &error) -> std::unique_ptr<AdbSyncService> {
error = Status::FromErrorString("Sync service unavailable");
return nullptr;
});
@@ -283,7 +283,7 @@ TEST_F(PlatformAndroidTest, GetFile_NetworkTimeout_PropagatesErrorCorrectly) {
Return(ByMove(AdbClientUP(adb_client)))));
EXPECT_CALL(*this, GetSyncService(_))
- .WillOnce([](Status &error) -> std::unique_ptr<AdbClient::SyncService> {
+ .WillOnce([](Status &error) -> std::unique_ptr<AdbSyncService> {
error = Status::FromErrorString("Sync service unavailable");
return nullptr;
});
@@ -294,12 +294,10 @@ TEST_F(PlatformAndroidTest, GetFile_NetworkTimeout_PropagatesErrorCorrectly) {
}
TEST_F(PlatformAndroidTest, SyncService_ConnectionFailsGracefully) {
- auto mock_conn = std::make_unique<MockConnection>(true); // Configure to fail
-
// Constructor should succeed even with a failing connection
- AdbClient::SyncService sync_service(std::move(mock_conn), "test-device");
+ AdbSyncService sync_service("test-device");
- // The service should report as not connected
+ // The service should report as not connected initially
EXPECT_FALSE(sync_service.IsConnected());
EXPECT_EQ(sync_service.GetDeviceId(), "test-device");
@@ -310,7 +308,6 @@ TEST_F(PlatformAndroidTest, SyncService_ConnectionFailsGracefully) {
Status result = sync_service.Stat(remote_file, mode, size, mtime);
EXPECT_TRUE(result.Fail());
- EXPECT_THAT(result.AsCString(), HasSubstr("connection failed"));
}
TEST_F(PlatformAndroidTest, GetRunAs_FormatsPackageNameCorrectly) {
@@ -355,7 +352,7 @@ TEST_F(PlatformAndroidTest, DownloadModuleSlice_ZeroOffset_CallsGetFileInsteadOf
.WillOnce(DoAll(WithArg<0>([](auto &arg) {
arg = Status::FromErrorString("Sync service unavailable");
}),
- Return(ByMove(std::unique_ptr<AdbClient::SyncService>()))));
+ Return(ByMove(std::unique_ptr<AdbSyncService>()))));
Status result = DownloadModuleSlice(FileSpec("/system/lib64/libc.so"), 0, 0, FileSpec("/tmp/libc.so"));
EXPECT_TRUE(result.Success());
>From b4f72a418c603ef7bf941c7313971310f1334270 Mon Sep 17 00:00:00 2001
From: Chad Smith <cs01 at users.noreply.github.com>
Date: Wed, 9 Jul 2025 22:24:25 -0700
Subject: [PATCH 7/8] fix headers
---
lldb/source/Plugins/Platform/Android/AdbClientUtils.cpp | 7 +++++++
lldb/source/Plugins/Platform/Android/AdbClientUtils.h | 2 +-
2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/lldb/source/Plugins/Platform/Android/AdbClientUtils.cpp b/lldb/source/Plugins/Platform/Android/AdbClientUtils.cpp
index 88144337b57c4..dd87826e21aa6 100644
--- a/lldb/source/Plugins/Platform/Android/AdbClientUtils.cpp
+++ b/lldb/source/Plugins/Platform/Android/AdbClientUtils.cpp
@@ -1,3 +1,10 @@
+//===-- AdbClientUtils.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 "lldb/Utility/Connection.h"
#include "AdbClientUtils.h"
#include "lldb/Utility/LLDBLog.h"
diff --git a/lldb/source/Plugins/Platform/Android/AdbClientUtils.h b/lldb/source/Plugins/Platform/Android/AdbClientUtils.h
index ec3c035345a72..0ca93450735f0 100644
--- a/lldb/source/Plugins/Platform/Android/AdbClientUtils.h
+++ b/lldb/source/Plugins/Platform/Android/AdbClientUtils.h
@@ -1,4 +1,4 @@
-//===-- AdbClientUtils.h ---------------------------------------*- C++ -*-===//
+//===-- AdbClientUtils.h --------------------------------------------------===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
>From 0537600afc780e898c757a74f815f0f06cb10e16 Mon Sep 17 00:00:00 2001
From: Chad Smith <cs01 at users.noreply.github.com>
Date: Thu, 10 Jul 2025 11:05:44 -0700
Subject: [PATCH 8/8] add log on reconnect
---
lldb/source/Plugins/Platform/Android/AdbClient.cpp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lldb/source/Plugins/Platform/Android/AdbClient.cpp b/lldb/source/Plugins/Platform/Android/AdbClient.cpp
index 54ab165921965..a4eaecfb20094 100644
--- a/lldb/source/Plugins/Platform/Android/AdbClient.cpp
+++ b/lldb/source/Plugins/Platform/Android/AdbClient.cpp
@@ -112,8 +112,8 @@ Status AdbClient::GetDevices(DeviceIDList &device_list) {
for (const auto &device : devices)
device_list.push_back(std::string(device.split('\t').first));
- // Force disconnect since ADB closes connection after host:devices response
- // is sent.
+ Log *log = GetLog(LLDBLog::Platform);
+ LLDB_LOGF(log, "Force reconnect since ADB closes connection after host:devices response");
m_conn = std::make_unique<ConnectionFileDescriptor>();
error = Connect();
return error;
More information about the lldb-commits
mailing list