From: Jakub Jelinek <jakub@redhat.com>
To: Ulrich Drepper <drepper@redhat.com>
Cc: Glibc hackers <libc-hacker@sources.redhat.com>
Subject: [PATCH] Fix relocation dependency handling
Date: Sun, 14 Apr 2002 13:25:00 -0000 [thread overview]
Message-ID: <20020414222546.Q32482@sunsite.ms.mff.cuni.cz> (raw)
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 <jakub@redhat.com>
* 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
next reply other threads:[~2002-04-14 20:25 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2002-04-14 13:25 Jakub Jelinek [this message]
2002-04-14 23:36 ` Ulrich Drepper
2002-04-15 1:29 ` Andreas Schwab
2002-04-15 10:01 ` Ulrich Drepper
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20020414222546.Q32482@sunsite.ms.mff.cuni.cz \
--to=jakub@redhat.com \
--cc=drepper@redhat.com \
--cc=libc-hacker@sources.redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).