diff --git a/linker/linker.cpp b/linker/linker.cpp index 7421b1d17..80ff5831f 100644 --- a/linker/linker.cpp +++ b/linker/linker.cpp @@ -41,6 +41,7 @@ #include #include +#include #include // 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 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& 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; } diff --git a/tests/dlext_test.cpp b/tests/dlext_test.cpp index 44b899e91..9d8b71d0a 100644 --- a/tests/dlext_test.cpp +++ b/tests/dlext_test.cpp @@ -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(dlsym(handle_, "getRandomNumber")); - ASSERT_DL_NOTNULL(f); - EXPECT_EQ(4, f()); + uint32_t* taxicab_number = reinterpret_cast(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(dlsym(handle, "getRandomNumber")); - ASSERT_TRUE(fn != nullptr); - EXPECT_EQ(4, fn()); + uint32_t* taxicab_number = reinterpret_cast(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(dlsym(handle, "dlopen_testlib_taxicab_number")); + ASSERT_DL_NOTNULL(taxicab_number); + EXPECT_EQ(1729U, *taxicab_number); + dlclose(handle); } diff --git a/tests/libs/Android.build.dlext_testzip.mk b/tests/libs/Android.build.dlext_testzip.mk index 1e89411c5..93df21369 100644 --- a/tests/libs/Android.build.dlext_testzip.mk +++ b/tests/libs/Android.build.dlext_testzip.mk @@ -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 $@ diff --git a/tests/libs/Android.mk b/tests/libs/Android.mk index b13400e84..3391d79fe 100644 --- a/tests/libs/Android.mk +++ b/tests/libs/Android.mk @@ -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 # ----------------------------------------------------------------------------