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 <c...@users.noreply.github.com>
Date: Mon, 23 Jun 2025 11:07:00 -0700
Subject: [PATCH 1/2] [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 <c...@users.noreply.github.com>
Date: Fri, 27 Jun 2025 11:52:30 -0700
Subject: [PATCH 2/2] 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());
}
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
