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

[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] 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] 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 "llvm/

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

2023-06-16 Thread Kazuki Sakamoto via Phabricator via lldb-commits

splhack updated this revision to Diff 532165.
splhack added a comment.

rebase


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 "llvm/Support/Endian.h"
+
+using namespace lldb_private;
+using namespace llvm::support;
+
+na

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

2023-06-15 Thread Kazuki Sakamoto via Phabricator via lldb-commits

splhack updated this revision to Diff 531992.
splhack added a comment.

rebase


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 "llvm/Support/Endian.h"
+
+using namespace lldb_private;
+using namespace llvm::support;
+
+na

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

2023-06-15 Thread Kazuki Sakamoto via Phabricator via lldb-commits

splhack updated this revision to Diff 531974.
splhack added a comment.

Added eFileKindInvalid with assert


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 "llvm/Support/Endian.h"
+
+using namespace lldb_private;
+using na

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

2023-06-15 Thread Kazuki Sakamoto via Phabricator via lldb-commits

splhack added inline comments.



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp:1336
+  // "zip_path!/so_path". Resolve the zip file path, .so file offset and size.
+  ZipFileResolver::FileKind file_kind;
+  std::string file_path;

clayborg wrote:
> might be good to init this to an invalid value in case code changes over time?
yup, sounds good. will add with assert.


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] D152759: [lldb][Android] Support zip .so file

2023-06-15 Thread Kazuki Sakamoto via Phabricator via lldb-commits

splhack updated this revision to Diff 531969.
splhack added a comment.

Rename `ResolveBionicPath` to `ResolveSharedLibraryPath`.


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 "llvm/Support/Endian.h"
+
+using namespace

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

2023-06-15 Thread Greg Clayton via Phabricator via lldb-commits

clayborg added inline comments.



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp:1336
+  // "zip_path!/so_path". Resolve the zip file path, .so file offset and size.
+  ZipFileResolver::FileKind file_kind;
+  std::string file_path;

might be good to init this to an invalid value in case code changes over time?


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] D152759: [lldb][Android] Support zip .so file

2023-06-15 Thread Kazuki Sakamoto via Phabricator via lldb-commits

splhack added inline comments.



Comment at: lldb/include/lldb/Host/common/ZipFileResolver.h:30
+
+  static bool ResolveBionicPath(const FileSpec &file_spec, FileKind &file_kind,
+std::string &file_path,

mib wrote:
> mib wrote:
> > This function name sounds like the the bionic linker is in the zip file 
> > which is not the case IIUC. I'm fine with adding a zip file resolver in 
> > lldb but I'd prefer if it had a generic name, otherwise, this should be a 
> > plugin.
> what about this instead ?
Yes, should be good. will update this diff.

(Why it had "Bionic" because I thought the path encoding "zip_path!/lib_path" 
is bionic specific.)



Comment at: lldb/source/Utility/ZipFile.cpp:19-21
+// Zip headers.
+// https://pkware.cachefly.net/webdocs/casestudies/APPNOTE.TXT
+

mib wrote:
> Did you just copy & past the this file from somewhere else of did you 
> implement it yourself ?
I implemented this code.

logic
- Linear search the end of central directory record from the file end because 
it is located at the end of the file with comment (64KB max)
- Linear search the file from the central directory records that is pointed by 
the end of central directory record.
- Get the file offset and size from the local file header that is pointed by 
the central directory record
- Use unaligned_uint16_t/unaligned_uint32_t since Zip header is 1 byte aligned. 


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] D152759: [lldb][Android] Support zip .so file

2023-06-15 Thread Med Ismail Bennani via Phabricator via lldb-commits

mib added inline comments.



Comment at: lldb/include/lldb/Host/common/ZipFileResolver.h:30
+
+  static bool ResolveBionicPath(const FileSpec &file_spec, FileKind &file_kind,
+std::string &file_path,

mib wrote:
> This function name sounds like the the bionic linker is in the zip file which 
> is not the case IIUC. I'm fine with adding a zip file resolver in lldb but 
> I'd prefer if it had a generic name, otherwise, this should be a plugin.
what about this instead ?


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] D152759: [lldb][Android] Support zip .so file

2023-06-15 Thread Med Ismail Bennani via Phabricator via lldb-commits

mib added inline comments.



Comment at: lldb/include/lldb/Host/common/ZipFileResolver.h:30
+
+  static bool ResolveBionicPath(const FileSpec &file_spec, FileKind &file_kind,
+std::string &file_path,

This function name sounds like the the bionic linker is in the zip file which 
is not the case IIUC. I'm fine with adding a zip file resolver in lldb but I'd 
prefer if it had a generic name, otherwise, this should be a plugin.



Comment at: lldb/source/Utility/ZipFile.cpp:19-21
+// Zip headers.
+// https://pkware.cachefly.net/webdocs/casestudies/APPNOTE.TXT
+

Did you just copy & past the this file from somewhere else of did you implement 
it yourself ?


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] D152759: [lldb][Android] Support zip .so file

2023-06-15 Thread Alex Langford via Phabricator via lldb-commits

bulbazord accepted this revision.
bulbazord added a comment.
This revision is now accepted and ready to land.

Thanks for reworking it! You may want to wait a little bit before landing so 
others have some time to look over it, but I don't see anything that should 
prevent this from going in.


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] D152759: [lldb][Android] Support zip .so file

2023-06-14 Thread Kazuki Sakamoto via Phabricator via lldb-commits

splhack updated this revision to Diff 531576.
splhack added a comment.

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


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, ResolveBionicPathWithNormalFile) {
+  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::ResolveBionicPath(
+  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, ResolveBionicPathWithZipMissing) {
+  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::ResolveBionicPath(
+  file_spec, file_kind, file_path, file_offset, file_size));
+}
+
+TEST_F(ZipFileResolverTest, ResolveBionicPathWithZipExisting) {
+  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::ResolveBionicPath(
+  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 "llvm/Support/Endian.h"
+
+using namespace lldb_private;
+using nam

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

2023-06-14 Thread Kazuki Sakamoto via Phabricator via lldb-commits

splhack updated this revision to Diff 531499.
splhack added a comment.

rebase


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, ResolveBionicPathWithNormalFile) {
+  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::ResolveBionicPath(
+  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, ResolveBionicPathWithZipMissing) {
+  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::ResolveBionicPath(
+  file_spec, file_kind, file_path, file_offset, file_size));
+}
+
+TEST_F(ZipFileResolverTest, ResolveBionicPathWithZipExisting) {
+  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::ResolveBionicPath(
+  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 "llvm/Support/Endian.h"
+
+using namespace lldb_private;
+using namespace llvm::support;
+
+namespace {
+
+// Zip headers.
+// https://p

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

2023-06-14 Thread Kazuki Sakamoto via Phabricator via lldb-commits

splhack updated this revision to Diff 531394.
splhack added a comment.

remove 'Depends on' from commit message.


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, ResolveBionicPathWithNormalFile) {
+  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::ResolveBionicPath(
+  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, ResolveBionicPathWithZipMissing) {
+  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::ResolveBionicPath(
+  file_spec, file_kind, file_path, file_offset, file_size));
+}
+
+TEST_F(ZipFileResolverTest, ResolveBionicPathWithZipExisting) {
+  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::ResolveBionicPath(
+  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 "llvm/Support/Endian.h"
+
+using namespace lldb_private;
+using namespace llvm::support;
+
+namespace

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

2023-06-14 Thread Kazuki Sakamoto via Phabricator via lldb-commits

splhack added inline comments.



Comment at: lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp:261-264
+  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';

splhack wrote:
> bulbazord wrote:
> > Maybe it would be a good idea to centralize the `run-as` logic somewhere, 
> > right now it's pretty ad-hoc.
> Will look into these run-as things with D152494 centralize and if 
> plugin.platform.android.platform-run-as possible.
run-as is centralized in D152933


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] D152759: [lldb][Android] Support zip .so file

2023-06-14 Thread Kazuki Sakamoto via Phabricator via lldb-commits

splhack updated this revision to Diff 531384.
splhack added a comment.

rebase onto D152757 


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, ResolveBionicPathWithNormalFile) {
+  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::ResolveBionicPath(
+  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, ResolveBionicPathWithZipMissing) {
+  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::ResolveBionicPath(
+  file_spec, file_kind, file_path, file_offset, file_size));
+}
+
+TEST_F(ZipFileResolverTest, ResolveBionicPathWithZipExisting) {
+  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::ResolveBionicPath(
+  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 "llvm/Support/Endian.h"
+
+using namespace lldb_private;
+using namespace llvm::support;

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

2023-06-13 Thread Kazuki Sakamoto via Phabricator via lldb-commits

splhack updated this revision to Diff 531195.
splhack added a comment.

- ZipFile: zip file parser in Utility
  - include/lldb/Utility/ZipFile.h
  - source/Utility/ZipFile.cpp

- ZipFileResolver: bionic zip .so file resolver, depends on Host::FileSystem
  - include/lldb/Host/common/ZipFileResolver.h
  - source/Host/common/ZipFileResolver.cpp

- ZipFileResolverTest
  - unittests/Host/common


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, ResolveBionicPathWithNormalFile) {
+  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::ResolveBionicPath(
+  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, ResolveBionicPathWithZipMissing) {
+  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::ResolveBionicPath(
+  file_spec, file_kind, file_path, file_offset, file_size));
+}
+
+TEST_F(ZipFileResolverTest, ResolveBionicPathWithZipExisting) {
+  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::ResolveBionicPath(
+  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
+//
+//===---

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

2023-06-13 Thread Kazuki Sakamoto via Phabricator via lldb-commits

splhack added inline comments.



Comment at: lldb/source/Host/CMakeLists.txt:109-121
+
+set(ANDROID_SOURCES
+  android/ZipFile.cpp
+  android/HostInfoAndroid.cpp
+  )
 if (CMAKE_SYSTEM_NAME MATCHES "Android")
+  list(APPEND ANDROID_SOURCES

bulbazord wrote:
> splhack wrote:
> > bulbazord wrote:
> > > I don't think this is correct to do. lldbHost is different than other 
> > > libraries in that it's meant to provide functionality that lldb and 
> > > lldb-server needs to work on the host system. Unconditionally adding a 
> > > host subdirectory for android even when we're on Linux doesn't make sense 
> > > to do.
> > I agree with that, however, I think this is pretty much only way to unblock 
> > writing and running unit tests for the Android host system, which has been 
> > no tests at all. The AndroidPlatformTest D152855 requires this to run the 
> > tests on Linux, and Android is basically Linux, so, hope it still makes 
> > sense for the unit testing capability. (only android/LibcGlue.cpp is not 
> > buildable for Linux target.)
> I see. You want to be able to run the android host tests but that's not easy 
> to do right now. I think this is a reasonable thing to want to do (especially 
> since so much of android support in lldb is not well tested AFAIK).
> 
> Instead of making this functionality specific to android hosts, why not make 
> it possible to do on all platforms? This would do a few things:
> - It would make it easier to test on more than just Linux and Android 
> machines.
> - It would open up the possibility of being able to use an apk on the host 
> machine instead of needing to fetch it from the remote device via `adb shell 
> dd`. An optimization for sure, but for large shared objects this may be able 
> to improve performance.
> 
> What do you think?
Yeah, sounds great to me! will update move things around.
- include/lldb/Utility/ZipFile.h
- source/Utility/ZipFile.cpp
- `HostInfoAndroid::ResolveZipPath` -> `ZipFile::ResolveBionicZipPath` (this is 
bionic libc specific)


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] D152759: [lldb][Android] Support zip .so file

2023-06-13 Thread Alex Langford via Phabricator via lldb-commits

bulbazord added inline comments.



Comment at: lldb/source/Host/CMakeLists.txt:109-121
+
+set(ANDROID_SOURCES
+  android/ZipFile.cpp
+  android/HostInfoAndroid.cpp
+  )
 if (CMAKE_SYSTEM_NAME MATCHES "Android")
+  list(APPEND ANDROID_SOURCES

splhack wrote:
> bulbazord wrote:
> > I don't think this is correct to do. lldbHost is different than other 
> > libraries in that it's meant to provide functionality that lldb and 
> > lldb-server needs to work on the host system. Unconditionally adding a host 
> > subdirectory for android even when we're on Linux doesn't make sense to do.
> I agree with that, however, I think this is pretty much only way to unblock 
> writing and running unit tests for the Android host system, which has been no 
> tests at all. The AndroidPlatformTest D152855 requires this to run the tests 
> on Linux, and Android is basically Linux, so, hope it still makes sense for 
> the unit testing capability. (only android/LibcGlue.cpp is not buildable for 
> Linux target.)
I see. You want to be able to run the android host tests but that's not easy to 
do right now. I think this is a reasonable thing to want to do (especially 
since so much of android support in lldb is not well tested AFAIK).

Instead of making this functionality specific to android hosts, why not make it 
possible to do on all platforms? This would do a few things:
- It would make it easier to test on more than just Linux and Android machines.
- It would open up the possibility of being able to use an apk on the host 
machine instead of needing to fetch it from the remote device via `adb shell 
dd`. An optimization for sure, but for large shared objects this may be able to 
improve performance.

What do you think?



Comment at: lldb/source/Host/android/ZipFile.cpp:1
+//===-- ZipFile.cpp 
---===//
+//

BTW, I haven't looked at this file too closely yet, but I think a better 
candidate for its location would be `lldbUtility`. It doesn't rely on anything 
else from lldb other than something else in `lldbUtility`.


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] D152759: [lldb][Android] Support zip .so file

2023-06-13 Thread Kazuki Sakamoto via Phabricator via lldb-commits

splhack added a comment.

@bulbazord thanks for reviewing! will address the types, formats.




Comment at: lldb/source/Host/CMakeLists.txt:109-121
+
+set(ANDROID_SOURCES
+  android/ZipFile.cpp
+  android/HostInfoAndroid.cpp
+  )
 if (CMAKE_SYSTEM_NAME MATCHES "Android")
+  list(APPEND ANDROID_SOURCES

bulbazord wrote:
> I don't think this is correct to do. lldbHost is different than other 
> libraries in that it's meant to provide functionality that lldb and 
> lldb-server needs to work on the host system. Unconditionally adding a host 
> subdirectory for android even when we're on Linux doesn't make sense to do.
I agree with that, however, I think this is pretty much only way to unblock 
writing and running unit tests for the Android host system, which has been no 
tests at all. The AndroidPlatformTest D152855 requires this to run the tests on 
Linux, and Android is basically Linux, so, hope it still makes sense for the 
unit testing capability. (only android/LibcGlue.cpp is not buildable for Linux 
target.)



Comment at: lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp:261-264
+  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';

bulbazord wrote:
> Maybe it would be a good idea to centralize the `run-as` logic somewhere, 
> right now it's pretty ad-hoc.
Will look into these run-as things with D152494 centralize and if 
plugin.platform.android.platform-run-as possible.



Comment at: lldb/unittests/Host/CMakeLists.txt:42
+
+if (CMAKE_SYSTEM_NAME MATCHES "Linux|Android")
+  add_subdirectory(android)

bulbazord wrote:
> Why not just match on `"Android"`?
The reason is, as commented above, to run AndroidPlatformTest D152855 on Linux.


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] D152759: [lldb][Android] Support zip .so file

2023-06-13 Thread Alex Langford via Phabricator via lldb-commits

bulbazord added a comment.
Herald added a subscriber: JDevlieghere.

This is definitely useful to support, I had a few comments mostly about 
matching lldb's coding style.

Also, are you running clang-format on your patches? There are a few places 
where I'm not sure if the formatting is right.




Comment at: lldb/source/Host/CMakeLists.txt:109-121
+
+set(ANDROID_SOURCES
+  android/ZipFile.cpp
+  android/HostInfoAndroid.cpp
+  )
 if (CMAKE_SYSTEM_NAME MATCHES "Android")
+  list(APPEND ANDROID_SOURCES

I don't think this is correct to do. lldbHost is different than other libraries 
in that it's meant to provide functionality that lldb and lldb-server needs to 
work on the host system. Unconditionally adding a host subdirectory for android 
even when we're on Linux doesn't make sense to do.



Comment at: lldb/source/Host/android/HostInfoAndroid.cpp:107
+  // #opening-shared-libraries-directly-from-an-apk
+  const std::string kZipSeparator = "!/";
+  auto path = file_spec.GetPath();

You can avoid using `std::string` here (and invoking the constructor) by using 
an llvm::StringLiteral. Like so:
```
static constexpr llvm::StringLiteral k_zip_separator("!/");
```



Comment at: lldb/source/Host/android/HostInfoAndroid.cpp:108
+  const std::string kZipSeparator = "!/";
+  auto path = file_spec.GetPath();
+  auto pos = path.find(kZipSeparator);

It's not obvious what the type of `path` is, please write out the type instead 
of using `auto`.



Comment at: lldb/source/Host/android/HostInfoAndroid.cpp:119-126
+  auto zip_path = path.substr(0, pos);
+  auto so_path = path.substr(pos + kZipSeparator.size());
+
+  auto zip_file_spec = FileSpec(zip_path);
+  auto zip_file_size = FileSystem::Instance().GetByteSize(zip_file_spec);
+  auto zip_data = FileSystem::Instance().CreateDataBuffer(zip_file_spec,
+  zip_file_size,

It's not obvious what the types of these are (except for `zip_file_spec`), 
please write out the types explicitly.



Comment at: lldb/source/Host/android/HostInfoAndroid.cpp:126
+  zip_file_size,
+  0);
+  if (ZipFile::Find(zip_data, so_path, file_offset, file_size)) {

It's not obvious what 0 is as the parameter here, please make it look something 
like this:
`/* offset = */ 0);`



Comment at: lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp:261-264
+  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';

Maybe it would be a good idea to centralize the `run-as` logic somewhere, right 
now it's pretty ad-hoc.



Comment at: lldb/unittests/Host/CMakeLists.txt:42
+
+if (CMAKE_SYSTEM_NAME MATCHES "Linux|Android")
+  add_subdirectory(android)

Why not just match on `"Android"`?


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] D152759: [lldb][Android] Support zip .so file

2023-06-12 Thread Kazuki Sakamoto via Phabricator via lldb-commits

splhack created this revision.
Herald added subscribers: danielkiss, krytarowski.
Herald added a project: All.
splhack added reviewers: clayborg, labath, lanza, srhines.
splhack published this revision for review.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

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.

Depends on D152494  and D152712 
 and D152757 



Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D152759

Files:
  lldb/include/lldb/Host/android/HostInfoAndroid.h
  lldb/source/Host/CMakeLists.txt
  lldb/source/Host/android/HostInfoAndroid.cpp
  lldb/source/Host/android/ZipFile.cpp
  lldb/source/Host/android/ZipFile.h
  lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
  lldb/unittests/Host/CMakeLists.txt
  lldb/unittests/Host/android/CMakeLists.txt
  lldb/unittests/Host/android/HostInfoAndroidTest.cpp
  lldb/unittests/Host/android/Inputs/zip-test.zip

Index: lldb/unittests/Host/android/HostInfoAndroidTest.cpp
===
--- /dev/null
+++ lldb/unittests/Host/android/HostInfoAndroidTest.cpp
@@ -0,0 +1,67 @@
+//===-- HostInfoAndroidTest.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/android/HostInfoAndroid.h"
+#include "TestingSupport/SubsystemRAII.h"
+#include "TestingSupport/TestUtilities.h"
+#include "gtest/gtest.h"
+
+using namespace lldb_private;
+using namespace llvm;
+
+namespace {
+class HostInfoAndroidTest : 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(HostInfoAndroidTest, ResolveZipPathWithNormalFile) {
+  const FileSpec file_spec("/system/lib64/libtest.so");
+
+  std::string file_path;
+  lldb::offset_t file_offset;
+  lldb::offset_t file_size;
+  ASSERT_TRUE(HostInfoAndroid::ResolveZipPath(file_spec, file_path,
+  file_offset, file_size));
+
+  EXPECT_EQ(file_path, file_spec.GetPath());
+  EXPECT_EQ(file_offset, 0UL);
+  EXPECT_EQ(file_size, 0UL);
+}
+
+TEST_F(HostInfoAndroidTest, ResolveZipPathWithZipMissing) {
+  const std::string zip_path = TestZipPath();
+  const FileSpec file_spec(zip_path + "!/lib/arm64-v8a/libmissing.so");
+
+  std::string file_path;
+  lldb::offset_t file_offset;
+  lldb::offset_t file_size;
+  ASSERT_FALSE(HostInfoAndroid::ResolveZipPath(file_spec, file_path,
+   file_offset, file_size));
+}
+
+TEST_F(HostInfoAndroidTest, ResolveZipPathWithZipExisting) {
+  const std::string zip_path = TestZipPath();
+  const FileSpec file_spec(zip_path + "!/lib/arm64-v8a/libzip-test.so");
+
+  std::string file_path;
+  lldb::offset_t file_offset;
+  lldb::offset_t file_size;
+  ASSERT_TRUE(HostInfoAndroid::ResolveZipPath(file_spec, file_path,
+  file_offset, file_size));
+
+  EXPECT_EQ(file_path, zip_path);
+  EXPECT_EQ(file_offset, 4096UL);
+  EXPECT_EQ(file_size, 3600UL);
+}
Index: lldb/unittests/Host/android/CMakeLists.txt
===
--- /dev/null
+++ lldb/unittests/Host/android/CMakeLists.txt
@@ -0,0 +1,15 @@
+set (FILES
+  HostInfoAndroidTest.cpp
+)
+
+add_lldb_unittest(HostAndroidTests
+  ${FILES}
+  LINK_LIBS
+lldbHost
+lldbUtilityHelpers
+  )
+
+set(test_inputs
+  zip-test.zip
+  )
+add_unittest_inputs(HostAndroidTests "${test_inputs}")
Index: lldb/unittests/Host/CMakeLists.txt
===
--- lldb/unittests/Host/CMakeLists.txt
+++ lldb/unittests/Host/CMakeLists.txt
@@ -38

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

24 matches

Site Navigation

Mail list logo

Footer information