From 0caf70e98e8cf109d0116c774147c568b369fd27 Mon Sep 17 00:00:00 2001 From: Elliott Hughes Date: Wed, 23 Jul 2014 11:38:38 -0700 Subject: [PATCH 01/13] Fix belated review comments on syslog change. Bug: 14292866 (cherry picked from commit afe6360627ef3f0e9bc8f45535fbfae3354f3ae0) Change-Id: I8e3cc6b37b2539e51a27261ffb5d6e58266ce11d --- libc/bionic/libc_logging.cpp | 3 +- libc/bionic/syslog.cpp | 6 +-- libc/include/syslog.h | 77 +++++++++++++++++++----------------- 3 files changed, 45 insertions(+), 41 deletions(-) diff --git a/libc/bionic/libc_logging.cpp b/libc/bionic/libc_logging.cpp index 88d979091..cdbf7236c 100644 --- a/libc/bionic/libc_logging.cpp +++ b/libc/bionic/libc_logging.cpp @@ -230,6 +230,7 @@ static void SendRepeat(Out& o, char ch, int count) { /* Perform formatted output to an output target 'o' */ template static void out_vformat(Out& o, const char* format, va_list args) { + int caller_errno = errno; int nn = 0; for (;;) { @@ -380,7 +381,7 @@ static void out_vformat(Out& o, const char* format, va_list args) { buffer[1] = '\0'; } else if (c == 'm') { // syslog-like %m for strerror(errno). - str = strerror(errno); + str = strerror(caller_errno); } else { __assert(__FILE__, __LINE__, "conversion specifier unsupported"); } diff --git a/libc/bionic/syslog.cpp b/libc/bionic/syslog.cpp index 7e153eb16..29f892a20 100644 --- a/libc/bionic/syslog.cpp +++ b/libc/bionic/syslog.cpp @@ -14,9 +14,8 @@ * limitations under the License. */ -#include - #include +#include #include "private/libc_logging.h" @@ -24,6 +23,7 @@ static const char* syslog_log_tag = NULL; static int syslog_priority_mask = 0xff; void closelog() { + syslog_log_tag = NULL; } void openlog(const char* log_tag, int /*options*/, int /*facility*/) { @@ -61,7 +61,7 @@ void vsyslog(int priority, const char* fmt, va_list args) { // What's our Android log priority? priority &= LOG_PRIMASK; int android_log_priority; - if (priority < LOG_ERR) { + if (priority <= LOG_ERR) { android_log_priority = ANDROID_LOG_ERROR; } else if (priority == LOG_WARNING) { android_log_priority = ANDROID_LOG_WARN; diff --git a/libc/include/syslog.h b/libc/include/syslog.h index f396feca7..cbceab287 100644 --- a/libc/include/syslog.h +++ b/libc/include/syslog.h @@ -35,45 +35,48 @@ __BEGIN_DECLS -#define LOG_EMERG 0 -#define LOG_ALERT 1 -#define LOG_CRIT 2 -#define LOG_ERR 3 -#define LOG_WARNING 4 -#define LOG_NOTICE 5 -#define LOG_INFO 6 -#define LOG_DEBUG 7 +/* Priorities are translated to Android log priorities as shown. */ +#define LOG_EMERG 0 /* ERROR */ +#define LOG_ALERT 1 /* ERROR */ +#define LOG_CRIT 2 /* ERROR */ +#define LOG_ERR 3 /* ERROR */ +#define LOG_WARNING 4 /* WARN */ +#define LOG_NOTICE 5 /* INFO */ +#define LOG_INFO 6 /* INFO */ +#define LOG_DEBUG 7 /* DEBUG */ -#define LOG_PRIMASK 7 -#define LOG_PRI(x) ((x) & LOG_PRIMASK) +#define LOG_PRIMASK 7 +#define LOG_PRI(x) ((x) & LOG_PRIMASK) -#define LOG_KERN 0000 -#define LOG_USER 0010 -#define LOG_MAIL 0020 -#define LOG_DAEMON 0030 -#define LOG_AUTH 0040 -#define LOG_SYSLOG 0050 -#define LOG_LPR 0060 -#define LOG_NEWS 0070 -#define LOG_UUCP 0100 -#define LOG_CRON 0110 -#define LOG_AUTHPRIV 0120 -#define LOG_FTP 0130 -#define LOG_LOCAL0 0200 -#define LOG_LOCAL1 0210 -#define LOG_LOCAL2 0220 -#define LOG_LOCAL3 0230 -#define LOG_LOCAL4 0240 -#define LOG_LOCAL5 0250 -#define LOG_LOCAL6 0260 -#define LOG_LOCAL7 0270 +/* Facilities are currently ignored on Android. */ +#define LOG_KERN 0000 +#define LOG_USER 0010 +#define LOG_MAIL 0020 +#define LOG_DAEMON 0030 +#define LOG_AUTH 0040 +#define LOG_SYSLOG 0050 +#define LOG_LPR 0060 +#define LOG_NEWS 0070 +#define LOG_UUCP 0100 +#define LOG_CRON 0110 +#define LOG_AUTHPRIV 0120 +#define LOG_FTP 0130 +#define LOG_LOCAL0 0200 +#define LOG_LOCAL1 0210 +#define LOG_LOCAL2 0220 +#define LOG_LOCAL3 0230 +#define LOG_LOCAL4 0240 +#define LOG_LOCAL5 0250 +#define LOG_LOCAL6 0260 +#define LOG_LOCAL7 0270 -#define LOG_FACMASK 01770 -#define LOG_FAC(x) (((x) >> 3) & (LOG_FACMASK >> 3)) +#define LOG_FACMASK 01770 +#define LOG_FAC(x) (((x) >> 3) & (LOG_FACMASK >> 3)) #define LOG_MASK(pri) (1 << (pri)) #define LOG_UPTO(pri) ((1 << ((pri)+1)) - 1) +/* openlog(3) flags are currently ignored on Android. */ #define LOG_PID 0x01 #define LOG_CONS 0x02 #define LOG_ODELAY 0x04 @@ -81,11 +84,11 @@ __BEGIN_DECLS #define LOG_NOWAIT 0x10 #define LOG_PERROR 0x20 -extern void closelog(void); -extern void openlog(const char*, int, int); -extern int setlogmask(int); -extern void syslog(int, const char*, ...) __printflike(2, 3); -extern void vsyslog(int, const char*, va_list) __printflike(2, 0); +void closelog(void); +void openlog(const char*, int, int); +int setlogmask(int); +void syslog(int, const char*, ...) __printflike(2, 3); +void vsyslog(int, const char*, va_list) __printflike(2, 0); __END_DECLS From 4514aa630c8cace57a71268eabff687fcdf13ca0 Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Wed, 23 Jul 2014 14:43:44 -0700 Subject: [PATCH 02/13] HACK: remove %m support from printf. The change that added this support causes a cpu hard lock on one device. This code clearly isn't at fault, but disabling it to unblock until we can find a real fix. Bug: 16484311 Change-Id: I33834dc49d959ae403b10d2c7cad12ae2950f772 --- libc/bionic/libc_logging.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/libc/bionic/libc_logging.cpp b/libc/bionic/libc_logging.cpp index cdbf7236c..1fb8a84ab 100644 --- a/libc/bionic/libc_logging.cpp +++ b/libc/bionic/libc_logging.cpp @@ -230,7 +230,9 @@ static void SendRepeat(Out& o, char ch, int count) { /* Perform formatted output to an output target 'o' */ template static void out_vformat(Out& o, const char* format, va_list args) { +#if 0 int caller_errno = errno; +#endif int nn = 0; for (;;) { @@ -379,9 +381,11 @@ static void out_vformat(Out& o, const char* format, va_list args) { } else if (c == '%') { buffer[0] = '%'; buffer[1] = '\0'; +#if 0 } else if (c == 'm') { // syslog-like %m for strerror(errno). str = strerror(caller_errno); +#endif } else { __assert(__FILE__, __LINE__, "conversion specifier unsupported"); } From 11837629693e520452672e0eae28d3ce71f80ed6 Mon Sep 17 00:00:00 2001 From: Christopher Ferris Date: Thu, 24 Jul 2014 17:52:23 -0700 Subject: [PATCH 03/13] Use libunwindbacktrace for debug malloc code. Create a method of disabling the debug allocation code paths so that it's possible to use the libunwindbacktrace library without any modifications. Use this path to create and destroy the maps for the process. It's not stricly necessary in the init code since the symbols are not modified until after the initialize calls. Also, remove the debug_XXX source files that doesn't need to be in libc.so. Fix the maps reading code since it was completely broken for 64 bit. Bug: 16408686 (cherry picked from commit 861c0ef37bcfcae56d88572cb01c18bcfe1faded) Change-Id: I04445f0cf9a1e85172b64d57df92eb7939ce2332 --- libc/Android.mk | 8 ++-- libc/bionic/debug_mapinfo.cpp | 53 +++++++++++++++---------- libc/bionic/debug_mapinfo.h | 4 +- libc/bionic/debug_stacktrace.cpp | 13 +++++- libc/bionic/malloc_debug_check.cpp | 43 ++++++++++++++++++++ libc/bionic/malloc_debug_disable.h | 64 ++++++++++++++++++++++++++++++ libc/bionic/malloc_debug_leak.cpp | 41 +++++++++++++++++++ 7 files changed, 198 insertions(+), 28 deletions(-) create mode 100644 libc/bionic/malloc_debug_disable.h diff --git a/libc/Android.mk b/libc/Android.mk index 1fb5e84ae..8326dbbc8 100644 --- a/libc/Android.mk +++ b/libc/Android.mk @@ -982,8 +982,6 @@ LOCAL_SRC_FILES := \ $(libc_arch_dynamic_src_files) \ $(libc_static_common_src_files) \ bionic/malloc_debug_common.cpp \ - bionic/debug_mapinfo.cpp \ - bionic/debug_stacktrace.cpp \ bionic/libc_init_dynamic.cpp \ bionic/NetdClient.cpp \ @@ -1049,7 +1047,10 @@ LOCAL_CFLAGS := \ LOCAL_CONLYFLAGS := $(libc_common_conlyflags) LOCAL_CPPFLAGS := $(libc_common_cppflags) -LOCAL_C_INCLUDES := $(libc_common_c_includes) +# Make sure that unwind.h comes from libunwind. +LOCAL_C_INCLUDES := \ + external/libunwind/include \ + $(libc_common_c_includes) \ LOCAL_SRC_FILES := \ bionic/debug_mapinfo.cpp \ @@ -1064,6 +1065,7 @@ LOCAL_ADDITIONAL_DEPENDENCIES := $(libc_common_additional_dependencies) LOCAL_SHARED_LIBRARIES := libc libdl LOCAL_SYSTEM_SHARED_LIBRARIES := +LOCAL_WHOLE_STATIC_LIBRARIES := libunwindbacktrace LOCAL_ALLOW_UNDEFINED_SYMBOLS := true # Don't install on release build diff --git a/libc/bionic/debug_mapinfo.cpp b/libc/bionic/debug_mapinfo.cpp index 17276ce40..d83799afa 100644 --- a/libc/bionic/debug_mapinfo.cpp +++ b/libc/bionic/debug_mapinfo.cpp @@ -26,39 +26,48 @@ * SUCH DAMAGE. */ +#include +#include #include #include #include -#include #include "debug_mapinfo.h" +#include "malloc_debug_disable.h" -// 6f000000-6f01e000 rwxp 00000000 00:0c 16389419 /system/lib/libcomposer.so -// 012345678901234567890123456789012345678901234567890123456789 -// 0 1 2 3 4 5 - +// Format of /proc//maps: +// 6f000000-6f01e000 rwxp 00000000 00:0c 16389419 /system/lib/libcomposer.so static mapinfo_t* parse_maps_line(char* line) { - int len = strlen(line); + uintptr_t start; + uintptr_t end; + int name_pos; + if (sscanf(line, "%" PRIxPTR "-%" PRIxPTR " %*4s %*x %*x:%*x %*d%n", &start, + &end, &name_pos) < 2) { + return NULL; + } - if (len < 1) return 0; - line[--len] = 0; - - if (len < 50) return 0; - if (line[20] != 'x') return 0; - - mapinfo_t* mi = static_cast( - mmap(NULL, sizeof(mapinfo_t) + (len - 47), PROT_READ | PROT_WRITE, MAP_ANON | MAP_PRIVATE, -1, 0)); - if (mi == MAP_FAILED) return 0; - - mi->start = strtoul(line, 0, 16); - mi->end = strtoul(line + 9, 0, 16); - mi->next = 0; - strcpy(mi->name, line + 49); + while (isspace(line[name_pos])) { + name_pos += 1; + } + const char* name = line + name_pos; + size_t name_len = strlen(name); + if (name_len && name[name_len - 1] == '\n') { + name_len -= 1; + } + mapinfo_t* mi = reinterpret_cast(calloc(1, sizeof(mapinfo_t) + name_len + 1)); + if (mi) { + mi->start = start; + mi->end = end; + memcpy(mi->name, name, name_len); + mi->name[name_len] = '\0'; + } return mi; } __LIBC_HIDDEN__ mapinfo_t* mapinfo_create(pid_t pid) { + ScopedDisableDebugCalls disable; + struct mapinfo_t* milist = NULL; char data[1024]; // Used to read lines as well as to construct the filename. snprintf(data, sizeof(data), "/proc/%d/maps", pid); @@ -77,10 +86,12 @@ __LIBC_HIDDEN__ mapinfo_t* mapinfo_create(pid_t pid) { } __LIBC_HIDDEN__ void mapinfo_destroy(mapinfo_t* mi) { + ScopedDisableDebugCalls disable; + while (mi != NULL) { mapinfo_t* del = mi; mi = mi->next; - munmap(del, sizeof(mapinfo_t) + strlen(del->name) + 2); + free(del); } } diff --git a/libc/bionic/debug_mapinfo.h b/libc/bionic/debug_mapinfo.h index cccd2e374..926b37762 100644 --- a/libc/bionic/debug_mapinfo.h +++ b/libc/bionic/debug_mapinfo.h @@ -33,8 +33,8 @@ struct mapinfo_t { struct mapinfo_t* next; - unsigned start; - unsigned end; + uintptr_t start; + uintptr_t end; char name[]; }; diff --git a/libc/bionic/debug_stacktrace.cpp b/libc/bionic/debug_stacktrace.cpp index 713e76113..b86e2afdc 100644 --- a/libc/bionic/debug_stacktrace.cpp +++ b/libc/bionic/debug_stacktrace.cpp @@ -35,6 +35,7 @@ #include #include "debug_mapinfo.h" +#include "malloc_debug_disable.h" #include "private/libc_logging.h" #if defined(__LP64__) @@ -56,6 +57,8 @@ typedef char* (*DemanglerFn)(const char*, char*, size_t*, int*); static DemanglerFn g_demangler_fn = NULL; __LIBC_HIDDEN__ void backtrace_startup() { + ScopedDisableDebugCalls disable; + g_map_info = mapinfo_create(getpid()); g_demangler = dlopen("libgccdemangle.so", RTLD_NOW); if (g_demangler != NULL) { @@ -65,6 +68,8 @@ __LIBC_HIDDEN__ void backtrace_startup() { } __LIBC_HIDDEN__ void backtrace_shutdown() { + ScopedDisableDebugCalls disable; + mapinfo_destroy(g_map_info); dlclose(g_demangler); } @@ -98,7 +103,7 @@ static _Unwind_Reason_Code trace_function(__unwind_context* context, void* arg) return _URC_NO_REASON; } -#ifdef __arm__ +#if defined(__arm__) /* * The instruction pointer is pointing at the instruction after the bl(x), and * the _Unwind_Backtrace routine already masks the Thumb mode indicator (LSB @@ -121,12 +126,16 @@ static _Unwind_Reason_Code trace_function(__unwind_context* context, void* arg) } __LIBC_HIDDEN__ int get_backtrace(uintptr_t* frames, size_t max_depth) { + ScopedDisableDebugCalls disable; + stack_crawl_state_t state(frames, max_depth); _Unwind_Backtrace(trace_function, &state); return state.frame_count; } __LIBC_HIDDEN__ void log_backtrace(uintptr_t* frames, size_t frame_count) { + ScopedDisableDebugCalls disable; + uintptr_t self_bt[16]; if (frames == NULL) { frame_count = get_backtrace(self_bt, 16); @@ -146,7 +155,7 @@ __LIBC_HIDDEN__ void log_backtrace(uintptr_t* frames, size_t frame_count) { symbol = info.dli_sname; } - uintptr_t rel_pc; + uintptr_t rel_pc = offset; const mapinfo_t* mi = (g_map_info != NULL) ? mapinfo_find(g_map_info, frames[i], &rel_pc) : NULL; const char* soname = (mi != NULL) ? mi->name : info.dli_fname; if (soname == NULL) { diff --git a/libc/bionic/malloc_debug_check.cpp b/libc/bionic/malloc_debug_check.cpp index 1c63d4dcf..94ba6f5ec 100644 --- a/libc/bionic/malloc_debug_check.cpp +++ b/libc/bionic/malloc_debug_check.cpp @@ -49,6 +49,7 @@ #include "debug_mapinfo.h" #include "debug_stacktrace.h" #include "malloc_debug_common.h" +#include "malloc_debug_disable.h" #include "private/bionic_macros.h" #include "private/libc_logging.h" #include "private/ScopedPthreadMutexLocker.h" @@ -331,6 +332,9 @@ static inline void add_to_backlog(hdr_t* hdr) { extern "C" void* chk_malloc(size_t bytes) { // log_message("%s: %s\n", __FILE__, __FUNCTION__); + if (DebugCallsDisabled()) { + return g_malloc_dispatch->malloc(bytes); + } size_t size = sizeof(hdr_t) + bytes + sizeof(ftr_t); if (size < bytes) { // Overflow @@ -348,6 +352,10 @@ extern "C" void* chk_malloc(size_t bytes) { } extern "C" void* chk_memalign(size_t alignment, size_t bytes) { + if (DebugCallsDisabled()) { + return g_malloc_dispatch->memalign(alignment, bytes); + } + if (alignment <= MALLOC_ALIGNMENT) { return chk_malloc(bytes); } @@ -386,6 +394,9 @@ extern "C" void* chk_memalign(size_t alignment, size_t bytes) { extern "C" void chk_free(void* ptr) { // log_message("%s: %s\n", __FILE__, __FUNCTION__); + if (DebugCallsDisabled()) { + return g_malloc_dispatch->free(ptr); + } if (!ptr) /* ignore free(NULL) */ return; @@ -421,6 +432,9 @@ extern "C" void chk_free(void* ptr) { extern "C" void* chk_realloc(void* ptr, size_t bytes) { // log_message("%s: %s\n", __FILE__, __FUNCTION__); + if (DebugCallsDisabled()) { + return g_malloc_dispatch->realloc(ptr, bytes); + } if (!ptr) { return chk_malloc(bytes); @@ -496,6 +510,10 @@ extern "C" void* chk_realloc(void* ptr, size_t bytes) { extern "C" void* chk_calloc(size_t nmemb, size_t bytes) { // log_message("%s: %s\n", __FILE__, __FUNCTION__); + if (DebugCallsDisabled()) { + return g_malloc_dispatch->calloc(nmemb, bytes); + } + size_t total_bytes = nmemb * bytes; size_t size = sizeof(hdr_t) + total_bytes + sizeof(ftr_t); if (size < total_bytes || (nmemb && SIZE_MAX / nmemb < bytes)) { // Overflow @@ -513,6 +531,10 @@ extern "C" void* chk_calloc(size_t nmemb, size_t bytes) { } extern "C" size_t chk_malloc_usable_size(const void* ptr) { + if (DebugCallsDisabled()) { + return g_malloc_dispatch->malloc_usable_size(ptr); + } + // malloc_usable_size returns 0 for NULL and unknown blocks. if (ptr == NULL) return 0; @@ -529,6 +551,10 @@ extern "C" struct mallinfo chk_mallinfo() { } extern "C" int chk_posix_memalign(void** memptr, size_t alignment, size_t size) { + if (DebugCallsDisabled()) { + return g_malloc_dispatch->posix_memalign(memptr, alignment, size); + } + if (!powerof2(alignment)) { return EINVAL; } @@ -538,7 +564,12 @@ extern "C" int chk_posix_memalign(void** memptr, size_t alignment, size_t size) return (*memptr != NULL) ? 0 : ENOMEM; } +#if defined(HAVE_DEPRECATED_MALLOC_FUNCS) extern "C" void* chk_pvalloc(size_t bytes) { + if (DebugCallsDisabled()) { + return g_malloc_dispatch->pvalloc(bytes); + } + size_t pagesize = getpagesize(); size_t size = BIONIC_ALIGN(bytes, pagesize); if (size < bytes) { // Overflow @@ -548,10 +579,16 @@ extern "C" void* chk_pvalloc(size_t bytes) { } extern "C" void* chk_valloc(size_t size) { + if (DebugCallsDisabled()) { + return g_malloc_dispatch->valloc(size); + } return chk_memalign(getpagesize(), size); } +#endif static void ReportMemoryLeaks() { + ScopedDisableDebugCalls disable; + // Use /proc/self/exe link to obtain the program name for logging // purposes. If it's not available, we set it to "". char exe[PATH_MAX]; @@ -585,10 +622,14 @@ static void ReportMemoryLeaks() { } } +pthread_key_t g_debug_calls_disabled; + extern "C" bool malloc_debug_initialize(HashTable* hash_table, const MallocDebug* malloc_dispatch) { g_hash_table = hash_table; g_malloc_dispatch = malloc_dispatch; + pthread_key_create(&g_debug_calls_disabled, NULL); + char debug_backlog[PROP_VALUE_MAX]; if (__system_property_get("libc.debug.malloc.backlog", debug_backlog)) { g_malloc_debug_backlog = atoi(debug_backlog); @@ -605,4 +646,6 @@ extern "C" void malloc_debug_finalize(int malloc_debug_level) { ReportMemoryLeaks(); } backtrace_shutdown(); + + pthread_setspecific(g_debug_calls_disabled, NULL); } diff --git a/libc/bionic/malloc_debug_disable.h b/libc/bionic/malloc_debug_disable.h new file mode 100644 index 000000000..9503128c4 --- /dev/null +++ b/libc/bionic/malloc_debug_disable.h @@ -0,0 +1,64 @@ +/* + * Copyright (C) 2014 The Android Open Source Project + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in + * the documentation and/or other materials provided with the + * distribution. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS + * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE + * COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, + * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, + * BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS + * OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED + * AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, + * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT + * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF + * SUCH DAMAGE. + */ + +#ifndef MALLOC_DEBUG_DISABLE_H +#define MALLOC_DEBUG_DISABLE_H + +#include + +#include "private/bionic_macros.h" + +// ============================================================================= +// Used to disable the debug allocation calls. +// ============================================================================= +extern pthread_key_t g_debug_calls_disabled; + +static inline bool DebugCallsDisabled() { + return pthread_getspecific(g_debug_calls_disabled) != NULL; +} + +class ScopedDisableDebugCalls { + public: + ScopedDisableDebugCalls() : disabled_(DebugCallsDisabled()) { + if (!disabled_) { + pthread_setspecific(g_debug_calls_disabled, reinterpret_cast(1)); + } + } + ~ScopedDisableDebugCalls() { + if (!disabled_) { + pthread_setspecific(g_debug_calls_disabled, NULL); + } + } + + private: + bool disabled_; + + DISALLOW_COPY_AND_ASSIGN(ScopedDisableDebugCalls); +}; + +#endif // MALLOC_DEBUG_DISABLE_H diff --git a/libc/bionic/malloc_debug_leak.cpp b/libc/bionic/malloc_debug_leak.cpp index d9824f09f..7926a1f27 100644 --- a/libc/bionic/malloc_debug_leak.cpp +++ b/libc/bionic/malloc_debug_leak.cpp @@ -48,6 +48,7 @@ #include "debug_stacktrace.h" #include "malloc_debug_common.h" +#include "malloc_debug_disable.h" #include "private/bionic_macros.h" #include "private/libc_logging.h" @@ -267,6 +268,7 @@ extern "C" int fill_posix_memalign(void** memptr, size_t alignment, size_t size) return (*memptr != NULL) ? 0 : ENOMEM; } +#if defined(HAVE_DEPRECATED_MALLOC_FUNCS) extern "C" void* fill_pvalloc(size_t bytes) { size_t pagesize = getpagesize(); size_t size = BIONIC_ALIGN(bytes, pagesize); @@ -279,6 +281,7 @@ extern "C" void* fill_pvalloc(size_t bytes) { extern "C" void* fill_valloc(size_t size) { return fill_memalign(getpagesize(), size); } +#endif // ============================================================================= // malloc leak functions @@ -287,6 +290,10 @@ extern "C" void* fill_valloc(size_t size) { static uint32_t MEMALIGN_GUARD = 0xA1A41520; extern "C" void* leak_malloc(size_t bytes) { + if (DebugCallsDisabled()) { + return g_malloc_dispatch->malloc(bytes); + } + // allocate enough space infront of the allocation to store the pointer for // the alloc structure. This will making free'ing the structer really fast! @@ -319,6 +326,10 @@ extern "C" void* leak_malloc(size_t bytes) { } extern "C" void leak_free(void* mem) { + if (DebugCallsDisabled()) { + return g_malloc_dispatch->free(mem); + } + if (mem == NULL) { return; } @@ -355,6 +366,10 @@ extern "C" void leak_free(void* mem) { } extern "C" void* leak_calloc(size_t n_elements, size_t elem_size) { + if (DebugCallsDisabled()) { + return g_malloc_dispatch->calloc(n_elements, elem_size); + } + // Fail on overflow - just to be safe even though this code runs only // within the debugging C library, not the production one. if (n_elements && SIZE_MAX / n_elements < elem_size) { @@ -370,6 +385,10 @@ extern "C" void* leak_calloc(size_t n_elements, size_t elem_size) { } extern "C" void* leak_realloc(void* oldMem, size_t bytes) { + if (DebugCallsDisabled()) { + return g_malloc_dispatch->realloc(oldMem, bytes); + } + if (oldMem == NULL) { return leak_malloc(bytes); } @@ -398,6 +417,10 @@ extern "C" void* leak_realloc(void* oldMem, size_t bytes) { } extern "C" void* leak_memalign(size_t alignment, size_t bytes) { + if (DebugCallsDisabled()) { + return g_malloc_dispatch->memalign(alignment, bytes); + } + // we can just use malloc if (alignment <= MALLOC_ALIGNMENT) { return leak_malloc(bytes); @@ -439,6 +462,10 @@ extern "C" void* leak_memalign(size_t alignment, size_t bytes) { } extern "C" size_t leak_malloc_usable_size(const void* mem) { + if (DebugCallsDisabled()) { + return g_malloc_dispatch->malloc_usable_size(mem); + } + if (mem != NULL) { // Check the guard to make sure it is valid. const AllocationEntry* header = const_to_header((void*)mem); @@ -467,6 +494,10 @@ extern "C" struct mallinfo leak_mallinfo() { } extern "C" int leak_posix_memalign(void** memptr, size_t alignment, size_t size) { + if (DebugCallsDisabled()) { + return g_malloc_dispatch->posix_memalign(memptr, alignment, size); + } + if (!powerof2(alignment)) { return EINVAL; } @@ -476,7 +507,12 @@ extern "C" int leak_posix_memalign(void** memptr, size_t alignment, size_t size) return (*memptr != NULL) ? 0 : ENOMEM; } +#if defined(HAVE_DEPRECATED_MALLOC_FUNCS) extern "C" void* leak_pvalloc(size_t bytes) { + if (DebugCallsDisabled()) { + return g_malloc_dispatch->pvalloc(bytes); + } + size_t pagesize = getpagesize(); size_t size = BIONIC_ALIGN(bytes, pagesize); if (size < bytes) { // Overflow @@ -486,5 +522,10 @@ extern "C" void* leak_pvalloc(size_t bytes) { } extern "C" void* leak_valloc(size_t size) { + if (DebugCallsDisabled()) { + return g_malloc_dispatch->valloc(size); + } + return leak_memalign(getpagesize(), size); } +#endif From b5e08542840d5722defae3e750d49a7d5ce6ccc9 Mon Sep 17 00:00:00 2001 From: Christopher Ferris Date: Thu, 7 Aug 2014 16:21:21 -0700 Subject: [PATCH 04/13] Add a way to disable backtracing in malloc debug. The property libc.debug.malloc.nobacktrace set to non-zero disables getting backtracing when using mode 1 or mode 10. Bug: 16874447 Change-Id: I7650ba9f4385b5110b743cab01e877fc69545b3c --- libc/bionic/malloc_debug_backtrace.h | 37 +++++++++++ libc/bionic/malloc_debug_check.cpp | 93 ++++++++++++++++++---------- libc/bionic/malloc_debug_leak.cpp | 3 +- 3 files changed, 98 insertions(+), 35 deletions(-) create mode 100644 libc/bionic/malloc_debug_backtrace.h diff --git a/libc/bionic/malloc_debug_backtrace.h b/libc/bionic/malloc_debug_backtrace.h new file mode 100644 index 000000000..774548b59 --- /dev/null +++ b/libc/bionic/malloc_debug_backtrace.h @@ -0,0 +1,37 @@ +/* + * Copyright (C) 2014 The Android Open Source Project + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in + * the documentation and/or other materials provided with the + * distribution. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS + * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE + * COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, + * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, + * BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS + * OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED + * AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, + * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT + * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF + * SUCH DAMAGE. + */ + +#ifndef MALLOC_DEBUG_BACKTRACE_H +#define MALLOC_DEBUG_BACKTRACE_H + +extern bool g_backtrace_enabled; + +#define GET_BACKTRACE(bt, depth) \ + (g_backtrace_enabled ? get_backtrace(bt, depth) : 0) + +#endif // MALLOC_DEBUG_BACKTRACE_H diff --git a/libc/bionic/malloc_debug_check.cpp b/libc/bionic/malloc_debug_check.cpp index 94ba6f5ec..dee03fae1 100644 --- a/libc/bionic/malloc_debug_check.cpp +++ b/libc/bionic/malloc_debug_check.cpp @@ -48,6 +48,7 @@ #include "debug_mapinfo.h" #include "debug_stacktrace.h" +#include "malloc_debug_backtrace.h" #include "malloc_debug_common.h" #include "malloc_debug_disable.h" #include "private/bionic_macros.h" @@ -123,6 +124,10 @@ static pthread_mutex_t backlog_lock = PTHREAD_MUTEX_INITIALIZER; // It determines the size of the backlog we use to detect multiple frees. static unsigned g_malloc_debug_backlog = 100; +// This variable is set to false if the property libc.debug.malloc.nobacktrace +// is set to non-zero. +__LIBC_HIDDEN__ bool g_backtrace_enabled = true; + __LIBC_HIDDEN__ HashTable* g_hash_table; __LIBC_HIDDEN__ const MallocDebug* g_malloc_dispatch; @@ -273,7 +278,7 @@ static inline int check_allocation_locked(hdr_t* hdr, int* safe) { valid = check_guards(hdr, safe); } - if (!valid && *safe) { + if (!valid && *safe && g_backtrace_enabled) { log_message("+++ ALLOCATION %p SIZE %d ALLOCATED HERE:\n", user(hdr), hdr->size); log_backtrace(hdr->bt, hdr->bt_depth); @@ -344,7 +349,7 @@ extern "C" void* chk_malloc(size_t bytes) { hdr_t* hdr = static_cast(g_malloc_dispatch->malloc(size)); if (hdr) { hdr->base = hdr; - hdr->bt_depth = get_backtrace(hdr->bt, MAX_BACKTRACE_DEPTH); + hdr->bt_depth = GET_BACKTRACE(hdr->bt, MAX_BACKTRACE_DEPTH); add(hdr, bytes); return user(hdr); } @@ -385,7 +390,7 @@ extern "C" void* chk_memalign(size_t alignment, size_t bytes) { hdr_t* hdr = meta(reinterpret_cast(ptr)); hdr->base = base; - hdr->bt_depth = get_backtrace(hdr->bt, MAX_BACKTRACE_DEPTH); + hdr->bt_depth = GET_BACKTRACE(hdr->bt, MAX_BACKTRACE_DEPTH); add(hdr, bytes); return user(hdr); } @@ -405,27 +410,31 @@ extern "C" void chk_free(void* ptr) { if (del(hdr) < 0) { uintptr_t bt[MAX_BACKTRACE_DEPTH]; - int depth = get_backtrace(bt, MAX_BACKTRACE_DEPTH); + int depth = GET_BACKTRACE(bt, MAX_BACKTRACE_DEPTH); if (hdr->tag == BACKLOG_TAG) { log_message("+++ ALLOCATION %p SIZE %d BYTES MULTIPLY FREED!\n", user(hdr), hdr->size); - log_message("+++ ALLOCATION %p SIZE %d ALLOCATED HERE:\n", - user(hdr), hdr->size); - log_backtrace(hdr->bt, hdr->bt_depth); - /* hdr->freed_bt_depth should be nonzero here */ - log_message("+++ ALLOCATION %p SIZE %d FIRST FREED HERE:\n", - user(hdr), hdr->size); - log_backtrace(hdr->freed_bt, hdr->freed_bt_depth); - log_message("+++ ALLOCATION %p SIZE %d NOW BEING FREED HERE:\n", - user(hdr), hdr->size); - log_backtrace(bt, depth); + if (g_backtrace_enabled) { + log_message("+++ ALLOCATION %p SIZE %d ALLOCATED HERE:\n", + user(hdr), hdr->size); + log_backtrace(hdr->bt, hdr->bt_depth); + /* hdr->freed_bt_depth should be nonzero here */ + log_message("+++ ALLOCATION %p SIZE %d FIRST FREED HERE:\n", + user(hdr), hdr->size); + log_backtrace(hdr->freed_bt, hdr->freed_bt_depth); + log_message("+++ ALLOCATION %p SIZE %d NOW BEING FREED HERE:\n", + user(hdr), hdr->size); + log_backtrace(bt, depth); + } } else { log_message("+++ ALLOCATION %p IS CORRUPTED OR NOT ALLOCATED VIA TRACKER!\n", user(hdr)); - log_backtrace(bt, depth); + if (g_backtrace_enabled) { + log_backtrace(bt, depth); + } } } else { - hdr->freed_bt_depth = get_backtrace(hdr->freed_bt, MAX_BACKTRACE_DEPTH); + hdr->freed_bt_depth = GET_BACKTRACE(hdr->freed_bt, MAX_BACKTRACE_DEPTH); add_to_backlog(hdr); } } @@ -451,22 +460,24 @@ extern "C" void* chk_realloc(void* ptr, size_t bytes) { if (del(hdr) < 0) { uintptr_t bt[MAX_BACKTRACE_DEPTH]; - int depth = get_backtrace(bt, MAX_BACKTRACE_DEPTH); + int depth = GET_BACKTRACE(bt, MAX_BACKTRACE_DEPTH); if (hdr->tag == BACKLOG_TAG) { log_message("+++ REALLOCATION %p SIZE %d OF FREED MEMORY!\n", user(hdr), bytes, hdr->size); - log_message("+++ ALLOCATION %p SIZE %d ALLOCATED HERE:\n", - user(hdr), hdr->size); - log_backtrace(hdr->bt, hdr->bt_depth); - /* hdr->freed_bt_depth should be nonzero here */ - log_message("+++ ALLOCATION %p SIZE %d FIRST FREED HERE:\n", - user(hdr), hdr->size); - log_backtrace(hdr->freed_bt, hdr->freed_bt_depth); - log_message("+++ ALLOCATION %p SIZE %d NOW BEING REALLOCATED HERE:\n", - user(hdr), hdr->size); - log_backtrace(bt, depth); + if (g_backtrace_enabled) { + log_message("+++ ALLOCATION %p SIZE %d ALLOCATED HERE:\n", + user(hdr), hdr->size); + log_backtrace(hdr->bt, hdr->bt_depth); + /* hdr->freed_bt_depth should be nonzero here */ + log_message("+++ ALLOCATION %p SIZE %d FIRST FREED HERE:\n", + user(hdr), hdr->size); + log_backtrace(hdr->freed_bt, hdr->freed_bt_depth); + log_message("+++ ALLOCATION %p SIZE %d NOW BEING REALLOCATED HERE:\n", + user(hdr), hdr->size); + log_backtrace(bt, depth); + } - /* We take the memory out of the backlog and fall through so the + /* We take the memory out of the backlog and fall through so the * reallocation below succeeds. Since we didn't really free it, we * can default to this behavior. */ @@ -474,7 +485,9 @@ extern "C" void* chk_realloc(void* ptr, size_t bytes) { } else { log_message("+++ REALLOCATION %p SIZE %d IS CORRUPTED OR NOT ALLOCATED VIA TRACKER!\n", user(hdr), bytes); - log_backtrace(bt, depth); + if (g_backtrace_enabled) { + log_backtrace(bt, depth); + } // just get a whole new allocation and leak the old one return g_malloc_dispatch->realloc(0, bytes); // return realloc(user(hdr), bytes); // assuming it was allocated externally @@ -501,7 +514,7 @@ extern "C" void* chk_realloc(void* ptr, size_t bytes) { } if (hdr) { hdr->base = hdr; - hdr->bt_depth = get_backtrace(hdr->bt, MAX_BACKTRACE_DEPTH); + hdr->bt_depth = GET_BACKTRACE(hdr->bt, MAX_BACKTRACE_DEPTH); add(hdr, bytes); return user(hdr); } @@ -523,7 +536,7 @@ extern "C" void* chk_calloc(size_t nmemb, size_t bytes) { hdr_t* hdr = static_cast(g_malloc_dispatch->calloc(1, size)); if (hdr) { hdr->base = hdr; - hdr->bt_depth = get_backtrace(hdr->bt, MAX_BACKTRACE_DEPTH); + hdr->bt_depth = GET_BACKTRACE(hdr->bt, MAX_BACKTRACE_DEPTH); add(hdr, total_bytes); return user(hdr); } @@ -611,7 +624,7 @@ static void ReportMemoryLeaks() { hdr_t* block = head; log_message("+++ %s leaked block of size %d at %p (leak %d of %d)", exe, block->size, user(block), index++, total); - if (del_leak(block, &safe)) { + if (del_leak(block, &safe) && g_backtrace_enabled) { /* safe == 1, because the allocation is valid */ log_backtrace(block->bt, block->bt_depth); } @@ -636,7 +649,17 @@ extern "C" bool malloc_debug_initialize(HashTable* hash_table, const MallocDebug info_log("%s: setting backlog length to %d\n", getprogname(), g_malloc_debug_backlog); } - backtrace_startup(); + // Check if backtracing should be disabled. + char env[PROP_VALUE_MAX]; + if (__system_property_get("libc.debug.malloc.nobacktrace", env) && atoi(env) != 0) { + g_backtrace_enabled = false; + __libc_format_log(ANDROID_LOG_INFO, "libc", "not gathering backtrace information\n"); + } + + if (g_backtrace_enabled) { + backtrace_startup(); + } + return true; } @@ -645,7 +668,9 @@ extern "C" void malloc_debug_finalize(int malloc_debug_level) { if (malloc_debug_level == 10) { ReportMemoryLeaks(); } - backtrace_shutdown(); + if (g_backtrace_enabled) { + backtrace_shutdown(); + } pthread_setspecific(g_debug_calls_disabled, NULL); } diff --git a/libc/bionic/malloc_debug_leak.cpp b/libc/bionic/malloc_debug_leak.cpp index 7926a1f27..837dccc5a 100644 --- a/libc/bionic/malloc_debug_leak.cpp +++ b/libc/bionic/malloc_debug_leak.cpp @@ -47,6 +47,7 @@ #include #include "debug_stacktrace.h" +#include "malloc_debug_backtrace.h" #include "malloc_debug_common.h" #include "malloc_debug_disable.h" @@ -311,7 +312,7 @@ extern "C" void* leak_malloc(size_t bytes) { ScopedPthreadMutexLocker locker(&g_hash_table->lock); uintptr_t backtrace[BACKTRACE_SIZE]; - size_t numEntries = get_backtrace(backtrace, BACKTRACE_SIZE); + size_t numEntries = GET_BACKTRACE(backtrace, BACKTRACE_SIZE); AllocationEntry* header = reinterpret_cast(base); header->entry = record_backtrace(backtrace, numEntries, bytes); From a0108accb212fd916fb776e6675cc6261d1b1433 Mon Sep 17 00:00:00 2001 From: Christopher Ferris Date: Fri, 15 Aug 2014 18:42:58 -0700 Subject: [PATCH 05/13] Fix leak_realloc, copy entire allocation. Bug: 16874447 Change-Id: Ie54a73fd75529961195fa5173d9116d0ae897b03 --- libc/bionic/malloc_debug_leak.cpp | 64 +++++++++++++++---------------- 1 file changed, 31 insertions(+), 33 deletions(-) diff --git a/libc/bionic/malloc_debug_leak.cpp b/libc/bionic/malloc_debug_leak.cpp index 27b671432..df0f997e5 100644 --- a/libc/bionic/malloc_debug_leak.cpp +++ b/libc/bionic/malloc_debug_leak.cpp @@ -385,6 +385,36 @@ extern "C" void* leak_calloc(size_t n_elements, size_t elem_size) { return ptr; } +extern "C" size_t leak_malloc_usable_size(const void* mem) { + if (DebugCallsDisabled()) { + return g_malloc_dispatch->malloc_usable_size(mem); + } + + if (mem == NULL) { + return 0; + } + + // Check the guard to make sure it is valid. + const AllocationEntry* header = const_to_header(mem); + + if (header->guard == MEMALIGN_GUARD) { + // If this is a memalign'd pointer, then grab the header from + // entry. + header = const_to_header(header->entry); + } else if (header->guard != GUARD) { + debug_log("WARNING bad header guard: '0x%x'! and invalid entry: %p\n", + header->guard, header->entry); + return 0; + } + + size_t ret = g_malloc_dispatch->malloc_usable_size(header); + if (ret != 0) { + // The usable area starts at 'mem' and stops at 'header+ret'. + return reinterpret_cast(header) + ret - reinterpret_cast(mem); + } + return 0; +} + extern "C" void* leak_realloc(void* oldMem, size_t bytes) { if (DebugCallsDisabled()) { return g_malloc_dispatch->realloc(oldMem, bytes); @@ -408,7 +438,7 @@ extern "C" void* leak_realloc(void* oldMem, size_t bytes) { newMem = leak_malloc(bytes); if (newMem != NULL) { - size_t oldSize = header->entry->size & ~SIZE_FLAG_MASK; + size_t oldSize = leak_malloc_usable_size(oldMem); size_t copySize = (oldSize <= bytes) ? oldSize : bytes; memcpy(newMem, oldMem, copySize); leak_free(oldMem); @@ -462,38 +492,6 @@ extern "C" void* leak_memalign(size_t alignment, size_t bytes) { return base; } -extern "C" size_t leak_malloc_usable_size(const void* mem) { - if (DebugCallsDisabled()) { - return g_malloc_dispatch->malloc_usable_size(mem); - } - - if (mem != NULL) { - // Check the guard to make sure it is valid. - const AllocationEntry* header = const_to_header((void*)mem); - - if (header->guard == MEMALIGN_GUARD) { - // If this is a memalign'd pointer, then grab the header from - // entry. - header = const_to_header(header->entry); - } else if (header->guard != GUARD) { - debug_log("WARNING bad header guard: '0x%x'! and invalid entry: %p\n", - header->guard, header->entry); - return 0; - } - - // TODO: Temporary workaround to avoid a crash b/16874447. - return header->entry->size & ~SIZE_FLAG_MASK; -#if 0 - size_t ret = g_malloc_dispatch->malloc_usable_size(header); - if (ret != 0) { - // The usable area starts at 'mem' and stops at 'header+ret'. - return reinterpret_cast(header) + ret - reinterpret_cast(mem); - } -#endif - } - return 0; -} - extern "C" struct mallinfo leak_mallinfo() { return g_malloc_dispatch->mallinfo(); } From 20dc3f8fa4b192d902d58f496ae15ff33faa78ac Mon Sep 17 00:00:00 2001 From: Christopher Ferris Date: Tue, 26 Aug 2014 20:48:11 -0700 Subject: [PATCH 06/13] Replace snprintf calls in linker. When enabling debug malloc, the snprintf calls in the linker fails to update the buffer. The problem is that snprintf makes a call to pthread_getspecific that returns a valid pointer, but the data it points to is zero. This should never happen and causes the snprintf to stop and do nothing. Temporarily replace snprintf with a different implementation to work around this issue. Bug: 16874447 Bug: 17302493 Change-Id: I7a500f28adf153150cf2812fae745ff41f1c48d3 --- linker/debugger.cpp | 4 ++-- linker/linker.cpp | 16 +++++++++++++++- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/linker/debugger.cpp b/linker/debugger.cpp index 079682cab..c31615171 100644 --- a/linker/debugger.cpp +++ b/linker/debugger.cpp @@ -170,9 +170,9 @@ static void log_signal_summary(int signum, const siginfo_t* info) { if (info != NULL) { // For a rethrown signal, this si_code will be right and the one debuggerd shows will // always be SI_TKILL. - snprintf(code_desc, sizeof(code_desc), ", code %d", info->si_code); + __libc_format_buffer(code_desc, sizeof(code_desc), ", code %d", info->si_code); if (has_address) { - snprintf(addr_desc, sizeof(addr_desc), ", fault addr %p", info->si_addr); + __libc_format_buffer(addr_desc, sizeof(addr_desc), ", fault addr %p", info->si_addr); } } __libc_format_log(ANDROID_LOG_FATAL, "libc", diff --git a/linker/linker.cpp b/linker/linker.cpp index 52eb56ab8..2d8e07eb4 100644 --- a/linker/linker.cpp +++ b/linker/linker.cpp @@ -866,7 +866,21 @@ static void soinfo_unload(soinfo* si) { } void do_android_get_LD_LIBRARY_PATH(char* buffer, size_t buffer_size) { - snprintf(buffer, buffer_size, "%s:%s", kDefaultLdPaths[0], kDefaultLdPaths[1]); + // Use basic string manipulation calls to avoid snprintf. + // snprintf indirectly calls pthread_getspecific to get the size of a buffer. + // When debug malloc is enabled, this call returns 0. This in turn causes + // snprintf to do nothing, which causes libraries to fail to load. + // See b/17302493 for further details. + // Once the above bug is fixed, this code can be modified to use + // snprintf again. + size_t required_len = strlen(kDefaultLdPaths[0]) + strlen(kDefaultLdPaths[1]) + 2; + if (buffer_size < required_len) { + __libc_fatal("android_get_LD_LIBRARY_PATH failed, buffer too small: buffer len %zu, required len %zu", + buffer_size, required_len); + } + char* end = stpcpy(buffer, kDefaultLdPaths[0]); + *end = ':'; + strcpy(end + 1, kDefaultLdPaths[1]); } void do_android_update_LD_LIBRARY_PATH(const char* ld_library_path) { From b9f21a08f3590848cdda581c02b8b35808f6b3f8 Mon Sep 17 00:00:00 2001 From: Dmitriy Ivanov Date: Mon, 3 Nov 2014 21:13:55 -0800 Subject: [PATCH 07/13] Revert "Fix arm64 and arm builds." This reverts commit 445111a1c977e94a4233efd54f3690defa4a7582. Bug: 18222321 Bug: 18211780 Change-Id: I4fa9e1b63ec9b528f8bfed73c2ec15046c43a2fe --- tests/Android.mk | 14 ++++---------- tests/dlfcn_test.cpp | 4 ++-- tests/libs/Android.mk | 2 +- 3 files changed, 7 insertions(+), 13 deletions(-) diff --git a/tests/Android.mk b/tests/Android.mk index 5a1127b96..6423df183 100644 --- a/tests/Android.mk +++ b/tests/Android.mk @@ -248,11 +248,8 @@ bionic-unit-tests_shared_libraries_target := \ libdl \ libpagemap \ libdl_preempt_test_1 \ - libdl_preempt_test_2 - -ifneq ($(filter $(TARGET_ARCH),arm arm64),$(TARGET_ARCH)) -bionic-unit-tests_shared_libraries_target += libdl_test_df_1_global -endif + libdl_preempt_test_2 \ + libdl_test_df_1_global module := bionic-unit-tests module_tag := optional @@ -296,11 +293,8 @@ bionic-unit-tests-glibc_src_files := \ bionic-unit-tests-glibc_shared_libraries := \ libdl_preempt_test_1 \ - libdl_preempt_test_2 - -ifneq ($(filter $(TARGET_ARCH),arm arm64),$(TARGET_ARCH)) -bionic-unit-tests-glibc_shared_libraries += libdl_test_df_1_global -endif + libdl_preempt_test_2 \ + libdl_test_df_1_global bionic-unit-tests-glibc_whole_static_libraries := \ libBionicStandardTests \ diff --git a/tests/dlfcn_test.cpp b/tests/dlfcn_test.cpp index 2f0d430f6..a7fe2f852 100644 --- a/tests/dlfcn_test.cpp +++ b/tests/dlfcn_test.cpp @@ -500,7 +500,7 @@ TEST(dlfcn, dlopen_nodelete_dt_flags_1) { } TEST(dlfcn, dlsym_df_1_global) { -#if !defined(__arm__) && !defined(__aarch64__) +#if !defined(__arm__) void* handle = dlopen("libtest_dlsym_df_1_global.so", RTLD_NOW); ASSERT_TRUE(handle != nullptr) << dlerror(); int (*get_answer)(); @@ -509,7 +509,7 @@ TEST(dlfcn, dlsym_df_1_global) { ASSERT_EQ(42, get_answer()); ASSERT_EQ(0, dlclose(handle)); #else - GTEST_LOG_(INFO) << "This test does nothing on arm/arm64 (to be reenabled once b/18137520 or b/18130452 are fixed).\n"; + GTEST_LOG_(INFO) << "This test does nothing on arm (to be reenabled once b/18137520 or b/18130452 are fixed).\n"; #endif } diff --git a/tests/libs/Android.mk b/tests/libs/Android.mk index 7d959d484..f45e5a80f 100644 --- a/tests/libs/Android.mk +++ b/tests/libs/Android.mk @@ -311,7 +311,7 @@ include $(LOCAL_PATH)/Android.build.testlib.mk # Library with DF_1_GLOBAL # ----------------------------------------------------------------------------- # TODO: re-enable arm once b/18137520 or b/18130452 are fixed -ifeq ($(filter $(TARGET_ARCH),arm arm64),) +ifeq ($(filter $(TARGET_ARCH),arm),) libdl_test_df_1_global_src_files := dl_df_1_global.cpp libdl_test_df_1_global_ldflags := -fuse-ld=bfd -Wl,-z,global module := libdl_test_df_1_global From 86fdf8f09bcc64a4f2033cd6463c8fc1a1ec70a1 Mon Sep 17 00:00:00 2001 From: Dmitriy Ivanov Date: Mon, 3 Nov 2014 21:14:07 -0800 Subject: [PATCH 08/13] Revert "Fix symbol lookup order during relocation" This reverts commit 976402cca13a1f4f3aa988fd301575e134ef5f2c. Bug: 18222321 Bug: 18211780 Change-Id: Iafdd3d843db7b1cf288be9a0232022816622c944 --- linker/linked_list.h | 12 --- linker/linker.cpp | 162 +++++++++++----------------- linker/linker.h | 23 ++-- tests/Android.mk | 13 +-- tests/dl_test.cpp | 72 ------------- tests/dlfcn_test.cpp | 14 --- tests/libs/Android.mk | 36 ------- tests/libs/dl_df_1_global.cpp | 19 ---- tests/libs/dl_df_1_use_global.cpp | 23 ---- tests/libs/dl_preempt_library_1.cpp | 45 -------- tests/libs/dl_preempt_library_2.cpp | 37 ------- 11 files changed, 71 insertions(+), 385 deletions(-) delete mode 100644 tests/dl_test.cpp delete mode 100644 tests/libs/dl_df_1_global.cpp delete mode 100644 tests/libs/dl_df_1_use_global.cpp delete mode 100644 tests/libs/dl_preempt_library_1.cpp delete mode 100644 tests/libs/dl_preempt_library_2.cpp diff --git a/linker/linked_list.h b/linker/linked_list.h index b08806161..72a32b4ba 100644 --- a/linker/linked_list.h +++ b/linker/linked_list.h @@ -36,12 +36,6 @@ class LinkedList { clear(); } - LinkedList(LinkedList&& that) { - this->head_ = that.head_; - this->tail_ = that.tail_; - that.head_ = that.tail_ = nullptr; - } - void push_front(T* const element) { LinkedListEntry* new_entry = Allocator::alloc(); new_entry->next = head_; @@ -146,12 +140,6 @@ class LinkedList { return false; } - static LinkedList make_list(T* const element) { - LinkedList one_element_list; - one_element_list.push_back(element); - return one_element_list; - } - private: LinkedListEntry* head_; LinkedListEntry* tail_; diff --git a/linker/linker.cpp b/linker/linker.cpp index ef6e3e127..f8963b59c 100644 --- a/linker/linker.cpp +++ b/linker/linker.cpp @@ -282,7 +282,7 @@ static void protect_data(int protection) { g_soinfo_links_allocator.protect_all(protection); } -static soinfo* soinfo_alloc(const char* name, struct stat* file_stat, off64_t file_offset, uint32_t rtld_flags) { +static soinfo* soinfo_alloc(const char* name, struct stat* file_stat, off64_t file_offset, int rtld_flags) { if (strlen(name) >= SOINFO_NAME_LEN) { DL_ERR("library name \"%s\" too long", name); return nullptr; @@ -481,8 +481,7 @@ static unsigned elfhash(const char* _name) { return h; } -static ElfW(Sym)* soinfo_do_lookup(soinfo* si_from, const char* name, soinfo** si_found_in, - const soinfo::soinfo_list_t& global_group, const soinfo::soinfo_list_t& local_group) { +static ElfW(Sym)* soinfo_do_lookup(soinfo* si, const char* name, soinfo** lsi, const soinfo::soinfo_list_t& local_group) { unsigned elf_hash = elfhash(name); ElfW(Sym)* s = nullptr; @@ -497,40 +496,49 @@ static ElfW(Sym)* soinfo_do_lookup(soinfo* si_from, const char* name, soinfo** s * Note that this is unlikely since static linker avoids generating * relocations for -Bsymbolic linked dynamic executables. */ - if (si_from->has_DT_SYMBOLIC) { - DEBUG("%s: looking up %s in local scope (DT_SYMBOLIC)", si_from->name, name); - s = soinfo_elf_lookup(si_from, elf_hash, name); + if (si->has_DT_SYMBOLIC) { + DEBUG("%s: looking up %s in local scope (DT_SYMBOLIC)", si->name, name); + s = soinfo_elf_lookup(si, elf_hash, name); if (s != nullptr) { - *si_found_in = si_from; + *lsi = si; } } - // 1. Look for it in global_group - if (s == nullptr) { - global_group.visit([&](soinfo* global_si) { - DEBUG("%s: looking up %s in %s (from global group)", si_from->name, name, global_si->name); - s = soinfo_elf_lookup(global_si, elf_hash, name); + if (s == nullptr && somain != nullptr) { + // 1. Look for it in the main executable unless we already did. + if (si != somain || !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); if (s != nullptr) { - *si_found_in = global_si; - return false; + *lsi = somain; } + } - return true; - }); + // 2. Look for it in the ld_preloads + if (s == nullptr) { + for (int i = 0; g_ld_preloads[i] != NULL; i++) { + s = soinfo_elf_lookup(g_ld_preloads[i], elf_hash, name); + if (s != nullptr) { + *lsi = g_ld_preloads[i]; + break; + } + } + } } - // 2. Look for it in the local group + // 3. Look for it in the local group if (s == nullptr) { local_group.visit([&](soinfo* local_si) { - if (local_si == si_from && si_from->has_DT_SYMBOLIC) { + if (local_si == si && si->has_DT_SYMBOLIC) { // we already did this - skip return true; } - DEBUG("%s: looking up %s in %s (from local group)", si_from->name, name, local_si->name); + DEBUG("%s: looking up %s in %s (from local group)", si->name, name, local_si->name); s = soinfo_elf_lookup(local_si, elf_hash, name); if (s != nullptr) { - *si_found_in = local_si; + *lsi = local_si; return false; } @@ -541,9 +549,9 @@ static ElfW(Sym)* soinfo_do_lookup(soinfo* si_from, const char* name, soinfo** s if (s != nullptr) { TRACE_TYPE(LOOKUP, "si %s sym %s s->st_value = %p, " "found in %s, base = %p, load bias = %p", - si_from->name, name, reinterpret_cast(s->st_value), - (*si_found_in)->name, reinterpret_cast((*si_found_in)->base), - reinterpret_cast((*si_found_in)->load_bias)); + si->name, name, reinterpret_cast(s->st_value), + (*lsi)->name, reinterpret_cast((*lsi)->base), + reinterpret_cast((*lsi)->load_bias)); } return s; @@ -908,24 +916,6 @@ static bool is_recursive(soinfo* si, soinfo* parent) { }); } -// TODO: this is slightly unusual way to construct -// the global group for relocation. Not every RTLD_GLOBAL -// library is included in this group for backwards-compatibility -// reasons. -// -// This group consists of the main executable, LD_PRELOADs -// and libraries with the DF_1_GLOBAL flag set. -static soinfo::soinfo_list_t make_global_group() { - soinfo::soinfo_list_t global_group; - for (soinfo* si = somain; si != nullptr; si = si->next) { - if ((si->get_dt_flags_1() & DF_1_GLOBAL) != 0) { - global_group.push_back(si); - } - } - - return global_group; -} - static bool find_libraries(soinfo* start_with, const char* const library_names[], size_t library_names_count, soinfo* soinfos[], soinfo* ld_preloads[], size_t ld_preloads_count, int rtld_flags, const android_dlextinfo* extinfo) { // Step 0: prepare. @@ -935,9 +925,6 @@ static bool find_libraries(soinfo* start_with, const char* const library_names[] load_tasks.push_back(LoadTask::create(name, start_with)); } - // Construct global_group. - soinfo::soinfo_list_t global_group = make_global_group(); - // If soinfos array is null allocate one on stack. // The array is needed in case of failure; for example // when library_names[] = {libone.so, libtwo.so} and libone.so @@ -986,11 +973,6 @@ static bool find_libraries(soinfo* start_with, const char* const library_names[] // When ld_preloads is not null, the first // ld_preloads_count libs are in fact ld_preloads. if (ld_preloads != nullptr && soinfos_count < ld_preloads_count) { - // Add LD_PRELOADed libraries to the global group for future runs. - // There is no need to explicitly add them to the global group - // for this run because they are going to appear in the local - // group in the correct order. - si->set_dt_flags_1(si->get_dt_flags_1() | DF_1_GLOBAL); ld_preloads[soinfos_count] = si; } @@ -1011,7 +993,7 @@ static bool find_libraries(soinfo* start_with, const char* const library_names[] bool linked = local_group.visit([&](soinfo* si) { if ((si->flags & FLAG_LINKED) == 0) { - if (!si->LinkImage(global_group, local_group, extinfo)) { + if (!si->LinkImage(local_group, extinfo)) { return false; } si->flags |= FLAG_LINKED; @@ -1146,7 +1128,7 @@ static ElfW(Addr) call_ifunc_resolver(ElfW(Addr) resolver_addr) { } #if defined(USE_RELA) -int soinfo::Relocate(ElfW(Rela)* rela, unsigned count, const soinfo_list_t& global_group, const soinfo_list_t& local_group) { +int soinfo::Relocate(ElfW(Rela)* rela, unsigned count, const soinfo_list_t& local_group) { for (size_t idx = 0; idx < count; ++idx, ++rela) { unsigned type = ELFW(R_TYPE)(rela->r_info); unsigned sym = ELFW(R_SYM)(rela->r_info); @@ -1164,7 +1146,7 @@ int soinfo::Relocate(ElfW(Rela)* rela, unsigned count, const soinfo_list_t& glob if (sym != 0) { sym_name = get_string(symtab[sym].st_name); - s = soinfo_do_lookup(this, sym_name, &lsi, global_group,local_group); + s = soinfo_do_lookup(this, sym_name, &lsi, local_group); if (s == nullptr) { // We only allow an undefined symbol if this is a weak reference... s = &symtab[sym]; @@ -1423,7 +1405,7 @@ int soinfo::Relocate(ElfW(Rela)* rela, unsigned count, const soinfo_list_t& glob } #else // REL, not RELA. -int soinfo::Relocate(ElfW(Rel)* rel, unsigned count, const soinfo_list_t& global_group, const soinfo_list_t& local_group) { +int soinfo::Relocate(ElfW(Rel)* rel, unsigned count, const soinfo_list_t& local_group) { for (size_t idx = 0; idx < count; ++idx, ++rel) { unsigned type = ELFW(R_TYPE)(rel->r_info); // TODO: don't use unsigned for 'sym'. Use uint32_t or ElfW(Addr) instead. @@ -1442,7 +1424,7 @@ int soinfo::Relocate(ElfW(Rel)* rel, unsigned count, const soinfo_list_t& global if (sym != 0) { sym_name = get_string(symtab[sym].st_name); - s = soinfo_do_lookup(this, sym_name, &lsi, global_group, local_group); + s = soinfo_do_lookup(this, sym_name, &lsi, local_group); if (s == nullptr) { // We only allow an undefined symbol if this is a weak reference... s = &symtab[sym]; @@ -1628,7 +1610,7 @@ int soinfo::Relocate(ElfW(Rel)* rel, unsigned count, const soinfo_list_t& global #endif #if defined(__mips__) -static bool mips_relocate_got(soinfo* si, const soinfo::soinfo_list_t& global_group, const soinfo::soinfo_list_t& local_group) { +static bool mips_relocate_got(soinfo* si, const soinfo::soinfo_list_t& local_group) { ElfW(Addr)** got = si->plt_got; if (got == nullptr) { return true; @@ -1661,7 +1643,7 @@ static bool mips_relocate_got(soinfo* si, const soinfo::soinfo_list_t& global_gr // This is an undefined reference... try to locate it. const char* sym_name = si->get_string(sym->st_name); soinfo* lsi = nullptr; - ElfW(Sym)* s = soinfo_do_lookup(si, sym_name, &lsi, global_group, local_group); + ElfW(Sym)* s = soinfo_do_lookup(si, sym_name, &lsi, local_group); if (s == nullptr) { // We only allow an undefined symbol if this is a weak reference. s = &symtab[g]; @@ -1801,7 +1783,7 @@ void soinfo::remove_all_links() { children.clear(); } -dev_t soinfo::get_st_dev() const { +dev_t soinfo::get_st_dev() { if (has_min_version(0)) { return st_dev; } @@ -1809,7 +1791,7 @@ dev_t soinfo::get_st_dev() const { return 0; }; -ino_t soinfo::get_st_ino() const { +ino_t soinfo::get_st_ino() { if (has_min_version(0)) { return st_ino; } @@ -1817,7 +1799,7 @@ ino_t soinfo::get_st_ino() const { return 0; } -off64_t soinfo::get_file_offset() const { +off64_t soinfo::get_file_offset() { if (has_min_version(1)) { return file_offset; } @@ -1825,7 +1807,7 @@ off64_t soinfo::get_file_offset() const { return 0; } -uint32_t soinfo::get_rtld_flags() const { +int soinfo::get_rtld_flags() { if (has_min_version(1)) { return rtld_flags; } @@ -1833,27 +1815,6 @@ uint32_t soinfo::get_rtld_flags() const { return 0; } -uint32_t soinfo::get_dt_flags_1() const { - if (has_min_version(1)) { - return dt_flags_1; - } - - return 0; -} -void soinfo::set_dt_flags_1(uint32_t dt_flags_1) { - if (has_min_version(1)) { - if ((dt_flags_1 & DF_1_GLOBAL) != 0) { - rtld_flags |= RTLD_GLOBAL; - } - - if ((dt_flags_1 & DF_1_NODELETE) != 0) { - rtld_flags |= RTLD_NODELETE; - } - - this->dt_flags_1 = dt_flags_1; - } -} - // This is a return on get_children()/get_parents() if // 'this->flags' does not have FLAG_NEW_SOINFO set. static soinfo::soinfo_list_t g_empty_list; @@ -1891,9 +1852,8 @@ const char* soinfo::get_string(ElfW(Word) index) const { } bool soinfo::can_unload() const { - return (get_rtld_flags() & (RTLD_NODELETE | RTLD_GLOBAL)) == 0; + return (rtld_flags & (RTLD_NODELETE | RTLD_GLOBAL)) == 0; } - /* Force any of the closed stdin, stdout and stderr to be associated with /dev/null. */ static int nullify_closed_stdio() { @@ -2194,9 +2154,16 @@ bool soinfo::PrelinkImage() { break; case DT_FLAGS_1: - set_dt_flags_1(d->d_un.d_val); + if ((d->d_un.d_val & DF_1_GLOBAL) != 0) { + rtld_flags |= RTLD_GLOBAL; + } - if ((d->d_un.d_val & ~SUPPORTED_DT_FLAGS_1) != 0) { + if ((d->d_un.d_val & DF_1_NODELETE) != 0) { + rtld_flags |= RTLD_NODELETE; + } + // TODO: Implement other flags + + if ((d->d_un.d_val & ~(DF_1_NOW | DF_1_GLOBAL | DF_1_NODELETE)) != 0) { DL_WARN("Unsupported flags DT_FLAGS_1=%p", reinterpret_cast(d->d_un.d_val)); } break; @@ -2269,7 +2236,7 @@ bool soinfo::PrelinkImage() { return true; } -bool soinfo::LinkImage(const soinfo_list_t& global_group, const soinfo_list_t& local_group, const android_dlextinfo* extinfo) { +bool soinfo::LinkImage(const soinfo_list_t& local_group, const android_dlextinfo* extinfo) { #if !defined(__LP64__) if (has_text_relocations) { @@ -2288,33 +2255,33 @@ bool soinfo::LinkImage(const soinfo_list_t& global_group, const soinfo_list_t& l #if defined(USE_RELA) if (rela != nullptr) { DEBUG("[ relocating %s ]", name); - if (Relocate(rela, rela_count, global_group, local_group)) { + if (Relocate(rela, rela_count, local_group)) { return false; } } if (plt_rela != nullptr) { DEBUG("[ relocating %s plt ]", name); - if (Relocate(plt_rela, plt_rela_count, global_group, local_group)) { + if (Relocate(plt_rela, plt_rela_count, local_group)) { return false; } } #else if (rel != nullptr) { DEBUG("[ relocating %s ]", name); - if (Relocate(rel, rel_count, global_group, local_group)) { + if (Relocate(rel, rel_count, local_group)) { return false; } } if (plt_rel != nullptr) { DEBUG("[ relocating %s plt ]", name); - if (Relocate(plt_rel, plt_rel_count, global_group, local_group)) { + if (Relocate(plt_rel, plt_rel_count, local_group)) { return false; } } #endif #if defined(__mips__) - if (!mips_relocate_got(this, global_group, local_group)) { + if (!mips_relocate_got(this, local_group)) { return false; } #endif @@ -2381,7 +2348,7 @@ static void add_vdso(KernelArgumentBlock& args __unused) { si->load_bias = get_elf_exec_load_bias(ehdr_vdso); si->PrelinkImage(); - si->LinkImage(g_empty_list, soinfo::soinfo_list_t::make_list(si), nullptr); + si->LinkImage(g_empty_list, nullptr); #endif } @@ -2512,9 +2479,6 @@ static ElfW(Addr) __linker_init_post_relocation(KernelArgumentBlock& args, ElfW( si->PrelinkImage(); - // add somain to global group - si->set_dt_flags_1(si->get_dt_flags_1() | DF_1_GLOBAL); - // Load ld_preloads and dependencies. StringLinkedList needed_library_name_list; size_t needed_libraries_count = 0; @@ -2658,13 +2622,7 @@ extern "C" ElfW(Addr) __linker_init(void* raw_args) { linker_so.phnum = elf_hdr->e_phnum; linker_so.flags |= FLAG_LINKER; - // This might not be obvious... The reasons why we pass g_empty_list - // in place of local_group here are (1) we do not really need it, because - // linker is built with DT_SYMBOLIC and therefore relocates its symbols against - // itself without having to look into local_group and (2) allocators - // are not yet initialized, and therefore we cannot use linked_list.push_* - // functions at this point. - if (!(linker_so.PrelinkImage() && linker_so.LinkImage(g_empty_list, g_empty_list, nullptr))) { + if (!(linker_so.PrelinkImage() && linker_so.LinkImage(g_empty_list, nullptr))) { // It would be nice to print an error message, but if the linker // can't link itself, there's no guarantee that we'll be able to // call write() (because it involves a GOT reference). We may as diff --git a/linker/linker.h b/linker/linker.h index 0a98b40dc..222aca11e 100644 --- a/linker/linker.h +++ b/linker/linker.h @@ -89,9 +89,7 @@ #define FLAG_LINKER 0x00000010 // The linker itself #define FLAG_NEW_SOINFO 0x40000000 // new soinfo format -#define SUPPORTED_DT_FLAGS_1 (DF_1_NOW | DF_1_GLOBAL | DF_1_NODELETE) - -#define SOINFO_VERSION 1 +#define SOINFO_VERSION 0 #define SOINFO_NAME_LEN 128 @@ -209,18 +207,16 @@ struct soinfo { void CallDestructors(); void CallPreInitConstructors(); bool PrelinkImage(); - bool LinkImage(const soinfo_list_t& global_group, const soinfo_list_t& local_group, const android_dlextinfo* extinfo); + bool LinkImage(const soinfo_list_t& local_group, const android_dlextinfo* extinfo); void add_child(soinfo* child); void remove_all_links(); - ino_t get_st_ino() const; - dev_t get_st_dev() const; - off64_t get_file_offset() const; + ino_t get_st_ino(); + dev_t get_st_dev(); + off64_t get_file_offset(); - uint32_t get_rtld_flags() const; - uint32_t get_dt_flags_1() const; - void set_dt_flags_1(uint32_t dt_flags_1); + int get_rtld_flags(); soinfo_list_t& get_children(); soinfo_list_t& get_parents(); @@ -238,9 +234,9 @@ struct soinfo { void CallArray(const char* array_name, linker_function_t* functions, size_t count, bool reverse); void CallFunction(const char* function_name, linker_function_t function); #if defined(USE_RELA) - int Relocate(ElfW(Rela)* rela, unsigned count, const soinfo_list_t& global_group, const soinfo_list_t& local_group); + int Relocate(ElfW(Rela)* rela, unsigned count, const soinfo_list_t& local_group); #else - int Relocate(ElfW(Rel)* rel, unsigned count, const soinfo_list_t& global_group, const soinfo_list_t& local_group); + int Relocate(ElfW(Rel)* rel, unsigned count, const soinfo_list_t& local_group); #endif private: @@ -258,8 +254,7 @@ struct soinfo { // version >= 1 off64_t file_offset; - uint32_t rtld_flags; - uint32_t dt_flags_1; + int rtld_flags; size_t strtab_size; friend soinfo* get_libdl_info(); diff --git a/tests/Android.mk b/tests/Android.mk index 6423df183..8b0b0a0e0 100644 --- a/tests/Android.mk +++ b/tests/Android.mk @@ -225,7 +225,6 @@ bionic-unit-tests_whole_static_libraries := \ bionic-unit-tests_src_files := \ atexit_test.cpp \ - dl_test.cpp \ dlext_test.cpp \ dlfcn_test.cpp \ @@ -238,7 +237,8 @@ bionic-unit-tests_conlyflags := \ bionic-unit-tests_cppflags := $(test_cppflags) bionic-unit-tests_ldflags := \ - -Wl,--export-dynamic + -Wl,--export-dynamic \ + -Wl,-u,DlSymTestFunction \ bionic-unit-tests_c_includes := \ bionic/libc \ @@ -247,9 +247,6 @@ bionic-unit-tests_c_includes := \ bionic-unit-tests_shared_libraries_target := \ libdl \ libpagemap \ - libdl_preempt_test_1 \ - libdl_preempt_test_2 \ - libdl_test_df_1_global module := bionic-unit-tests module_tag := optional @@ -289,12 +286,6 @@ ifeq ($(HOST_OS)-$(HOST_ARCH),$(filter $(HOST_OS)-$(HOST_ARCH),linux-x86 linux-x bionic-unit-tests-glibc_src_files := \ atexit_test.cpp \ dlfcn_test.cpp \ - dl_test.cpp \ - -bionic-unit-tests-glibc_shared_libraries := \ - libdl_preempt_test_1 \ - libdl_preempt_test_2 \ - libdl_test_df_1_global bionic-unit-tests-glibc_whole_static_libraries := \ libBionicStandardTests \ diff --git a/tests/dl_test.cpp b/tests/dl_test.cpp deleted file mode 100644 index 74c7b510f..000000000 --- a/tests/dl_test.cpp +++ /dev/null @@ -1,72 +0,0 @@ -/* - * Copyright (C) 2012 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 -#include -#include -#include - -#include - -extern "C" int main_global_default_serial() { - return 3370318; -} - -extern "C" int main_global_protected_serial() { - return 2716057; -} - -// The following functions are defined in DT_NEEDED -// libdl_preempt_test.so library. - -// This one calls main_global_default_serial -extern "C" int main_global_default_get_serial(); - -// This one calls main_global_protected_serial -extern "C" int main_global_protected_get_serial(); - -// This one calls lib_global_default_serial -extern "C" int lib_global_default_get_serial(); - -// This one calls lib_global_protected_serial -extern "C" int lib_global_protected_get_serial(); - -// This test verifies that the global default function -// main_global_default_serial() is preempted by -// the function defined above. -TEST(dl, main_preempts_global_default) { - ASSERT_EQ(3370318, main_global_default_get_serial()); -} - -// This one makes sure that the global protected -// symbols do not get preempted -TEST(dl, main_does_not_preempt_global_protected) { - ASSERT_EQ(3370318, main_global_protected_get_serial()); -} - -// check same things for lib -TEST(dl, lib_preempts_global_default) { - ASSERT_EQ(3370318, lib_global_default_get_serial()); -} - -TEST(dl, lib_does_not_preempt_global_protected) { - ASSERT_EQ(3370318, lib_global_protected_get_serial()); -} - -// TODO: Add tests for LD_PRELOADs diff --git a/tests/dlfcn_test.cpp b/tests/dlfcn_test.cpp index a7fe2f852..e604f5abe 100644 --- a/tests/dlfcn_test.cpp +++ b/tests/dlfcn_test.cpp @@ -499,20 +499,6 @@ TEST(dlfcn, dlopen_nodelete_dt_flags_1) { ASSERT_TRUE(!is_unloaded); } -TEST(dlfcn, dlsym_df_1_global) { -#if !defined(__arm__) - void* handle = dlopen("libtest_dlsym_df_1_global.so", RTLD_NOW); - ASSERT_TRUE(handle != nullptr) << dlerror(); - int (*get_answer)(); - get_answer = reinterpret_cast(dlsym(handle, "dl_df_1_global_get_answer")); - ASSERT_TRUE(get_answer != nullptr) << dlerror(); - ASSERT_EQ(42, get_answer()); - ASSERT_EQ(0, dlclose(handle)); -#else - GTEST_LOG_(INFO) << "This test does nothing on arm (to be reenabled once b/18137520 or b/18130452 are fixed).\n"; -#endif -} - TEST(dlfcn, dlopen_failure) { void* self = dlopen("/does/not/exist", RTLD_NOW); ASSERT_TRUE(self == NULL); diff --git a/tests/libs/Android.mk b/tests/libs/Android.mk index f45e5a80f..05e7113ba 100644 --- a/tests/libs/Android.mk +++ b/tests/libs/Android.mk @@ -289,42 +289,6 @@ libtest_atexit_src_files := \ module := libtest_atexit include $(LOCAL_PATH)/Android.build.testlib.mk -# ----------------------------------------------------------------------------- -# This library is used by dl_load test to check symbol preempting -# by main executable -# ----------------------------------------------------------------------------- -libdl_preempt_test_1_src_files := dl_preempt_library_1.cpp - -module := libdl_preempt_test_1 -include $(LOCAL_PATH)/Android.build.testlib.mk - -# ----------------------------------------------------------------------------- -# This library is used by dl_load test to check symbol preempting -# by libdl_preempt_test_1.so -# ----------------------------------------------------------------------------- -libdl_preempt_test_2_src_files := dl_preempt_library_2.cpp - -module := libdl_preempt_test_2 -include $(LOCAL_PATH)/Android.build.testlib.mk - -# ----------------------------------------------------------------------------- -# Library with DF_1_GLOBAL -# ----------------------------------------------------------------------------- -# TODO: re-enable arm once b/18137520 or b/18130452 are fixed -ifeq ($(filter $(TARGET_ARCH),arm),) -libdl_test_df_1_global_src_files := dl_df_1_global.cpp -libdl_test_df_1_global_ldflags := -fuse-ld=bfd -Wl,-z,global -module := libdl_test_df_1_global -include $(LOCAL_PATH)/Android.build.testlib.mk -endif - -# ----------------------------------------------------------------------------- -# Library using symbol from libdl_test_df_1_global -# ----------------------------------------------------------------------------- -libtest_dlsym_df_1_global_src_files := dl_df_1_use_global.cpp -module := libtest_dlsym_df_1_global -include $(LOCAL_PATH)/Android.build.testlib.mk - # ----------------------------------------------------------------------------- # Library with weak function # ----------------------------------------------------------------------------- diff --git a/tests/libs/dl_df_1_global.cpp b/tests/libs/dl_df_1_global.cpp deleted file mode 100644 index 39856fd50..000000000 --- a/tests/libs/dl_df_1_global.cpp +++ /dev/null @@ -1,19 +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. - */ - -extern "C" int dl_df_1_global_get_answer_impl() { - return 42; -} diff --git a/tests/libs/dl_df_1_use_global.cpp b/tests/libs/dl_df_1_use_global.cpp deleted file mode 100644 index e14910d1c..000000000 --- a/tests/libs/dl_df_1_use_global.cpp +++ /dev/null @@ -1,23 +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. - */ - -extern "C" int __attribute__((weak)) dl_df_1_global_get_answer_impl() { - return 0; -} - -extern "C" int dl_df_1_global_get_answer() { - return dl_df_1_global_get_answer_impl(); -} diff --git a/tests/libs/dl_preempt_library_1.cpp b/tests/libs/dl_preempt_library_1.cpp deleted file mode 100644 index b4d81d5ac..000000000 --- a/tests/libs/dl_preempt_library_1.cpp +++ /dev/null @@ -1,45 +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. - */ - -// This one should be preempted by the function -// defined in the main executable. -extern "C" int __attribute__((weak)) main_global_default_serial() { - return 2716057; -} - -// Even though this one is defined by the main -// executable it should not be preempted -// because of protected visibility -extern "C" int __attribute__((weak, visibility("protected"))) main_global_protected_serial() { - return 3370318; -} - -extern "C" int main_global_default_get_serial() { - return main_global_default_serial(); -} - -extern "C" int main_global_protected_get_serial() { - return main_global_protected_serial(); -} - -// Trying to preempt functions from a DT_NEEDED .so -extern "C" int lib_global_default_serial() { - return 3370318; -} - -extern "C" int lib_global_protected_serial() { - return 2716057; -} diff --git a/tests/libs/dl_preempt_library_2.cpp b/tests/libs/dl_preempt_library_2.cpp deleted file mode 100644 index 8df9a1667..000000000 --- a/tests/libs/dl_preempt_library_2.cpp +++ /dev/null @@ -1,37 +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. - */ - -// This one should be preempted by the function -// defined in libdl_preempt_test_1.so -extern "C" int __attribute__((weak)) lib_global_default_serial() { - return 2716057; -} - -// Even though this one is defined by -// libdl_preempt_test_1.so it should not be -// preempted because of protected visibility -extern "C" int __attribute__((weak,visibility("protected"))) lib_global_protected_serial() { - return 3370318; -} - -extern "C" int lib_global_default_get_serial() { - return lib_global_default_serial(); -} - -extern "C" int lib_global_protected_get_serial() { - return lib_global_protected_serial(); -} - From 8b952f55527a988d63ea277bf560c0527993f8d5 Mon Sep 17 00:00:00 2001 From: Dmitriy Ivanov Date: Mon, 3 Nov 2014 21:14:31 -0800 Subject: [PATCH 09/13] Revert "Fix mips build" This reverts commit bf3d5ef5fd240d4c5fbde1b32f9084dbc720840b. Bug: 18222321 Bug: 18211780 Change-Id: I902ed888197b358c77303f1acb6d5ffd7ae6dcd3 --- linker/linker.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/linker/linker.cpp b/linker/linker.cpp index f8963b59c..c63ac35bb 100644 --- a/linker/linker.cpp +++ b/linker/linker.cpp @@ -2281,7 +2281,7 @@ bool soinfo::LinkImage(const soinfo_list_t& local_group, const android_dlextinfo #endif #if defined(__mips__) - if (!mips_relocate_got(this, local_group)) { + if (!mips_relocate_got(this)) { return false; } #endif From 8bf7353b79c3249951831378a6605fd9e8a7def1 Mon Sep 17 00:00:00 2001 From: Dmitriy Ivanov Date: Mon, 3 Nov 2014 21:15:15 -0800 Subject: [PATCH 10/13] Revert "Remove unnecessary lookups during relocations" This reverts commit 6442dbd3bcadbd5e522465743a8d8cf56338ae1c. Bug: 18222321 Bug: 18211780 Change-Id: I87b18a32238a1f75afe56149221b6691f50d9f56 --- linker/linker.cpp | 27 ++++++++++++++++++++++----- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/linker/linker.cpp b/linker/linker.cpp index c63ac35bb..eb1a483ae 100644 --- a/linker/linker.cpp +++ b/linker/linker.cpp @@ -530,11 +530,6 @@ static ElfW(Sym)* soinfo_do_lookup(soinfo* si, const char* name, soinfo** lsi, c // 3. Look for it in the local group if (s == nullptr) { local_group.visit([&](soinfo* local_si) { - if (local_si == si && si->has_DT_SYMBOLIC) { - // we already did this - skip - return true; - } - DEBUG("%s: looking up %s in %s (from local group)", si->name, name, local_si->name); s = soinfo_elf_lookup(local_si, elf_hash, name); if (s != nullptr) { @@ -546,6 +541,28 @@ static ElfW(Sym)* soinfo_do_lookup(soinfo* si, const char* name, soinfo** lsi, c }); } + // 4. Look for it in this library (unless we already did it because of DT_SYMBOLIC) + if (s == nullptr && !si->has_DT_SYMBOLIC) { + DEBUG("%s: looking up %s in local scope", si->name, name); + s = soinfo_elf_lookup(si, elf_hash, name); + if (s != nullptr) { + *lsi = si; + } + } + + // 5. Dependencies + if (s == nullptr) { + si->get_children().visit([&](soinfo* child) { + DEBUG("%s: looking up %s in %s", si->name, name, child->name); + s = soinfo_elf_lookup(child, elf_hash, name); + if (s != nullptr) { + *lsi = child; + return false; + } + return true; + }); + } + if (s != nullptr) { TRACE_TYPE(LOOKUP, "si %s sym %s s->st_value = %p, " "found in %s, base = %p, load bias = %p", From 189ac9f14280d495d3d6f3c196f895e7b8e6098f Mon Sep 17 00:00:00 2001 From: Dmitriy Ivanov Date: Mon, 3 Nov 2014 21:15:25 -0800 Subject: [PATCH 11/13] Revert "Fix relocation to look for symbols in local group" This reverts commit fd2747bb585fc51b5ad56db09c0e9b66c7091a92. Bug: 18222321 Bug: 18211780 Change-Id: I2d4ebab1e73b7277161af76b99f8249825b22d65 --- linker/linked_list.h | 4 +- linker/linker.cpp | 193 +++++++----------- linker/linker.h | 6 +- tests/dlfcn_test.cpp | 186 ++--------------- .../Android.build.dlopen_check_order_dlsym.mk | 90 -------- ...lopen_check_order_reloc_main_executable.mk | 56 ----- ...build.dlopen_check_order_reloc_siblings.mk | 133 ------------ tests/libs/Android.mk | 75 ++++++- .../libs/dlopen_check_order_reloc_answer.cpp | 23 --- .../dlopen_check_order_reloc_answer_impl.cpp | 19 -- ...dlopen_check_order_reloc_nephew_answer.cpp | 21 -- .../dlopen_check_order_reloc_root_answer.cpp | 21 -- ...pen_check_order_reloc_root_answer_impl.cpp | 19 -- ...m_answer.cpp => dlopen_testlib_answer.cpp} | 4 +- 14 files changed, 162 insertions(+), 688 deletions(-) delete mode 100644 tests/libs/Android.build.dlopen_check_order_dlsym.mk delete mode 100644 tests/libs/Android.build.dlopen_check_order_reloc_main_executable.mk delete mode 100644 tests/libs/Android.build.dlopen_check_order_reloc_siblings.mk delete mode 100644 tests/libs/dlopen_check_order_reloc_answer.cpp delete mode 100644 tests/libs/dlopen_check_order_reloc_answer_impl.cpp delete mode 100644 tests/libs/dlopen_check_order_reloc_nephew_answer.cpp delete mode 100644 tests/libs/dlopen_check_order_reloc_root_answer.cpp delete mode 100644 tests/libs/dlopen_check_order_reloc_root_answer_impl.cpp rename tests/libs/{dlopen_check_order_dlsym_answer.cpp => dlopen_testlib_answer.cpp} (87%) diff --git a/linker/linked_list.h b/linker/linked_list.h index 72a32b4ba..4e62e208f 100644 --- a/linker/linked_list.h +++ b/linker/linked_list.h @@ -86,7 +86,7 @@ class LinkedList { } template - void for_each(F action) const { + void for_each(F action) { visit([&] (T* si) { action(si); return true; @@ -94,7 +94,7 @@ class LinkedList { } template - bool visit(F action) const { + bool visit(F action) { for (LinkedListEntry* e = head_; e != nullptr; e = e->next) { if (!action(e->element)) { return false; diff --git a/linker/linker.cpp b/linker/linker.cpp index eb1a483ae..41557e231 100644 --- a/linker/linker.cpp +++ b/linker/linker.cpp @@ -415,7 +415,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(const soinfo* si, unsigned hash, const char* name) { +static ElfW(Sym)* soinfo_elf_lookup(soinfo* si, unsigned hash, const char* name) { ElfW(Sym)* symtab = si->symtab; TRACE_TYPE(LOOKUP, "SEARCH %s in %s@%p %x %zd", @@ -481,7 +481,7 @@ static unsigned elfhash(const char* _name) { return h; } -static ElfW(Sym)* soinfo_do_lookup(soinfo* si, const char* name, soinfo** lsi, const soinfo::soinfo_list_t& local_group) { +static ElfW(Sym)* soinfo_do_lookup(soinfo* si, const char* name, soinfo** lsi) { unsigned elf_hash = elfhash(name); ElfW(Sym)* s = nullptr; @@ -527,21 +527,16 @@ static ElfW(Sym)* soinfo_do_lookup(soinfo* si, const char* name, soinfo** lsi, c } } - // 3. Look for it in the local group - if (s == nullptr) { - local_group.visit([&](soinfo* local_si) { - DEBUG("%s: looking up %s in %s (from local group)", si->name, name, local_si->name); - s = soinfo_elf_lookup(local_si, elf_hash, name); - if (s != nullptr) { - *lsi = local_si; - return false; - } + /* Look for symbols in the local scope (the object who is + * searching). This happens with C++ templates on x86 for some + * reason. + * + * Notes on weak symbols: + * The ELF specs are ambiguous about treatment of weak definitions in + * dynamic linking. Some systems return the first definition found + * and some the first non-weak definition. This is system dependent. + * Here we return the first definition found for simplicity. */ - return true; - }); - } - - // 4. Look for it in this library (unless we already did it because of DT_SYMBOLIC) if (s == nullptr && !si->has_DT_SYMBOLIC) { DEBUG("%s: looking up %s in local scope", si->name, name); s = soinfo_elf_lookup(si, elf_hash, name); @@ -550,7 +545,6 @@ static ElfW(Sym)* soinfo_do_lookup(soinfo* si, const char* name, soinfo** lsi, c } } - // 5. Dependencies if (s == nullptr) { si->get_children().visit([&](soinfo* child) { DEBUG("%s: looking up %s in %s", si->name, name, child->name); @@ -649,61 +643,33 @@ typedef linked_list_t StringLinkedList; typedef linked_list_t LoadTaskList; -// This function walks down the tree of soinfo dependencies -// in breadth-first order and -// * calls action(soinfo* si) for each node, and -// * terminates walk if action returns false. -// -// walk_dependencies_tree returns false if walk was terminated -// by the action and true otherwise. -template -static bool walk_dependencies_tree(soinfo* root_soinfos[], size_t root_soinfos_size, F action) { +// 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) { SoinfoLinkedList visit_list; SoinfoLinkedList visited; - for (size_t i = 0; i < root_soinfos_size; ++i) { - visit_list.push_back(root_soinfos[i]); - } - - soinfo* si; - while ((si = visit_list.pop_front()) != nullptr) { - if (visited.contains(si)) { + visit_list.push_back(si); + soinfo* current_soinfo; + while ((current_soinfo = visit_list.pop_front()) != nullptr) { + if (visited.contains(current_soinfo)) { continue; } - if (!action(si)) { - return false; + ElfW(Sym)* result = soinfo_elf_lookup(current_soinfo, elfhash(name), name); + + if (result != nullptr) { + *found = current_soinfo; + return result; } + visited.push_back(current_soinfo); - visited.push_back(si); - - si->get_children().for_each([&](soinfo* child) { + current_soinfo->get_children().for_each([&](soinfo* child) { visit_list.push_back(child); }); } - return true; -} - - -// 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) { - ElfW(Sym)* result = nullptr; - uint32_t elf_hash = elfhash(name); - - - walk_dependencies_tree(&si, 1, [&](soinfo* current_soinfo) { - result = soinfo_elf_lookup(current_soinfo, elf_hash, name); - if (result != nullptr) { - *found = current_soinfo; - return false; - } - - return true; - }); - - return result; + return nullptr; } /* This is used by dlsym(3) to performs a global symbol lookup. If the @@ -933,30 +899,19 @@ static bool is_recursive(soinfo* si, soinfo* parent) { }); } -static bool find_libraries(soinfo* start_with, const char* const library_names[], size_t library_names_count, soinfo* soinfos[], - soinfo* ld_preloads[], size_t ld_preloads_count, int rtld_flags, const android_dlextinfo* extinfo) { +static bool find_libraries(const char* const library_names[], size_t library_names_size, soinfo* soinfos[], + soinfo* ld_preloads[], size_t ld_preloads_size, int rtld_flags, const android_dlextinfo* extinfo) { // Step 0: prepare. LoadTaskList load_tasks; - for (size_t i = 0; i < library_names_count; ++i) { + for (size_t i = 0; i < library_names_size; ++i) { const char* name = library_names[i]; - load_tasks.push_back(LoadTask::create(name, start_with)); + load_tasks.push_back(LoadTask::create(name, nullptr)); } - // If soinfos array is null allocate one on stack. - // The array is needed in case of failure; for example - // when library_names[] = {libone.so, libtwo.so} and libone.so - // is loaded correctly but libtwo.so failed for some reason. - // In this case libone.so should be unloaded on return. - // See also implementation of failure_guard below. - - if (soinfos == nullptr) { - size_t soinfos_size = sizeof(soinfo*)*library_names_count; - soinfos = reinterpret_cast(alloca(soinfos_size)); - memset(soinfos, 0, soinfos_size); - } - - // list of libraries to link - see step 2. - size_t soinfos_count = 0; + // Libraries added to this list in reverse order so that we can + // start linking from bottom-up - see step 2. + SoinfoLinkedList found_libs; + size_t soinfos_size = 0; auto failure_guard = make_scope_guard([&]() { // Housekeeping @@ -964,7 +919,7 @@ static bool find_libraries(soinfo* start_with, const char* const library_names[] LoadTask::deleter(t); }); - for (size_t i = 0; iadd_child(si); } + found_libs.push_front(si); - // When ld_preloads is not null, the first - // ld_preloads_count libs are in fact ld_preloads. - if (ld_preloads != nullptr && soinfos_count < ld_preloads_count) { - ld_preloads[soinfos_count] = si; + // When ld_preloads is not null first + // ld_preloads_size libs are in fact ld_preloads. + if (ld_preloads != nullptr && soinfos_size < ld_preloads_size) { + ld_preloads[soinfos_size] = si; } - if (soinfos_count < library_names_count) { - soinfos[soinfos_count++] = si; + if (soinfos_sizeflags & FLAG_LINKED) == 0) { - if (!si->LinkImage(local_group, extinfo)) { + if (!si->LinkImage(extinfo)) { return false; } si->flags |= FLAG_LINKED; } - - return true; - }); - - if (linked) { - failure_guard.disable(); } - return linked; + // All is well - found_libs and load_tasks are empty at this point + // and all libs are successfully linked. + failure_guard.disable(); + return true; } static soinfo* find_library(const char* name, int rtld_flags, const android_dlextinfo* extinfo) { @@ -1034,7 +979,7 @@ static soinfo* find_library(const char* name, int rtld_flags, const android_dlex soinfo* si; - if (!find_libraries(nullptr, &name, 1, &si, nullptr, 0, rtld_flags, extinfo)) { + if (!find_libraries(&name, 1, &si, nullptr, 0, rtld_flags, extinfo)) { return nullptr; } @@ -1145,7 +1090,7 @@ static ElfW(Addr) call_ifunc_resolver(ElfW(Addr) resolver_addr) { } #if defined(USE_RELA) -int soinfo::Relocate(ElfW(Rela)* rela, unsigned count, const soinfo_list_t& local_group) { +int soinfo::Relocate(ElfW(Rela)* rela, unsigned count) { for (size_t idx = 0; idx < count; ++idx, ++rela) { unsigned type = ELFW(R_TYPE)(rela->r_info); unsigned sym = ELFW(R_SYM)(rela->r_info); @@ -1163,7 +1108,7 @@ int soinfo::Relocate(ElfW(Rela)* rela, unsigned count, const soinfo_list_t& loca if (sym != 0) { sym_name = get_string(symtab[sym].st_name); - s = soinfo_do_lookup(this, sym_name, &lsi, local_group); + s = soinfo_do_lookup(this, sym_name, &lsi); if (s == nullptr) { // We only allow an undefined symbol if this is a weak reference... s = &symtab[sym]; @@ -1422,7 +1367,7 @@ int soinfo::Relocate(ElfW(Rela)* rela, unsigned count, const soinfo_list_t& loca } #else // REL, not RELA. -int soinfo::Relocate(ElfW(Rel)* rel, unsigned count, const soinfo_list_t& local_group) { +int soinfo::Relocate(ElfW(Rel)* rel, unsigned count) { for (size_t idx = 0; idx < count; ++idx, ++rel) { unsigned type = ELFW(R_TYPE)(rel->r_info); // TODO: don't use unsigned for 'sym'. Use uint32_t or ElfW(Addr) instead. @@ -1441,7 +1386,7 @@ int soinfo::Relocate(ElfW(Rel)* rel, unsigned count, const soinfo_list_t& local_ if (sym != 0) { sym_name = get_string(symtab[sym].st_name); - s = soinfo_do_lookup(this, sym_name, &lsi, local_group); + s = soinfo_do_lookup(this, sym_name, &lsi); if (s == nullptr) { // We only allow an undefined symbol if this is a weak reference... s = &symtab[sym]; @@ -1627,7 +1572,7 @@ int soinfo::Relocate(ElfW(Rel)* rel, unsigned count, const soinfo_list_t& local_ #endif #if defined(__mips__) -static bool mips_relocate_got(soinfo* si, const soinfo::soinfo_list_t& local_group) { +static bool mips_relocate_got(soinfo* si) { ElfW(Addr)** got = si->plt_got; if (got == nullptr) { return true; @@ -1660,7 +1605,7 @@ static bool mips_relocate_got(soinfo* si, const soinfo::soinfo_list_t& local_gro // This is an undefined reference... try to locate it. const char* sym_name = si->get_string(sym->st_name); soinfo* lsi = nullptr; - ElfW(Sym)* s = soinfo_do_lookup(si, sym_name, &lsi, local_group); + ElfW(Sym)* s = soinfo_do_lookup(si, sym_name, &lsi); if (s == nullptr) { // We only allow an undefined symbol if this is a weak reference. s = &symtab[g]; @@ -2253,7 +2198,7 @@ bool soinfo::PrelinkImage() { return true; } -bool soinfo::LinkImage(const soinfo_list_t& local_group, const android_dlextinfo* extinfo) { +bool soinfo::LinkImage(const android_dlextinfo* extinfo) { #if !defined(__LP64__) if (has_text_relocations) { @@ -2272,26 +2217,26 @@ bool soinfo::LinkImage(const soinfo_list_t& local_group, const android_dlextinfo #if defined(USE_RELA) if (rela != nullptr) { DEBUG("[ relocating %s ]", name); - if (Relocate(rela, rela_count, local_group)) { + if (Relocate(rela, rela_count)) { return false; } } if (plt_rela != nullptr) { DEBUG("[ relocating %s plt ]", name); - if (Relocate(plt_rela, plt_rela_count, local_group)) { + if (Relocate(plt_rela, plt_rela_count)) { return false; } } #else if (rel != nullptr) { DEBUG("[ relocating %s ]", name); - if (Relocate(rel, rel_count, local_group)) { + if (Relocate(rel, rel_count)) { return false; } } if (plt_rel != nullptr) { DEBUG("[ relocating %s plt ]", name); - if (Relocate(plt_rel, plt_rel_count, local_group)) { + if (Relocate(plt_rel, plt_rel_count)) { return false; } } @@ -2365,7 +2310,7 @@ static void add_vdso(KernelArgumentBlock& args __unused) { si->load_bias = get_elf_exec_load_bias(ehdr_vdso); si->PrelinkImage(); - si->LinkImage(g_empty_list, nullptr); + si->LinkImage(nullptr); #endif } @@ -2511,11 +2456,21 @@ static ElfW(Addr) __linker_init_post_relocation(KernelArgumentBlock& args, ElfW( }); const char* needed_library_names[needed_libraries_count]; + soinfo* needed_library_si[needed_libraries_count]; memset(needed_library_names, 0, sizeof(needed_library_names)); needed_library_name_list.copy_to_array(needed_library_names, needed_libraries_count); - if (needed_libraries_count > 0 && !find_libraries(si, needed_library_names, needed_libraries_count, nullptr, g_ld_preloads, ld_preloads_count, RTLD_GLOBAL, nullptr)) { + if (needed_libraries_count > 0 && !find_libraries(needed_library_names, needed_libraries_count, needed_library_si, g_ld_preloads, ld_preloads_count, RTLD_GLOBAL, nullptr)) { + __libc_format_fd(2, "CANNOT LINK EXECUTABLE DEPENDENCIES: %s\n", linker_get_error_buffer()); + exit(EXIT_FAILURE); + } + + for (size_t i = 0; iadd_child(needed_library_si[i]); + } + + if (!si->LinkImage(nullptr)) { __libc_format_fd(2, "CANNOT LINK EXECUTABLE: %s\n", linker_get_error_buffer()); exit(EXIT_FAILURE); } @@ -2639,7 +2594,7 @@ extern "C" ElfW(Addr) __linker_init(void* raw_args) { linker_so.phnum = elf_hdr->e_phnum; linker_so.flags |= FLAG_LINKER; - if (!(linker_so.PrelinkImage() && linker_so.LinkImage(g_empty_list, nullptr))) { + if (!(linker_so.PrelinkImage() && linker_so.LinkImage(nullptr))) { // It would be nice to print an error message, but if the linker // can't link itself, there's no guarantee that we'll be able to // call write() (because it involves a GOT reference). We may as diff --git a/linker/linker.h b/linker/linker.h index 222aca11e..ebb4793af 100644 --- a/linker/linker.h +++ b/linker/linker.h @@ -207,7 +207,7 @@ struct soinfo { void CallDestructors(); void CallPreInitConstructors(); bool PrelinkImage(); - bool LinkImage(const soinfo_list_t& local_group, const android_dlextinfo* extinfo); + bool LinkImage(const android_dlextinfo* extinfo); void add_child(soinfo* child); void remove_all_links(); @@ -234,9 +234,9 @@ struct soinfo { void CallArray(const char* array_name, linker_function_t* functions, size_t count, bool reverse); void CallFunction(const char* function_name, linker_function_t function); #if defined(USE_RELA) - int Relocate(ElfW(Rela)* rela, unsigned count, const soinfo_list_t& local_group); + int Relocate(ElfW(Rela)* rela, unsigned count); #else - int Relocate(ElfW(Rel)* rel, unsigned count, const soinfo_list_t& local_group); + int Relocate(ElfW(Rel)* rel, unsigned count); #endif private: diff --git a/tests/dlfcn_test.cpp b/tests/dlfcn_test.cpp index e604f5abe..f1ec0f131 100644 --- a/tests/dlfcn_test.cpp +++ b/tests/dlfcn_test.cpp @@ -162,39 +162,39 @@ TEST(dlfcn, dlopen_check_relocation_dt_needed_order) { ASSERT_EQ(1, fn()); } -TEST(dlfcn, dlopen_check_order_dlsym) { +TEST(dlfcn, dlopen_check_order) { // Here is how the test library and its dt_needed // libraries are arranged // - // libtest_check_order_children.so + // libtest_check_order.so // | - // +-> ..._1_left.so + // +-> libtest_check_order_1_left.so // | | - // | +-> ..._a.so + // | +-> libtest_check_order_a.so // | | - // | +-> ...r_b.so + // | +-> libtest_check_order_b.so // | - // +-> ..._2_right.so + // +-> libtest_check_order_2_right.so // | | - // | +-> ..._d.so + // | +-> libtest_check_order_d.so // | | - // | +-> ..._b.so + // | +-> libtest_check_order_b.so // | - // +-> ..._3_c.so + // +-> libtest_check_order_3_c.so // // load order should be (1, 2, 3, a, b, d) // // get_answer() is defined in (2, 3, a, b, c) // get_answer2() is defined in (b, d) - void* sym = dlsym(RTLD_DEFAULT, "check_order_dlsym_get_answer"); + void* sym = dlsym(RTLD_DEFAULT, "dlopen_test_get_answer"); ASSERT_TRUE(sym == nullptr); - void* handle = dlopen("libtest_check_order_dlsym.so", RTLD_NOW | RTLD_GLOBAL); - ASSERT_TRUE(handle != nullptr) << dlerror(); + void* handle = dlopen("libtest_check_order.so", RTLD_NOW | RTLD_GLOBAL); + ASSERT_TRUE(handle != nullptr); typedef int (*fn_t) (void); fn_t fn, fn2; - fn = reinterpret_cast(dlsym(RTLD_DEFAULT, "check_order_dlsym_get_answer")); + fn = reinterpret_cast(dlsym(RTLD_DEFAULT, "dlopen_test_get_answer")); ASSERT_TRUE(fn != NULL) << dlerror(); - fn2 = reinterpret_cast(dlsym(RTLD_DEFAULT, "check_order_dlsym_get_answer2")); + fn2 = reinterpret_cast(dlsym(RTLD_DEFAULT, "dlopen_test_get_answer2")); ASSERT_TRUE(fn2 != NULL) << dlerror(); ASSERT_EQ(42, fn()); @@ -202,163 +202,6 @@ TEST(dlfcn, dlopen_check_order_dlsym) { dlclose(handle); } -TEST(dlfcn, dlopen_check_order_reloc_siblings) { - // This is how this one works: - // we lookup and call get_answer which is defined in '_2.so' - // and in turn calls external get_answer_impl() defined in _1.so and in '_[a-f].so' - // the correct _impl() is implemented by '_a.so'; - // - // Note that this is test for RTLD_LOCAL (TODO: test for GLOBAL?) - // - // Here is the picture: - // - // libtest_check_order_reloc_siblings.so - // | - // +-> ..._1.so <- empty - // | | - // | +-> ..._a.so <- exports correct answer_impl() - // | | - // | +-> ..._b.so <- every other letter exporting incorrect one. - // | - // +-> ..._2.so <- empty - // | | - // | +-> ..._c.so - // | | - // | +-> ..._d.so - // | - // +-> ..._3.so <- empty - // | - // +-> ..._e.so - // | - // +-> ..._f.so <- exports get_answer() that calls get_anser_impl(); - // implements incorrect get_answer_impl() - - void* handle = dlopen("libtest_check_order_reloc_siblings.so", RTLD_NOW | RTLD_NOLOAD); - ASSERT_TRUE(handle == nullptr); -#ifdef __BIONIC__ - // TODO: glibc returns nullptr on dlerror() here. Is it bug? - ASSERT_STREQ("dlopen failed: library \"libtest_check_order_reloc_siblings.so\" wasn't loaded and RTLD_NOLOAD prevented it", dlerror()); -#endif - - handle = dlopen("libtest_check_order_reloc_siblings.so", RTLD_NOW | RTLD_LOCAL); - ASSERT_TRUE(handle != nullptr) << dlerror(); - - typedef int (*fn_t) (void); - fn_t fn = reinterpret_cast(dlsym(handle, "check_order_reloc_get_answer")); - ASSERT_TRUE(fn != nullptr) << dlerror(); - ASSERT_EQ(42, fn()); - - ASSERT_EQ(0, dlclose(handle)); -} - -TEST(dlfcn, dlopen_check_order_reloc_siblings_with_preload) { - // This test uses the same library as dlopen_check_order_reloc_siblings. - // Unlike dlopen_check_order_reloc_siblings it preloads - // libtest_check_order_reloc_siblings_1.so (first dependency) prior to - // dlopen(libtest_check_order_reloc_siblings.so) - - void* handle = dlopen("libtest_check_order_reloc_siblings.so", RTLD_NOW | RTLD_NOLOAD); - ASSERT_TRUE(handle == nullptr); - handle = dlopen("libtest_check_order_reloc_siblings_1.so", RTLD_NOW | RTLD_NOLOAD); - ASSERT_TRUE(handle == nullptr); - - void* handle_for_1 = dlopen("libtest_check_order_reloc_siblings_1.so", RTLD_NOW | RTLD_LOCAL); - ASSERT_TRUE(handle_for_1 != nullptr) << dlerror(); - - handle = dlopen("libtest_check_order_reloc_siblings.so", RTLD_NOW | RTLD_LOCAL); - ASSERT_TRUE(handle != nullptr) << dlerror(); - - ASSERT_EQ(0, dlclose(handle_for_1)); - - typedef int (*fn_t) (void); - fn_t fn = reinterpret_cast(dlsym(handle, "check_order_reloc_get_answer")); - ASSERT_TRUE(fn != nullptr) << dlerror(); - ASSERT_EQ(42, fn()); - - ASSERT_EQ(0, dlclose(handle)); -} - -TEST(dlfcn, dlopen_check_order_reloc_nephew) { - // This is how this one works: - // we lookup and call nephew_get_answer which is defined in '_2.so' - // and in turn calls external get_answer_impl() defined in '_[a-f].so' - // the correct _impl() is implemented by '_a.so'; - // - // Here is the picture: - // - // libtest_check_order_reloc_siblings.so - // | - // +-> ..._1.so <- empty - // | | - // | +-> ..._a.so <- exports correct answer_impl() - // | | - // | +-> ..._b.so <- every other letter exporting incorrect one. - // | - // +-> ..._2.so <- empty - // | | - // | +-> ..._c.so - // | | - // | +-> ..._d.so - // | - // +-> ..._3.so <- nephew_get_answer() that calls get_answer_impl(); - // | - // +-> ..._e.so - // | - // +-> ..._f.so - - void* handle = dlopen("libtest_check_order_reloc_siblings.so", RTLD_NOW | RTLD_NOLOAD); - ASSERT_TRUE(handle == nullptr); -#ifdef __BIONIC__ - // TODO: glibc returns nullptr on dlerror() here. Is it bug? - ASSERT_STREQ("dlopen failed: library \"libtest_check_order_reloc_siblings.so\" wasn't loaded and RTLD_NOLOAD prevented it", dlerror()); -#endif - - handle = dlopen("libtest_check_order_reloc_siblings.so", RTLD_NOW | RTLD_LOCAL); - ASSERT_TRUE(handle != nullptr) << dlerror(); - - typedef int (*fn_t) (void); - fn_t fn = reinterpret_cast(dlsym(handle, "check_order_reloc_nephew_get_answer")); - ASSERT_TRUE(fn != nullptr) << dlerror(); - ASSERT_EQ(42, fn()); - - ASSERT_EQ(0, dlclose(handle)); -} - -extern "C" int check_order_reloc_root_get_answer_impl() { - return 42; -} - -TEST(dlfcn, dlopen_check_order_reloc_main_executable) { - // This is how this one works: - // we lookup and call get_answer3 which is defined in 'root.so' - // and in turn calls external root_get_answer_impl() defined in _2.so and - // above the correct _impl() is one in the executable. - // - // libtest_check_order_reloc_root.so - // | - // +-> ..._1.so <- empty - // | - // +-> ..._2.so <- gives incorrect answer for answer_main_impl() - // - - void* handle = dlopen("libtest_check_order_reloc_root.so", RTLD_NOW | RTLD_NOLOAD); - ASSERT_TRUE(handle == nullptr); -#ifdef __BIONIC__ - // TODO: glibc returns nullptr on dlerror() here. Is it bug? - ASSERT_STREQ("dlopen failed: library \"libtest_check_order_reloc_root.so\" wasn't loaded and RTLD_NOLOAD prevented it", dlerror()); -#endif - - handle = dlopen("libtest_check_order_reloc_root.so", RTLD_NOW | RTLD_LOCAL); - ASSERT_TRUE(handle != nullptr) << dlerror(); - - typedef int (*fn_t) (void); - fn_t fn = reinterpret_cast(dlsym(handle, "check_order_reloc_root_get_answer")); - ASSERT_TRUE(fn != nullptr) << dlerror(); - ASSERT_EQ(42, fn()); - - ASSERT_EQ(0, dlclose(handle)); -} - TEST(dlfcn, dlopen_check_rtld_local) { void* sym = dlsym(RTLD_DEFAULT, "dlopen_testlib_simple_func"); ASSERT_TRUE(sym == nullptr); @@ -499,6 +342,7 @@ TEST(dlfcn, dlopen_nodelete_dt_flags_1) { ASSERT_TRUE(!is_unloaded); } + TEST(dlfcn, dlopen_failure) { void* self = dlopen("/does/not/exist", RTLD_NOW); ASSERT_TRUE(self == NULL); diff --git a/tests/libs/Android.build.dlopen_check_order_dlsym.mk b/tests/libs/Android.build.dlopen_check_order_dlsym.mk deleted file mode 100644 index 73d8c1a83..000000000 --- a/tests/libs/Android.build.dlopen_check_order_dlsym.mk +++ /dev/null @@ -1,90 +0,0 @@ -# -# Copyright (C) 2012 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. -# - -# ----------------------------------------------------------------------------- -# Libraries used by dlfcn tests to verify correct load order: -# libtest_check_order_2_right.so -# ----------------------------------------------------------------------------- -libtest_check_order_dlsym_2_right_src_files := \ - dlopen_check_order_dlsym_answer.cpp - -libtest_check_order_dlsym_2_right_cflags := -D__ANSWER=42 -module := libtest_check_order_dlsym_2_right -include $(LOCAL_PATH)/Android.build.testlib.mk - -# ----------------------------------------------------------------------------- -# libtest_check_order_a.so -# ----------------------------------------------------------------------------- -libtest_check_order_dlsym_a_src_files := \ - dlopen_check_order_dlsym_answer.cpp - -libtest_check_order_dlsym_a_cflags := -D__ANSWER=1 -module := libtest_check_order_dlsym_a -include $(LOCAL_PATH)/Android.build.testlib.mk - -# ----------------------------------------------------------------------------- -# libtest_check_order_b.so -# ----------------------------------------------------------------------------- -libtest_check_order_dlsym_b_src_files := \ - dlopen_check_order_dlsym_answer.cpp - -libtest_check_order_dlsym_b_cflags := -D__ANSWER=2 -D__ANSWER2=43 -module := libtest_check_order_dlsym_b -include $(LOCAL_PATH)/Android.build.testlib.mk - -# ----------------------------------------------------------------------------- -# libtest_check_order_c.so -# ----------------------------------------------------------------------------- -libtest_check_order_dlsym_3_c_src_files := \ - dlopen_check_order_dlsym_answer.cpp - -libtest_check_order_dlsym_3_c_cflags := -D__ANSWER=3 -module := libtest_check_order_dlsym_3_c -include $(LOCAL_PATH)/Android.build.testlib.mk - -# ----------------------------------------------------------------------------- -# libtest_check_order_d.so -# ----------------------------------------------------------------------------- -libtest_check_order_dlsym_d_src_files := \ - dlopen_check_order_dlsym_answer.cpp - -libtest_check_order_dlsym_d_shared_libraries := libtest_check_order_dlsym_b -libtest_check_order_dlsym_d_cflags := -D__ANSWER=4 -D__ANSWER2=4 -module := libtest_check_order_dlsym_d -include $(LOCAL_PATH)/Android.build.testlib.mk - -# ----------------------------------------------------------------------------- -# libtest_check_order_left.so -# ----------------------------------------------------------------------------- -libtest_check_order_dlsym_1_left_src_files := \ - empty.cpp - -libtest_check_order_dlsym_1_left_shared_libraries := libtest_check_order_dlsym_a libtest_check_order_dlsym_b - -module := libtest_check_order_dlsym_1_left -include $(LOCAL_PATH)/Android.build.testlib.mk - -# ----------------------------------------------------------------------------- -# libtest_check_order.so -# ----------------------------------------------------------------------------- -libtest_check_order_dlsym_src_files := \ - empty.cpp - -libtest_check_order_dlsym_shared_libraries := libtest_check_order_dlsym_1_left \ - libtest_check_order_dlsym_2_right libtest_check_order_dlsym_3_c - -module := libtest_check_order_dlsym -include $(LOCAL_PATH)/Android.build.testlib.mk diff --git a/tests/libs/Android.build.dlopen_check_order_reloc_main_executable.mk b/tests/libs/Android.build.dlopen_check_order_reloc_main_executable.mk deleted file mode 100644 index 639696b25..000000000 --- a/tests/libs/Android.build.dlopen_check_order_reloc_main_executable.mk +++ /dev/null @@ -1,56 +0,0 @@ -# -# Copyright (C) 2012 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. -# - -# ----------------------------------------------------------------------------- -# Libraries used by dlfcn tests to verify correct relocation order: -# libtest_check_order_reloc_root*.so -# ----------------------------------------------------------------------------- - - -# ----------------------------------------------------------------------------- -# ..._1.so - empty -# ----------------------------------------------------------------------------- -libtest_check_order_reloc_root_1_src_files := \ - empty.cpp - - -module := libtest_check_order_reloc_root_1 -include $(LOCAL_PATH)/Android.build.testlib.mk - - -# ----------------------------------------------------------------------------- -# ..._2.so - this one has the incorrect answer -# ----------------------------------------------------------------------------- -libtest_check_order_reloc_root_2_src_files := \ - dlopen_check_order_reloc_root_answer_impl.cpp - -libtest_check_order_reloc_root_2_cflags := -D__ANSWER=2 - -module := libtest_check_order_reloc_root_2 -include $(LOCAL_PATH)/Android.build.testlib.mk - -# ----------------------------------------------------------------------------- -# libtest_check_order_reloc_root.so <- implements get_answer3() -# ----------------------------------------------------------------------------- -libtest_check_order_reloc_root_src_files := \ - dlopen_check_order_reloc_root_answer.cpp - -libtest_check_order_reloc_root_shared_libraries := \ - libtest_check_order_reloc_root_1 \ - libtest_check_order_reloc_root_2 - -module := libtest_check_order_reloc_root -include $(LOCAL_PATH)/Android.build.testlib.mk diff --git a/tests/libs/Android.build.dlopen_check_order_reloc_siblings.mk b/tests/libs/Android.build.dlopen_check_order_reloc_siblings.mk deleted file mode 100644 index 0f1a2b4da..000000000 --- a/tests/libs/Android.build.dlopen_check_order_reloc_siblings.mk +++ /dev/null @@ -1,133 +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. -# - -# ----------------------------------------------------------------------------- -# Libraries used by dlfcn tests to verify correct relocation order: -# libtest_check_order_reloc_siblings*.so -# ----------------------------------------------------------------------------- - -# ----------------------------------------------------------------------------- -# ..._1.so - empty -# ----------------------------------------------------------------------------- -libtest_check_order_reloc_siblings_1_src_files := \ - empty.cpp - -libtest_check_order_reloc_siblings_1_shared_libraries := \ - libtest_check_order_reloc_siblings_a \ - libtest_check_order_reloc_siblings_b - -module := libtest_check_order_reloc_siblings_1 -include $(LOCAL_PATH)/Android.build.testlib.mk - - -# ----------------------------------------------------------------------------- -# ..._2.so - empty -# ----------------------------------------------------------------------------- -libtest_check_order_reloc_siblings_2_src_files := \ - empty.cpp - -libtest_check_order_reloc_siblings_2_shared_libraries := \ - libtest_check_order_reloc_siblings_c \ - libtest_check_order_reloc_siblings_d - -module := libtest_check_order_reloc_siblings_2 -include $(LOCAL_PATH)/Android.build.testlib.mk - -# ----------------------------------------------------------------------------- -# ..._3.so - get_answer2(); -# ----------------------------------------------------------------------------- -libtest_check_order_reloc_siblings_3_src_files := \ - dlopen_check_order_reloc_nephew_answer.cpp - -libtest_check_order_reloc_siblings_3_shared_libraries := \ - libtest_check_order_reloc_siblings_e \ - libtest_check_order_reloc_siblings_f - -module := libtest_check_order_reloc_siblings_3 -include $(LOCAL_PATH)/Android.build.testlib.mk - -# ----------------------------------------------------------------------------- -# ..._a.so <- correct impl -# ----------------------------------------------------------------------------- -libtest_check_order_reloc_siblings_a_src_files := \ - dlopen_check_order_reloc_answer_impl.cpp - -libtest_check_order_reloc_siblings_a_cflags := -D__ANSWER=42 -module := libtest_check_order_reloc_siblings_a -include $(LOCAL_PATH)/Android.build.testlib.mk - -# ----------------------------------------------------------------------------- -# ..._b.so -# ----------------------------------------------------------------------------- -libtest_check_order_reloc_siblings_b_src_files := \ - dlopen_check_order_reloc_answer_impl.cpp - -libtest_check_order_reloc_siblings_b_cflags := -D__ANSWER=1 -module := libtest_check_order_reloc_siblings_b -include $(LOCAL_PATH)/Android.build.testlib.mk - -# ----------------------------------------------------------------------------- -# ..._c.so -# ----------------------------------------------------------------------------- -libtest_check_order_reloc_siblings_c_src_files := \ - dlopen_check_order_reloc_answer_impl.cpp - -libtest_check_order_reloc_siblings_c_cflags := -D__ANSWER=2 -module := libtest_check_order_reloc_siblings_c -include $(LOCAL_PATH)/Android.build.testlib.mk - -# ----------------------------------------------------------------------------- -# ..._d.so -# ----------------------------------------------------------------------------- -libtest_check_order_reloc_siblings_d_src_files := \ - dlopen_check_order_reloc_answer_impl.cpp - -libtest_check_order_reloc_siblings_d_cflags := -D__ANSWER=3 -module := libtest_check_order_reloc_siblings_d -include $(LOCAL_PATH)/Android.build.testlib.mk - -# ----------------------------------------------------------------------------- -# ..._e.so -# ----------------------------------------------------------------------------- -libtest_check_order_reloc_siblings_e_src_files := \ - dlopen_check_order_reloc_answer_impl.cpp - -libtest_check_order_reloc_siblings_e_cflags := -D__ANSWER=4 -module := libtest_check_order_reloc_siblings_e -include $(LOCAL_PATH)/Android.build.testlib.mk - -# ----------------------------------------------------------------------------- -# ..._f.so <- get_answer() -# ----------------------------------------------------------------------------- -libtest_check_order_reloc_siblings_f_src_files := \ - dlopen_check_order_reloc_answer.cpp - -module := libtest_check_order_reloc_siblings_f -include $(LOCAL_PATH)/Android.build.testlib.mk - -# ----------------------------------------------------------------------------- -# libtest_check_order_reloc_siblings.so -# ----------------------------------------------------------------------------- -libtest_check_order_reloc_siblings_src_files := \ - empty.cpp - -libtest_check_order_reloc_siblings_shared_libraries := \ - libtest_check_order_reloc_siblings_1 \ - libtest_check_order_reloc_siblings_2 \ - libtest_check_order_reloc_siblings_3 - -module := libtest_check_order_reloc_siblings -include $(LOCAL_PATH)/Android.build.testlib.mk diff --git a/tests/libs/Android.mk b/tests/libs/Android.mk index 05e7113ba..175a63539 100644 --- a/tests/libs/Android.mk +++ b/tests/libs/Android.mk @@ -21,9 +21,6 @@ common_cppflags += -std=gnu++11 common_additional_dependencies := \ $(LOCAL_PATH)/Android.mk \ $(LOCAL_PATH)/Android.build.dlext_testzip.mk \ - $(LOCAL_PATH)/Android.build.dlopen_check_order_dlsym.mk \ - $(LOCAL_PATH)/Android.build.dlopen_check_order_reloc_siblings.mk \ - $(LOCAL_PATH)/Android.build.dlopen_check_order_reloc_main_executable.mk \ $(LOCAL_PATH)/Android.build.testlib.mk \ $(TEST_PATH)/Android.build.mk @@ -153,19 +150,79 @@ module := libtest_nodelete_dt_flags_1 include $(LOCAL_PATH)/Android.build.testlib.mk # ----------------------------------------------------------------------------- -# Build libtest_check_order_dlsym.so with its dependencies. +# Libraries used by dlfcn tests to verify correct load order: +# libtest_check_order_2_right.so # ----------------------------------------------------------------------------- -include $(LOCAL_PATH)/Android.build.dlopen_check_order_dlsym.mk +libtest_check_order_2_right_src_files := \ + dlopen_testlib_answer.cpp + +libtest_check_order_2_right_cflags := -D__ANSWER=42 +module := libtest_check_order_2_right +include $(LOCAL_PATH)/Android.build.testlib.mk # ----------------------------------------------------------------------------- -# Build libtest_check_order_siblings.so with its dependencies. +# libtest_check_order_a.so # ----------------------------------------------------------------------------- -include $(LOCAL_PATH)/Android.build.dlopen_check_order_reloc_siblings.mk +libtest_check_order_a_src_files := \ + dlopen_testlib_answer.cpp + +libtest_check_order_a_cflags := -D__ANSWER=1 +module := libtest_check_order_a +include $(LOCAL_PATH)/Android.build.testlib.mk # ----------------------------------------------------------------------------- -# Build libtest_check_order_root.so with its dependencies. +# libtest_check_order_b.so # ----------------------------------------------------------------------------- -include $(LOCAL_PATH)/Android.build.dlopen_check_order_reloc_main_executable.mk +libtest_check_order_b_src_files := \ + dlopen_testlib_answer.cpp + +libtest_check_order_b_cflags := -D__ANSWER=2 -D__ANSWER2=43 +module := libtest_check_order_b +include $(LOCAL_PATH)/Android.build.testlib.mk + +# ----------------------------------------------------------------------------- +# libtest_check_order_c.so +# ----------------------------------------------------------------------------- +libtest_check_order_3_c_src_files := \ + dlopen_testlib_answer.cpp + +libtest_check_order_3_c_cflags := -D__ANSWER=3 +module := libtest_check_order_3_c +include $(LOCAL_PATH)/Android.build.testlib.mk + +# ----------------------------------------------------------------------------- +# libtest_check_order_d.so +# ----------------------------------------------------------------------------- +libtest_check_order_d_src_files := \ + dlopen_testlib_answer.cpp + +libtest_check_order_d_shared_libraries := libtest_check_order_b +libtest_check_order_d_cflags := -D__ANSWER=4 -D__ANSWER2=4 +module := libtest_check_order_d +include $(LOCAL_PATH)/Android.build.testlib.mk + +# ----------------------------------------------------------------------------- +# libtest_check_order_left.so +# ----------------------------------------------------------------------------- +libtest_check_order_1_left_src_files := \ + empty.cpp + +libtest_check_order_1_left_shared_libraries := libtest_check_order_a libtest_check_order_b + +module := libtest_check_order_1_left +include $(LOCAL_PATH)/Android.build.testlib.mk + +# ----------------------------------------------------------------------------- +# libtest_check_order.so +# ----------------------------------------------------------------------------- +libtest_check_order_src_files := \ + empty.cpp + +libtest_check_order_shared_libraries := libtest_check_order_1_left \ + libtest_check_order_2_right libtest_check_order_3_c + +module := libtest_check_order +include $(LOCAL_PATH)/Android.build.testlib.mk # ----------------------------------------------------------------------------- # Library with dependency loop used by dlfcn tests diff --git a/tests/libs/dlopen_check_order_reloc_answer.cpp b/tests/libs/dlopen_check_order_reloc_answer.cpp deleted file mode 100644 index 036670bee..000000000 --- a/tests/libs/dlopen_check_order_reloc_answer.cpp +++ /dev/null @@ -1,23 +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. - */ - -extern "C" int __attribute__((weak)) check_order_reloc_get_answer_impl() { - return 0; -} - -extern "C" int check_order_reloc_get_answer() { - return check_order_reloc_get_answer_impl(); -} diff --git a/tests/libs/dlopen_check_order_reloc_answer_impl.cpp b/tests/libs/dlopen_check_order_reloc_answer_impl.cpp deleted file mode 100644 index 324b905da..000000000 --- a/tests/libs/dlopen_check_order_reloc_answer_impl.cpp +++ /dev/null @@ -1,19 +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. - */ - -extern "C" int check_order_reloc_get_answer_impl() { - return __ANSWER; -} diff --git a/tests/libs/dlopen_check_order_reloc_nephew_answer.cpp b/tests/libs/dlopen_check_order_reloc_nephew_answer.cpp deleted file mode 100644 index 065d1bef7..000000000 --- a/tests/libs/dlopen_check_order_reloc_nephew_answer.cpp +++ /dev/null @@ -1,21 +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. - */ - -extern "C" int check_order_reloc_get_answer_impl(); - -extern "C" int check_order_reloc_nephew_get_answer() { - return check_order_reloc_get_answer_impl(); -} diff --git a/tests/libs/dlopen_check_order_reloc_root_answer.cpp b/tests/libs/dlopen_check_order_reloc_root_answer.cpp deleted file mode 100644 index b21abd77a..000000000 --- a/tests/libs/dlopen_check_order_reloc_root_answer.cpp +++ /dev/null @@ -1,21 +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. - */ - -extern "C" int check_order_reloc_root_get_answer_impl(); - -extern "C" int check_order_reloc_root_get_answer() { - return check_order_reloc_root_get_answer_impl(); -} diff --git a/tests/libs/dlopen_check_order_reloc_root_answer_impl.cpp b/tests/libs/dlopen_check_order_reloc_root_answer_impl.cpp deleted file mode 100644 index 25fb9aca9..000000000 --- a/tests/libs/dlopen_check_order_reloc_root_answer_impl.cpp +++ /dev/null @@ -1,19 +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. - */ - -extern "C" int check_order_reloc_root_get_answer_impl() { - return __ANSWER; -} diff --git a/tests/libs/dlopen_check_order_dlsym_answer.cpp b/tests/libs/dlopen_testlib_answer.cpp similarity index 87% rename from tests/libs/dlopen_check_order_dlsym_answer.cpp rename to tests/libs/dlopen_testlib_answer.cpp index 2ae6cf79e..a4d75046e 100644 --- a/tests/libs/dlopen_check_order_dlsym_answer.cpp +++ b/tests/libs/dlopen_testlib_answer.cpp @@ -14,12 +14,12 @@ * limitations under the License. */ -extern "C" int check_order_dlsym_get_answer() { +extern "C" int dlopen_test_get_answer() { return __ANSWER; } #ifdef __ANSWER2 -extern "C" int check_order_dlsym_get_answer2() { +extern "C" int dlopen_test_get_answer2() { return __ANSWER2; } #endif From d84897d4a30d5521e7f16d986142b634ef28541a Mon Sep 17 00:00:00 2001 From: Dmitriy Ivanov Date: Mon, 3 Nov 2014 22:08:31 -0800 Subject: [PATCH 12/13] Revert "Add RTLD_NODELETE flag support" This reverts commit c87f65d2cd0690d81665f8b241c1d763f72b6f80. Bug: 18222321 Bug: 18211780 Change-Id: I00252e26a28a41ab9f1e2dd3b32f0f80d86297f1 --- libc/include/dlfcn.h | 1 - linker/linker.cpp | 16 +---- linker/linker.h | 18 +++-- tests/dlfcn_test.cpp | 80 ----------------------- tests/libs/Android.mk | 29 -------- tests/libs/dlopen_nodelete_1.cpp | 31 --------- tests/libs/dlopen_nodelete_2.cpp | 31 --------- tests/libs/dlopen_nodelete_dt_flags_1.cpp | 30 --------- 8 files changed, 10 insertions(+), 226 deletions(-) delete mode 100644 tests/libs/dlopen_nodelete_1.cpp delete mode 100644 tests/libs/dlopen_nodelete_2.cpp delete mode 100644 tests/libs/dlopen_nodelete_dt_flags_1.cpp diff --git a/libc/include/dlfcn.h b/libc/include/dlfcn.h index afa76878f..8dde08cf5 100644 --- a/libc/include/dlfcn.h +++ b/libc/include/dlfcn.h @@ -64,7 +64,6 @@ enum { RTLD_GLOBAL = 2, #endif RTLD_NOLOAD = 4, - RTLD_NODELETE = 0x01000, }; #if defined (__LP64__) diff --git a/linker/linker.cpp b/linker/linker.cpp index 41557e231..636541297 100644 --- a/linker/linker.cpp +++ b/linker/linker.cpp @@ -987,11 +987,6 @@ static soinfo* find_library(const char* name, int rtld_flags, const android_dlex } static void soinfo_unload(soinfo* si) { - if (!si->can_unload()) { - TRACE("not unloading '%s' - the binary is flagged with NODELETE", si->name); - return; - } - if (si->ref_count == 1) { TRACE("unloading '%s'", si->name); si->CallDestructors(); @@ -1050,7 +1045,7 @@ void do_android_update_LD_LIBRARY_PATH(const char* ld_library_path) { } soinfo* do_dlopen(const char* name, int flags, const android_dlextinfo* extinfo) { - if ((flags & ~(RTLD_NOW|RTLD_LAZY|RTLD_LOCAL|RTLD_GLOBAL|RTLD_NODELETE|RTLD_NOLOAD)) != 0) { + if ((flags & ~(RTLD_NOW|RTLD_LAZY|RTLD_LOCAL|RTLD_GLOBAL|RTLD_NOLOAD)) != 0) { DL_ERR("invalid flags to dlopen: %x", flags); return nullptr; } @@ -1813,9 +1808,6 @@ const char* soinfo::get_string(ElfW(Word) index) const { return strtab + index; } -bool soinfo::can_unload() const { - return (rtld_flags & (RTLD_NODELETE | RTLD_GLOBAL)) == 0; -} /* Force any of the closed stdin, stdout and stderr to be associated with /dev/null. */ static int nullify_closed_stdio() { @@ -2119,13 +2111,9 @@ bool soinfo::PrelinkImage() { if ((d->d_un.d_val & DF_1_GLOBAL) != 0) { rtld_flags |= RTLD_GLOBAL; } - - if ((d->d_un.d_val & DF_1_NODELETE) != 0) { - rtld_flags |= RTLD_NODELETE; - } // TODO: Implement other flags - if ((d->d_un.d_val & ~(DF_1_NOW | DF_1_GLOBAL | DF_1_NODELETE)) != 0) { + if ((d->d_un.d_val & ~(DF_1_NOW | DF_1_GLOBAL)) != 0) { DL_WARN("Unsupported flags DT_FLAGS_1=%p", reinterpret_cast(d->d_un.d_val)); } break; diff --git a/linker/linker.h b/linker/linker.h index ebb4793af..6329efda6 100644 --- a/linker/linker.h +++ b/linker/linker.h @@ -134,7 +134,7 @@ struct soinfo { #endif soinfo* next; - uint32_t flags; + unsigned flags; private: const char* strtab; @@ -143,8 +143,8 @@ struct soinfo { size_t nbucket; size_t nchain; - uint32_t* bucket; - uint32_t* chain; + unsigned* bucket; + unsigned* chain; #if defined(__mips__) || !defined(__LP64__) // This is only used by mips and mips64, but needs to be here for @@ -179,12 +179,12 @@ struct soinfo { #if defined(__arm__) // ARM EABI section used for stack unwinding. - uint32_t* ARM_exidx; + unsigned* ARM_exidx; size_t ARM_exidx_count; #elif defined(__mips__) - uint32_t mips_symtabno; - uint32_t mips_local_gotno; - uint32_t mips_gotsym; + unsigned mips_symtabno; + unsigned mips_local_gotno; + unsigned mips_gotsym; #endif size_t ref_count; @@ -224,12 +224,10 @@ struct soinfo { ElfW(Addr) resolve_symbol_address(ElfW(Sym)* s); const char* get_string(ElfW(Word) index) const; - bool can_unload() const; bool inline has_min_version(uint32_t min_version) const { return (flags & FLAG_NEW_SOINFO) != 0 && version >= min_version; } - private: void CallArray(const char* array_name, linker_function_t* functions, size_t count, bool reverse); void CallFunction(const char* function_name, linker_function_t function); @@ -260,7 +258,7 @@ struct soinfo { friend soinfo* get_libdl_info(); }; -soinfo* get_libdl_info(); +extern soinfo* get_libdl_info(); void do_android_get_LD_LIBRARY_PATH(char*, size_t); void do_android_update_LD_LIBRARY_PATH(const char* ld_library_path); diff --git a/tests/dlfcn_test.cpp b/tests/dlfcn_test.cpp index f1ec0f131..c9c856a37 100644 --- a/tests/dlfcn_test.cpp +++ b/tests/dlfcn_test.cpp @@ -232,15 +232,10 @@ TEST(dlfcn, dlopen_check_rtld_global) { ASSERT_TRUE(sym == nullptr); void* handle = dlopen("libtest_simple.so", RTLD_NOW | RTLD_GLOBAL); - ASSERT_TRUE(handle != nullptr) << dlerror(); sym = dlsym(RTLD_DEFAULT, "dlopen_testlib_simple_func"); ASSERT_TRUE(sym != nullptr) << dlerror(); ASSERT_TRUE(reinterpret_cast(sym)()); dlclose(handle); - - // RTLD_GLOBAL implies RTLD_NODELETE, let's check that - void* sym_after_dlclose = dlsym(RTLD_DEFAULT, "dlopen_testlib_simple_func"); - ASSERT_EQ(sym, sym_after_dlclose); } // libtest_with_dependency_loop.so -> libtest_with_dependency_loop_a.so -> @@ -268,81 +263,6 @@ TEST(dlfcn, dlopen_check_loop) { #endif } -TEST(dlfcn, dlopen_nodelete) { - static bool is_unloaded = false; - - void* handle = dlopen("libtest_nodelete_1.so", RTLD_NOW | RTLD_NODELETE); - ASSERT_TRUE(handle != nullptr) << dlerror(); - void (*set_unload_flag_ptr)(bool*); - set_unload_flag_ptr = reinterpret_cast(dlsym(handle, "dlopen_nodelete_1_set_unload_flag_ptr")); - ASSERT_TRUE(set_unload_flag_ptr != nullptr) << dlerror(); - set_unload_flag_ptr(&is_unloaded); - - uint32_t* taxicab_number = reinterpret_cast(dlsym(handle, "dlopen_nodelete_1_taxicab_number")); - ASSERT_TRUE(taxicab_number != nullptr) << dlerror(); - ASSERT_EQ(1729U, *taxicab_number); - *taxicab_number = 2; - - dlclose(handle); - ASSERT_TRUE(!is_unloaded); - - uint32_t* taxicab_number_after_dlclose = reinterpret_cast(dlsym(handle, "dlopen_nodelete_1_taxicab_number")); - ASSERT_EQ(taxicab_number_after_dlclose, taxicab_number); - ASSERT_EQ(2U, *taxicab_number_after_dlclose); - - - handle = dlopen("libtest_nodelete_1.so", RTLD_NOW); - uint32_t* taxicab_number2 = reinterpret_cast(dlsym(handle, "dlopen_nodelete_1_taxicab_number")); - ASSERT_EQ(taxicab_number2, taxicab_number); - - ASSERT_EQ(2U, *taxicab_number2); - - dlclose(handle); - ASSERT_TRUE(!is_unloaded); -} - -TEST(dlfcn, dlopen_nodelete_on_second_dlopen) { - static bool is_unloaded = false; - - void* handle = dlopen("libtest_nodelete_2.so", RTLD_NOW); - ASSERT_TRUE(handle != nullptr) << dlerror(); - void (*set_unload_flag_ptr)(bool*); - set_unload_flag_ptr = reinterpret_cast(dlsym(handle, "dlopen_nodelete_2_set_unload_flag_ptr")); - ASSERT_TRUE(set_unload_flag_ptr != nullptr) << dlerror(); - set_unload_flag_ptr(&is_unloaded); - - uint32_t* taxicab_number = reinterpret_cast(dlsym(handle, "dlopen_nodelete_2_taxicab_number")); - ASSERT_TRUE(taxicab_number != nullptr) << dlerror(); - - ASSERT_EQ(1729U, *taxicab_number); - *taxicab_number = 2; - - // This RTLD_NODELETE should be ignored - void* handle1 = dlopen("libtest_nodelete_2.so", RTLD_NOW | RTLD_NODELETE); - ASSERT_TRUE(handle1 != nullptr) << dlerror(); - ASSERT_EQ(handle, handle1); - - dlclose(handle1); - dlclose(handle); - - ASSERT_TRUE(is_unloaded); -} - -TEST(dlfcn, dlopen_nodelete_dt_flags_1) { - static bool is_unloaded = false; - - void* handle = dlopen("libtest_nodelete_dt_flags_1.so", RTLD_NOW); - ASSERT_TRUE(handle != nullptr) << dlerror(); - void (*set_unload_flag_ptr)(bool*); - set_unload_flag_ptr = reinterpret_cast(dlsym(handle, "dlopen_nodelete_dt_flags_1_set_unload_flag_ptr")); - ASSERT_TRUE(set_unload_flag_ptr != nullptr) << dlerror(); - set_unload_flag_ptr(&is_unloaded); - - dlclose(handle); - ASSERT_TRUE(!is_unloaded); -} - - TEST(dlfcn, dlopen_failure) { void* self = dlopen("/does/not/exist", RTLD_NOW); ASSERT_TRUE(self == NULL); diff --git a/tests/libs/Android.mk b/tests/libs/Android.mk index 175a63539..af3e070a8 100644 --- a/tests/libs/Android.mk +++ b/tests/libs/Android.mk @@ -120,35 +120,6 @@ libtest_simple_src_files := \ module := libtest_simple include $(LOCAL_PATH)/Android.build.testlib.mk -# ----------------------------------------------------------------------------- -# Library used by dlfcn nodelete tests -# ----------------------------------------------------------------------------- -libtest_nodelete_1_src_files := \ - dlopen_nodelete_1.cpp - -module := libtest_nodelete_1 -include $(LOCAL_PATH)/Android.build.testlib.mk - -# ----------------------------------------------------------------------------- -# Library used by dlfcn nodelete tests -# ----------------------------------------------------------------------------- -libtest_nodelete_2_src_files := \ - dlopen_nodelete_2.cpp - -module := libtest_nodelete_2 -include $(LOCAL_PATH)/Android.build.testlib.mk - -# ----------------------------------------------------------------------------- -# Library used by dlfcn nodelete tests -# ----------------------------------------------------------------------------- -libtest_nodelete_dt_flags_1_src_files := \ - dlopen_nodelete_dt_flags_1.cpp - -libtest_nodelete_dt_flags_1_ldflags := -Wl,-z,nodelete - -module := libtest_nodelete_dt_flags_1 -include $(LOCAL_PATH)/Android.build.testlib.mk - # ----------------------------------------------------------------------------- # Libraries used by dlfcn tests to verify correct load order: # libtest_check_order_2_right.so diff --git a/tests/libs/dlopen_nodelete_1.cpp b/tests/libs/dlopen_nodelete_1.cpp deleted file mode 100644 index 943897815..000000000 --- a/tests/libs/dlopen_nodelete_1.cpp +++ /dev/null @@ -1,31 +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 - -uint32_t dlopen_nodelete_1_taxicab_number = 1729; -static bool* unload_flag_ptr = nullptr; - -extern "C" void dlopen_nodelete_1_set_unload_flag_ptr(bool* ptr) { - unload_flag_ptr = ptr; -} - -static void __attribute__((destructor)) unload_guard() { - if (unload_flag_ptr != nullptr) { - *unload_flag_ptr = true; - } -} diff --git a/tests/libs/dlopen_nodelete_2.cpp b/tests/libs/dlopen_nodelete_2.cpp deleted file mode 100644 index b5ab5c1ae..000000000 --- a/tests/libs/dlopen_nodelete_2.cpp +++ /dev/null @@ -1,31 +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 - -uint32_t dlopen_nodelete_2_taxicab_number = 1729; -static bool* unload_flag_ptr = nullptr; - -extern "C" void dlopen_nodelete_2_set_unload_flag_ptr(bool* ptr) { - unload_flag_ptr = ptr; -} - -static void __attribute__((destructor)) unload_guard() { - if (unload_flag_ptr != nullptr) { - *unload_flag_ptr = true; - } -} diff --git a/tests/libs/dlopen_nodelete_dt_flags_1.cpp b/tests/libs/dlopen_nodelete_dt_flags_1.cpp deleted file mode 100644 index 39c0a7ea6..000000000 --- a/tests/libs/dlopen_nodelete_dt_flags_1.cpp +++ /dev/null @@ -1,30 +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 - -static bool* unload_flag_ptr = nullptr; - -extern "C" void dlopen_nodelete_dt_flags_1_set_unload_flag_ptr(bool* ptr) { - unload_flag_ptr = ptr; -} - -static void __attribute__((destructor)) unload_guard() { - if (unload_flag_ptr != nullptr) { - *unload_flag_ptr = true; - } -} From e4ae96ffd3bf23b510c53701b14555cfd0274499 Mon Sep 17 00:00:00 2001 From: Dmitriy Ivanov Date: Mon, 3 Nov 2014 22:12:19 -0800 Subject: [PATCH 13/13] Revert "Fix dlsym() to take into account RTLD_GLOBAL/LOCAL" This reverts commit c85e82dde5c4b2accc50a9e17740b9005dfbae6a. Bug: 18222321 Bug: 18211780 Change-Id: I32f4048bd5ea85dc8a3dfccce8cf141b241ab692 --- linker/dlfcn.cpp | 2 +- linker/linker.cpp | 48 ++++++++++------------------ linker/linker.h | 4 +-- tests/dlfcn_test.cpp | 36 --------------------- tests/libs/dlopen_testlib_simple.cpp | 2 +- 5 files changed, 20 insertions(+), 72 deletions(-) diff --git a/linker/dlfcn.cpp b/linker/dlfcn.cpp index 367179d8d..3eb5bea22 100644 --- a/linker/dlfcn.cpp +++ b/linker/dlfcn.cpp @@ -232,7 +232,7 @@ static unsigned g_libdl_chains[] = { 0, 2, 3, 4, 5, 6, 7, 8, 9, 10, 0 }; static unsigned g_libdl_chains[] = { 0, 2, 3, 4, 5, 6, 7, 8, 9, 0 }; #endif -static soinfo __libdl_info("libdl.so", nullptr, 0, RTLD_GLOBAL); +static soinfo __libdl_info("libdl.so", nullptr, 0); // This is used by the dynamic linker. Every process gets these symbols for free. soinfo* get_libdl_info() { diff --git a/linker/linker.cpp b/linker/linker.cpp index 636541297..35c8cbdc8 100644 --- a/linker/linker.cpp +++ b/linker/linker.cpp @@ -282,13 +282,13 @@ static void protect_data(int protection) { g_soinfo_links_allocator.protect_all(protection); } -static soinfo* soinfo_alloc(const char* name, struct stat* file_stat, off64_t file_offset, int rtld_flags) { +static soinfo* soinfo_alloc(const char* name, struct stat* file_stat, off64_t file_offset) { if (strlen(name) >= SOINFO_NAME_LEN) { DL_ERR("library name \"%s\" too long", name); return nullptr; } - soinfo* si = new (g_soinfo_allocator.alloc()) soinfo(name, file_stat, file_offset, rtld_flags); + soinfo* si = new (g_soinfo_allocator.alloc()) soinfo(name, file_stat, file_offset); sonext->next = si; sonext = si; @@ -452,7 +452,7 @@ static ElfW(Sym)* soinfo_elf_lookup(soinfo* si, unsigned hash, const char* name) return nullptr; } -soinfo::soinfo(const char* name, const struct stat* file_stat, off64_t file_offset, int rtld_flags) { +soinfo::soinfo(const char* name, const struct stat* file_stat, off64_t file_offset) { memset(this, 0, sizeof(*this)); strlcpy(this->name, name, sizeof(this->name)); @@ -464,8 +464,6 @@ soinfo::soinfo(const char* name, const struct stat* file_stat, off64_t file_offs this->st_ino = file_stat->st_ino; this->file_offset = file_offset; } - - this->rtld_flags = rtld_flags; } static unsigned elfhash(const char* _name) { @@ -686,10 +684,6 @@ ElfW(Sym)* dlsym_linear_lookup(const char* name, soinfo** found, soinfo* start) ElfW(Sym)* s = nullptr; for (soinfo* si = start; (s == nullptr) && (si != nullptr); si = si->next) { - if ((si->get_rtld_flags() & RTLD_GLOBAL) == 0) { - continue; - } - s = soinfo_elf_lookup(si, elf_hash, name); if (s != nullptr) { *found = si; @@ -780,7 +774,7 @@ static void for_each_dt_needed(const soinfo* si, F action) { } } -static soinfo* load_library(LoadTaskList& load_tasks, const char* name, int rtld_flags, const android_dlextinfo* extinfo) { +static soinfo* load_library(LoadTaskList& load_tasks, const char* name, int dlflags, const android_dlextinfo* extinfo) { int fd = -1; off64_t file_offset = 0; ScopedFd file_guard(-1); @@ -825,7 +819,7 @@ static soinfo* load_library(LoadTaskList& load_tasks, const char* name, int rtld } } - if ((rtld_flags & RTLD_NOLOAD) != 0) { + if ((dlflags & RTLD_NOLOAD) != 0) { DL_ERR("library \"%s\" wasn't loaded and RTLD_NOLOAD prevented it", name); return nullptr; } @@ -836,7 +830,7 @@ static soinfo* load_library(LoadTaskList& load_tasks, const char* name, int rtld return nullptr; } - soinfo* si = soinfo_alloc(SEARCH_NAME(name), &file_stat, file_offset, rtld_flags); + soinfo* si = soinfo_alloc(SEARCH_NAME(name), &file_stat, file_offset); if (si == nullptr) { return nullptr; } @@ -868,7 +862,7 @@ static soinfo *find_loaded_library_by_name(const char* name) { return nullptr; } -static soinfo* find_library_internal(LoadTaskList& load_tasks, const char* name, int rtld_flags, const android_dlextinfo* extinfo) { +static soinfo* find_library_internal(LoadTaskList& load_tasks, const char* name, int dlflags, const android_dlextinfo* extinfo) { soinfo* si = find_loaded_library_by_name(name); @@ -876,7 +870,7 @@ static soinfo* find_library_internal(LoadTaskList& load_tasks, const char* name, // of this fact is done by load_library. if (si == nullptr) { TRACE("[ '%s' has not been found by name. Trying harder...]", name); - si = load_library(load_tasks, name, rtld_flags, extinfo); + si = load_library(load_tasks, name, dlflags, extinfo); } return si; @@ -900,7 +894,7 @@ static bool is_recursive(soinfo* si, soinfo* parent) { } static bool find_libraries(const char* const library_names[], size_t library_names_size, soinfo* soinfos[], - soinfo* ld_preloads[], size_t ld_preloads_size, int rtld_flags, const android_dlextinfo* extinfo) { + soinfo* ld_preloads[], size_t ld_preloads_size, int dlflags, const android_dlextinfo* extinfo) { // Step 0: prepare. LoadTaskList load_tasks; for (size_t i = 0; i < library_names_size; ++i) { @@ -926,7 +920,7 @@ static bool find_libraries(const char* const library_names[], size_t library_nam // Step 1: load and pre-link all DT_NEEDED libraries in breadth first order. for (LoadTask::unique_ptr task(load_tasks.pop_front()); task.get() != nullptr; task.reset(load_tasks.pop_front())) { - soinfo* si = find_library_internal(load_tasks, task->get_name(), rtld_flags, extinfo); + soinfo* si = find_library_internal(load_tasks, task->get_name(), dlflags, extinfo); if (si == nullptr) { return false; } @@ -971,7 +965,7 @@ static bool find_libraries(const char* const library_names[], size_t library_nam return true; } -static soinfo* find_library(const char* name, int rtld_flags, const android_dlextinfo* extinfo) { +static soinfo* find_library(const char* name, int dlflags, const android_dlextinfo* extinfo) { if (name == nullptr) { somain->ref_count++; return somain; @@ -979,7 +973,7 @@ static soinfo* find_library(const char* name, int rtld_flags, const android_dlex soinfo* si; - if (!find_libraries(&name, 1, &si, nullptr, 0, rtld_flags, extinfo)) { + if (!find_libraries(&name, 1, &si, nullptr, 0, dlflags, extinfo)) { return nullptr; } @@ -1764,14 +1758,6 @@ off64_t soinfo::get_file_offset() { return 0; } -int soinfo::get_rtld_flags() { - if (has_min_version(1)) { - return rtld_flags; - } - - return 0; -} - // This is a return on get_children()/get_parents() if // 'this->flags' does not have FLAG_NEW_SOINFO set. static soinfo::soinfo_list_t g_empty_list; @@ -2289,7 +2275,7 @@ static void add_vdso(KernelArgumentBlock& args __unused) { return; } - soinfo* si = soinfo_alloc("[vdso]", nullptr, 0, 0); + soinfo* si = soinfo_alloc("[vdso]", nullptr, 0); si->phdr = reinterpret_cast(reinterpret_cast(ehdr_vdso) + ehdr_vdso->e_phoff); si->phnum = ehdr_vdso->e_phnum; @@ -2310,7 +2296,7 @@ static void add_vdso(KernelArgumentBlock& args __unused) { #else #define LINKER_PATH "/system/bin/linker" #endif -static soinfo linker_soinfo_for_gdb(LINKER_PATH, nullptr, 0, 0); +static soinfo linker_soinfo_for_gdb(LINKER_PATH, nullptr, 0); /* gdb expects the linker to be in the debug shared object list. * Without this, gdb has trouble locating the linker's ".text" @@ -2374,7 +2360,7 @@ static ElfW(Addr) __linker_init_post_relocation(KernelArgumentBlock& args, ElfW( INFO("[ android linker & debugger ]"); - soinfo* si = soinfo_alloc(args.argv[0], nullptr, 0, RTLD_GLOBAL); + soinfo* si = soinfo_alloc(args.argv[0], nullptr, 0); if (si == nullptr) { exit(EXIT_FAILURE); } @@ -2449,7 +2435,7 @@ static ElfW(Addr) __linker_init_post_relocation(KernelArgumentBlock& args, ElfW( memset(needed_library_names, 0, sizeof(needed_library_names)); needed_library_name_list.copy_to_array(needed_library_names, needed_libraries_count); - if (needed_libraries_count > 0 && !find_libraries(needed_library_names, needed_libraries_count, needed_library_si, g_ld_preloads, ld_preloads_count, RTLD_GLOBAL, nullptr)) { + if (needed_libraries_count > 0 && !find_libraries(needed_library_names, needed_libraries_count, needed_library_si, g_ld_preloads, ld_preloads_count, 0, nullptr)) { __libc_format_fd(2, "CANNOT LINK EXECUTABLE DEPENDENCIES: %s\n", linker_get_error_buffer()); exit(EXIT_FAILURE); } @@ -2562,7 +2548,7 @@ extern "C" ElfW(Addr) __linker_init(void* raw_args) { ElfW(Ehdr)* elf_hdr = reinterpret_cast(linker_addr); ElfW(Phdr)* phdr = reinterpret_cast(linker_addr + elf_hdr->e_phoff); - soinfo linker_so("[dynamic linker]", nullptr, 0, 0); + soinfo linker_so("[dynamic linker]", nullptr, 0); // If the linker is not acting as PT_INTERP entry_point is equal to // _start. Which means that the linker is running as an executable and diff --git a/linker/linker.h b/linker/linker.h index 6329efda6..fa38c7fef 100644 --- a/linker/linker.h +++ b/linker/linker.h @@ -201,7 +201,7 @@ struct soinfo { #endif bool has_DT_SYMBOLIC; - soinfo(const char* name, const struct stat* file_stat, off64_t file_offset, int rtld_flags); + soinfo(const char* name, const struct stat* file_stat, off64_t file_offset); void CallConstructors(); void CallDestructors(); @@ -216,8 +216,6 @@ struct soinfo { dev_t get_st_dev(); off64_t get_file_offset(); - int get_rtld_flags(); - soinfo_list_t& get_children(); soinfo_list_t& get_parents(); diff --git a/tests/dlfcn_test.cpp b/tests/dlfcn_test.cpp index c9c856a37..e24af13c0 100644 --- a/tests/dlfcn_test.cpp +++ b/tests/dlfcn_test.cpp @@ -202,42 +202,6 @@ TEST(dlfcn, dlopen_check_order) { dlclose(handle); } -TEST(dlfcn, dlopen_check_rtld_local) { - void* sym = dlsym(RTLD_DEFAULT, "dlopen_testlib_simple_func"); - ASSERT_TRUE(sym == nullptr); - - // implicit RTLD_LOCAL - void* handle = dlopen("libtest_simple.so", RTLD_NOW); - sym = dlsym(RTLD_DEFAULT, "dlopen_testlib_simple_func"); - ASSERT_TRUE(sym == nullptr); - ASSERT_SUBSTR("undefined symbol: dlopen_testlib_simple_func", dlerror()); - sym = dlsym(handle, "dlopen_testlib_simple_func"); - ASSERT_TRUE(sym != nullptr); - ASSERT_TRUE(reinterpret_cast(sym)()); - dlclose(handle); - - // explicit RTLD_LOCAL - handle = dlopen("libtest_simple.so", RTLD_NOW | RTLD_LOCAL); - sym = dlsym(RTLD_DEFAULT, "dlopen_testlib_simple_func"); - ASSERT_TRUE(sym == nullptr); - ASSERT_SUBSTR("undefined symbol: dlopen_testlib_simple_func", dlerror()); - sym = dlsym(handle, "dlopen_testlib_simple_func"); - ASSERT_TRUE(sym != nullptr); - ASSERT_TRUE(reinterpret_cast(sym)()); - dlclose(handle); -} - -TEST(dlfcn, dlopen_check_rtld_global) { - void* sym = dlsym(RTLD_DEFAULT, "dlopen_testlib_simple_func"); - ASSERT_TRUE(sym == nullptr); - - void* handle = dlopen("libtest_simple.so", RTLD_NOW | RTLD_GLOBAL); - sym = dlsym(RTLD_DEFAULT, "dlopen_testlib_simple_func"); - ASSERT_TRUE(sym != nullptr) << dlerror(); - ASSERT_TRUE(reinterpret_cast(sym)()); - dlclose(handle); -} - // libtest_with_dependency_loop.so -> libtest_with_dependency_loop_a.so -> // libtest_with_dependency_loop_b.so -> libtest_with_dependency_loop_c.so -> // libtest_with_dependency_loop_a.so diff --git a/tests/libs/dlopen_testlib_simple.cpp b/tests/libs/dlopen_testlib_simple.cpp index 32269557a..bf750b2a6 100644 --- a/tests/libs/dlopen_testlib_simple.cpp +++ b/tests/libs/dlopen_testlib_simple.cpp @@ -19,6 +19,6 @@ uint32_t dlopen_testlib_taxicab_number = 1729; -extern "C" bool dlopen_testlib_simple_func() { +bool dlopen_testlib_simple_func() { return true; }