diff --git a/linker/dlfcn.cpp b/linker/dlfcn.cpp index 8ebf3576b..efb829e7a 100644 --- a/linker/dlfcn.cpp +++ b/linker/dlfcn.cpp @@ -111,7 +111,8 @@ void* dlsym(void* handle, const char* symbol) { sym = dlsym_linear_lookup(symbol, &found, caller_si->next, caller_si); } } else { - sym = dlsym_handle_lookup(reinterpret_cast(handle), &found, symbol, caller_si); + found = reinterpret_cast(handle); + sym = dlsym_handle_lookup(found, symbol, caller_si); } if (sym != NULL && sym->st_shndx != 0) { diff --git a/linker/linked_list.h b/linker/linked_list.h index 7f8c90160..52af0f110 100644 --- a/linker/linked_list.h +++ b/linker/linked_list.h @@ -31,45 +31,13 @@ struct LinkedListEntry { template class LinkedList { public: - LinkedList() : head_(nullptr), tail_(nullptr) {} + LinkedList() : head_(nullptr) {} void push_front(T* const element) { LinkedListEntry* new_entry = Allocator::alloc(); new_entry->next = head_; new_entry->element = element; head_ = new_entry; - if (tail_ == nullptr) { - tail_ = new_entry; - } - } - - void push_back(T* const element) { - LinkedListEntry* new_entry = Allocator::alloc(); - new_entry->next = nullptr; - new_entry->element = element; - if (tail_ == nullptr) { - tail_ = head_ = new_entry; - } else { - tail_->next = new_entry; - tail_ = new_entry; - } - } - - T* pop_front() { - if (head_ == nullptr) { - return nullptr; - } - - LinkedListEntry* entry = head_; - T* element = entry->element; - head_ = entry->next; - Allocator::free(entry); - - if (head_ == nullptr) { - tail_ = nullptr; - } - - return element; } void clear() { @@ -78,8 +46,6 @@ class LinkedList { head_ = head_->next; Allocator::free(p); } - - tail_ = nullptr; } template @@ -102,7 +68,6 @@ class LinkedList { private: LinkedListEntry* head_; - LinkedListEntry* tail_; DISALLOW_COPY_AND_ASSIGN(LinkedList); }; diff --git a/linker/linker.cpp b/linker/linker.cpp index f8b35d7a8..59b99383e 100644 --- a/linker/linker.cpp +++ b/linker/linker.cpp @@ -469,10 +469,6 @@ static ElfW(Sym)* soinfo_elf_lookup(soinfo* si, unsigned hash, const char* name, } } - TRACE_TYPE(LOOKUP, "NOT FOUND %s in %s@%p %x %zd", - name, si->name, reinterpret_cast(si->base), hash, hash % si->nbucket); - - return NULL; } @@ -589,43 +585,18 @@ done: return NULL; } -// Another soinfo list allocator to use in dlsym. We don't reuse -// SoinfoListAllocator because it is write-protected most of the time. -static LinkerAllocator> g_soinfo_list_allocator_rw; -class SoinfoListAllocatorRW { - public: - static LinkedListEntry* alloc() { - return g_soinfo_list_allocator_rw.alloc(); - } +/* This is used by dlsym(3). It performs symbol lookup only within the + specified soinfo object and not in any of its dependencies. - static void free(LinkedListEntry* ptr) { - g_soinfo_list_allocator_rw.free(ptr); - } -}; - -// This is used by dlsym(3). It performs symbol lookup only within the -// specified soinfo object and its dependencies in breadth first order. -ElfW(Sym)* dlsym_handle_lookup(soinfo* si, soinfo** found, const char* name, soinfo* caller) { - LinkedList visit_list; - visit_list.push_back(si); - soinfo* current_soinfo; - while ((current_soinfo = visit_list.pop_front()) != nullptr) { - ElfW(Sym)* result = soinfo_elf_lookup(current_soinfo, elfhash(name), name, - caller == current_soinfo ? SymbolLookupScope::kAllowLocal : SymbolLookupScope::kExcludeLocal); - - if (result != nullptr) { - *found = current_soinfo; - visit_list.clear(); - return result; - } - - current_soinfo->get_children().for_each([&](soinfo* child) { - visit_list.push_back(child); - }); - } - - visit_list.clear(); - return nullptr; + TODO: Only looking in the specified soinfo seems wrong. dlsym(3) says + that it should do a breadth first search through the dependency + tree. This agrees with the ELF spec (aka System V Application + Binary Interface) where in Chapter 5 it discuss resolving "Shared + Object Dependencies" in breadth first search order. + */ +ElfW(Sym)* dlsym_handle_lookup(soinfo* si, const char* name, soinfo* caller) { + return soinfo_elf_lookup(si, elfhash(name), name, + caller == si ? SymbolLookupScope::kAllowLocal : SymbolLookupScope::kExcludeLocal); } /* This is used by dlsym(3) to performs a global symbol lookup. If the diff --git a/linker/linker.h b/linker/linker.h index 03672b2c6..e1112e6e6 100644 --- a/linker/linker.h +++ b/linker/linker.h @@ -238,7 +238,7 @@ ElfW(Sym)* dlsym_linear_lookup(const char* name, soinfo** found, soinfo* start, soinfo* find_containing_library(const void* addr); ElfW(Sym)* dladdr_find_symbol(soinfo* si, const void* addr); -ElfW(Sym)* dlsym_handle_lookup(soinfo* si, soinfo** found, const char* name, soinfo* caller_si); +ElfW(Sym)* dlsym_handle_lookup(soinfo* si, const char* name, soinfo* caller_si); void debuggerd_init(); extern "C" abort_msg_t* g_abort_message; diff --git a/linker/tests/linked_list_test.cpp b/linker/tests/linked_list_test.cpp index b9816fa10..31ec7d505 100644 --- a/linker/tests/linked_list_test.cpp +++ b/linker/tests/linked_list_test.cpp @@ -95,23 +95,3 @@ TEST(linked_list, simple) { ASSERT_TRUE(free_called); ASSERT_EQ("", test_list_to_string(list)); } - -TEST(linked_list, push_pop) { - test_list_t list; - list.push_front("b"); - list.push_front("a"); - ASSERT_EQ("ab", test_list_to_string(list)); - list.push_back("c"); - ASSERT_EQ("abc", test_list_to_string(list)); - ASSERT_EQ("a", list.pop_front()); - ASSERT_EQ("bc", test_list_to_string(list)); - ASSERT_EQ("b", list.pop_front()); - ASSERT_EQ("c", test_list_to_string(list)); - ASSERT_EQ("c", list.pop_front()); - ASSERT_EQ("", test_list_to_string(list)); - ASSERT_TRUE(list.pop_front() == nullptr); - list.push_back("r"); - ASSERT_EQ("r", test_list_to_string(list)); - ASSERT_EQ("r", list.pop_front()); - ASSERT_TRUE(list.pop_front() == nullptr); -} diff --git a/tests/dlfcn_test.cpp b/tests/dlfcn_test.cpp index 9bc25574b..f056fb692 100644 --- a/tests/dlfcn_test.cpp +++ b/tests/dlfcn_test.cpp @@ -62,9 +62,10 @@ TEST(dlfcn, dlsym_in_self) { ASSERT_EQ(0, dlclose(self)); } -#if defined(__arm__) -// This seems to be working only for arm. -// Others platforms optimize LOCAL PROTECTED symbols. +#if !defined(__LP64__) +// Current compiler/static linker used for aarch64 +// platform optimizes LOCAL PROTECTED symbol +// in libtest_local_symbol.so out of existence TEST(dlfcn, dlsym_local_symbol) { void* handle = dlopen("libtest_local_symbol.so", RTLD_NOW); ASSERT_TRUE(handle != NULL); @@ -77,23 +78,9 @@ TEST(dlfcn, dlsym_local_symbol) { f = reinterpret_cast(dlsym(handle, "dlsym_local_symbol_get_taxicab_number_using_dlsym")); ASSERT_TRUE(f != NULL); ASSERT_EQ(1729U, f()); - dlclose(handle); } #endif -TEST(dlfcn, dlsym_with_dependencies) { - void* handle = dlopen("libtest_with_dependency.so", RTLD_NOW); - ASSERT_TRUE(handle != NULL); - dlerror(); - // This symbol is in DT_NEEDED library. - void* sym = dlsym(handle, "getRandomNumber"); - ASSERT_TRUE(sym != NULL); - int (*fn)(void); - fn = reinterpret_cast(sym); - EXPECT_EQ(4, fn()); - dlclose(handle); -} - TEST(dlfcn, dlopen_noload) { void* handle = dlopen("libtest_simple.so", RTLD_NOW | RTLD_NOLOAD); ASSERT_TRUE(handle == NULL); diff --git a/tests/libs/Android.mk b/tests/libs/Android.mk index 7ed3e7b07..a374e4839 100644 --- a/tests/libs/Android.mk +++ b/tests/libs/Android.mk @@ -101,19 +101,6 @@ build_type := target build_target := SHARED_LIBRARY include $(TEST_PATH)/Android.build.mk -# ----------------------------------------------------------------------------- -# Library with dependency used by dlfcn tests -# ----------------------------------------------------------------------------- -libtest_with_dependency_src_files := \ - dlopen_testlib_simple.cpp - -libtest_with_dependency_shared_libraries := libdlext_test - -module := libtest_with_dependency -build_type := target -build_target := SHARED_LIBRARY -include $(TEST_PATH)/Android.build.mk - # ----------------------------------------------------------------------------- # Library used to test local symbol lookup # -----------------------------------------------------------------------------