Revert "Fix dlsym(3) to do breadth first search."
This reverts commit 422106a24d620af4be58e8d92a2e9b7b6167b72d.
Change-Id: I9e26a6933d10eb30438b521450f2010997ca5aee
diff --git a/linker/dlfcn.cpp b/linker/dlfcn.cpp
index 8ebf357..efb829e 100644
--- a/linker/dlfcn.cpp
+++ b/linker/dlfcn.cpp
@@ -111,7 +111,8 @@
sym = dlsym_linear_lookup(symbol, &found, caller_si->next, caller_si);
}
} else {
- sym = dlsym_handle_lookup(reinterpret_cast<soinfo*>(handle), &found, symbol, caller_si);
+ found = reinterpret_cast<soinfo*>(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 7f8c901..52af0f1 100644
--- a/linker/linked_list.h
+++ b/linker/linked_list.h
@@ -31,45 +31,13 @@
template<typename T, typename Allocator>
class LinkedList {
public:
- LinkedList() : head_(nullptr), tail_(nullptr) {}
+ LinkedList() : head_(nullptr) {}
void push_front(T* const element) {
LinkedListEntry<T>* 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<T>* 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<T>* 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 @@
head_ = head_->next;
Allocator::free(p);
}
-
- tail_ = nullptr;
}
template<typename F>
@@ -102,7 +68,6 @@
private:
LinkedListEntry<T>* head_;
- LinkedListEntry<T>* tail_;
DISALLOW_COPY_AND_ASSIGN(LinkedList);
};
diff --git a/linker/linker.cpp b/linker/linker.cpp
index f8b35d7..59b9938 100644
--- a/linker/linker.cpp
+++ b/linker/linker.cpp
@@ -469,10 +469,6 @@
}
}
- TRACE_TYPE(LOOKUP, "NOT FOUND %s in %s@%p %x %zd",
- name, si->name, reinterpret_cast<void*>(si->base), hash, hash % si->nbucket);
-
-
return NULL;
}
@@ -589,43 +585,18 @@
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<LinkedListEntry<soinfo>> g_soinfo_list_allocator_rw;
-class SoinfoListAllocatorRW {
- public:
- static LinkedListEntry<soinfo>* 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<soinfo>* 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<soinfo, SoinfoListAllocatorRW> 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 03672b2..e1112e6 100644
--- a/linker/linker.h
+++ b/linker/linker.h
@@ -238,7 +238,7 @@
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 b9816fa..31ec7d5 100644
--- a/linker/tests/linked_list_test.cpp
+++ b/linker/tests/linked_list_test.cpp
@@ -95,23 +95,3 @@
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 9bc2557..f056fb6 100644
--- a/tests/dlfcn_test.cpp
+++ b/tests/dlfcn_test.cpp
@@ -62,9 +62,10 @@
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 @@
f = reinterpret_cast<uint32_t (*)(void)>(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<int (*)(void)>(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 7ed3e7b..a374e48 100644
--- a/tests/libs/Android.mk
+++ b/tests/libs/Android.mk
@@ -102,19 +102,6 @@
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
# -----------------------------------------------------------------------------
libtest_local_symbol_src_files := \
