Revert "Fix dlsym(3) to do breadth first search."linaro-armv8-14.08

This reverts commit aa0f2bdbc22d4b7aec5d3f8f5f01eaeaa13414c2.

7 files changed, 19 insertions, 128 deletions

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<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 7f8c90160..52af0f110 100644
--- a/linker/linked_list.h
+++ b/linker/linked_list.h
@@ -31,45 +31,13 @@ struct LinkedListEntry {
 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 @@ class LinkedList {
       head_ = head_->next;
       Allocator::free(p);
     }
-
-    tail_ = nullptr;
   }
 
   template<typename F>
@@ -102,7 +68,6 @@ class LinkedList {
 
  private:
   LinkedListEntry<T>* head_;
-  LinkedListEntry<T>* 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<void*>(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<LinkedListEntry<soinfo>> g_soinfo_list_allocator_rw;
-class SoinfoListAllocatorRW {
- public:
-  static LinkedListEntry<soinfo>* alloc() {
-    return g_soinfo_list_allocator_rw.alloc();
-  }
-
-  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);
+/* This is used by dlsym(3).  It performs symbol lookup only within the
+   specified soinfo object and not in any of its dependencies.
 
-    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<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 7ed3e7b07..a374e4839 100644
--- a/tests/libs/Android.mk
+++ b/tests/libs/Android.mk
@@ -102,19 +102,6 @@ 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
 # -----------------------------------------------------------------------------
 libtest_local_symbol_src_files := \