https://github.com/cs01 updated https://github.com/llvm/llvm-project/pull/145382
>From 4756820fa10e1c9b98616947780eb4dc83bc98e1 Mon Sep 17 00:00:00 2001 From: Chad Smith <c...@users.noreply.github.com> Date: Tue, 29 Jul 2025 10:00:02 -0700 Subject: [PATCH] refactor android platform --- .../Plugins/Platform/Android/AdbClient.cpp | 525 +++++++++--------- .../Plugins/Platform/Android/AdbClient.h | 106 ++-- .../Platform/Android/PlatformAndroid.cpp | 83 +-- .../Platform/Android/PlatformAndroid.h | 12 +- .../PlatformAndroidRemoteGDBServer.cpp | 11 +- .../Platform/Android/AdbClientTest.cpp | 124 ++++- .../Platform/Android/PlatformAndroidTest.cpp | 348 +++++++----- 7 files changed, 700 insertions(+), 509 deletions(-) diff --git a/lldb/source/Plugins/Platform/Android/AdbClient.cpp b/lldb/source/Plugins/Platform/Android/AdbClient.cpp index a179260ca15f6..b4b9ead31eb0d 100644 --- a/lldb/source/Plugins/Platform/Android/AdbClient.cpp +++ b/lldb/source/Plugins/Platform/Android/AdbClient.cpp @@ -8,61 +8,48 @@ #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/Host/posix/ConnectionFileDescriptorPosix.h" +#include "lldb/Utility/Connection.h" #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 "lldb/Utility/Status.h" #include "lldb/Utility/StreamString.h" #include "lldb/Utility/Timeout.h" +#include "llvm/ADT/SmallVector.h" +#include "llvm/ADT/StringRef.h" +#include "llvm/Support/FileUtilities.h" +#include <chrono> #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; - -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"; +using namespace llvm; + +const static char *kSocketNamespaceAbstract = "localabstract"; +const static char *kSocketNamespaceFileSystem = "localfilesystem"; +const 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; static Status ReadAllBytes(Connection &conn, void *buffer, size_t size) { - Status error; ConnectionStatus status; char *read_buffer = static_cast<char *>(buffer); @@ -81,90 +68,220 @@ static 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; } -Status AdbClient::CreateByDeviceID(const std::string &device_id, - AdbClient &adb) { - Status error; - std::string android_serial; - if (!device_id.empty()) - android_serial = device_id; - else if (const char *env_serial = std::getenv("ANDROID_SERIAL")) - android_serial = env_serial; +static Status ReadAdbMessage(Connection &conn, std::vector<char> &message) { + message.clear(); - if (android_serial.empty()) { - DeviceIDList connected_devices; - error = adb.GetDevices(connected_devices); - if (error.Fail()) - return error; + 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(); - if (connected_devices.size() != 1) - return Status::FromErrorStringWithFormat( - "Expected a single connected device, got instead %zu - try " - "setting 'ANDROID_SERIAL'", - connected_devices.size()); - adb.SetDeviceID(connected_devices.front()); - } else { - adb.SetDeviceID(android_serial); - } return error; } -AdbClient::AdbClient() = default; - -AdbClient::AdbClient(const std::string &device_id) : m_device_id(device_id) {} +static 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); -AdbClient::~AdbClient() = default; + std::vector<char> error_message; + auto error = ReadAdbMessage(conn, error_message); + if (!error.Success()) + return error; -void AdbClient::SetDeviceID(const std::string &device_id) { - m_device_id = device_id; + 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); } -const std::string &AdbClient::GetDeviceID() const { return m_device_id; } +static Status ReadResponseStatus(Connection &conn) { + char response_id[5]; -Status AdbClient::Connect() { + 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; +} + +static Status SendAdbMessage(Connection &conn, const std::string &packet) { Status error; - m_conn = std::make_unique<ConnectionFileDescriptor>(); + + 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; +} + +static Status ConnectToAdb(Connection &conn) { std::string port = "5037"; - if (const char *env_port = std::getenv("ANDROID_ADB_SERVER_PORT")) { + 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); + 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; } -Status AdbClient::GetDevices(DeviceIDList &device_list) { - device_list.clear(); - - auto error = SendMessage("host:devices"); +static Status EnterSyncMode(Connection &conn) { + auto error = SendAdbMessage(conn, "sync:"); if (error.Fail()) return error; - error = ReadResponseStatus(); + return ReadResponseStatus(conn); +} + +static 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; + + auto error = SendAdbMessage(conn, msg.str()); if (error.Fail()) return error; - std::vector<char> in_buffer; - error = ReadMessage(in_buffer); + return ReadResponseStatus(conn); +} + +Expected<std::string> AdbClient::ResolveDeviceID(StringRef device_id) { + StringRef preferred_serial; + if (!device_id.empty()) { + preferred_serial = device_id; + } else if (const char *env_serial = std::getenv("ANDROID_SERIAL")) { + preferred_serial = env_serial; + } - llvm::StringRef response(&in_buffer[0], in_buffer.size()); - llvm::SmallVector<llvm::StringRef, 4> devices; - response.split(devices, "\n", -1, false); + if (preferred_serial.empty()) { + DeviceIDList connected_devices; - for (const auto &device : devices) - device_list.push_back(std::string(device.split('\t').first)); + auto GetDevices = [](DeviceIDList &device_list) -> Status { + device_list.clear(); - // Force disconnect since ADB closes connection after host:devices response - // is sent. - m_conn.reset(); - return error; + // Create temporary ADB client for this operation only + auto temp_conn = std::make_unique<ConnectionFileDescriptor>(); + auto error = ConnectToAdb(*temp_conn); + if (error.Fail()) + return error; + + // NOTE: ADB closes the connection after host:devices response. + // The connection is no longer valid + error = SendAdbMessage(*temp_conn, "host:devices"); + if (error.Fail()) + return error; + + error = ReadResponseStatus(*temp_conn); + if (error.Fail()) + return error; + + std::vector<char> in_buffer; + error = ReadAdbMessage(*temp_conn, in_buffer); + + StringRef response(&in_buffer[0], in_buffer.size()); + SmallVector<StringRef, 4> devices; + response.split(devices, "\n", -1, false); + + for (const auto &device : devices) + device_list.push_back(std::string(device.split('\t').first)); + return error; + }; + + Status error = GetDevices(connected_devices); + if (error.Fail()) + return error.ToError(); + + if (connected_devices.size() != 1) + return createStringError(inconvertibleErrorCode(), + "Expected a single connected device, got instead %zu - try " + "setting 'ANDROID_SERIAL'", + connected_devices.size()); + + std::string resolved_device_id = std::move(connected_devices.front()); + Log *log = GetLog(LLDBLog::Platform); + LLDB_LOGF(log, "AdbClient::ResolveDeviceID Resolved device ID: %s", + resolved_device_id.c_str()); + return resolved_device_id; + } + + std::string resolved_device_id = preferred_serial.str(); + Log *log = GetLog(LLDBLog::Platform); + LLDB_LOGF(log, "AdbClient::ResolveDeviceID Resolved device ID: %s", + resolved_device_id.c_str()); + return resolved_device_id; +} + +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() { + Log *log = GetLog(LLDBLog::Platform); + LLDB_LOGF(log, + "AdbClient::~AdbClient() - Destroying AdbClient for device: %s", + m_device_id.c_str()); +} + +const std::string &AdbClient::GetDeviceID() const { return m_device_id; } + +Status AdbClient::Connect() { + if (m_conn->IsConnected()) + return Status(); + + return ConnectToAdb(*m_conn); } Status AdbClient::SetPortForwarding(const uint16_t local_port, @@ -177,7 +294,7 @@ Status AdbClient::SetPortForwarding(const uint16_t local_port, if (error.Fail()) return error; - return ReadResponseStatus(); + return ReadResponseStatus(*m_conn); } Status @@ -196,7 +313,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) { @@ -207,56 +324,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(); - - 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()); } Status AdbClient::ReadMessageStream(std::vector<char> &message, @@ -264,6 +338,9 @@ 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]; @@ -282,87 +359,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::SwitchDeviceTransport() { - std::ostringstream msg; - msg << "host:transport:" << m_device_id; - - auto error = SendMessage(msg.str()); - if (error.Fail()) - return error; - - 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() { - auto error = SendMessage("sync:", false); - if (error.Fail()) - return error; - - return ReadResponseStatus(); -} - -Status AdbClient::ReadAllBytes(void *buffer, size_t size) { - return ::ReadAllBytes(*m_conn, buffer, size); -} - Status AdbClient::internalShell(const char *command, milliseconds timeout, std::vector<char> &output_buf) { output_buf.clear(); - auto error = SwitchDeviceTransport(); + auto error = SelectTargetDevice(*m_conn, m_device_id); 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); - error = SendMessage(std::string(adb_command.GetString()), false); + error = SendAdbMessage(*m_conn, std::string(adb_command.GetString())); if (error.Fail()) return error; - error = ReadResponseStatus(); + error = ReadResponseStatus(*m_conn); if (error.Fail()) return error; @@ -417,18 +429,8 @@ Status AdbClient::ShellToFile(const char *command, milliseconds timeout, return Status(); } -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))); - - return sync_service; -} - -Status AdbClient::SyncService::internalPullFile(const FileSpec &remote_file, - const FileSpec &local_file) { +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); @@ -462,8 +464,8 @@ Status AdbClient::SyncService::internalPullFile(const FileSpec &remote_file, return error; } -Status AdbClient::SyncService::internalPushFile(const FileSpec &local_file, - const FileSpec &remote_file) { +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()) @@ -487,7 +489,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; @@ -500,7 +504,7 @@ Status AdbClient::SyncService::internalPushFile(const FileSpec &local_file, error.AsCString()); if (response_id == kFAIL) { std::string error_message(data_len, 0); - error = ReadAllBytes(&error_message[0], data_len); + error = ReadAllBytes(*m_conn, &error_message[0], data_len); if (error.Fail()) return Status::FromErrorStringWithFormat( "Failed to read DONE error message: %s", error.AsCString()); @@ -518,9 +522,8 @@ Status AdbClient::SyncService::internalPushFile(const FileSpec &local_file, return error; } -Status AdbClient::SyncService::internalStat(const FileSpec &remote_file, - uint32_t &mode, uint32_t &size, - uint32_t &mtime) { +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()); @@ -532,7 +535,7 @@ Status AdbClient::SyncService::internalStat(const FileSpec &remote_file, static const size_t response_len = stat_len + (sizeof(uint32_t) * 3); std::vector<char> buffer(response_len); - error = ReadAllBytes(&buffer[0], buffer.size()); + error = ReadAllBytes(*m_conn, &buffer[0], buffer.size()); if (error.Fail()) return Status::FromErrorStringWithFormat("Failed to read response: %s", error.AsCString()); @@ -555,51 +558,57 @@ Status AdbClient::SyncService::internalStat(const FileSpec &remote_file, return Status(); } -Status AdbClient::SyncService::PullFile(const FileSpec &remote_file, - const FileSpec &local_file) { +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 AdbClient::SyncService::PushFile(const FileSpec &local_file, - const FileSpec &remote_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 AdbClient::SyncService::Stat(const FileSpec &remote_file, uint32_t &mode, - uint32_t &size, uint32_t &mtime) { +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 AdbClient::SyncService::IsConnected() const { +bool AdbSyncService::IsConnected() const { return m_conn && m_conn->IsConnected(); } -AdbClient::SyncService::SyncService(std::unique_ptr<Connection> &&conn) - : m_conn(std::move(conn)) {} - -Status -AdbClient::SyncService::executeCommand(const std::function<Status()> &cmd) { - if (!m_conn) - return Status::FromErrorString("SyncService is disconnected"); +AdbSyncService::AdbSyncService(const std::string device_id) + : m_device_id(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) { Status error = cmd(); - if (error.Fail()) - m_conn.reset(); - return error; } -AdbClient::SyncService::~SyncService() = default; +AdbSyncService::~AdbSyncService() { + Log *log = GetLog(LLDBLog::Platform); + LLDB_LOGF(log, + "AdbSyncService::~AdbSyncService() - Destroying AdbSyncService for " + "device: %s", + m_device_id.c_str()); +} -Status AdbClient::SyncService::SendSyncRequest(const char *request_id, - const uint32_t data_len, - const void *data) { +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); @@ -615,11 +624,11 @@ Status AdbClient::SyncService::SendSyncRequest(const char *request_id, return error; } -Status AdbClient::SyncService::ReadSyncHeader(std::string &response_id, - uint32_t &data_len) { +Status AdbSyncService::ReadSyncHeader(std::string &response_id, + uint32_t &data_len) { char buffer[kSyncPacketLen]; - auto error = ReadAllBytes(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 *)); @@ -630,8 +639,7 @@ Status AdbClient::SyncService::ReadSyncHeader(std::string &response_id, return error; } -Status AdbClient::SyncService::PullFileChunk(std::vector<char> &buffer, - bool &eof) { +Status AdbSyncService::PullFileChunk(std::vector<char> &buffer, bool &eof) { buffer.clear(); std::string response_id; @@ -642,14 +650,14 @@ Status AdbClient::SyncService::PullFileChunk(std::vector<char> &buffer, if (response_id == kDATA) { buffer.resize(data_len, 0); - error = ReadAllBytes(&buffer[0], data_len); + 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(&error_message[0], data_len); + error = ReadAllBytes(*m_conn, &error_message[0], data_len); if (error.Fail()) return Status::FromErrorStringWithFormat( "Failed to read pull error message: %s", error.AsCString()); @@ -662,6 +670,15 @@ Status AdbClient::SyncService::PullFileChunk(std::vector<char> &buffer, return Status(); } -Status AdbClient::SyncService::ReadAllBytes(void *buffer, size_t size) { - return ::ReadAllBytes(*m_conn, buffer, size); +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/AdbClient.h b/lldb/source/Plugins/Platform/Android/AdbClient.h index 851c09957bd4a..20ab05f35a4ea 100644 --- a/lldb/source/Plugins/Platform/Android/AdbClient.h +++ b/lldb/source/Plugins/Platform/Android/AdbClient.h @@ -10,6 +10,7 @@ #define LLDB_SOURCE_PLUGINS_PLATFORM_ANDROID_ADBCLIENT_H #include "lldb/Utility/Status.h" +#include "llvm/Support/Error.h" #include <chrono> #include <functional> #include <list> @@ -32,50 +33,14 @@ class AdbClient { using DeviceIDList = std::list<std::string>; - class SyncService { - friend class AdbClient; - - public: - 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 Stat(const FileSpec &remote_file, uint32_t &mode, - uint32_t &size, uint32_t &mtime); - - bool IsConnected() const; - - protected: - explicit SyncService(std::unique_ptr<Connection> &&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 ReadAllBytes(void *buffer, size_t size); - - 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; - }; - - static Status CreateByDeviceID(const std::string &device_id, AdbClient &adb); + /// Resolves a device identifier to its canonical form. + /// + /// \param device_id the device identifier to resolve (may be empty). + /// + /// \returns Expected<std::string> containing the resolved device ID on + /// success, or an Error if the device ID cannot be resolved or + /// is ambiguous. + static llvm::Expected<std::string> ResolveDeviceID(llvm::StringRef device_id); AdbClient(); explicit AdbClient(const std::string &device_id); @@ -84,8 +49,6 @@ class AdbClient { const std::string &GetDeviceID() const; - Status GetDevices(DeviceIDList &device_list); - Status SetPortForwarding(const uint16_t local_port, const uint16_t remote_port); @@ -102,39 +65,52 @@ class AdbClient { std::chrono::milliseconds timeout, const FileSpec &output_file_spec); - virtual std::unique_ptr<SyncService> GetSyncService(Status &error); - - Status SwitchDeviceTransport(); - -private: Status Connect(); - void SetDeviceID(const std::string &device_id); - - Status SendMessage(const std::string &packet, const bool reconnect = true); - +private: 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 internalShell(const char *command, std::chrono::milliseconds timeout, + std::vector<char> &output_buf); - Status ReadResponseStatus(); + std::string m_device_id; + std::unique_ptr<Connection> m_conn; +}; - Status Sync(); +class AdbSyncService { +public: + explicit AdbSyncService(const std::string device_id); + virtual ~AdbSyncService(); + Status SetupSyncConnection(); - Status StartSync(); + 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; - Status internalShell(const char *command, std::chrono::milliseconds timeout, - std::vector<char> &output_buf); + const std::string &GetDeviceId() const { return m_device_id; } - 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 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::string m_device_id; std::unique_ptr<Connection> m_conn; + std::string m_device_id; }; } // namespace platform_android diff --git a/lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp b/lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp index 5bc9cc133fbd3..5fdc43f316395 100644 --- a/lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp +++ b/lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp @@ -9,10 +9,8 @@ #include "lldb/Core/Module.h" #include "lldb/Core/PluginManager.h" #include "lldb/Core/Section.h" -#include "lldb/Host/HostInfo.h" #include "lldb/Utility/LLDBLog.h" #include "lldb/Utility/Log.h" -#include "lldb/Utility/Scalar.h" #include "lldb/Utility/UriParser.h" #include "lldb/ValueObject/ValueObject.h" @@ -26,6 +24,7 @@ using namespace lldb; using namespace lldb_private; using namespace lldb_private::platform_android; using namespace std::chrono; +using namespace llvm; LLDB_PLUGIN_DEFINE(PlatformAndroid) @@ -194,12 +193,10 @@ Status PlatformAndroid::ConnectRemote(Args &args) { auto error = PlatformLinux::ConnectRemote(args); if (error.Success()) { - AdbClient adb; - error = AdbClient::CreateByDeviceID(m_device_id, adb); - if (error.Fail()) - return error; - - m_device_id = adb.GetDeviceID(); + auto resolved_device_id_or_error = AdbClient::ResolveDeviceID(m_device_id); + if (!resolved_device_id_or_error) + return Status::FromError(resolved_device_id_or_error.takeError()); + m_device_id = *resolved_device_id_or_error; } return error; } @@ -209,36 +206,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 +284,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"); @@ -424,7 +440,7 @@ PlatformAndroid::GetLibdlFunctionDeclarations(lldb_private::Process *process) { 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 (auto *name : dl_open_names) { target.GetImages().FindFunctionSymbols( ConstString(name), eFunctionNameTypeFull, matching_symbols); if (matching_symbols.GetSize()) { @@ -445,11 +461,8 @@ PlatformAndroid::GetLibdlFunctionDeclarations(lldb_private::Process *process) { } PlatformAndroid::AdbClientUP PlatformAndroid::GetAdbClient(Status &error) { - AdbClientUP adb(std::make_unique<AdbClient>(m_device_id)); - if (adb) - error.Clear(); - else - error = Status::FromErrorString("Failed to create AdbClient"); + AdbClientUP adb = std::make_unique<AdbClient>(m_device_id); + error = adb->Connect(); return adb; } @@ -473,14 +486,10 @@ 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(); - - AdbClientUP adb(GetAdbClient(error)); +std::unique_ptr<AdbSyncService> PlatformAndroid::GetSyncService(Status &error) { + auto sync_service = std::make_unique<AdbSyncService>(m_device_id); + error = sync_service->SetupSyncConnection(); if (error.Fail()) return nullptr; - m_adb_sync_svc = adb->GetSyncService(error); - return (error.Success()) ? m_adb_sync_svc.get() : nullptr; + return sync_service; } diff --git a/lldb/source/Plugins/Platform/Android/PlatformAndroid.h b/lldb/source/Plugins/Platform/Android/PlatformAndroid.h index 5602edf73c1d3..d32fe70b30b5f 100644 --- a/lldb/source/Plugins/Platform/Android/PlatformAndroid.h +++ b/lldb/source/Plugins/Platform/Android/PlatformAndroid.h @@ -73,16 +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<AdbSyncService> GetSyncService(Status &error); private: - AdbClient::SyncService *GetSyncService(Status &error); - - std::unique_ptr<AdbClient::SyncService> m_adb_sync_svc; std::string m_device_id; uint32_t m_sdk_version; }; diff --git a/lldb/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.cpp b/lldb/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.cpp index 0cf64807ec0d6..461ee8e3b1826 100644 --- a/lldb/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.cpp +++ b/lldb/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.cpp @@ -21,6 +21,7 @@ using namespace lldb; using namespace lldb_private; using namespace platform_android; +using namespace llvm; static const lldb::pid_t g_remote_platform_pid = 0; // Alias for the process id of lldb-platform @@ -32,12 +33,12 @@ static Status ForwardPortWithAdb( std::string &device_id) { Log *log = GetLog(LLDBLog::Platform); - AdbClient adb; - auto error = AdbClient::CreateByDeviceID(device_id, adb); - if (error.Fail()) - return error; + auto resolved_device_id_or_error = AdbClient::ResolveDeviceID(device_id); + if (!resolved_device_id_or_error) + return Status::FromError(resolved_device_id_or_error.takeError()); + device_id = *resolved_device_id_or_error; - device_id = adb.GetDeviceID(); + AdbClient adb(device_id); LLDB_LOGF(log, "Connected to Android device \"%s\"", device_id.c_str()); if (remote_port != 0) { diff --git a/lldb/unittests/Platform/Android/AdbClientTest.cpp b/lldb/unittests/Platform/Android/AdbClientTest.cpp index 0808b96f69fc8..719b7ca63801b 100644 --- a/lldb/unittests/Platform/Android/AdbClientTest.cpp +++ b/lldb/unittests/Platform/Android/AdbClientTest.cpp @@ -6,8 +6,11 @@ // //===----------------------------------------------------------------------===// -#include "gtest/gtest.h" #include "Plugins/Platform/Android/AdbClient.h" +#include "lldb/Host/Socket.h" +#include "lldb/Host/common/TCPSocket.h" +#include "gtest/gtest.h" +#include <chrono> #include <cstdlib> static void set_env(const char *var, const char *value) { @@ -20,32 +23,117 @@ static void set_env(const char *var, const char *value) { using namespace lldb; using namespace lldb_private; - -namespace lldb_private { -namespace platform_android { +using namespace lldb_private::platform_android; class AdbClientTest : public ::testing::Test { public: - void SetUp() override { set_env("ANDROID_SERIAL", ""); } + void SetUp() override { + set_env("ANDROID_SERIAL", ""); + set_env("ANDROID_ADB_SERVER_PORT", ""); + } - void TearDown() override { set_env("ANDROID_SERIAL", ""); } + void TearDown() override { + set_env("ANDROID_SERIAL", ""); + set_env("ANDROID_ADB_SERVER_PORT", ""); + } }; -TEST(AdbClientTest, CreateByDeviceId) { - AdbClient adb; - Status error = AdbClient::CreateByDeviceID("device1", adb); - EXPECT_TRUE(error.Success()); - EXPECT_EQ("device1", adb.GetDeviceID()); +TEST_F(AdbClientTest, ResolveDeviceId_ExplicitDeviceId) { + auto result = AdbClient::ResolveDeviceID("device1"); + EXPECT_TRUE(static_cast<bool>(result)); + EXPECT_EQ("device1", *result); } -TEST(AdbClientTest, CreateByDeviceId_ByEnvVar) { +TEST_F(AdbClientTest, ResolveDeviceId_ByEnvVar) { set_env("ANDROID_SERIAL", "device2"); - AdbClient adb; - Status error = AdbClient::CreateByDeviceID("", adb); - EXPECT_TRUE(error.Success()); - EXPECT_EQ("device2", adb.GetDeviceID()); + auto result = AdbClient::ResolveDeviceID(""); + EXPECT_TRUE(static_cast<bool>(result)); + EXPECT_EQ("device2", *result); +} + +TEST_F(AdbClientTest, ResolveDeviceId_PrefersExplicitOverEnvVar) { + set_env("ANDROID_SERIAL", "env_device"); + + // Explicit device ID should take precedence over environment variable + auto result = AdbClient::ResolveDeviceID("explicit_device"); + EXPECT_TRUE(static_cast<bool>(result)); + EXPECT_EQ("explicit_device", *result); +} + +TEST_F(AdbClientTest, AdbClient_Constructor_StoresDeviceId) { + AdbClient client("test_device_123"); + EXPECT_EQ(client.GetDeviceID(), "test_device_123"); +} + +TEST_F(AdbClientTest, AdbClient_DefaultConstructor) { + AdbClient client; + EXPECT_EQ(client.GetDeviceID(), ""); } -} // end namespace platform_android -} // end namespace lldb_private +TEST_F(AdbClientTest, AdbSyncService_Constructor_StoresDeviceId) { + AdbSyncService sync("device123"); + EXPECT_EQ(sync.GetDeviceId(), "device123"); +} + +TEST_F(AdbClientTest, AdbSyncService_OperationsFailWhenNotConnected) { + AdbSyncService sync_service("test_device"); + + // Verify service is not connected initially + EXPECT_FALSE(sync_service.IsConnected()); + + // File operations should fail when not connected + FileSpec remote_file("/data/test.txt"); + FileSpec local_file("/tmp/test.txt"); + uint32_t mode, size, mtime; + + Status stat_result = sync_service.Stat(remote_file, mode, size, mtime); + EXPECT_TRUE(stat_result.Fail()); + + Status pull_result = sync_service.PullFile(remote_file, local_file); + EXPECT_TRUE(pull_result.Fail()); + + Status push_result = sync_service.PushFile(local_file, remote_file); + EXPECT_TRUE(push_result.Fail()); +} + +static uint16_t FindUnusedPort() { + auto temp_socket = std::make_unique<TCPSocket>(true); + Status error = temp_socket->Listen("localhost:0", 1); + if (error.Fail()) { + return 0; // fallback + } + uint16_t port = temp_socket->GetLocalPortNumber(); + temp_socket.reset(); // Close the socket to free the port + return port; +} + +TEST_F(AdbClientTest, RealTcpConnection) { + uint16_t unused_port = FindUnusedPort(); + ASSERT_NE(unused_port, 0) << "Failed to find an unused port"; + + std::string port_str = std::to_string(unused_port); + setenv("ANDROID_ADB_SERVER_PORT", port_str.c_str(), 1); + + AdbClient client; + const auto status1 = client.Connect(); + EXPECT_FALSE(status1.Success()) + << "Connection should fail when no server is listening on port " + << unused_port; + + // now start a server on the port and try again + auto listen_socket = std::make_unique<TCPSocket>(true); + std::string listen_address = "localhost:" + port_str; + Status error = listen_socket->Listen(listen_address.c_str(), 5); + ASSERT_TRUE(error.Success()) << "Failed to create listening socket on port " + << unused_port << ": " << error.AsCString(); + + // Verify the socket is listening on the expected port + ASSERT_EQ(listen_socket->GetLocalPortNumber(), unused_port) + << "Socket is not listening on the expected port"; + + const auto status2 = client.Connect(); + EXPECT_TRUE(status2.Success()) + << "Connection should succeed when server is listening on port " + << unused_port; +} diff --git a/lldb/unittests/Platform/Android/PlatformAndroidTest.cpp b/lldb/unittests/Platform/Android/PlatformAndroidTest.cpp index d021562d94d28..83aecfebb5bab 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,212 +18,312 @@ using namespace testing; namespace { -class MockSyncService : public AdbClient::SyncService { -public: - MockSyncService() : SyncService(std::unique_ptr<Connection>()) {} - - 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)); -}; - -typedef std::unique_ptr<AdbClient::SyncService> SyncServiceUP; - class MockAdbClient : public AdbClient { public: - explicit MockAdbClient() : AdbClient("mock") {} + explicit MockAdbClient() : 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<AdbSyncService> { + 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<AdbSyncService>(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"); }), Return(ByMove(AdbClientUP())))); - EXPECT_TRUE( - DownloadModuleSlice( - FileSpec("/system/app/Test/Test.apk!/lib/arm64-v8a/libtest.so"), 4096, - 3600, FileSpec()) - .Fail()); -} - -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) - .WillOnce(Return(Status())); + Status result = DownloadModuleSlice( + FileSpec("/system/app/Test/Test.apk!/lib/arm64-v8a/libtest.so"), 4096, + 3600, FileSpec("/tmp/libtest.so")); - auto adb_client = new MockAdbClient(); - EXPECT_CALL(*adb_client, GetSyncService(_)) - .Times(1) - .WillOnce(Return(ByMove(SyncServiceUP(sync_service)))); - - EXPECT_CALL(*this, GetAdbClient(_)) - .Times(1) - .WillOnce(Return(ByMove(AdbClientUP(adb_client)))); - - EXPECT_TRUE( - DownloadModuleSlice(FileSpec("/system/lib64/libc.so"), 0, 0, FileSpec()) - .Success()); + EXPECT_TRUE(result.Fail()); + EXPECT_THAT(result.AsCString(), HasSubstr("Failed to create AdbClient")); } -TEST_F(PlatformAndroidTest, DownloadModuleSliceWithZipFile) { - auto adb_client = new MockAdbClient(); +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"), _, _)) - .Times(1) .WillOnce(Return(Status())); + 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/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_ZipFileWithRunAs_UsesRunAsCommand) { + auto *adb_client = new MockAdbClient(); EXPECT_CALL(*adb_client, 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()) - .Times(1) .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, 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, + 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("dd if='/system/app/Large.apk' " + "iflag=skip_bytes,count_bytes " + "skip=104857600 count=52428800 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(GetFile(FileSpec("/data/local/tmp/test"), 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, 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()))); +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())); + + EXPECT_CALL(*this, GetPropertyPackageName()) + .WillOnce(Return(llvm::StringRef(""))); - auto adb_client0 = new MockAdbClient(); - EXPECT_CALL(*adb_client0, GetSyncService(_)) - .Times(1) - .WillOnce(Return(ByMove(SyncServiceUP(sync_service)))); + 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<AdbSyncService> { + error = Status::FromErrorString("Sync service unavailable"); + return nullptr; + }); + + Status result = + GetFile(FileSpec("/data/local/tmp/test"), FileSpec("/tmp/test")); + EXPECT_TRUE(result.Success()); +} - auto adb_client1 = new MockAdbClient(); +TEST_F(PlatformAndroidTest, GetFile_WithRunAs_UsesRunAsInShellCommand) { + auto *adb_client = new MockAdbClient(); EXPECT_CALL( - *adb_client1, - ShellToFile(StrEq("cat '/data/data/com.example.app/lib-main/libtest.so'"), + *adb_client, + ShellToFile(StrEq("run-as 'com.example.app' " + "cat '/data/data/com.example.app/lib-main/libtest.so'"), _, _)) - .Times(1) .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)))); + .WillOnce(DoAll(WithArg<0>([](auto &arg) { arg.Clear(); }), + Return(ByMove(AdbClientUP(adb_client))))); + + EXPECT_CALL(*this, GetSyncService(_)) + .WillOnce([](Status &error) -> std::unique_ptr<AdbSyncService> { + error = Status::FromErrorString("Sync service unavailable"); + return nullptr; + }); - EXPECT_TRUE( + Status result = GetFile(FileSpec("/data/data/com.example.app/lib-main/libtest.so"), - FileSpec()) - .Success()); + FileSpec("/tmp/libtest.so")); + EXPECT_TRUE(result.Success()); } -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()))); +TEST_F(PlatformAndroidTest, GetFile_FilenameWithSingleQuotes_Rejected) { + EXPECT_CALL(*this, GetSyncService(_)) + .WillOnce([](Status &error) -> std::unique_ptr<AdbSyncService> { + error = Status::FromErrorString("Sync service unavailable"); + return nullptr; + }); - auto adb_client0 = new MockAdbClient(); - EXPECT_CALL(*adb_client0, GetSyncService(_)) - .Times(1) - .WillOnce(Return(ByMove(SyncServiceUP(sync_service)))); + Status result = + GetFile(FileSpec("/test/file'with'quotes"), FileSpec("/tmp/output")); - 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) + 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, 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<AdbSyncService> { + 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) { + // Constructor should succeed even with a failing connection + AdbSyncService sync_service("test-device"); + + // The service should report as not connected initially + 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()); +} + +TEST_F(PlatformAndroidTest, GetRunAs_FormatsPackageNameCorrectly) { + // Empty package name + EXPECT_CALL(*this, GetPropertyPackageName()) + .WillOnce(Return(llvm::StringRef(""))); + EXPECT_EQ(this->GetRunAs(), ""); + + // Valid package name + EXPECT_CALL(*this, GetPropertyPackageName()) + .WillOnce(Return(llvm::StringRef("com.example.test"))); + EXPECT_EQ(this->GetRunAs(), "run-as 'com.example.test' "); +} + +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)))); + .WillOnce(DoAll(WithArg<0>([](auto &arg) { arg.Clear(); }), + Return(ByMove(AdbClientUP(adb_client))))); - EXPECT_TRUE( - GetFile(FileSpec("/data/data/com.example.app/lib-main/libtest.so"), - FileSpec()) - .Success()); + // 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<AdbSyncService>())))); + + 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