From 9419420919ea846bbad5510850c7aaec95021648 Mon Sep 17 00:00:00 2001 From: Dmitriy Ivanov Date: Mon, 18 Aug 2014 15:08:51 -0700 Subject: [PATCH] Revert "Add support for protected local symbol lookup." This reverts commit d97e9f546ea195686a78e539315b273393609b9e. Bug: 17107521 Change-Id: I2b81ce2b5a4a2d166133a2626e49d81b6aef3672 --- linker/dlfcn.cpp | 25 +++++++----- linker/linker.cpp | 38 +++++++----------- linker/linker.h | 4 +- tests/dlfcn_test.cpp | 19 --------- tests/libs/Android.mk | 15 -------- tests/libs/dlsym_local_symbol.map | 22 ----------- tests/libs/dlsym_local_symbol_private.cpp | 24 ------------ tests/libs/dlsym_local_symbol_public.cpp | 47 ----------------------- 8 files changed, 31 insertions(+), 163 deletions(-) delete mode 100644 tests/libs/dlsym_local_symbol.map delete mode 100644 tests/libs/dlsym_local_symbol_private.cpp delete mode 100644 tests/libs/dlsym_local_symbol_public.cpp 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(handle), &found, symbol, caller_si); + sym = dlsym_handle_lookup(reinterpret_cast(handle), &found, symbol); } - if (sym != NULL && sym->st_shndx != 0) { - return reinterpret_cast(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(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(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(s->st_value), - static_cast(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 visit_list; LinkedList 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(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 @@ -114,21 +114,6 @@ 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 := \ - 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 -#include -#include - -// 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 -#include -#include - -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(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; -}