* [PATCH] Fix relocation dependency handling
@ 2002-04-14 13:25 Jakub Jelinek
2002-04-14 23:36 ` Ulrich Drepper
0 siblings, 1 reply; 4+ messages in thread
From: Jakub Jelinek @ 2002-04-14 13:25 UTC (permalink / raw)
To: Ulrich Drepper; +Cc: Glibc hackers
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Fix relocation dependency handling
2002-04-14 13:25 [PATCH] Fix relocation dependency handling Jakub Jelinek
@ 2002-04-14 23:36 ` Ulrich Drepper
2002-04-15 1:29 ` Andreas Schwab
0 siblings, 1 reply; 4+ messages in thread
From: Ulrich Drepper @ 2002-04-14 23:36 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Glibc hackers
[-- Attachment #1: Type: text/plain, Size: 596 bytes --]
On Sun, 2002-04-14 at 13:25, Jakub Jelinek wrote:
> 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.
This patch looks OK, I've applied it. Thanks,
--
---------------. ,-. 1325 Chesapeake Terrace
Ulrich Drepper \ ,-------------------' \ Sunnyvale, CA 94089 USA
Red Hat `--' drepper at redhat.com `------------------------
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 232 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Fix relocation dependency handling
2002-04-14 23:36 ` Ulrich Drepper
@ 2002-04-15 1:29 ` Andreas Schwab
2002-04-15 10:01 ` Ulrich Drepper
0 siblings, 1 reply; 4+ messages in thread
From: Andreas Schwab @ 2002-04-15 1:29 UTC (permalink / raw)
To: Ulrich Drepper; +Cc: Jakub Jelinek, Glibc hackers
Ulrich Drepper <drepper@redhat.com> writes:
|> On Sun, 2002-04-14 at 13:25, Jakub Jelinek wrote:
|>
|> > 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.
|>
|> This patch looks OK, I've applied it. Thanks,
See also http://sources.redhat.com/ml/libc-hacker/2002-02/msg00121.html.
Why didn't you just check in this patch? It even contains a test!
Andreas.
--
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE GmbH, Deutschherrnstr. 15-19, D-90429 Nürnberg
Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2002-04-15 17:01 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2002-04-14 13:25 [PATCH] Fix relocation dependency handling Jakub Jelinek
2002-04-14 23:36 ` Ulrich Drepper
2002-04-15 1:29 ` Andreas Schwab
2002-04-15 10:01 ` Ulrich Drepper
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).