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);
-}