[Lldb-commits] [PATCH] D152759: [lldb][Android] Support zip .so file

2023-06-20 Thread Kazuki Sakamoto via Phabricator via lldb-commits 
splhack added a comment.

Fixed Windows test failures in D153390 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152759/new/

https://reviews.llvm.org/D152759

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D153390: [lldb][Windows] Fix ZipFileResolver tests

2023-06-20 Thread Kazuki Sakamoto via Phabricator via lldb-commits 
splhack created this revision.
Herald added a project: All.
splhack requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

D152759  introduced the Android .zip so file 
support, but it only considered
POSIX path. The code also runs on Windows, so the path could be Windows path.
Support both patterns on Windows.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D153390

Files:
  lldb/source/Host/common/ZipFileResolver.cpp


Index: lldb/source/Host/common/ZipFileResolver.cpp
===
--- lldb/source/Host/common/ZipFileResolver.cpp
+++ lldb/source/Host/common/ZipFileResolver.cpp
@@ -25,6 +25,15 @@
   static constexpr llvm::StringLiteral k_zip_separator("!/");
   std::string path(file_spec.GetPath());
   size_t pos = path.find(k_zip_separator);
+
+#if defined(_WIN32)
+  // When the file_spec is resolved as a Windows path, the zip .so path will be
+  // "zip_path!\so_path". Support both patterns on Windows.
+  static constexpr llvm::StringLiteral k_zip_separator_win("!\\");
+  if (pos == std::string::npos)
+pos = path.find(k_zip_separator_win);
+#endif
+
   if (pos == std::string::npos) {
 // This file_spec does not contain the zip separator.
 // Treat this file_spec as a normal file.
@@ -40,6 +49,12 @@
   std::string zip_path(path.substr(0, pos));
   std::string so_path(path.substr(pos + k_zip_separator.size()));
 
+#if defined(_WIN32)
+  // Replace the .so path to use POSIX file separator for file searching inside
+  // the zip file.
+  std::replace(so_path.begin(), so_path.end(), '\\', '/');
+#endif
+
   // Try to find the .so file from the zip file.
   FileSpec zip_file_spec(zip_path);
   uint64_t zip_file_size = FileSystem::Instance().GetByteSize(zip_file_spec);


Index: lldb/source/Host/common/ZipFileResolver.cpp
===
--- lldb/source/Host/common/ZipFileResolver.cpp
+++ lldb/source/Host/common/ZipFileResolver.cpp
@@ -25,6 +25,15 @@
   static constexpr llvm::StringLiteral k_zip_separator("!/");
   std::string path(file_spec.GetPath());
   size_t pos = path.find(k_zip_separator);
+
+#if defined(_WIN32)
+  // When the file_spec is resolved as a Windows path, the zip .so path will be
+  // "zip_path!\so_path". Support both patterns on Windows.
+  static constexpr llvm::StringLiteral k_zip_separator_win("!\\");
+  if (pos == std::string::npos)
+pos = path.find(k_zip_separator_win);
+#endif
+
   if (pos == std::string::npos) {
 // This file_spec does not contain the zip separator.
 // Treat this file_spec as a normal file.
@@ -40,6 +49,12 @@
   std::string zip_path(path.substr(0, pos));
   std::string so_path(path.substr(pos + k_zip_separator.size()));
 
+#if defined(_WIN32)
+  // Replace the .so path to use POSIX file separator for file searching inside
+  // the zip file.
+  std::replace(so_path.begin(), so_path.end(), '\\', '/');
+#endif
+
   // Try to find the .so file from the zip file.
   FileSpec zip_file_spec(zip_path);
   uint64_t zip_file_size = FileSystem::Instance().GetByteSize(zip_file_spec);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D151765: [lldb] Introduce the FileSpecBuilder abstraction

2023-06-20 Thread Greg Clayton via Phabricator via lldb-commits 
clayborg added a comment.

The idea behind FileSpec containing two ConstStrings, one for the directory and 
one for he filename is for lookup performance when searching thousands of line 
tables for a given file and line when setting breakpoints.

Currently we pass in a FileSpec + line number and we expect fast searching. 
This is currently done by asking each CompileUnit to find a matching file in 
its support files array and to return a matching index. If just the filename is 
filled in, then we compare the just the ConstString for m_filename which is 
really quick for ConstString values. Then we compare all or part of the 
m_directory if the path is a full path or if it is relative. But at least the 
filename comparison is very fast. If we get a valid index back, then we know to 
search the compile unit's full line table for any matching entries by looking 
for any line entries with a matching file index. IF the file index is invalid, 
we don't materialize the line table at all for a compile unit.

Maybe we want the notion of a "FileSpec" (which could contain a llvm::Vector 
class) and a ConstFileSpec (which would contain two ConstString objects like 
FileSpec currently has). And we make anyone that needs quick searching, like 
the support files for a compile unit, use the new ConstFileSpec class.

If you change anything in the FileSpec class, please test setting breakpoints 
with a really large codebase to ensure we don't regress performance.

If we go the FileSpecBuilder route, we probably want to remove all FileSpec 
path modification methods and force people to use this class by converting a 
FileSpec to a FileSpecBuilder, manipulating the path with the FileSpecBuilder 
calls, and then extracting the FileSpec object from the FileSpecBuilder at the 
end.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151765/new/

https://reviews.llvm.org/D151765

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D152759: [lldb][Android] Support zip .so file

2023-06-20 Thread Kazuki Sakamoto via Phabricator via lldb-commits 
splhack added a comment.

ResolveSharedLibraryPathWithZipExisting and 
ResolveSharedLibraryPathWithZipMissing are failing on Windows. Looking into.
https://lab.llvm.org/buildbot/#/builders/219/builds/3674


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152759/new/

https://reviews.llvm.org/D152759

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D152933: [lldb][Android] Add platform.plugin.remote-android.package-name

2023-06-20 Thread Kazuki Sakamoto via Phabricator via lldb-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rGfabd16c7460e: [lldb][Android] Add 
platform.plugin.remote-android.package-name (authored by splhack).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152933/new/

https://reviews.llvm.org/D152933

Files:
  lldb/source/Plugins/Platform/Android/CMakeLists.txt
  lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp
  lldb/source/Plugins/Platform/Android/PlatformAndroid.h
  lldb/source/Plugins/Platform/Android/PlatformAndroidProperties.td
  lldb/unittests/Platform/Android/PlatformAndroidTest.cpp

Index: lldb/unittests/Platform/Android/PlatformAndroidTest.cpp
===
--- lldb/unittests/Platform/Android/PlatformAndroidTest.cpp
+++ lldb/unittests/Platform/Android/PlatformAndroidTest.cpp
@@ -49,6 +49,7 @@
   }
 
   MOCK_METHOD1(GetAdbClient, AdbClientUP(Status ));
+  MOCK_METHOD0(GetPropertyPackageName, llvm::StringRef());
 };
 
 } // namespace
@@ -112,6 +113,32 @@
   .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, 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());
+}
+
 TEST_F(PlatformAndroidTest, GetFileWithNormalFile) {
   auto sync_service = new MockSyncService();
   EXPECT_CALL(*sync_service, Stat(FileSpec("/data/local/tmp/test"), _, _, _))
@@ -164,3 +191,40 @@
   FileSpec())
   .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(;
+
+  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)
+  .WillOnce(Return(Status()));
+
+  EXPECT_CALL(*this, GetPropertyPackageName())
+  .Times(1)
+  .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());
+}
Index: lldb/source/Plugins/Platform/Android/PlatformAndroidProperties.td
===
--- /dev/null
+++ lldb/source/Plugins/Platform/Android/PlatformAndroidProperties.td
@@ -0,0 +1,9 @@
+include "../../../../include/lldb/Core/PropertiesBase.td"
+
+let Definition = "android" in {
+  def PlatformPackageName: Property<"package-name", "String">,
+Global,
+DefaultStringValue<"">,
+Desc<"Specify package name to run adb shell command with 'run-as' as the "
+ "package user when necessary (e.g. to get file with 'cat' and 'dd').">;
+}
Index: lldb/source/Plugins/Platform/Android/PlatformAndroid.h
===
--- lldb/source/Plugins/Platform/Android/PlatformAndroid.h
+++ lldb/source/Plugins/Platform/Android/PlatformAndroid.h
@@ -30,6 +30,8 @@
   // lldb_private::PluginInterface functions
   static lldb::PlatformSP CreateInstance(bool force, const ArchSpec *arch);
 
+  static void DebuggerInitialize(lldb_private::Debugger );
+
   static llvm::StringRef GetPluginNameStatic(bool is_host) {
 return is_host ? Platform::GetHostPlatformName() : "remote-android";
   }
@@ -73,6 +75,10 @@
   typedef std::unique_ptr AdbClientUP;
   virtual AdbClientUP GetAdbClient(Status );
 
+  virtual llvm::StringRef GetPropertyPackageName();
+
+  std::string GetRunAs();
+
 private:
   AdbClient::SyncService *GetSyncService(Status );
 
Index:

[Lldb-commits] [lldb] fabd16c - [lldb][Android] Add platform.plugin.remote-android.package-name

2023-06-20 Thread Kazuki Sakamoto via lldb-commits 

Author: Kazuki Sakamoto
Date: 2023-06-20T16:17:21-07:00
New Revision: fabd16c7460e6cfae3106155ec79142bfa54db2e

URL: 
https://github.com/llvm/llvm-project/commit/fabd16c7460e6cfae3106155ec79142bfa54db2e
DIFF: 
https://github.com/llvm/llvm-project/commit/fabd16c7460e6cfae3106155ec79142bfa54db2e.diff

LOG: [lldb][Android] Add platform.plugin.remote-android.package-name

When LLDB fails to pull file from a package directory due to security
constraint, user needs to set the package name to
'platform.plugin.remote-android.package-name' property to run shell commands
as the package user. (e.g. to get file with 'cat' and 'dd').

https://cs.android.com/android/platform/superproject/+/master:
system/core/run-as/run-as.cpp;l=39-61;
drc=4a77a84a55522a3b122f9c63ef0d0b8a6a131627

Differential Revision: https://reviews.llvm.org/D152933

Added: 
lldb/source/Plugins/Platform/Android/PlatformAndroidProperties.td

Modified: 
lldb/source/Plugins/Platform/Android/CMakeLists.txt
lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp
lldb/source/Plugins/Platform/Android/PlatformAndroid.h
lldb/unittests/Platform/Android/PlatformAndroidTest.cpp

Removed: 




diff  --git a/lldb/source/Plugins/Platform/Android/CMakeLists.txt 
b/lldb/source/Plugins/Platform/Android/CMakeLists.txt
index 5abb51a0b94ac..518041b6cb316 100644
--- a/lldb/source/Plugins/Platform/Android/CMakeLists.txt
+++ b/lldb/source/Plugins/Platform/Android/CMakeLists.txt
@@ -1,3 +1,11 @@
+lldb_tablegen(PlatformAndroidProperties.inc -gen-lldb-property-defs
+  SOURCE PlatformAndroidProperties.td
+  TARGET LLDBPluginPlatformAndroidPropertiesGen)
+
+lldb_tablegen(PlatformAndroidPropertiesEnum.inc -gen-lldb-property-enum-defs
+  SOURCE PlatformAndroidProperties.td
+  TARGET LLDBPluginPlatformAndroidPropertiesEnumGen)
+
 add_lldb_library(lldbPluginPlatformAndroid PLUGIN
   AdbClient.cpp
   PlatformAndroid.cpp
@@ -11,3 +19,7 @@ add_lldb_library(lldbPluginPlatformAndroid PLUGIN
   LINK_COMPONENTS
 Support
   )
+
+add_dependencies(lldbPluginPlatformAndroid
+  LLDBPluginPlatformAndroidPropertiesGen
+  LLDBPluginPlatformAndroidPropertiesEnumGen)

diff  --git a/lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp 
b/lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp
index 36ca784d5ebf1..a1d2924203f13 100644
--- a/lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp
+++ b/lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp
@@ -29,10 +29,36 @@ using namespace std::chrono;
 
 LLDB_PLUGIN_DEFINE(PlatformAndroid)
 
-static uint32_t g_initialize_count = 0;
-static const unsigned int g_android_default_cache_size =
+namespace {
+
+#define LLDB_PROPERTIES_android
+#include "PlatformAndroidProperties.inc"
+
+enum {
+#define LLDB_PROPERTIES_android
+#include "PlatformAndroidPropertiesEnum.inc"
+};
+
+class PluginProperties : public Properties {
+public:
+  PluginProperties() {
+m_collection_sp = std::make_shared(
+ConstString(PlatformAndroid::GetPluginNameStatic(false)));
+m_collection_sp->Initialize(g_android_properties);
+  }
+};
+
+static PluginProperties () {
+  static PluginProperties g_settings;
+  return g_settings;
+}
+
+uint32_t g_initialize_count = 0;
+const unsigned int g_android_default_cache_size =
 2048; // Fits inside 4k adb packet.
 
+} // end of anonymous namespace
+
 void PlatformAndroid::Initialize() {
   PlatformLinux::Initialize();
 
@@ -45,7 +71,7 @@ void PlatformAndroid::Initialize() {
 PluginManager::RegisterPlugin(
 PlatformAndroid::GetPluginNameStatic(false),
 PlatformAndroid::GetPluginDescriptionStatic(false),
-PlatformAndroid::CreateInstance);
+PlatformAndroid::CreateInstance, PlatformAndroid::DebuggerInitialize);
   }
 }
 
@@ -128,6 +154,16 @@ PlatformSP PlatformAndroid::CreateInstance(bool force, 
const ArchSpec *arch) {
   return PlatformSP();
 }
 
+void PlatformAndroid::DebuggerInitialize(Debugger ) {
+  if (!PluginManager::GetSettingForPlatformPlugin(
+  debugger, ConstString(GetPluginNameStatic(false {
+PluginManager::CreateSettingForPlatformPlugin(
+debugger, GetGlobalProperties().GetValueProperties(),
+"Properties for the Android platform plugin.",
+/*is_global_property=*/true);
+  }
+}
+
 PlatformAndroid::PlatformAndroid(bool is_host)
 : PlatformLinux(is_host), m_sdk_version(0) {}
 
@@ -206,7 +242,8 @@ Status PlatformAndroid::GetFile(const FileSpec ,
 return error;
 
   char cmd[PATH_MAX];
-  snprintf(cmd, sizeof(cmd), "cat '%s'", source_file.c_str());
+  snprintf(cmd, sizeof(cmd), "%scat '%s'", GetRunAs().c_str(),
+   source_file.c_str());
 
   return adb->ShellToFile(cmd, minutes(1), destination);
 }
@@ -260,9 +297,9 @@ Status PlatformAndroid::DownloadModuleSlice(const FileSpec 
_file_spec,
   // Use 'shell dd' to download the file slice with the offset and size.
   char cmd[PATH_MAX];
   snprintf(cmd,

[Lldb-commits] [PATCH] D152855: [lldb][Android] Add PlatformAndroidTest

2023-06-20 Thread Kazuki Sakamoto via Phabricator via lldb-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rG49f55b025d81: [lldb][Android] Add PlatformAndroidTest 
(authored by splhack).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152855/new/

https://reviews.llvm.org/D152855

Files:
  lldb/source/Plugins/Platform/Android/AdbClient.h
  lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp
  lldb/source/Plugins/Platform/Android/PlatformAndroid.h
  lldb/unittests/Platform/Android/CMakeLists.txt
  lldb/unittests/Platform/Android/PlatformAndroidTest.cpp

Index: lldb/unittests/Platform/Android/PlatformAndroidTest.cpp
===
--- /dev/null
+++ lldb/unittests/Platform/Android/PlatformAndroidTest.cpp
@@ -0,0 +1,166 @@
+//===-- 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 "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"
+
+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::unique_ptr()) {}
+
+  MOCK_METHOD2(PullFile,
+   Status(const FileSpec _file, const FileSpec _file));
+  MOCK_METHOD4(Stat, Status(const FileSpec _file, uint32_t ,
+uint32_t , uint32_t ));
+};
+
+typedef std::unique_ptr SyncServiceUP;
+
+class MockAdbClient : public AdbClient {
+public:
+  explicit MockAdbClient() : AdbClient("mock") {}
+
+  MOCK_METHOD3(ShellToFile,
+   Status(const char *command, std::chrono::milliseconds timeout,
+  const FileSpec _file_spec));
+  MOCK_METHOD1(GetSyncService, SyncServiceUP(Status ));
+};
+
+class PlatformAndroidTest : public PlatformAndroid, public ::testing::Test {
+public:
+  PlatformAndroidTest() : PlatformAndroid(false) {
+m_remote_platform_sp = PlatformSP(new PlatformAndroidRemoteGDBServer());
+  }
+
+  MOCK_METHOD1(GetAdbClient, AdbClientUP(Status ));
+};
+
+} // namespace
+
+TEST_F(PlatformAndroidTest, DownloadModuleSliceWithAdbClientError) {
+  EXPECT_CALL(*this, GetAdbClient(_))
+  .Times(1)
+  .WillOnce(DoAll(WithArg<0>([](auto ) {
+arg = Status("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()));
+
+  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());
+}
+
+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(AdbClientUP(adb_client;
+
+  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,

[Lldb-commits] [lldb] 49f55b0 - [lldb][Android] Add PlatformAndroidTest

2023-06-20 Thread Kazuki Sakamoto via lldb-commits 

Author: Kazuki Sakamoto
Date: 2023-06-20T16:15:02-07:00
New Revision: 49f55b025d81823fa7e2b287c8930a8304483e5a

URL: 
https://github.com/llvm/llvm-project/commit/49f55b025d81823fa7e2b287c8930a8304483e5a
DIFF: 
https://github.com/llvm/llvm-project/commit/49f55b025d81823fa7e2b287c8930a8304483e5a.diff

LOG: [lldb][Android] Add PlatformAndroidTest

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

introduce PlatformAndroidTest with the capability of mocking adb client.

Differential Revision: https://reviews.llvm.org/D152855

Added: 
lldb/unittests/Platform/Android/PlatformAndroidTest.cpp

Modified: 
lldb/source/Plugins/Platform/Android/AdbClient.h
lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp
lldb/source/Plugins/Platform/Android/PlatformAndroid.h
lldb/unittests/Platform/Android/CMakeLists.txt

Removed: 




diff  --git a/lldb/source/Plugins/Platform/Android/AdbClient.h 
b/lldb/source/Plugins/Platform/Android/AdbClient.h
index 04ad3819f57f8..851c09957bd4a 100644
--- a/lldb/source/Plugins/Platform/Android/AdbClient.h
+++ b/lldb/source/Plugins/Platform/Android/AdbClient.h
@@ -36,20 +36,22 @@ class AdbClient {
 friend class AdbClient;
 
   public:
-~SyncService();
+virtual ~SyncService();
 
-Status PullFile(const FileSpec _file, const FileSpec _file);
+virtual Status PullFile(const FileSpec _file,
+const FileSpec _file);
 
 Status PushFile(const FileSpec _file, const FileSpec _file);
 
-Status Stat(const FileSpec _file, uint32_t , uint32_t ,
-uint32_t );
+virtual Status Stat(const FileSpec _file, uint32_t ,
+uint32_t , uint32_t );
 
 bool IsConnected() const;
 
-  private:
+  protected:
 explicit SyncService(std::unique_ptr &);
 
+  private:
 Status SendSyncRequest(const char *request_id, const uint32_t data_len,
const void *data);
 
@@ -78,7 +80,7 @@ class AdbClient {
   AdbClient();
   explicit AdbClient(const std::string _id);
 
-  ~AdbClient();
+  virtual ~AdbClient();
 
   const std::string () const;
 
@@ -96,10 +98,11 @@ class AdbClient {
   Status Shell(const char *command, std::chrono::milliseconds timeout,
std::string *output);
 
-  Status ShellToFile(const char *command, std::chrono::milliseconds timeout,
- const FileSpec _file_spec);
+  virtual Status ShellToFile(const char *command,
+ std::chrono::milliseconds timeout,
+ const FileSpec _file_spec);
 
-  std::unique_ptr GetSyncService(Status );
+  virtual std::unique_ptr GetSyncService(Status );
 
   Status SwitchDeviceTransport();
 

diff  --git a/lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp 
b/lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp
index 4b09e8b253412..36ca784d5ebf1 100644
--- a/lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp
+++ b/lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp
@@ -201,12 +201,14 @@ Status PlatformAndroid::GetFile(const FileSpec ,
 
   // mode == 0 can signify that adbd cannot access the file due security
   // constraints - try "cat ..." as a fallback.
-  AdbClient adb(m_device_id);
+  AdbClientUP adb(GetAdbClient(error));
+  if (error.Fail())
+return error;
 
   char cmd[PATH_MAX];
   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 ,
@@ -250,7 +252,10 @@ Status PlatformAndroid::DownloadModuleSlice(const FileSpec 
_file_spec,
   if (pos != std::string::npos)
 source_file = source_file.substr(0, pos);
 
-  AdbClient adb(m_device_id);
+  Status error;
+  AdbClientUP adb(GetAdbClient(error));
+  if (error.Fail())
+return error;
 
   // Use 'shell dd' to download the file slice with the offset and size.
   char cmd[PATH_MAX];
@@ -259,7 +264,7 @@ Status PlatformAndroid::DownloadModuleSlice(const FileSpec 
_file_spec,
"skip=%" PRIu64 " count=%" PRIu64 " status=none",
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() {
@@ -283,9 +288,12 @@ uint32_t PlatformAndroid::GetSdkVersion() {
 return m_sdk_version;
 
   std::string version_string;
-  AdbClient adb(m_device_id);
-  Status error =
-  adb.Shell("getprop ro.build.version.sdk", seconds(5), _string);
+  Status error;
+  AdbClientUP adb(GetAdbClient(error));
+  if (error.Fail())
+return 0;
+  error =
+  adb->Shell("getprop ro.build.version.sdk", seconds(5), _string);
   version_string = llvm::StringRef(version_string).trim().str();
 
   if (error.Fail() || version_string.empty()) {
@@ -321,10 +329,13 @@

[Lldb-commits] [PATCH] D152759: [lldb][Android] Support zip .so file

2023-06-20 Thread Kazuki Sakamoto via Phabricator via lldb-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rG12dee9d3cd76: [lldb][Android] Support zip .so file (authored 
by splhack).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152759/new/

https://reviews.llvm.org/D152759

Files:
  lldb/include/lldb/Host/common/ZipFileResolver.h
  lldb/include/lldb/Utility/ZipFile.h
  lldb/source/Host/CMakeLists.txt
  lldb/source/Host/common/ZipFileResolver.cpp
  lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
  lldb/source/Utility/CMakeLists.txt
  lldb/source/Utility/ZipFile.cpp
  lldb/unittests/Host/CMakeLists.txt
  lldb/unittests/Host/common/CMakeLists.txt
  lldb/unittests/Host/common/Inputs/zip-test.zip
  lldb/unittests/Host/common/ZipFileResolverTest.cpp

Index: lldb/unittests/Host/common/ZipFileResolverTest.cpp
===
--- /dev/null
+++ lldb/unittests/Host/common/ZipFileResolverTest.cpp
@@ -0,0 +1,72 @@
+//===-- ZipFileResolverTest.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/Host/common/ZipFileResolver.h"
+#include "TestingSupport/SubsystemRAII.h"
+#include "TestingSupport/TestUtilities.h"
+#include "gtest/gtest.h"
+
+using namespace lldb_private;
+using namespace llvm;
+
+namespace {
+class ZipFileResolverTest : public ::testing::Test {
+  SubsystemRAII subsystems;
+};
+
+std::string TestZipPath() {
+  FileSpec zip_spec(GetInputFilePath("zip-test.zip"));
+  FileSystem::Instance().Resolve(zip_spec);
+  return zip_spec.GetPath();
+}
+} // namespace
+
+TEST_F(ZipFileResolverTest, ResolveSharedLibraryPathWithNormalFile) {
+  const FileSpec file_spec("/system/lib64/libtest.so");
+
+  ZipFileResolver::FileKind file_kind;
+  std::string file_path;
+  lldb::offset_t file_offset;
+  lldb::offset_t file_size;
+  ASSERT_TRUE(ZipFileResolver::ResolveSharedLibraryPath(
+  file_spec, file_kind, file_path, file_offset, file_size));
+
+  EXPECT_EQ(file_kind, ZipFileResolver::FileKind::eFileKindNormal);
+  EXPECT_EQ(file_path, file_spec.GetPath());
+  EXPECT_EQ(file_offset, 0UL);
+  EXPECT_EQ(file_size, 0UL);
+}
+
+TEST_F(ZipFileResolverTest, ResolveSharedLibraryPathWithZipMissing) {
+  const std::string zip_path = TestZipPath();
+  const FileSpec file_spec(zip_path + "!/lib/arm64-v8a/libmissing.so");
+
+  ZipFileResolver::FileKind file_kind;
+  std::string file_path;
+  lldb::offset_t file_offset;
+  lldb::offset_t file_size;
+  ASSERT_FALSE(ZipFileResolver::ResolveSharedLibraryPath(
+  file_spec, file_kind, file_path, file_offset, file_size));
+}
+
+TEST_F(ZipFileResolverTest, ResolveSharedLibraryPathWithZipExisting) {
+  const std::string zip_path = TestZipPath();
+  const FileSpec file_spec(zip_path + "!/lib/arm64-v8a/libzip-test.so");
+
+  ZipFileResolver::FileKind file_kind;
+  std::string file_path;
+  lldb::offset_t file_offset;
+  lldb::offset_t file_size;
+  ASSERT_TRUE(ZipFileResolver::ResolveSharedLibraryPath(
+  file_spec, file_kind, file_path, file_offset, file_size));
+
+  EXPECT_EQ(file_kind, ZipFileResolver::FileKind::eFileKindZip);
+  EXPECT_EQ(file_path, zip_path);
+  EXPECT_EQ(file_offset, 4096UL);
+  EXPECT_EQ(file_size, 3600UL);
+}
Index: lldb/unittests/Host/common/CMakeLists.txt
===
--- /dev/null
+++ lldb/unittests/Host/common/CMakeLists.txt
@@ -0,0 +1,15 @@
+set (FILES
+  ZipFileResolverTest.cpp
+)
+
+add_lldb_unittest(HostCommonTests
+  ${FILES}
+  LINK_LIBS
+lldbHost
+lldbUtilityHelpers
+  )
+
+set(test_inputs
+  zip-test.zip
+  )
+add_unittest_inputs(HostCommonTests "${test_inputs}")
Index: lldb/unittests/Host/CMakeLists.txt
===
--- lldb/unittests/Host/CMakeLists.txt
+++ lldb/unittests/Host/CMakeLists.txt
@@ -38,3 +38,5 @@
 LLVMTestingSupport
 LLVMTargetParser
   )
+
+add_subdirectory(common)
Index: lldb/source/Utility/ZipFile.cpp
===
--- /dev/null
+++ lldb/source/Utility/ZipFile.cpp
@@ -0,0 +1,180 @@
+//===-- ZipFile.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/ZipFile.h"
+#include "lldb/Utility/DataBuffer.h"
+#include "lldb/Utility/FileSpec.h"
+#include

[Lldb-commits] [lldb] 12dee9d - [lldb][Android] Support zip .so file

2023-06-20 Thread Kazuki Sakamoto via lldb-commits 

Author: Kazuki Sakamoto
Date: 2023-06-20T15:21:46-07:00
New Revision: 12dee9d3cd762d9754e2adadffa13c1cce85cf07

URL: 
https://github.com/llvm/llvm-project/commit/12dee9d3cd762d9754e2adadffa13c1cce85cf07
DIFF: 
https://github.com/llvm/llvm-project/commit/12dee9d3cd762d9754e2adadffa13c1cce85cf07.diff

LOG: [lldb][Android] Support zip .so file

In Android API level 23 and above, dynamic loader is able to load .so file
directly from APK, which is zip file.
https://android.googlesource.com/platform/bionic/+/master/
android-changes-for-ndk-developers.md#
opening-shared-libraries-directly-from-an-apk

The .so file is page aligned and uncompressed, so
ObjectFileELF::GetModuleSpecifications works with .so file offset and size
directly from zip file without extracting it. (D152757)

GDBRemoteCommunicationServerCommon::GetModuleInfo returns a module spec to LLDB
with "zip_path!/so_path" file spec, which is passed through from Android
dynamic loader, and the .so file offset and size.

PlatformAndroid::DownloadModuleSlice uses 'shell dd' to download the .so file
slice from the zip file with the .so file offset and size.

Differential Revision: https://reviews.llvm.org/D152759

Added: 
lldb/include/lldb/Host/common/ZipFileResolver.h
lldb/include/lldb/Utility/ZipFile.h
lldb/source/Host/common/ZipFileResolver.cpp
lldb/source/Utility/ZipFile.cpp
lldb/unittests/Host/common/CMakeLists.txt
lldb/unittests/Host/common/Inputs/zip-test.zip
lldb/unittests/Host/common/ZipFileResolverTest.cpp

Modified: 
lldb/source/Host/CMakeLists.txt
lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp

lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
lldb/source/Utility/CMakeLists.txt
lldb/unittests/Host/CMakeLists.txt

Removed: 




diff  --git a/lldb/include/lldb/Host/common/ZipFileResolver.h 
b/lldb/include/lldb/Host/common/ZipFileResolver.h
new file mode 100644
index 0..ec7151f3e57c9
--- /dev/null
+++ b/lldb/include/lldb/Host/common/ZipFileResolver.h
@@ -0,0 +1,40 @@
+//===-- ZipFileResolver.h ---*- C++ 
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLDB_HOST_ZIPFILERESOLVER_H
+#define LLDB_HOST_ZIPFILERESOLVER_H
+
+#include "lldb/lldb-private.h"
+
+namespace lldb_private {
+
+/// In Android API level 23 and above, bionic dynamic linker is able to load
+/// .so file directly from APK or .zip file. This is a utility class to resolve
+/// the file spec in order to get the zip path and the .so file offset and size
+/// if the file spec contains "zip_path!/so_path".
+/// https://android.googlesource.com/platform/bionic/+/master/
+/// android-changes-for-ndk-developers.md#
+/// opening-shared-libraries-directly-from-an-apk
+class ZipFileResolver {
+public:
+  enum FileKind {
+eFileKindInvalid = 0,
+eFileKindNormal,
+eFileKindZip,
+  };
+
+  static bool ResolveSharedLibraryPath(const FileSpec _spec,
+   FileKind _kind,
+   std::string _path,
+   lldb::offset_t _file_offset,
+   lldb::offset_t _file_size);
+};
+
+} // end of namespace lldb_private
+
+#endif // LLDB_HOST_ZIPFILERESOLVER_H

diff  --git a/lldb/include/lldb/Utility/ZipFile.h 
b/lldb/include/lldb/Utility/ZipFile.h
new file mode 100644
index 0..cddd6c44ca638
--- /dev/null
+++ b/lldb/include/lldb/Utility/ZipFile.h
@@ -0,0 +1,30 @@
+//===-- ZipFile.h ---*- C++ 
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLDB_UTILITY_ZIPFILE_H
+#define LLDB_UTILITY_ZIPFILE_H
+
+#include "lldb/lldb-private.h"
+
+namespace lldb_private {
+
+/// In Android API level 23 and above, bionic dynamic linker is able to load
+/// .so file directly from APK or .zip file. This is a utility class to find
+/// .so file offset and size from zip file.
+/// https://android.googlesource.com/platform/bionic/+/master/
+/// android-changes-for-ndk-developers.md#
+/// opening-shared-libraries-directly-from-an-apk
+class ZipFile {
+public:
+  static bool Find(lldb::DataBufferSP zip_data, const llvm::StringRef 
file_path,
+   lldb::offset_t _offset, lldb::offset_t _size);
+};
+
+} // end of namespace lldb_private
+
+#endif // LLDB_UTILITY_ZIPFILE_H

diff  --git

[Lldb-commits] [PATCH] D152757: [lldb][ObjectFileELF] Set ModuleSpec file offset and size

2023-06-20 Thread Kazuki Sakamoto via Phabricator via lldb-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rGb0b9605a544d: [lldb][ObjectFileELF] Set ModuleSpec file 
offset and size (authored by splhack).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152757/new/

https://reviews.llvm.org/D152757

Files:
  lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
  lldb/unittests/ObjectFile/ELF/CMakeLists.txt
  lldb/unittests/ObjectFile/ELF/Inputs/liboffset-test.so
  lldb/unittests/ObjectFile/ELF/Inputs/offset-test.bin
  lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp


Index: lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp
===
--- lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp
+++ lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp
@@ -156,6 +156,39 @@
   EXPECT_EQ(Spec.GetUUID(), Uuid);
 }
 
+TEST_F(ObjectFileELFTest, GetModuleSpecifications_OffsetSizeWithNormalFile) {
+  std::string SO = GetInputFilePath("liboffset-test.so");
+  ModuleSpecList Specs;
+  ASSERT_EQ(1u, ObjectFile::GetModuleSpecifications(FileSpec(SO), 0, 0, 
Specs));
+  ModuleSpec Spec;
+  ASSERT_TRUE(Specs.GetModuleSpecAtIndex(0, Spec)) ;
+  UUID Uuid;
+  Uuid.SetFromStringRef("7D6E4738");
+  EXPECT_EQ(Spec.GetUUID(), Uuid);
+  EXPECT_EQ(Spec.GetObjectOffset(), 0UL);
+  EXPECT_EQ(Spec.GetObjectSize(), 3600UL);
+  EXPECT_EQ(FileSystem::Instance().GetByteSize(FileSpec(SO)), 3600UL);
+}
+
+TEST_F(ObjectFileELFTest, GetModuleSpecifications_OffsetSizeWithOffsetFile) {
+  // The contents of offset-test.bin are
+  // -0-1023: \0
+  // - 1024-4623: liboffset-test.so (offset: 1024, size: 3600, CRC32: 7D6E4738)
+  // - 4624-4639: \0
+  std::string SO = GetInputFilePath("offset-test.bin");
+  ModuleSpecList Specs;
+  ASSERT_EQ(
+  1u, ObjectFile::GetModuleSpecifications(FileSpec(SO), 1024, 3600, 
Specs));
+  ModuleSpec Spec;
+  ASSERT_TRUE(Specs.GetModuleSpecAtIndex(0, Spec)) ;
+  UUID Uuid;
+  Uuid.SetFromStringRef("7D6E4738");
+  EXPECT_EQ(Spec.GetUUID(), Uuid);
+  EXPECT_EQ(Spec.GetObjectOffset(), 1024UL);
+  EXPECT_EQ(Spec.GetObjectSize(), 3600UL);
+  EXPECT_EQ(FileSystem::Instance().GetByteSize(FileSpec(SO)), 4640UL);
+}
+
 TEST_F(ObjectFileELFTest, GetSymtab_NoSymEntryPointArmThumbAddressClass) {
   /*
   // nosym-entrypoint-arm-thumb.s
Index: lldb/unittests/ObjectFile/ELF/CMakeLists.txt
===
--- lldb/unittests/ObjectFile/ELF/CMakeLists.txt
+++ lldb/unittests/ObjectFile/ELF/CMakeLists.txt
@@ -11,5 +11,7 @@
 
 set(test_inputs
   early-section-headers.so
+  liboffset-test.so
+  offset-test.bin
   )
 add_unittest_inputs(ObjectFileELFTests "${test_inputs}")
Index: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
===
--- lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -555,6 +555,14 @@
 if (header.Parse(data, _offset)) {
   if (data_sp) {
 ModuleSpec spec(file);
+// In Android API level 23 and above, bionic dynamic linker is able to
+// load .so file directly from zip file. In that case, .so file is
+// page aligned and uncompressed, and this module spec should retain 
the
+// .so file offset and file size to pass through the information from
+// lldb-server to LLDB. For normal file, file_offset should be 0,
+// length should be the size of the file.
+spec.SetObjectOffset(file_offset);
+spec.SetObjectSize(length);
 
 const uint32_t sub_type = subTypeFromElfHeader(header);
 spec.GetArchitecture().SetArchitecture(
@@ -586,8 +594,12 @@
   __FUNCTION__, file.GetPath().c_str());
   }
 
+  // When ELF file does not contain GNU build ID, the later code will
+  // calculate CRC32 with this data_sp file_offset and length. It is
+  // important for Android zip .so file, which is a slice of a file,
+  // to not access the outside of the file slice range.
   if (data_sp->GetByteSize() < length)
-data_sp = MapFileData(file, -1, file_offset);
+data_sp = MapFileData(file, length, file_offset);
   if (data_sp)
 data.SetData(data_sp);
   // In case there is header extension in the section #0, the header we


Index: lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp
===
--- lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp
+++ lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp
@@ -156,6 +156,39 @@
   EXPECT_EQ(Spec.GetUUID(), Uuid);
 }
 
+TEST_F(ObjectFileELFTest, GetModuleSpecifications_OffsetSizeWithNormalFile) {
+  std::string SO = GetInputFilePath("liboffset-test.so");
+  ModuleSpecList Specs;
+  ASSERT_EQ(1u, ObjectFile::GetModuleSpecifications(FileSpec(SO), 0, 0, Specs));
+

[Lldb-commits] [lldb] b0b9605 - [lldb][ObjectFileELF] Set ModuleSpec file offset and size

2023-06-20 Thread Kazuki Sakamoto via lldb-commits 

Author: Kazuki Sakamoto
Date: 2023-06-20T15:00:09-07:00
New Revision: b0b9605a544dbf26940a48777af401a419f4e4f1

URL: 
https://github.com/llvm/llvm-project/commit/b0b9605a544dbf26940a48777af401a419f4e4f1
DIFF: 
https://github.com/llvm/llvm-project/commit/b0b9605a544dbf26940a48777af401a419f4e4f1.diff

LOG: [lldb][ObjectFileELF] Set ModuleSpec file offset and size

In Android API level 23 and above, dynamic loader is able to load .so file
directly from APK.
https://android.googlesource.com/platform/bionic/+/master/
android-changes-for-ndk-developers.md#
opening-shared-libraries-directly-from-an-apk

ObjectFileELF::GetModuleSpecifications will load a .so file, which is page
aligned and uncompressed, directly from a zip file. However it does not
set the .so file offset and size to the ModuleSpec. Also crc32 calculation
uses more data than the .so file size.

Set the .so file offset and size to the ModuleSpec, and set the size to
MapFileData length argument. For normal file, file_offset should be zero,
and length should be the size of the file.

Differential Revision: https://reviews.llvm.org/D152757

Added: 
lldb/unittests/ObjectFile/ELF/Inputs/liboffset-test.so
lldb/unittests/ObjectFile/ELF/Inputs/offset-test.bin

Modified: 
lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
lldb/unittests/ObjectFile/ELF/CMakeLists.txt
lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp

Removed: 




diff  --git a/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp 
b/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
index 82171dee221ec..727c1fa96af64 100644
--- a/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ b/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -555,6 +555,14 @@ size_t ObjectFileELF::GetModuleSpecifications(
 if (header.Parse(data, _offset)) {
   if (data_sp) {
 ModuleSpec spec(file);
+// In Android API level 23 and above, bionic dynamic linker is able to
+// load .so file directly from zip file. In that case, .so file is
+// page aligned and uncompressed, and this module spec should retain 
the
+// .so file offset and file size to pass through the information from
+// lldb-server to LLDB. For normal file, file_offset should be 0,
+// length should be the size of the file.
+spec.SetObjectOffset(file_offset);
+spec.SetObjectSize(length);
 
 const uint32_t sub_type = subTypeFromElfHeader(header);
 spec.GetArchitecture().SetArchitecture(
@@ -586,8 +594,12 @@ size_t ObjectFileELF::GetModuleSpecifications(
   __FUNCTION__, file.GetPath().c_str());
   }
 
+  // When ELF file does not contain GNU build ID, the later code will
+  // calculate CRC32 with this data_sp file_offset and length. It is
+  // important for Android zip .so file, which is a slice of a file,
+  // to not access the outside of the file slice range.
   if (data_sp->GetByteSize() < length)
-data_sp = MapFileData(file, -1, file_offset);
+data_sp = MapFileData(file, length, file_offset);
   if (data_sp)
 data.SetData(data_sp);
   // In case there is header extension in the section #0, the header we

diff  --git a/lldb/unittests/ObjectFile/ELF/CMakeLists.txt 
b/lldb/unittests/ObjectFile/ELF/CMakeLists.txt
index 4c59ca109a57e..68a78bab543c1 100644
--- a/lldb/unittests/ObjectFile/ELF/CMakeLists.txt
+++ b/lldb/unittests/ObjectFile/ELF/CMakeLists.txt
@@ -11,5 +11,7 @@ add_lldb_unittest(ObjectFileELFTests
 
 set(test_inputs
   early-section-headers.so
+  liboffset-test.so
+  offset-test.bin
   )
 add_unittest_inputs(ObjectFileELFTests "${test_inputs}")

diff  --git a/lldb/unittests/ObjectFile/ELF/Inputs/liboffset-test.so 
b/lldb/unittests/ObjectFile/ELF/Inputs/liboffset-test.so
new file mode 100644
index 0..3aa98db24991f
Binary files /dev/null and 
b/lldb/unittests/ObjectFile/ELF/Inputs/liboffset-test.so 
diff er

diff  --git a/lldb/unittests/ObjectFile/ELF/Inputs/offset-test.bin 
b/lldb/unittests/ObjectFile/ELF/Inputs/offset-test.bin
new file mode 100644
index 0..7de9ecf091e72
Binary files /dev/null and 
b/lldb/unittests/ObjectFile/ELF/Inputs/offset-test.bin 
diff er

diff  --git a/lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp 
b/lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp
index 0381870f4e0a5..80abc5b80f84d 100644
--- a/lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp
+++ b/lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp
@@ -156,6 +156,39 @@ TEST_F(ObjectFileELFTest, 
GetModuleSpecifications_EarlySectionHeaders) {
   EXPECT_EQ(Spec.GetUUID(), Uuid);
 }
 
+TEST_F(ObjectFileELFTest, GetModuleSpecifications_OffsetSizeWithNormalFile) {
+  std::string SO = GetInputFilePath("liboffset-test.so");
+  ModuleSpecList Specs;
+  ASSERT_EQ(1u,

[Lldb-commits] [PATCH] D152855: [lldb][Android] Add PlatformAndroidTest

2023-06-20 Thread Kazuki Sakamoto via Phabricator via lldb-commits 
splhack added a comment.

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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152855/new/

https://reviews.llvm.org/D152855

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D152855: [lldb][Android] Add PlatformAndroidTest

2023-06-20 Thread Greg Clayton via Phabricator via lldb-commits 
clayborg accepted this revision.
clayborg added a comment.
This revision is now accepted and ready to land.

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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152855/new/

https://reviews.llvm.org/D152855

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D152855: [lldb][Android] Add PlatformAndroidTest

2023-06-20 Thread Greg Clayton via Phabricator via lldb-commits 
clayborg added inline comments.



Comment at: lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp:204
   // constraints - try "cat ..." as a fallback.
-  AdbClient adb(m_device_id);
+  AdbClientUP adb(GetAdbClient(error));
+  if (error.Fail())

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



Comment at: lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp:404
 
+PlatformAndroid::AdbClientUP PlatformAndroid::GetAdbClient(Status ) {
+  AdbClientUP adb(std::make_unique(m_device_id));

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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152855/new/

https://reviews.llvm.org/D152855

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits