From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 4522 invoked by alias); 14 Apr 2002 20:25:55 -0000 Mailing-List: contact libc-hacker-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-hacker-owner@sources.redhat.com Received: (qmail 4505 invoked from network); 14 Apr 2002 20:25:53 -0000 Received: from unknown (HELO sunsite.mff.cuni.cz) (195.113.19.66) by sources.redhat.com with SMTP; 14 Apr 2002 20:25:53 -0000 Received: (from jakub@localhost) by sunsite.mff.cuni.cz (8.11.6/8.11.6) id g3EKPkL31548; Sun, 14 Apr 2002 22:25:46 +0200 Date: Sun, 14 Apr 2002 13:25:00 -0000 From: Jakub Jelinek To: Ulrich Drepper Cc: Glibc hackers Subject: [PATCH] Fix relocation dependency handling Message-ID: <20020414222546.Q32482@sunsite.ms.mff.cuni.cz> Reply-To: Jakub Jelinek Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.2.5.1i X-SW-Source: 2002-04/txt/msg00054.txt.bz2 Hi! The following is an attempt to fix http://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=63422 The problem is that at least if GL(dl_dynamic_weak) != 0 (but I think in STV_PROTECTED handling this might happen as well), we don't call add_dependency at all if we don't find a non-weak definition. We resolve to the first weak dependency, and if this one is in a dlopened library and that library is later on dlclosed, we end up with relocations pointing to unmapped memory. Particularly: 28774: symbol=_t24__default_alloc_template2b1i0._S_node_allocator_lock; lookup in file=./a.out 28774: symbol=_t24__default_alloc_template2b1i0._S_node_allocator_lock; lookup in file=/usr/lib/libSDL-1.2.so.0 28774: symbol=_t24__default_alloc_template2b1i0._S_node_allocator_lock; lookup in file=/lib/i686/libpthread.so.0 28774: symbol=_t24__default_alloc_template2b1i0._S_node_allocator_lock; lookup in file=/lib/i686/libc.so.6 28774: symbol=_t24__default_alloc_template2b1i0._S_node_allocator_lock; lookup in file=/lib/i686/libm.so.6 28774: symbol=_t24__default_alloc_template2b1i0._S_node_allocator_lock; lookup in file=/usr/X11R6/lib/libX11.so.6 28774: symbol=_t24__default_alloc_template2b1i0._S_node_allocator_lock; lookup in file=/usr/X11R6/lib/libXext.so.6 28774: symbol=_t24__default_alloc_template2b1i0._S_node_allocator_lock; lookup in file=/lib/libdl.so.2 28774: symbol=_t24__default_alloc_template2b1i0._S_node_allocator_lock; lookup in file=/lib/ld-linux.so.2 28774: symbol=_t24__default_alloc_template2b1i0._S_node_allocator_lock; lookup in file=/usr/lib/libartscbackend.so.0 28774: symbol=_t24__default_alloc_template2b1i0._S_node_allocator_lock; lookup in file=/usr/lib/libartsflow.so.1 28774: symbol=_t24__default_alloc_template2b1i0._S_node_allocator_lock; lookup in file=/usr/lib/libsoundserver_idl.so.1 28774: symbol=_t24__default_alloc_template2b1i0._S_node_allocator_lock; lookup in file=/usr/lib/libkmedia2_idl.so.1 28774: symbol=_t24__default_alloc_template2b1i0._S_node_allocator_lock; lookup in file=/usr/lib/libaudiofile.so.0 28774: symbol=_t24__default_alloc_template2b1i0._S_node_allocator_lock; lookup in file=/usr/lib/libartsflow_idl.so.1 28774: symbol=_t24__default_alloc_template2b1i0._S_node_allocator_lock; lookup in file=/usr/lib/libmcop.so.1 28774: symbol=_t24__default_alloc_template2b1i0._S_node_allocator_lock; lookup in file=/lib/libresolv.so.2 28774: symbol=_t24__default_alloc_template2b1i0._S_node_allocator_lock; lookup in file=/lib/libdl.so.2 28774: symbol=_t24__default_alloc_template2b1i0._S_node_allocator_lock; lookup in file=/usr/lib/libstdc++-libc6.2-2.so.3 28774: symbol=_t24__default_alloc_template2b1i0._S_node_allocator_lock; lookup in file=/lib/i686/libm.so.6 28774: symbol=_t24__default_alloc_template2b1i0._S_node_allocator_lock; lookup in file=/lib/i686/libc.so.6 28774: symbol=_t24__default_alloc_template2b1i0._S_node_allocator_lock; lookup in file=/lib/ld-linux.so.2 28774: binding file /usr/lib/libmcop.so.1 to /usr/lib/libartscbackend.so.0: normal symbol `_t24__default_alloc_template2b1i0._S_node_allocator_lock' (note no 00746: file=/usr/lib/libartscbackend.so.0; needed by /usr/lib/libmcop.so.1 (relocation dependency) line neither here nor anywhere before this place). The patch below moves the add_dependency call and related code to the end of the functions to the point where we unconditionally return current_value as result. So far I've just lightly tested it on glibc tree from 4 days ago, ie. predating your dl-lookup.c etc. changes, and it worked for this case as expected. While forward porting it to current dl-lookup.c, I've noticed you haven't changed the last argument in the recursive calls to _dl_lookup_*symbol if add_dependency fails. Previously it was passing 0 since code above it ensured ! explicit, so it was just a more efficient way of passing explicit. But now that explicit is (flags & DL_LOOKUP_ADD_DEPENDENCY) != 0, I think we need to pass in flags, not 0 (both because of the additional bit fields and because 0 now means former !explicit. 2002-04-14 Jakub Jelinek * elf/dl-lookup.c (_dl_lookup_symbol): Move add_dependency call to the end of the function. Pass original flags to recursive call if add_dependency failed. (_dl_lookup_versioned_symbol): Likewise. --- libc/elf/dl-lookup.c.jj Sun Apr 14 22:14:09 2002 +++ libc/elf/dl-lookup.c Sun Apr 14 22:17:53 2002 @@ -228,24 +228,7 @@ _dl_lookup_symbol (const char *undef_nam for (scope = symbol_scope; *scope; ++scope) if (do_lookup (undef_name, hash, *ref, ¤t_value, *scope, 0, flags, NULL, type_class)) - { - /* We have to check whether this would bind UNDEF_MAP to an object - in the global scope which was dynamically loaded. In this case - we have to prevent the latter from being unloaded unless the - UNDEF_MAP object is also unloaded. */ - if (__builtin_expect (current_value.m->l_type == lt_loaded, 0) - /* Don't do this for explicit lookups as opposed to implicit - runtime lookups. */ - && (flags & DL_LOOKUP_ADD_DEPENDENCY) != 0 - /* Add UNDEF_MAP to the dependencies. */ - && add_dependency (undef_map, current_value.m) < 0) - /* Something went wrong. Perhaps the object we tried to reference - was just removed. Try finding another definition. */ - return INTUSE(_dl_lookup_symbol) (undef_name, undef_map, ref, - symbol_scope, type_class, 0); - - break; - } + break; if (__builtin_expect (current_value.s == NULL, 0)) { @@ -282,6 +265,21 @@ _dl_lookup_symbol (const char *undef_nam } } + /* We have to check whether this would bind UNDEF_MAP to an object + in the global scope which was dynamically loaded. In this case + we have to prevent the latter from being unloaded unless the + UNDEF_MAP object is also unloaded. */ + if (__builtin_expect (current_value.m->l_type == lt_loaded, 0) + /* Don't do this for explicit lookups as opposed to implicit + runtime lookups. */ + && (flags & DL_LOOKUP_ADD_DEPENDENCY) != 0 + /* Add UNDEF_MAP to the dependencies. */ + && add_dependency (undef_map, current_value.m) < 0) + /* Something went wrong. Perhaps the object we tried to reference + was just removed. Try finding another definition. */ + return INTUSE(_dl_lookup_symbol) (undef_name, undef_map, ref, + symbol_scope, type_class, flags); + if (__builtin_expect (GL(dl_debug_mask) & (DL_DEBUG_BINDINGS|DL_DEBUG_PRELINK), 0)) _dl_debug_bindings (undef_name, undef_map, ref, symbol_scope, @@ -395,26 +393,7 @@ _dl_lookup_versioned_symbol (const char int res = do_lookup_versioned (undef_name, hash, *ref, ¤t_value, *scope, 0, version, NULL, type_class); if (res > 0) - { - /* We have to check whether this would bind UNDEF_MAP to an object - in the global scope which was dynamically loaded. In this case - we have to prevent the latter from being unloaded unless the - UNDEF_MAP object is also unloaded. */ - if (__builtin_expect (current_value.m->l_type == lt_loaded, 0) - /* Don't do this for explicit lookups as opposed to implicit - runtime lookups. */ - && flags != 0 - /* Add UNDEF_MAP to the dependencies. */ - && add_dependency (undef_map, current_value.m) < 0) - /* Something went wrong. Perhaps the object we tried to reference - was just removed. Try finding another definition. */ - return INTUSE(_dl_lookup_versioned_symbol) (undef_name, undef_map, - ref, symbol_scope, - version, type_class, - 0); - - break; - } + break; if (__builtin_expect (res, 0) < 0) { @@ -479,6 +458,22 @@ _dl_lookup_versioned_symbol (const char } } + /* We have to check whether this would bind UNDEF_MAP to an object + in the global scope which was dynamically loaded. In this case + we have to prevent the latter from being unloaded unless the + UNDEF_MAP object is also unloaded. */ + if (__builtin_expect (current_value.m->l_type == lt_loaded, 0) + /* Don't do this for explicit lookups as opposed to implicit + runtime lookups. */ + && flags != 0 + /* Add UNDEF_MAP to the dependencies. */ + && add_dependency (undef_map, current_value.m) < 0) + /* Something went wrong. Perhaps the object we tried to reference + was just removed. Try finding another definition. */ + return INTUSE(_dl_lookup_versioned_symbol) (undef_name, undef_map, + ref, symbol_scope, + version, type_class, flags); + if (__builtin_expect (GL(dl_debug_mask) & (DL_DEBUG_BINDINGS|DL_DEBUG_PRELINK), 0)) _dl_debug_bindings (undef_name, undef_map, ref, symbol_scope, Jakub