Revert "Add support for protected local symbol lookup."

This reverts commit d97e9f546ea195686a78e539315b273393609b9e.

Bug: 17107521
Change-Id: I2b81ce2b5a4a2d166133a2626e49d81b6aef3672

8 files changed, 31 insertions, 163 deletions

diff --git a/linker/dlfcn.cpp b/linker/dlfcn.cpp
index 8ebf3576b..5d6db8e29 100644
--- a/linker/dlfcn.cpp
+++ b/linker/dlfcn.cpp
@@ -100,22 +100,29 @@ void* dlsym(void* handle, const char* symbol) {
 
   soinfo* found = NULL;
   ElfW(Sym)* sym = NULL;
-  void* caller_addr = __builtin_return_address(0);
-  soinfo* caller_si = find_containing_library(caller_addr);
-
   if (handle == RTLD_DEFAULT) {
-    sym = dlsym_linear_lookup(symbol, &found, NULL, caller_si);
+    sym = dlsym_linear_lookup(symbol, &found, NULL);
   } else if (handle == RTLD_NEXT) {
+    void* caller_addr = __builtin_return_address(0);
+    soinfo* si = find_containing_library(caller_addr);
+
     sym = NULL;
-    if (caller_si && caller_si->next) {
-      sym = dlsym_linear_lookup(symbol, &found, caller_si->next, caller_si);
+    if (si && si->next) {
+      sym = dlsym_linear_lookup(symbol, &found, si->next);
     }
   } else {
-    sym = dlsym_handle_lookup(reinterpret_cast<soinfo*>(handle), &found, symbol, caller_si);
+    sym = dlsym_handle_lookup(reinterpret_cast<soinfo*>(handle), &found, symbol);
   }
 
-  if (sym != NULL && sym->st_shndx != 0) {
-    return reinterpret_cast<void*>(sym->st_value + found->load_bias);
+  if (sym != NULL) {
+    unsigned bind = ELF_ST_BIND(sym->st_info);
+
+    if ((bind == STB_GLOBAL || bind == STB_WEAK) && sym->st_shndx != 0) {
+      return reinterpret_cast<void*>(sym->st_value + found->load_bias);
+    }
+
+    __bionic_format_dlerror("symbol found but not global", symbol);
+    return NULL;
   } else {
     __bionic_format_dlerror("undefined symbol", symbol);
     return NULL;
diff --git a/linker/linker.cpp b/linker/linker.cpp
index 77fb70c29..52eb56ab8 100644
--- a/linker/linker.cpp
+++ b/linker/linker.cpp
@@ -125,11 +125,6 @@ enum RelocationKind {
     kRelocMax
 };
 
-enum class SymbolLookupScope {
-  kAllowLocal,
-  kExcludeLocal,
-};
-
 #if STATS
 struct linker_stats_t {
     int count[kRelocMax];
@@ -433,7 +428,7 @@ int dl_iterate_phdr(int (*cb)(dl_phdr_info* info, size_t size, void* data), void
     return rv;
 }
 
-static ElfW(Sym)* soinfo_elf_lookup(soinfo* si, unsigned hash, const char* name, const SymbolLookupScope& lookup_scope) {
+static ElfW(Sym)* soinfo_elf_lookup(soinfo* si, unsigned hash, const char* name) {
   ElfW(Sym)* symtab = si->symtab;
   const char* strtab = si->strtab;
 
@@ -444,6 +439,7 @@ static ElfW(Sym)* soinfo_elf_lookup(soinfo* si, unsigned hash, const char* name,
     ElfW(Sym)* s = symtab + n;
     if (strcmp(strtab + s->st_name, name)) continue;
 
+    /* only concern ourselves with global and weak symbol definitions */
     switch (ELF_ST_BIND(s->st_info)) {
       case STB_GLOBAL:
       case STB_WEAK:
@@ -456,13 +452,7 @@ static ElfW(Sym)* soinfo_elf_lookup(soinfo* si, unsigned hash, const char* name,
                  static_cast<size_t>(s->st_size));
         return s;
       case STB_LOCAL:
-        if (lookup_scope != SymbolLookupScope::kAllowLocal) {
-          continue;
-        }
-        TRACE_TYPE(LOOKUP, "FOUND LOCAL %s in %s (%p) %zd",
-                name, si->name, reinterpret_cast<void*>(s->st_value),
-                static_cast<size_t>(s->st_size));
-        return s;
+        continue;
       default:
         __libc_fatal("ERROR: Unexpected ST_BIND value: %d for '%s' in '%s'",
             ELF_ST_BIND(s->st_info), name, si->name);
@@ -500,7 +490,7 @@ static ElfW(Sym)* soinfo_do_lookup(soinfo* si, const char* name, soinfo** lsi, s
          */
 
         if (si == somain) {
-            s = soinfo_elf_lookup(si, elf_hash, name, SymbolLookupScope::kAllowLocal);
+            s = soinfo_elf_lookup(si, elf_hash, name);
             if (s != NULL) {
                 *lsi = si;
                 goto done;
@@ -517,7 +507,7 @@ static ElfW(Sym)* soinfo_do_lookup(soinfo* si, const char* name, soinfo** lsi, s
             if (!si->has_DT_SYMBOLIC) {
                 DEBUG("%s: looking up %s in executable %s",
                       si->name, name, somain->name);
-                s = soinfo_elf_lookup(somain, elf_hash, name, SymbolLookupScope::kExcludeLocal);
+                s = soinfo_elf_lookup(somain, elf_hash, name);
                 if (s != NULL) {
                     *lsi = somain;
                     goto done;
@@ -534,7 +524,7 @@ static ElfW(Sym)* soinfo_do_lookup(soinfo* si, const char* name, soinfo** lsi, s
              * and some the first non-weak definition.   This is system dependent.
              * Here we return the first definition found for simplicity.  */
 
-            s = soinfo_elf_lookup(si, elf_hash, name, SymbolLookupScope::kAllowLocal);
+            s = soinfo_elf_lookup(si, elf_hash, name);
             if (s != NULL) {
                 *lsi = si;
                 goto done;
@@ -548,7 +538,7 @@ static ElfW(Sym)* soinfo_do_lookup(soinfo* si, const char* name, soinfo** lsi, s
             if (si->has_DT_SYMBOLIC) {
                 DEBUG("%s: looking up %s in executable %s after local scope",
                       si->name, name, somain->name);
-                s = soinfo_elf_lookup(somain, elf_hash, name, SymbolLookupScope::kExcludeLocal);
+                s = soinfo_elf_lookup(somain, elf_hash, name);
                 if (s != NULL) {
                     *lsi = somain;
                     goto done;
@@ -559,7 +549,7 @@ static ElfW(Sym)* soinfo_do_lookup(soinfo* si, const char* name, soinfo** lsi, s
 
     /* Next, look for it in the preloads list */
     for (int i = 0; g_ld_preloads[i] != NULL; i++) {
-        s = soinfo_elf_lookup(g_ld_preloads[i], elf_hash, name, SymbolLookupScope::kExcludeLocal);
+        s = soinfo_elf_lookup(g_ld_preloads[i], elf_hash, name);
         if (s != NULL) {
             *lsi = g_ld_preloads[i];
             goto done;
@@ -569,7 +559,7 @@ static ElfW(Sym)* soinfo_do_lookup(soinfo* si, const char* name, soinfo** lsi, s
     for (int i = 0; needed[i] != NULL; i++) {
         DEBUG("%s: looking up %s in %s",
               si->name, name, needed[i]->name);
-        s = soinfo_elf_lookup(needed[i], elf_hash, name, SymbolLookupScope::kExcludeLocal);
+        s = soinfo_elf_lookup(needed[i], elf_hash, name);
         if (s != NULL) {
             *lsi = needed[i];
             goto done;
@@ -605,7 +595,7 @@ class SoinfoListAllocatorRW {
 
 // 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) {
+ElfW(Sym)* dlsym_handle_lookup(soinfo* si, soinfo** found, const char* name) {
   LinkedList<soinfo, SoinfoListAllocatorRW> visit_list;
   LinkedList<soinfo, SoinfoListAllocatorRW> visited;
   visit_list.push_back(si);
@@ -615,8 +605,7 @@ ElfW(Sym)* dlsym_handle_lookup(soinfo* si, soinfo** found, const char* name, soi
       continue;
     }
 
-    ElfW(Sym)* result = soinfo_elf_lookup(current_soinfo, elfhash(name), name,
-        caller == current_soinfo ? SymbolLookupScope::kAllowLocal : SymbolLookupScope::kExcludeLocal);
+    ElfW(Sym)* result = soinfo_elf_lookup(current_soinfo, elfhash(name), name);
 
     if (result != nullptr) {
       *found = current_soinfo;
@@ -641,7 +630,7 @@ ElfW(Sym)* dlsym_handle_lookup(soinfo* si, soinfo** found, const char* name, soi
    beginning of the global solist. Otherwise the search starts at the
    specified soinfo (for RTLD_NEXT).
  */
-ElfW(Sym)* dlsym_linear_lookup(const char* name, soinfo** found, soinfo* start, soinfo* caller) {
+ElfW(Sym)* dlsym_linear_lookup(const char* name, soinfo** found, soinfo* start) {
   unsigned elf_hash = elfhash(name);
 
   if (start == NULL) {
@@ -650,8 +639,7 @@ ElfW(Sym)* dlsym_linear_lookup(const char* name, soinfo** found, soinfo* start,
 
   ElfW(Sym)* s = NULL;
   for (soinfo* si = start; (s == NULL) && (si != NULL); si = si->next) {
-    s = soinfo_elf_lookup(si, elf_hash, name,
-        caller == si ? SymbolLookupScope::kAllowLocal : SymbolLookupScope::kExcludeLocal);
+    s = soinfo_elf_lookup(si, elf_hash, name);
     if (s != NULL) {
       *found = si;
       break;
diff --git a/linker/linker.h b/linker/linker.h
index 03672b2c6..374652eb8 100644
--- a/linker/linker.h
+++ b/linker/linker.h
@@ -234,11 +234,11 @@ void do_android_update_LD_LIBRARY_PATH(const char* ld_library_path);
 soinfo* do_dlopen(const char* name, int flags, const android_dlextinfo* extinfo);
 void do_dlclose(soinfo* si);
 
-ElfW(Sym)* dlsym_linear_lookup(const char* name, soinfo** found, soinfo* start, soinfo* caller_si);
+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, soinfo** found, const char* name);
 
 void debuggerd_init();
 extern "C" abort_msg_t* g_abort_message;
diff --git a/tests/dlfcn_test.cpp b/tests/dlfcn_test.cpp
index 9bc25574b..c2c9286f6 100644
--- a/tests/dlfcn_test.cpp
+++ b/tests/dlfcn_test.cpp
@@ -62,25 +62,6 @@ 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.
-TEST(dlfcn, dlsym_local_symbol) {
-  void* handle = dlopen("libtest_local_symbol.so", RTLD_NOW);
-  ASSERT_TRUE(handle != NULL);
-  dlerror();
-  void* sym = dlsym(handle, "private_taxicab_number");
-  ASSERT_TRUE(sym == NULL);
-  ASSERT_STREQ("undefined symbol: private_taxicab_number", dlerror());
-
-  uint32_t (*f)(void);
-  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);
diff --git a/tests/libs/Android.mk b/tests/libs/Android.mk
index 7ed3e7b07..75df539b9 100644
--- a/tests/libs/Android.mk
+++ b/tests/libs/Android.mk
@@ -115,21 +115,6 @@ build_target := SHARED_LIBRARY
 include $(TEST_PATH)/Android.build.mk
 
 # -----------------------------------------------------------------------------
-# Library used to test local symbol lookup
-# -----------------------------------------------------------------------------
-libtest_local_symbol_src_files := \
-    dlsym_local_symbol_private.cpp \
-    dlsym_local_symbol_public.cpp
-
-module := libtest_local_symbol
-build_target := SHARED_LIBRARY
-libtest_local_symbol_ldflags := -Wl,--version-script=$(LOCAL_PATH)/dlsym_local_symbol.map
-libtest_local_symbol_cppflags := -std=gnu++11
-libtest_local_symbol_shared_libraries_target := libdl
-build_type := target
-include $(TEST_PATH)/Android.build.mk
-
-# -----------------------------------------------------------------------------
 # Library used by atexit tests
 # -----------------------------------------------------------------------------
 
diff --git a/tests/libs/dlsym_local_symbol.map b/tests/libs/dlsym_local_symbol.map
deleted file mode 100644
index 58a2299a8..000000000
--- a/tests/libs/dlsym_local_symbol.map
+++ /dev/null
@@ -1,22 +0,0 @@
-/*
- * Copyright (C) 2014 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *      http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-LIBTEST_LOCAL_SYMBOL_1.0 {
-  global:
-    dlsym_local_symbol_get_taxicab_number;
-    dlsym_local_symbol_get_taxicab_number_using_dlsym;
-  local:
-    *;
-};
diff --git a/tests/libs/dlsym_local_symbol_private.cpp b/tests/libs/dlsym_local_symbol_private.cpp
deleted file mode 100644
index 2587508e9..000000000
--- a/tests/libs/dlsym_local_symbol_private.cpp
+++ /dev/null
@@ -1,24 +0,0 @@
-/*
- * Copyright (C) 2014 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *      http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-#include <stdlib.h>
-#include <dlfcn.h>
-#include <stdio.h>
-
-// This symbol is declared local in
-// the linker version map: libdlsym_local_symbol.map.
-// It should not be visible from the outside.
-extern "C" const uint32_t __attribute__ ((visibility ("protected"))) private_taxicab_number = 1729;
diff --git a/tests/libs/dlsym_local_symbol_public.cpp b/tests/libs/dlsym_local_symbol_public.cpp
deleted file mode 100644
index d9da32a59..000000000
--- a/tests/libs/dlsym_local_symbol_public.cpp
+++ /dev/null
@@ -1,47 +0,0 @@
-/*
- * Copyright (C) 2014 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *      http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-#include <stdlib.h>
-#include <dlfcn.h>
-#include <stdio.h>
-
-extern const uint32_t private_taxicab_number;
-
-extern "C" {
-uint32_t dlsym_local_symbol_get_taxicab_number();
-uint32_t dlsym_local_symbol_get_taxicab_number_using_dlsym();
-}
-
-uint32_t dlsym_local_symbol_get_taxicab_number() {
-  return private_taxicab_number;
-}
-
-// Let's make sure that dlsym works correctly for local symbol
-uint32_t dlsym_local_symbol_get_taxicab_number_using_dlsym() {
-  dlerror();
-  uint32_t* ptr = reinterpret_cast<uint32_t*>(dlsym(RTLD_DEFAULT, "private_taxicab_number"));
-  if (ptr == nullptr) {
-    const char* dlerr = dlerror();
-    if (dlerr != nullptr) {
-      fprintf(stderr, "dlsym error: %s\n", dlerr);
-    } else {
-      fprintf(stderr, "dlsym returned NULL with no dlerror.\n");
-    }
-    return 0;
-  }
-
-  return *ptr;
-}