Optimize dlopen from a zip file
This change makes dynamic linker reuse ZipArchiveHandles in ld_library_path on dlopen to optimize the lookup of dt_needed libraries. Bug: http://b/21960534 Change-Id: I65f897910d46dd2ffabdcb0b7842db2f127eee30
This commit is contained in:
		@@ -41,6 +41,7 @@
 | 
			
		||||
 | 
			
		||||
#include <new>
 | 
			
		||||
#include <string>
 | 
			
		||||
#include <unordered_map>
 | 
			
		||||
#include <vector>
 | 
			
		||||
 | 
			
		||||
// Private C library headers.
 | 
			
		||||
@@ -1121,7 +1122,50 @@ ElfW(Sym)* soinfo::elf_addr_lookup(const void* addr) {
 | 
			
		||||
  return nullptr;
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
static int open_library_in_zipfile(const char* const path,
 | 
			
		||||
class ZipArchiveCache {
 | 
			
		||||
 public:
 | 
			
		||||
  ZipArchiveCache() {}
 | 
			
		||||
  ~ZipArchiveCache();
 | 
			
		||||
 | 
			
		||||
  bool get_or_open(const char* zip_path, ZipArchiveHandle* handle);
 | 
			
		||||
 private:
 | 
			
		||||
  DISALLOW_COPY_AND_ASSIGN(ZipArchiveCache);
 | 
			
		||||
 | 
			
		||||
  std::unordered_map<std::string, ZipArchiveHandle> cache_;
 | 
			
		||||
};
 | 
			
		||||
 | 
			
		||||
bool ZipArchiveCache::get_or_open(const char* zip_path, ZipArchiveHandle* handle) {
 | 
			
		||||
  std::string key(zip_path);
 | 
			
		||||
 | 
			
		||||
  auto it = cache_.find(key);
 | 
			
		||||
  if (it != cache_.end()) {
 | 
			
		||||
    *handle = it->second;
 | 
			
		||||
    return true;
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
  int fd = TEMP_FAILURE_RETRY(open(zip_path, O_RDONLY | O_CLOEXEC));
 | 
			
		||||
  if (fd == -1) {
 | 
			
		||||
    return false;
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
  if (OpenArchiveFd(fd, "", handle) != 0) {
 | 
			
		||||
    // invalid zip-file (?)
 | 
			
		||||
    close(fd);
 | 
			
		||||
    return false;
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
  cache_[key] = *handle;
 | 
			
		||||
  return true;
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
ZipArchiveCache::~ZipArchiveCache() {
 | 
			
		||||
  for (auto it : cache_) {
 | 
			
		||||
    CloseArchive(it.second);
 | 
			
		||||
  }
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
static int open_library_in_zipfile(ZipArchiveCache* zip_archive_cache,
 | 
			
		||||
                                   const char* const path,
 | 
			
		||||
                                   off64_t* file_offset) {
 | 
			
		||||
  TRACE("Trying zip file open from path '%s'", path);
 | 
			
		||||
 | 
			
		||||
@@ -1150,16 +1194,12 @@ static int open_library_in_zipfile(const char* const path,
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
  ZipArchiveHandle handle;
 | 
			
		||||
  if (OpenArchiveFd(fd, "", &handle, false) != 0) {
 | 
			
		||||
  if (!zip_archive_cache->get_or_open(zip_path, &handle)) {
 | 
			
		||||
    // invalid zip-file (?)
 | 
			
		||||
    close(fd);
 | 
			
		||||
    return -1;
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
  auto archive_guard = make_scope_guard([&]() {
 | 
			
		||||
    CloseArchive(handle);
 | 
			
		||||
  });
 | 
			
		||||
 | 
			
		||||
  ZipEntry entry;
 | 
			
		||||
 | 
			
		||||
  if (FindEntry(handle, ZipString(file_path), &entry) != 0) {
 | 
			
		||||
@@ -1205,7 +1245,8 @@ static int open_library_on_default_path(const char* name, off64_t* file_offset)
 | 
			
		||||
  return -1;
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
static int open_library_on_paths(const char* name, off64_t* file_offset,
 | 
			
		||||
static int open_library_on_paths(ZipArchiveCache* zip_archive_cache,
 | 
			
		||||
                                 const char* name, off64_t* file_offset,
 | 
			
		||||
                                 const std::vector<std::string>& paths) {
 | 
			
		||||
  for (const auto& path_str : paths) {
 | 
			
		||||
    char buf[512];
 | 
			
		||||
@@ -1216,7 +1257,7 @@ static int open_library_on_paths(const char* name, off64_t* file_offset,
 | 
			
		||||
 | 
			
		||||
    int fd = -1;
 | 
			
		||||
    if (strstr(buf, kZipFileSeparator) != nullptr) {
 | 
			
		||||
      fd = open_library_in_zipfile(buf, file_offset);
 | 
			
		||||
      fd = open_library_in_zipfile(zip_archive_cache, buf, file_offset);
 | 
			
		||||
    }
 | 
			
		||||
 | 
			
		||||
    if (fd == -1) {
 | 
			
		||||
@@ -1234,13 +1275,15 @@ static int open_library_on_paths(const char* name, off64_t* file_offset,
 | 
			
		||||
  return -1;
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
static int open_library(const char* name, soinfo *needed_by, off64_t* file_offset) {
 | 
			
		||||
static int open_library(ZipArchiveCache* zip_archive_cache,
 | 
			
		||||
                        const char* name, soinfo *needed_by,
 | 
			
		||||
                        off64_t* file_offset) {
 | 
			
		||||
  TRACE("[ opening %s ]", name);
 | 
			
		||||
 | 
			
		||||
  // If the name contains a slash, we should attempt to open it directly and not search the paths.
 | 
			
		||||
  if (strchr(name, '/') != nullptr) {
 | 
			
		||||
    if (strstr(name, kZipFileSeparator) != nullptr) {
 | 
			
		||||
      int fd = open_library_in_zipfile(name, file_offset);
 | 
			
		||||
      int fd = open_library_in_zipfile(zip_archive_cache, name, file_offset);
 | 
			
		||||
      if (fd != -1) {
 | 
			
		||||
        return fd;
 | 
			
		||||
      }
 | 
			
		||||
@@ -1254,13 +1297,15 @@ static int open_library(const char* name, soinfo *needed_by, off64_t* file_offse
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
  // Otherwise we try LD_LIBRARY_PATH first, and fall back to the built-in well known paths.
 | 
			
		||||
  int fd = open_library_on_paths(name, file_offset, g_ld_library_paths);
 | 
			
		||||
  int fd = open_library_on_paths(zip_archive_cache, name, file_offset, g_ld_library_paths);
 | 
			
		||||
  if (fd == -1 && needed_by) {
 | 
			
		||||
    fd = open_library_on_paths(name, file_offset, needed_by->get_dt_runpath());
 | 
			
		||||
    fd = open_library_on_paths(zip_archive_cache, name, file_offset, needed_by->get_dt_runpath());
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
  if (fd == -1) {
 | 
			
		||||
    fd = open_library_on_default_path(name, file_offset);
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
  return fd;
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
@@ -1367,7 +1412,8 @@ static soinfo* load_library(int fd, off64_t file_offset,
 | 
			
		||||
  return si;
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
static soinfo* load_library(LoadTaskList& load_tasks, const char* name,
 | 
			
		||||
static soinfo* load_library(ZipArchiveCache* zip_archive_cache,
 | 
			
		||||
                            LoadTaskList& load_tasks, const char* name,
 | 
			
		||||
                            soinfo* needed_by, int rtld_flags,
 | 
			
		||||
                            const android_dlextinfo* extinfo) {
 | 
			
		||||
  if (extinfo != nullptr && (extinfo->flags & ANDROID_DLEXT_USE_LIBRARY_FD) != 0) {
 | 
			
		||||
@@ -1380,7 +1426,7 @@ static soinfo* load_library(LoadTaskList& load_tasks, const char* name,
 | 
			
		||||
 | 
			
		||||
  // Open the file.
 | 
			
		||||
  off64_t file_offset;
 | 
			
		||||
  int fd = open_library(name, needed_by, &file_offset);
 | 
			
		||||
  int fd = open_library(zip_archive_cache, name, needed_by, &file_offset);
 | 
			
		||||
  if (fd == -1) {
 | 
			
		||||
    DL_ERR("library \"%s\" not found", name);
 | 
			
		||||
    return nullptr;
 | 
			
		||||
@@ -1427,7 +1473,8 @@ static bool find_loaded_library_by_soname(const char* name, soinfo** candidate)
 | 
			
		||||
  return false;
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
static soinfo* find_library_internal(LoadTaskList& load_tasks, const char* name,
 | 
			
		||||
static soinfo* find_library_internal(ZipArchiveCache* zip_archive_cache,
 | 
			
		||||
                                     LoadTaskList& load_tasks, const char* name,
 | 
			
		||||
                                     soinfo* needed_by, int rtld_flags,
 | 
			
		||||
                                     const android_dlextinfo* extinfo) {
 | 
			
		||||
  soinfo* candidate;
 | 
			
		||||
@@ -1441,7 +1488,7 @@ static soinfo* find_library_internal(LoadTaskList& load_tasks, const char* name,
 | 
			
		||||
  TRACE("[ '%s' find_loaded_library_by_soname returned false (*candidate=%s@%p). Trying harder...]",
 | 
			
		||||
      name, candidate == nullptr ? "n/a" : candidate->get_realpath(), candidate);
 | 
			
		||||
 | 
			
		||||
  soinfo* si = load_library(load_tasks, name, needed_by, rtld_flags, extinfo);
 | 
			
		||||
  soinfo* si = load_library(zip_archive_cache, load_tasks, name, needed_by, rtld_flags, extinfo);
 | 
			
		||||
 | 
			
		||||
  // In case we were unable to load the library but there
 | 
			
		||||
  // is a candidate loaded under the same soname but different
 | 
			
		||||
@@ -1521,15 +1568,18 @@ static bool find_libraries(soinfo* start_with,
 | 
			
		||||
    }
 | 
			
		||||
  });
 | 
			
		||||
 | 
			
		||||
  ZipArchiveCache zip_archive_cache;
 | 
			
		||||
 | 
			
		||||
  // Step 1: load and pre-link all DT_NEEDED libraries in breadth first order.
 | 
			
		||||
  for (LoadTask::unique_ptr task(load_tasks.pop_front());
 | 
			
		||||
      task.get() != nullptr; task.reset(load_tasks.pop_front())) {
 | 
			
		||||
    soinfo* needed_by = task->get_needed_by();
 | 
			
		||||
    bool is_dt_needed = needed_by != nullptr && (needed_by != start_with || add_as_children);
 | 
			
		||||
 | 
			
		||||
    soinfo* si = find_library_internal(load_tasks, task->get_name(), needed_by,
 | 
			
		||||
                                       rtld_flags,
 | 
			
		||||
    soinfo* si = find_library_internal(&zip_archive_cache, load_tasks,
 | 
			
		||||
                                       task->get_name(), needed_by, rtld_flags,
 | 
			
		||||
                                       is_dt_needed ? nullptr : extinfo);
 | 
			
		||||
 | 
			
		||||
    if (si == nullptr) {
 | 
			
		||||
      return false;
 | 
			
		||||
    }
 | 
			
		||||
 
 | 
			
		||||
@@ -52,15 +52,15 @@ typedef int (*fn)(void);
 | 
			
		||||
#define LIBSIZE 1024*1024 // how much address space to reserve for it
 | 
			
		||||
 | 
			
		||||
#if defined(__LP64__)
 | 
			
		||||
#define LIBPATH_PREFIX "/nativetest64/libdlext_test_fd/"
 | 
			
		||||
#define LIBPATH_PREFIX "/nativetest64/"
 | 
			
		||||
#else
 | 
			
		||||
#define LIBPATH_PREFIX "/nativetest/libdlext_test_fd/"
 | 
			
		||||
#define LIBPATH_PREFIX "/nativetest/"
 | 
			
		||||
#endif
 | 
			
		||||
 | 
			
		||||
#define LIBPATH LIBPATH_PREFIX "libdlext_test_fd.so"
 | 
			
		||||
#define LIBZIPPATH LIBPATH_PREFIX "libdlext_test_fd_zipaligned.zip"
 | 
			
		||||
#define LIBPATH LIBPATH_PREFIX "libdlext_test_fd/libdlext_test_fd.so"
 | 
			
		||||
#define LIBZIPPATH LIBPATH_PREFIX "libdlext_test_zip/libdlext_test_zip_zipaligned.zip"
 | 
			
		||||
 | 
			
		||||
#define LIBZIP_OFFSET 2*PAGE_SIZE
 | 
			
		||||
#define LIBZIP_OFFSET PAGE_SIZE
 | 
			
		||||
 | 
			
		||||
class DlExtTest : public ::testing::Test {
 | 
			
		||||
protected:
 | 
			
		||||
@@ -131,9 +131,9 @@ TEST_F(DlExtTest, ExtInfoUseFdWithOffset) {
 | 
			
		||||
  handle_ = android_dlopen_ext(lib_path.c_str(), RTLD_NOW, &extinfo);
 | 
			
		||||
  ASSERT_DL_NOTNULL(handle_);
 | 
			
		||||
 | 
			
		||||
  fn f = reinterpret_cast<fn>(dlsym(handle_, "getRandomNumber"));
 | 
			
		||||
  ASSERT_DL_NOTNULL(f);
 | 
			
		||||
  EXPECT_EQ(4, f());
 | 
			
		||||
  uint32_t* taxicab_number = reinterpret_cast<uint32_t*>(dlsym(handle_, "dlopen_testlib_taxicab_number"));
 | 
			
		||||
  ASSERT_DL_NOTNULL(taxicab_number);
 | 
			
		||||
  EXPECT_EQ(1729U, *taxicab_number);
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
TEST_F(DlExtTest, ExtInfoUseFdWithInvalidOffset) {
 | 
			
		||||
@@ -163,7 +163,7 @@ TEST_F(DlExtTest, ExtInfoUseFdWithInvalidOffset) {
 | 
			
		||||
  ASSERT_TRUE(handle_ == nullptr);
 | 
			
		||||
  ASSERT_SUBSTR("dlopen failed: file offset for the library \"libname_placeholder\" is negative", dlerror());
 | 
			
		||||
 | 
			
		||||
  extinfo.library_fd_offset = PAGE_SIZE;
 | 
			
		||||
  extinfo.library_fd_offset = 0;
 | 
			
		||||
  handle_ = android_dlopen_ext("libname_ignored", RTLD_NOW, &extinfo);
 | 
			
		||||
  ASSERT_TRUE(handle_ == nullptr);
 | 
			
		||||
  ASSERT_EQ("dlopen failed: \"" + lib_realpath + "\" has bad ELF magic", dlerror());
 | 
			
		||||
@@ -218,13 +218,12 @@ TEST(dlext, android_dlopen_ext_force_load_soname_exception) {
 | 
			
		||||
TEST(dlfcn, dlopen_from_zip_absolute_path) {
 | 
			
		||||
  const std::string lib_path = std::string(getenv("ANDROID_DATA")) + LIBZIPPATH;
 | 
			
		||||
 | 
			
		||||
  void* handle = dlopen((lib_path + "!/libdir/libdlext_test_fd.so").c_str(), RTLD_NOW);
 | 
			
		||||
  void* handle = dlopen((lib_path + "!/libdir/libatest_simple_zip.so").c_str(), RTLD_NOW);
 | 
			
		||||
  ASSERT_TRUE(handle != nullptr) << dlerror();
 | 
			
		||||
 | 
			
		||||
  int (*fn)(void);
 | 
			
		||||
  fn = reinterpret_cast<int (*)(void)>(dlsym(handle, "getRandomNumber"));
 | 
			
		||||
  ASSERT_TRUE(fn != nullptr);
 | 
			
		||||
  EXPECT_EQ(4, fn());
 | 
			
		||||
  uint32_t* taxicab_number = reinterpret_cast<uint32_t*>(dlsym(handle, "dlopen_testlib_taxicab_number"));
 | 
			
		||||
  ASSERT_DL_NOTNULL(taxicab_number);
 | 
			
		||||
  EXPECT_EQ(1729U, *taxicab_number);
 | 
			
		||||
 | 
			
		||||
  dlclose(handle);
 | 
			
		||||
}
 | 
			
		||||
@@ -238,12 +237,12 @@ TEST(dlfcn, dlopen_from_zip_ld_library_path) {
 | 
			
		||||
 | 
			
		||||
  ASSERT_TRUE(android_update_LD_LIBRARY_PATH != nullptr) << dlerror();
 | 
			
		||||
 | 
			
		||||
  void* handle = dlopen("libdlext_test_fd.so", RTLD_NOW);
 | 
			
		||||
  void* handle = dlopen("libdlext_test_zip.so", RTLD_NOW);
 | 
			
		||||
  ASSERT_TRUE(handle == nullptr);
 | 
			
		||||
 | 
			
		||||
  android_update_LD_LIBRARY_PATH(lib_path.c_str());
 | 
			
		||||
 | 
			
		||||
  handle = dlopen("libdlext_test_fd.so", RTLD_NOW);
 | 
			
		||||
  handle = dlopen("libdlext_test_zip.so", RTLD_NOW);
 | 
			
		||||
  ASSERT_TRUE(handle != nullptr) << dlerror();
 | 
			
		||||
 | 
			
		||||
  int (*fn)(void);
 | 
			
		||||
@@ -251,6 +250,10 @@ TEST(dlfcn, dlopen_from_zip_ld_library_path) {
 | 
			
		||||
  ASSERT_TRUE(fn != nullptr);
 | 
			
		||||
  EXPECT_EQ(4, fn());
 | 
			
		||||
 | 
			
		||||
  uint32_t* taxicab_number = reinterpret_cast<uint32_t*>(dlsym(handle, "dlopen_testlib_taxicab_number"));
 | 
			
		||||
  ASSERT_DL_NOTNULL(taxicab_number);
 | 
			
		||||
  EXPECT_EQ(1729U, *taxicab_number);
 | 
			
		||||
 | 
			
		||||
  dlclose(handle);
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
 
 | 
			
		||||
@@ -21,21 +21,21 @@
 | 
			
		||||
include $(CLEAR_VARS)
 | 
			
		||||
 | 
			
		||||
LOCAL_MODULE_CLASS := SHARED_LIBRARIES
 | 
			
		||||
LOCAL_MODULE := libdlext_test_fd_zipaligned
 | 
			
		||||
LOCAL_MODULE := libdlext_test_zip_zipaligned
 | 
			
		||||
LOCAL_MODULE_SUFFIX := .zip
 | 
			
		||||
LOCAL_MODULE_TAGS := tests
 | 
			
		||||
LOCAL_MODULE_PATH := $($(bionic_2nd_arch_prefix)TARGET_OUT_DATA_NATIVE_TESTS)/libdlext_test_fd
 | 
			
		||||
LOCAL_MODULE_PATH := $($(bionic_2nd_arch_prefix)TARGET_OUT_DATA_NATIVE_TESTS)/libdlext_test_zip
 | 
			
		||||
LOCAL_2ND_ARCH_VAR_PREFIX := $(bionic_2nd_arch_prefix)
 | 
			
		||||
 | 
			
		||||
include $(BUILD_SYSTEM)/base_rules.mk
 | 
			
		||||
 | 
			
		||||
my_shared_libs := \
 | 
			
		||||
  $($(bionic_2nd_arch_prefix)TARGET_OUT_INTERMEDIATE_LIBRARIES)/libdlext_test_fd.so
 | 
			
		||||
  $($(bionic_2nd_arch_prefix)TARGET_OUT_INTERMEDIATE_LIBRARIES)/libdlext_test_zip.so \
 | 
			
		||||
  $($(bionic_2nd_arch_prefix)TARGET_OUT_INTERMEDIATE_LIBRARIES)/libatest_simple_zip.so
 | 
			
		||||
 | 
			
		||||
$(LOCAL_BUILT_MODULE): PRIVATE_ALIGNMENT := 4096 # PAGE_SIZE
 | 
			
		||||
$(LOCAL_BUILT_MODULE) : $(my_shared_libs) | $(ZIPALIGN)
 | 
			
		||||
	@echo "Zipalign $(PRIVATE_ALIGNMENT): $@"
 | 
			
		||||
	$(hide) rm -rf $(dir $@) && mkdir -p $(dir $@)/libdir
 | 
			
		||||
	$(hide) cp $^ $(dir $@)/libdir
 | 
			
		||||
	$(hide) (cd $(dir $@) && touch empty_file.txt && zip -qrD0 $(notdir $@).unaligned empty_file.txt libdir/*.so)
 | 
			
		||||
	$(hide) $(ZIPALIGN) $(PRIVATE_ALIGNMENT) $@.unaligned $@
 | 
			
		||||
	$(hide) $(ZIPALIGN) -p 4 $@.unaligned $@
 | 
			
		||||
 
 | 
			
		||||
@@ -126,6 +126,32 @@ build_type := target
 | 
			
		||||
build_target := SHARED_LIBRARY
 | 
			
		||||
include $(TEST_PATH)/Android.build.mk
 | 
			
		||||
 | 
			
		||||
 | 
			
		||||
# -----------------------------------------------------------------------------
 | 
			
		||||
# Libraries used by dlext tests for open from a zip-file
 | 
			
		||||
# -----------------------------------------------------------------------------
 | 
			
		||||
libdlext_test_zip_src_files := \
 | 
			
		||||
    dlext_test_library.cpp \
 | 
			
		||||
 | 
			
		||||
libdlext_test_zip_shared_libraries := libatest_simple_zip
 | 
			
		||||
 | 
			
		||||
libdlext_test_zip_install_to_out_data := true
 | 
			
		||||
module := libdlext_test_zip
 | 
			
		||||
module_tag := optional
 | 
			
		||||
build_type := target
 | 
			
		||||
build_target := SHARED_LIBRARY
 | 
			
		||||
include $(TEST_PATH)/Android.build.mk
 | 
			
		||||
 | 
			
		||||
libatest_simple_zip_src_files := \
 | 
			
		||||
    dlopen_testlib_simple.cpp
 | 
			
		||||
 | 
			
		||||
libatest_simple_zip_install_to_out_data := true
 | 
			
		||||
module := libatest_simple_zip
 | 
			
		||||
module_tag := optional
 | 
			
		||||
build_type := target
 | 
			
		||||
build_target := SHARED_LIBRARY
 | 
			
		||||
include $(TEST_PATH)/Android.build.mk
 | 
			
		||||
 | 
			
		||||
# ----------------------------------------------------------------------------
 | 
			
		||||
# Library with soname which does not match filename
 | 
			
		||||
# ----------------------------------------------------------------------------
 | 
			
		||||
 
 | 
			
		||||
		Reference in New Issue
	
	Block a user