[lldb][Android] Add PlatformAndroidTest

[splhack]

Summary

To test D152759 [lldb][Android] Support zip .so file

introduce PlatformAndroidTest with the capability of mocking adb client.

Depends on D152759

[splhack]

update depends

[splhack]

.

[splhack]

(not sure why build status got merge conflict, probably landing D152494 may resolve)

[splhack]

sync with D152759 new version

[splhack]

rebase onto D152759

[splhack]

remove 'Depend on' from commit message

[splhack]

rebase

[splhack]

Fixed diff dependencies in order to fix CI
https://reviews.llvm.org/B238938

[clayborg]

Very easy fix for this as suggested in code changes and this will be good to go

lldb/source/Plugins/Platform/Android/PlatformAndroid.h

73

Using the "SP" suffix is for std::shared_ptr, "UP" is for unique pointers.

[bulbazord]

lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp

213

GetAdbClient() calls make_unique which can fail. You'll want to add some checks when using all the adb objects in this file.

lldb/source/Plugins/Platform/Android/PlatformAndroid.h

73

SP -> shared_ptr. Was this supposed to be std::shared_ptr? If it's supposed to be a unique_ptr, you probably want AdbClientUP.

74

I feel a little strange about this one because it's possible to create 2 AdbClients from the same PlatformAndroid ID, both are unique pointers and essentially do the same thing. Maybe PlatformAndroid can own an AdbClient that it vends through GetAdbClient?

[splhack]

lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp

213

will do!

lldb/source/Plugins/Platform/Android/PlatformAndroid.h

73

@bulbazord @clayborg thanks, and oops didn't realize UP. will fix.

74

Yes, it's a bit strange how PlatformAndroid currently uses AdbClient. But, the scope of this diff is to unblock us first to write PlatformAndroid unittest with gmock + virtual. We will revisit for the structure change later.

[splhack]

• s/AdbClientSP/AdbClientUP/g
• Updated Status &error arg to GetAdbClient
• Added error checks in AndroidPlatform for GetAdbClient
• Added DownloadModuleSliceWithAdbClientError test for the GetAdbClient error

[clayborg]

lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp

204

Do we want the PlatformAndroid object to have a member variable that stores the AdbClientUP as a member variable so we don't need to recreate this all the time? If the object isn't expensive to create and destroy, no worries, but if it is, then we might want to have a member variable

402

We could cache the AdbClient object and return just a pointer here. The idea would be to have a member variable in PlatformAndroid object and then return just a "AdbClient *" from this function. This would stop us from creating and destroying a AdbClient object each time this is called.

[clayborg]

Lets get this patch in so we have testing. We can work on caching the AdbClient internally in an ivar of PlatformAndroid in follow up patches.

[splhack]

I will submit a diff to deal with how to use the AdbClient in PlatformAndroid later. Most likely we could use only one instance of AdbClient.

lldb/source/Plugins/Platform/Android/AdbClient.h

 //===-- AdbClient.h ---------------------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
public:
   };
 
   using DeviceIDList = std::list<std::string>;
 
   class SyncService {
     friend class AdbClient;
 
   public:
-    ~SyncService();
+    virtual ~SyncService();
 
-    Status PullFile(const FileSpec &remote_file, const FileSpec &local_file);
+    virtual Status PullFile(const FileSpec &remote_file,
+                            const FileSpec &local_file);
 
     Status PushFile(const FileSpec &local_file, const FileSpec &remote_file);
 
-    Status Stat(const FileSpec &remote_file, uint32_t &mode, uint32_t &size,
-                uint32_t &mtime);
+    virtual Status Stat(const FileSpec &remote_file, uint32_t &mode,
+                        uint32_t &size, uint32_t &mtime);
 
     bool IsConnected() const;
 
-  private:
+  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);
private:
     std::unique_ptr<Connection> m_conn;
   };
 
   static Status CreateByDeviceID(const std::string &device_id, AdbClient &adb);
 
   AdbClient();
   explicit AdbClient(const std::string &device_id);
 
-  ~AdbClient();
+  virtual ~AdbClient();
 
   const std::string &GetDeviceID() const;
 
   Status GetDevices(DeviceIDList &device_list);
 
   Status SetPortForwarding(const uint16_t local_port,
                            const uint16_t remote_port);
 
   Status SetPortForwarding(const uint16_t local_port,
                            llvm::StringRef remote_socket_name,
                            const UnixSocketNamespace socket_namespace);
 
   Status DeletePortForwarding(const uint16_t local_port);
 
   Status Shell(const char *command, std::chrono::milliseconds timeout,
                std::string *output);
 
-  Status ShellToFile(const char *command, std::chrono::milliseconds timeout,
+  virtual Status ShellToFile(const char *command,
+                             std::chrono::milliseconds timeout,
                              const FileSpec &output_file_spec);
 
-  std::unique_ptr<SyncService> GetSyncService(Status &error);
+  virtual std::unique_ptr<SyncService> GetSyncService(Status &error);
 
   Status SwitchDeviceTransport();
 
 private:
   Status Connect();
 
   void SetDeviceID(const std::string &device_id);
 

lldb/source/Plugins/Platform/Android/PlatformAndroid.h

 //===-- PlatformAndroid.h ---------------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
Status DownloadModuleSlice(const FileSpec &src_file_spec,
                              const FileSpec &dst_file_spec) override;
 
   Status DownloadSymbolFile(const lldb::ModuleSP &module_sp,
                             const FileSpec &dst_file_spec) override;
 
   llvm::StringRef
   GetLibdlFunctionDeclarations(lldb_private::Process *process) override;
 
+  typedef std::unique_ptr<AdbClient> AdbClientSP;
+  virtual AdbClientSP GetAdbClient();
+
 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;
 };
 
 } // namespace platofor_android
 } // namespace lldb_private
 
 #endif // LLDB_SOURCE_PLUGINS_PLATFORM_ANDROID_PLATFORMANDROID_H

lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp

 //===-- PlatformAndroid.cpp -----------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
Status PlatformAndroid::GetFile(const FileSpec &source,
   LLDB_LOGF(log, "Got mode == 0 on '%s': try to get file via 'shell cat'",
             source_file.c_str());
 
   if (strchr(source_file.c_str(), '\'') != nullptr)
     return Status("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.
-  AdbClient adb(m_device_id);
+  AdbClientSP adb(GetAdbClient());
 
   char cmd[PATH_MAX];
   if (const char *run_as = std::getenv("ANDROID_PLATFORM_RUN_AS"))
     snprintf(cmd, sizeof(cmd), "run-as '%s' cat '%s'", run_as,
              source_file.c_str());
   else
     snprintf(cmd, sizeof(cmd), "cat '%s'", source_file.c_str());
 
-  return adb.ShellToFile(cmd, minutes(1), destination);
+  return adb->ShellToFile(cmd, minutes(1), destination);
 }
 
 Status PlatformAndroid::PutFile(const FileSpec &source,
                                 const FileSpec &destination, uint32_t uid,
                                 uint32_t gid) {
   if (IsHost() || !m_remote_platform_sp)
     return PlatformLinux::PutFile(source, destination, uid, gid);
 
Status PlatformAndroid::DownloadModuleSlice(const FileSpec &src_file_spec,
 
   // For zip .so file, src_file_spec will be "zip_path!/so_path".
   // Extract "zip_path" from the source path.
   const std::string kZipSeparator = "!/";
   auto pos = source_file.find(kZipSeparator);
   if (pos != std::string::npos)
     source_file = source_file.substr(0, pos);
 
-  AdbClient adb(m_device_id);
+  AdbClientSP adb(GetAdbClient());
 
   // Use 'run-as' if necessary.
   char run_as_cmd[PATH_MAX];
   if (const char *run_as = std::getenv("ANDROID_PLATFORM_RUN_AS"))
     snprintf(run_as_cmd, sizeof(run_as_cmd), "run-as '%s' ", run_as);
   else
     run_as_cmd[0] = '\0';
 
   // Use 'shell dd' to download the file slice with the offset and size.
   char cmd[PATH_MAX * 2];
   snprintf(cmd, sizeof(cmd), "%sdd if='%s' iflag=skip_bytes,count_bytes "
            "skip=%" PRIu64 " count=%" PRIu64 " status=none",
            run_as_cmd, source_file.c_str(), src_offset, src_size);
 
-  return adb.ShellToFile(cmd, minutes(1), dst_file_spec);
+  return adb->ShellToFile(cmd, minutes(1), dst_file_spec);
 }
 
 Status PlatformAndroid::DisconnectRemote() {
   Status error = PlatformLinux::DisconnectRemote();
   if (error.Success()) {
     m_device_id.clear();
     m_sdk_version = 0;
   }
   return error;
 }
 
 uint32_t PlatformAndroid::GetDefaultMemoryCacheLineSize() {
   return g_android_default_cache_size;
 }
 
 uint32_t PlatformAndroid::GetSdkVersion() {
   if (!IsConnected())
     return 0;
 
   if (m_sdk_version != 0)
     return m_sdk_version;
 
   std::string version_string;
-  AdbClient adb(m_device_id);
+  AdbClientSP adb(GetAdbClient());
   Status error =
-      adb.Shell("getprop ro.build.version.sdk", seconds(5), &version_string);
+      adb->Shell("getprop ro.build.version.sdk", seconds(5), &version_string);
   version_string = llvm::StringRef(version_string).trim().str();
 
   if (error.Fail() || version_string.empty()) {
     Log *log = GetLog(LLDBLog::Platform);
     LLDB_LOGF(log, "Get SDK version failed. (error: %s, output: %s)",
               error.AsCString(), version_string.c_str());
     return 0;
   }
Status PlatformAndroid::DownloadSymbolFile(const lldb::ModuleSP &module_sp,
   if (GetSdkVersion() < 23)
     return Status("Symbol file generation only supported on SDK 23+");
 
   // If we already have symtab then we don't have to try and generate one
   if (module_sp->GetSectionList()->FindSectionByName(ConstString(".symtab")) !=
       nullptr)
     return Status("Symtab already available in the module");
 
-  AdbClient adb(m_device_id);
+  AdbClientSP adb(GetAdbClient());
   std::string tmpdir;
-  Status error = adb.Shell("mktemp --directory --tmpdir /data/local/tmp",
+  Status error = adb->Shell("mktemp --directory --tmpdir /data/local/tmp",
                            seconds(5), &tmpdir);
   if (error.Fail() || tmpdir.empty())
     return Status("Failed to generate temporary directory on the device (%s)",
                   error.AsCString());
   tmpdir = llvm::StringRef(tmpdir).trim().str();
 
   // Create file remover for the temporary directory created on the device
   std::unique_ptr<std::string, std::function<void(std::string *)>>
   tmpdir_remover(&tmpdir, [&adb](std::string *s) {
     StreamString command;
     command.Printf("rm -rf %s", s->c_str());
-    Status error = adb.Shell(command.GetData(), seconds(5), nullptr);
+    Status error = adb->Shell(command.GetData(), seconds(5), nullptr);
 
     Log *log = GetLog(LLDBLog::Platform);
     if (log && error.Fail())
       LLDB_LOGF(log, "Failed to remove temp directory: %s", error.AsCString());
   });
 
   FileSpec symfile_platform_filespec(tmpdir);
   symfile_platform_filespec.AppendPathComponent("symbolized.oat");
 
   // Execute oatdump on the remote device to generate a file with symtab
   StreamString command;
   command.Printf("oatdump --symbolize=%s --output=%s",
                  module_sp->GetPlatformFileSpec().GetPath(false).c_str(),
                  symfile_platform_filespec.GetPath(false).c_str());
-  error = adb.Shell(command.GetData(), minutes(1), nullptr);
+  error = adb->Shell(command.GetData(), minutes(1), nullptr);
   if (error.Fail())
     return Status("Oatdump failed: %s", error.AsCString());
 
   // Download the symbolfile from the remote device
   return GetFile(symfile_platform_filespec, dst_file_spec);
 }
 
 bool PlatformAndroid::GetRemoteOSVersion() {
return R"(
               extern "C" void* dlsym(void*, const char*) asm("__dl_dlsym");
               extern "C" int   dlclose(void*) asm("__dl_dlclose");
               extern "C" char* dlerror(void) asm("__dl_dlerror");
              )";
 
   return PlatformPOSIX::GetLibdlFunctionDeclarations(process);
 }
 
+PlatformAndroid::AdbClientSP PlatformAndroid::GetAdbClient() {
+  return std::make_unique<AdbClient>(m_device_id);
+}
+
 AdbClient::SyncService *PlatformAndroid::GetSyncService(Status &error) {
   if (m_adb_sync_svc && m_adb_sync_svc->IsConnected())
     return m_adb_sync_svc.get();
 
-  AdbClient adb(m_device_id);
-  m_adb_sync_svc = adb.GetSyncService(error);
+  AdbClientSP adb(GetAdbClient());
+  m_adb_sync_svc = adb->GetSyncService(error);
   return (error.Success()) ? m_adb_sync_svc.get() : nullptr;
 }

lldb/unittests/Platform/Android/CMakeLists.txt

 include_directories(${LLDB_SOURCE_DIR}/source/Plugins/Platform/Android)
 
 add_lldb_unittest(AdbClientTests
   AdbClientTest.cpp
+  PlatformAndroidTest.cpp
 
   LINK_LIBS
     lldbPluginPlatformAndroid
   )

lldb/unittests/Platform/Android/PlatformAndroidTest.cpp

+//===-- PlatformAndroidTest.cpp -------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "lldb/Utility/Connection.h"
+#include "Plugins/Platform/Android/PlatformAndroid.h"
+#include "Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.h"
+#include "TestingSupport/SubsystemRAII.h"
+#include "TestingSupport/TestUtilities.h"
+#include "gmock/gmock.h"
+
+using namespace lldb;
+using namespace lldb_private;
+using namespace lldb_private::platform_android;
+using namespace testing;
+
+namespace {
+
+class MockSyncService : public AdbClient::SyncService {
+public:
+  MockSyncService() : SyncService(std::move(m_mock_conn)) {}
+
+  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));
+
+private:
+  std::unique_ptr<Connection> m_mock_conn;
+};
+
+typedef std::unique_ptr<AdbClient::SyncService> SyncServiceSP;
+
+class MockAdbClient : public AdbClient {
+public:
+  explicit MockAdbClient() : AdbClient("mock") {}
+
+  MOCK_METHOD3(ShellToFile, Status(const char *command,
+                                   std::chrono::milliseconds timeout,
+                                   const FileSpec &output_file_spec));
+  MOCK_METHOD1(GetSyncService, SyncServiceSP(Status &error));
+};
+
+static void set_env(const char *var, const char *value) {
+#ifdef _WIN32
+  _putenv_s(var, value);
+#else
+  if (strlen(value) == 0)
+    unsetenv(var);
+  else
+    setenv(var, value, true);
+#endif
+}
+
+class PlatformAndroidTest : public PlatformAndroid, public ::testing::Test {
+public:
+  PlatformAndroidTest() : PlatformAndroid(false) {
+    m_remote_platform_sp = PlatformSP(new PlatformAndroidRemoteGDBServer());
+  }
+
+  MOCK_METHOD0(GetAdbClient, AdbClientSP());
+
+  void SetUp() override { set_env("ANDROID_PLATFORM_RUN_AS", ""); }
+
+  void TearDown() override { set_env("ANDROID_PLATFORM_RUN_AS", ""); }
+};
+
+} // namespace
+
+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()));
+
+  auto adb_client = new MockAdbClient();
+  EXPECT_CALL(*adb_client,
+       GetSyncService(_))
+       .Times(1).WillOnce(
+           Return(ByMove(std::move(SyncServiceSP(sync_service)))));
+
+  EXPECT_CALL(*this, GetAdbClient())
+      .Times(1).WillOnce(Return(ByMove(std::move(AdbClientSP(adb_client)))));
+
+  EXPECT_TRUE(
+      DownloadModuleSlice(FileSpec("/system/lib64/libc.so"), 0, 0, FileSpec())
+      .Success());
+}
+
+TEST_F(PlatformAndroidTest, DownloadModuleSliceWithZipFile) {
+  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, GetAdbClient())
+      .Times(1).WillOnce(Return(ByMove(std::move(AdbClientSP(adb_client)))));
+
+  EXPECT_TRUE(
+      DownloadModuleSlice(
+          FileSpec("/system/app/Test/Test.apk!/lib/arm64-v8a/libtest.so"),
+          4096, 3600, FileSpec())
+      .Success());
+}
+
+TEST_F(PlatformAndroidTest, DownloadModuleSliceWithZipFileAndRunAs) {
+  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, GetAdbClient())
+      .Times(1).WillOnce(Return(ByMove(std::move(AdbClientSP(adb_client)))));
+
+  set_env("ANDROID_PLATFORM_RUN_AS", "com.example.test");
+  EXPECT_TRUE(
+      DownloadModuleSlice(
+          FileSpec("/system/app/Test/Test.apk!/lib/arm64-v8a/libtest.so"),
+          4096, 3600, FileSpec())
+      .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).WillOnce(Return(Status()));
+
+  auto adb_client = new MockAdbClient();
+  EXPECT_CALL(*adb_client,
+       GetSyncService(_))
+       .Times(1).WillOnce(
+           Return(ByMove(std::move(SyncServiceSP(sync_service)))));
+
+  EXPECT_CALL(*this, GetAdbClient())
+      .Times(1).WillOnce(Return(ByMove(std::move(AdbClientSP(adb_client)))));
+
+  EXPECT_TRUE(
+      GetFile(FileSpec("/data/local/tmp/test"), FileSpec())
+      .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(std::move(SyncServiceSP(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).WillOnce(Return(Status()));
+
+  EXPECT_CALL(*this, GetAdbClient())
+      .Times(2)
+      .WillOnce(Return(ByMove(std::move(AdbClientSP(adb_client0)))))
+      .WillOnce(Return(ByMove(std::move(AdbClientSP(adb_client1)))));
+
+  EXPECT_TRUE(
+      GetFile(FileSpec("/data/data/com.example.app/lib-main/libtest.so"),
+              FileSpec())
+      .Success());
+}
+
+TEST_F(PlatformAndroidTest, GetFileWithRunAs) {
+  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(std::move(SyncServiceSP(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).WillOnce(Return(Status()));
+
+  EXPECT_CALL(*this, GetAdbClient())
+      .Times(2)
+      .WillOnce(Return(ByMove(std::move(AdbClientSP(adb_client0)))))
+      .WillOnce(Return(ByMove(std::move(AdbClientSP(adb_client1)))));
+
+  set_env("ANDROID_PLATFORM_RUN_AS", "com.example.app");
+  EXPECT_TRUE(
+      GetFile(FileSpec("/data/data/com.example.app/lib-main/libtest.so"),
+              FileSpec())
+      .Success());
+}