[PATCH] Consolidate object search in DSO in _dl_find_dso_for_object

classic Classic list List threaded Threaded
14 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] Consolidate object search in DSO in _dl_find_dso_for_object

Siddhesh Poyarekar-3
Hi,

As Carlos, suggested, here's a patch to consolidate the object search
loop in various places in current code into a separate function.
There are only 3 places where this consolidation is needed (4 with
__cxa_thread_atexit_impl); the other two have a completely different
logic.

Built and regression tested on x86_64.  OK to commit?

Siddhesh

        * elf/Versions (ld): Add _dl_find_dso_for_object.
        * elf/dl-addr.c (_dl_addr): Use _dl_find_dso_for_object.
        * elf/dl-open.c (_dl_find_dso_for_object): New function.
        (dl_open_worker): Use _dl_find_dso_for_object.
        * elf/dl-sym.c (do_sym): Likewise.
        * sysdeps/generic/ldsodefs.h: Declare _dl_find_dso_for_object.

diff --git a/elf/Versions b/elf/Versions
index 87e27c5..97615de 100644
--- a/elf/Versions
+++ b/elf/Versions
@@ -62,5 +62,6 @@ ld {
     _dl_debug_state;
     # Pointer protection.
     __pointer_chk_guard;
+    _dl_find_dso_for_object;
   }
 }
diff --git a/elf/dl-addr.c b/elf/dl-addr.c
index 91cc443..a533466 100644
--- a/elf/dl-addr.c
+++ b/elf/dl-addr.c
@@ -130,18 +130,14 @@ _dl_addr (const void *address, Dl_info *info,
   /* Protect against concurrent loads and unloads.  */
   __rtld_lock_lock_recursive (GL(dl_load_lock));
 
-  /* Find the highest-addressed object that ADDRESS is not below.  */
-  for (Lmid_t ns = 0; ns < GL(dl_nns); ++ns)
-    for (struct link_map *l = GL(dl_ns)[ns]._ns_loaded; l; l = l->l_next)
-      if (addr >= l->l_map_start && addr < l->l_map_end
-  && (l->l_contiguous || _dl_addr_inside_object (l, addr)))
- {
-  determine_info (addr, l, info, mapp, symbolp);
-  result = 1;
-  goto out;
- }
+  struct link_map *l = _dl_find_dso_for_object (addr);
+
+  if (l)
+    {
+      determine_info (addr, l, info, mapp, symbolp);
+      result = 1;
+    }
 
- out:
   __rtld_lock_unlock_recursive (GL(dl_load_lock));
 
   return result;
diff --git a/elf/dl-open.c b/elf/dl-open.c
index 67f7e73..1c2f864 100644
--- a/elf/dl-open.c
+++ b/elf/dl-open.c
@@ -165,6 +165,26 @@ add_to_global (struct link_map *new)
   return 0;
 }
 
+struct link_map *
+internal_function
+_dl_find_dso_for_object (const ElfW(Addr) addr)
+{
+  struct link_map *l;
+
+  /* Find the highest-addressed object that ADDR is not below.  */
+  for (Lmid_t ns = 0; ns < GL(dl_nns); ++ns)
+    for (l = GL(dl_ns)[ns]._ns_loaded; l != NULL; l = l->l_next)
+      if (addr >= l->l_map_start && addr < l->l_map_end
+  && (l->l_contiguous
+      || _dl_addr_inside_object (l, (ElfW(Addr)) addr)))
+ {
+  assert (ns == l->l_ns);
+  return l;
+ }
+  return NULL;
+}
+rtld_hidden_def (_dl_find_dso_for_object);
+
 static void
 dl_open_worker (void *a)
 {
@@ -194,20 +214,11 @@ dl_open_worker (void *a)
       call_map = GL(dl_ns)[LM_ID_BASE]._ns_loaded;
 #endif
 
-      struct link_map *l;
-      for (Lmid_t ns = 0; ns < GL(dl_nns); ++ns)
- for (l = GL(dl_ns)[ns]._ns_loaded; l != NULL; l = l->l_next)
-  if (caller_dlopen >= (const void *) l->l_map_start
-      && caller_dlopen < (const void *) l->l_map_end
-      && (l->l_contiguous
-  || _dl_addr_inside_object (l, (ElfW(Addr)) caller_dlopen)))
-    {
-      assert (ns == l->l_ns);
-      call_map = l;
-      goto found_caller;
-    }
+      struct link_map *l = _dl_find_dso_for_object ((ElfW(Addr)) caller_dlopen);
+
+      if (l)
+        call_map = l;
 
-    found_caller:
       if (args->nsid == __LM_ID_CALLER)
  {
 #ifndef SHARED
diff --git a/elf/dl-sym.c b/elf/dl-sym.c
index d2b4db7..05de6c1 100644
--- a/elf/dl-sym.c
+++ b/elf/dl-sym.c
@@ -91,20 +91,10 @@ do_sym (void *handle, const char *name, void *who,
   lookup_t result;
   ElfW(Addr) caller = (ElfW(Addr)) who;
 
+  struct link_map *l = _dl_find_dso_for_object (caller);
   /* If the address is not recognized the call comes from the main
      program (we hope).  */
-  struct link_map *match = GL(dl_ns)[LM_ID_BASE]._ns_loaded;
-
-  /* Find the highest-addressed object that CALLER is not below.  */
-  for (Lmid_t ns = 0; ns < GL(dl_nns); ++ns)
-    for (struct link_map *l = GL(dl_ns)[ns]._ns_loaded; l != NULL;
- l = l->l_next)
-      if (caller >= l->l_map_start && caller < l->l_map_end
-  && (l->l_contiguous || _dl_addr_inside_object (l, caller)))
- {
-  match = l;
-  break;
- }
+  struct link_map *match = l ? l : GL(dl_ns)[LM_ID_BASE]._ns_loaded;
 
   if (handle == RTLD_DEFAULT)
     {
diff --git a/sysdeps/generic/ldsodefs.h b/sysdeps/generic/ldsodefs.h
index d6350fa..01a2712 100644
--- a/sysdeps/generic/ldsodefs.h
+++ b/sysdeps/generic/ldsodefs.h
@@ -1006,6 +1006,10 @@ extern int _dl_addr_inside_object (struct link_map *l, const ElfW(Addr) addr)
 /* Show show of an object.  */
 extern void _dl_show_scope (struct link_map *new, int from);
 
+extern struct link_map *_dl_find_dso_for_object (const ElfW(Addr) addr)
+     internal_function;
+rtld_hidden_proto (_dl_find_dso_for_object)
+
 __END_DECLS
 
 #endif /* ldsodefs.h */
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Consolidate object search in DSO in _dl_find_dso_for_object

Andreas Jaeger-8
On 02/15/2013 12:43 PM, Siddhesh Poyarekar wrote:

> Hi,
>
> As Carlos, suggested, here's a patch to consolidate the object search
> loop in various places in current code into a separate function.
> There are only 3 places where this consolidation is needed (4 with
> __cxa_thread_atexit_impl); the other two have a completely different
> logic.
>
> Built and regression tested on x86_64.  OK to commit?
>
> Siddhesh
>
> * elf/Versions (ld): Add _dl_find_dso_for_object.
> * elf/dl-addr.c (_dl_addr): Use _dl_find_dso_for_object.
> * elf/dl-open.c (_dl_find_dso_for_object): New function.
> (dl_open_worker): Use _dl_find_dso_for_object.
> * elf/dl-sym.c (do_sym): Likewise.
> * sysdeps/generic/ldsodefs.h: Declare _dl_find_dso_for_object.
>
> diff --git a/elf/Versions b/elf/Versions
> index 87e27c5..97615de 100644
> --- a/elf/Versions
> +++ b/elf/Versions
> @@ -62,5 +62,6 @@ ld {
>       _dl_debug_state;
>       # Pointer protection.
>       __pointer_chk_guard;
> +    _dl_find_dso_for_object;
>     }
>   }

The above should be alphabetically, so please add at the proper place.

I didn't review the rest properly, just wanted to point this one out,
Andreas
--
  Andreas Jaeger aj@{suse.com,opensuse.org} Twitter/Identica: jaegerandi
   SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
    GF: Jeff Hawn,Jennifer Guild,Felix Imendörffer,HRB16746 (AG Nürnberg)
     GPG fingerprint = 93A3 365E CE47 B889 DF7F  FED1 389A 563C C272 A126
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Consolidate object search in DSO in _dl_find_dso_for_object

Siddhesh Poyarekar-3
On Fri, Feb 15, 2013 at 01:12:56PM +0100, Andreas Jaeger wrote:
> >      # Pointer protection.
> >      __pointer_chk_guard;
> >+    _dl_find_dso_for_object;
> >    }
> >  }
>
> The above should be alphabetically, so please add at the proper place.
>
> I didn't review the rest properly, just wanted to point this one out,

Thanks, I'll fix that.  Will wait for the rest of the review.

Siddhesh
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Consolidate object search in DSO in _dl_find_dso_for_object

Carlos O'Donell-6
In reply to this post by Siddhesh Poyarekar-3
On 02/15/2013 06:43 AM, Siddhesh Poyarekar wrote:
> Hi,
>
> As Carlos, suggested, here's a patch to consolidate the object search
> loop in various places in current code into a separate function.
> There are only 3 places where this consolidation is needed (4 with
> __cxa_thread_atexit_impl); the other two have a completely different
> logic.

Thanks for doing this clean up, I appreciate it.
 

> Built and regression tested on x86_64.  OK to commit?
>
> Siddhesh
>
> * elf/Versions (ld): Add _dl_find_dso_for_object.
> * elf/dl-addr.c (_dl_addr): Use _dl_find_dso_for_object.
> * elf/dl-open.c (_dl_find_dso_for_object): New function.
> (dl_open_worker): Use _dl_find_dso_for_object.
> * elf/dl-sym.c (do_sym): Likewise.
> * sysdeps/generic/ldsodefs.h: Declare _dl_find_dso_for_object.
>
> diff --git a/elf/Versions b/elf/Versions
> index 87e27c5..97615de 100644
> --- a/elf/Versions
> +++ b/elf/Versions
> @@ -62,5 +62,6 @@ ld {
>      _dl_debug_state;
>      # Pointer protection.
>      __pointer_chk_guard;
> +    _dl_find_dso_for_object;

While Andreas says the list should be alphabetical it doesn't look like it is :-)

I guess it would be a distinct patch to sort the list first?

How deep does the rabbit hole go?

>    }
>  }
> diff --git a/elf/dl-addr.c b/elf/dl-addr.c
> index 91cc443..a533466 100644
> --- a/elf/dl-addr.c
> +++ b/elf/dl-addr.c
> @@ -130,18 +130,14 @@ _dl_addr (const void *address, Dl_info *info,
>    /* Protect against concurrent loads and unloads.  */
>    __rtld_lock_lock_recursive (GL(dl_load_lock));
>  
> -  /* Find the highest-addressed object that ADDRESS is not below.  */
> -  for (Lmid_t ns = 0; ns < GL(dl_nns); ++ns)
> -    for (struct link_map *l = GL(dl_ns)[ns]._ns_loaded; l; l = l->l_next)
> -      if (addr >= l->l_map_start && addr < l->l_map_end
> -  && (l->l_contiguous || _dl_addr_inside_object (l, addr)))
> - {
> -  determine_info (addr, l, info, mapp, symbolp);
> -  result = 1;
> -  goto out;
> - }
> +  struct link_map *l = _dl_find_dso_for_object (addr);
> +
> +  if (l)
> +    {
> +      determine_info (addr, l, info, mapp, symbolp);
> +      result = 1;
> +    }
>  
> - out:
>    __rtld_lock_unlock_recursive (GL(dl_load_lock));

Good. And we get rid of a goto.

>  
>    return result;
> diff --git a/elf/dl-open.c b/elf/dl-open.c
> index 67f7e73..1c2f864 100644
> --- a/elf/dl-open.c
> +++ b/elf/dl-open.c
> @@ -165,6 +165,26 @@ add_to_global (struct link_map *new)
>    return 0;
>  }
>  

Add a function level comment saying what the function does,
what namespace it searches, for what, and what it returns
under which conditions.

> +struct link_map *
> +internal_function
> +_dl_find_dso_for_object (const ElfW(Addr) addr)
> +{
> +  struct link_map *l;
> +
> +  /* Find the highest-addressed object that ADDR is not below.  */
> +  for (Lmid_t ns = 0; ns < GL(dl_nns); ++ns)
> +    for (l = GL(dl_ns)[ns]._ns_loaded; l != NULL; l = l->l_next)
> +      if (addr >= l->l_map_start && addr < l->l_map_end
> +  && (l->l_contiguous
> +      || _dl_addr_inside_object (l, (ElfW(Addr)) addr)))
> + {
> +  assert (ns == l->l_ns);
> +  return l;
> + }
> +  return NULL;
> +}
> +rtld_hidden_def (_dl_find_dso_for_object);
> +

Looks good. Correct definition visibility.

>  static void
>  dl_open_worker (void *a)
>  {
> @@ -194,20 +214,11 @@ dl_open_worker (void *a)
>        call_map = GL(dl_ns)[LM_ID_BASE]._ns_loaded;
>  #endif
>  
> -      struct link_map *l;
> -      for (Lmid_t ns = 0; ns < GL(dl_nns); ++ns)
> - for (l = GL(dl_ns)[ns]._ns_loaded; l != NULL; l = l->l_next)
> -  if (caller_dlopen >= (const void *) l->l_map_start
> -      && caller_dlopen < (const void *) l->l_map_end
> -      && (l->l_contiguous
> -  || _dl_addr_inside_object (l, (ElfW(Addr)) caller_dlopen)))
> -    {
> -      assert (ns == l->l_ns);
> -      call_map = l;
> -      goto found_caller;
> -    }
> +      struct link_map *l = _dl_find_dso_for_object ((ElfW(Addr)) caller_dlopen);
> +
> +      if (l)
> +        call_map = l;
>  
> -    found_caller:

OK.

>        if (args->nsid == __LM_ID_CALLER)
>   {
>  #ifndef SHARED
> diff --git a/elf/dl-sym.c b/elf/dl-sym.c
> index d2b4db7..05de6c1 100644
> --- a/elf/dl-sym.c
> +++ b/elf/dl-sym.c
> @@ -91,20 +91,10 @@ do_sym (void *handle, const char *name, void *who,
>    lookup_t result;
>    ElfW(Addr) caller = (ElfW(Addr)) who;
>  
> +  struct link_map *l = _dl_find_dso_for_object (caller);
>    /* If the address is not recognized the call comes from the main
>       program (we hope).  */
> -  struct link_map *match = GL(dl_ns)[LM_ID_BASE]._ns_loaded;
> -
> -  /* Find the highest-addressed object that CALLER is not below.  */
> -  for (Lmid_t ns = 0; ns < GL(dl_nns); ++ns)
> -    for (struct link_map *l = GL(dl_ns)[ns]._ns_loaded; l != NULL;
> - l = l->l_next)
> -      if (caller >= l->l_map_start && caller < l->l_map_end
> -  && (l->l_contiguous || _dl_addr_inside_object (l, caller)))
> - {
> -  match = l;
> -  break;
> - }
> +  struct link_map *match = l ? l : GL(dl_ns)[LM_ID_BASE]._ns_loaded;

OK.

>  
>    if (handle == RTLD_DEFAULT)
>      {
> diff --git a/sysdeps/generic/ldsodefs.h b/sysdeps/generic/ldsodefs.h
> index d6350fa..01a2712 100644
> --- a/sysdeps/generic/ldsodefs.h
> +++ b/sysdeps/generic/ldsodefs.h
> @@ -1006,6 +1006,10 @@ extern int _dl_addr_inside_object (struct link_map *l, const ElfW(Addr) addr)
>  /* Show show of an object.  */
>  extern void _dl_show_scope (struct link_map *new, int from);
>  
> +extern struct link_map *_dl_find_dso_for_object (const ElfW(Addr) addr)
> +     internal_function;
> +rtld_hidden_proto (_dl_find_dso_for_object)
> +

OK.

>  __END_DECLS
>  
>  #endif /* ldsodefs.h */
>

I'd say OK to checkin with:
* Alphabetically sorted Version entries.
* Verbose comment for utility function.

Cheers,
Carlos.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Consolidate object search in DSO in _dl_find_dso_for_object

Siddhesh Poyarekar-3
On Fri, Feb 15, 2013 at 01:33:20PM -0500, Carlos O'Donell wrote:
>
> While Andreas says the list should be alphabetical it doesn't look like it is :-)
>
> I guess it would be a distinct patch to sort the list first?
>
> How deep does the rabbit hole go?

Actually now that I think of it, the Versions files don't need to be
sorted; it is the abilist files that *must* be sorted.

Siddhesh
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Consolidate object search in DSO in _dl_find_dso_for_object

Carlos O'Donell-6
On 02/15/2013 01:38 PM, Siddhesh Poyarekar wrote:

> On Fri, Feb 15, 2013 at 01:33:20PM -0500, Carlos O'Donell wrote:
>>
>> While Andreas says the list should be alphabetical it doesn't look like it is :-)
>>
>> I guess it would be a distinct patch to sort the list first?
>>
>> How deep does the rabbit hole go?
>
> Actually now that I think of it, the Versions files don't need to be
> sorted; it is the abilist files that *must* be sorted.

No, they don't *need* to be sorted, it's just a convenience thing.

This list in particular is not well sorted.

Cheers,
Carlos.

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Consolidate object search in DSO in _dl_find_dso_for_object

Andreas Jaeger-8
In reply to this post by Carlos O'Donell-6
On 02/15/2013 07:33 PM, Carlos O'Donell wrote:

> On 02/15/2013 06:43 AM, Siddhesh Poyarekar wrote:
>> Hi,
>>
>> As Carlos, suggested, here's a patch to consolidate the object search
>> loop in various places in current code into a separate function.
>> There are only 3 places where this consolidation is needed (4 with
>> __cxa_thread_atexit_impl); the other two have a completely different
>> logic.
>
> Thanks for doing this clean up, I appreciate it.
>
>> Built and regression tested on x86_64.  OK to commit?
>>
>> Siddhesh
>>
>> * elf/Versions (ld): Add _dl_find_dso_for_object.
>> * elf/dl-addr.c (_dl_addr): Use _dl_find_dso_for_object.
>> * elf/dl-open.c (_dl_find_dso_for_object): New function.
>> (dl_open_worker): Use _dl_find_dso_for_object.
>> * elf/dl-sym.c (do_sym): Likewise.
>> * sysdeps/generic/ldsodefs.h: Declare _dl_find_dso_for_object.
>>
>> diff --git a/elf/Versions b/elf/Versions
>> index 87e27c5..97615de 100644
>> --- a/elf/Versions
>> +++ b/elf/Versions
>> @@ -62,5 +62,6 @@ ld {
>>       _dl_debug_state;
>>       # Pointer protection.
>>       __pointer_chk_guard;
>> +    _dl_find_dso_for_object;
>
> While Andreas says the list should be alphabetical it doesn't look like it is :-)
>
> I guess it would be a distinct patch to sort the list first?
>
> How deep does the rabbit hole go?


Indeed, we seem not to be consistent here. Files like dirent/Versions
are nicely sorted but others are not ;(.

Andreas
--
  Andreas Jaeger aj@{suse.com,opensuse.org} Twitter/Identica: jaegerandi
   SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
    GF: Jeff Hawn,Jennifer Guild,Felix Imendörffer,HRB16746 (AG Nürnberg)
     GPG fingerprint = 93A3 365E CE47 B889 DF7F  FED1 389A 563C C272 A126
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Consolidate object search in DSO in _dl_find_dso_for_object

Carlos O'Donell-2
On Fri, Feb 15, 2013 at 2:12 PM, Andreas Jaeger <[hidden email]> wrote:
> Indeed, we seem not to be consistent here. Files like dirent/Versions are
> nicely sorted but others are not ;(.

Add it to Concensus on the wiki?

Cheers,
Carlos.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Consolidate object search in DSO in _dl_find_dso_for_object

Andreas Jaeger-8
On 02/16/2013 03:46 AM, Carlos O'Donell wrote:
> On Fri, Feb 15, 2013 at 2:12 PM, Andreas Jaeger <[hidden email]> wrote:
>> Indeed, we seem not to be consistent here. Files like dirent/Versions are
>> nicely sorted but others are not ;(.
>
> Add it to Concensus on the wiki?

Carlos,

What exactly do you propose? Adding a note that sorting these files is
obvious?

Andreas
--
  Andreas Jaeger aj@{suse.com,opensuse.org} Twitter/Identica: jaegerandi
   SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
    GF: Jeff Hawn,Jennifer Guild,Felix Imendörffer,HRB16746 (AG Nürnberg)
     GPG fingerprint = 93A3 365E CE47 B889 DF7F  FED1 389A 563C C272 A126
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Consolidate object search in DSO in _dl_find_dso_for_object

Siddhesh Poyarekar-3
In reply to this post by Carlos O'Donell-6
On Fri, Feb 15, 2013 at 01:33:20PM -0500, Carlos O'Donell wrote:
> I'd say OK to checkin with:
> * Alphabetically sorted Version entries.
> * Verbose comment for utility function.
>

This is what I finally committed:


diff --git a/elf/Versions b/elf/Versions
index d6b5e50..2383992 100644
--- a/elf/Versions
+++ b/elf/Versions
@@ -51,8 +51,8 @@ ld {
     # Those are in the dynamic linker, but used by libc.so.
     __libc_enable_secure;
     _dl_allocate_tls; _dl_allocate_tls_init;
-    _dl_argv; _dl_get_tls_static_info; _dl_deallocate_tls;
-    _dl_make_stack_executable; _dl_out_of_memory;
+    _dl_argv; _dl_find_dso_for_object; _dl_get_tls_static_info;
+    _dl_deallocate_tls; _dl_make_stack_executable; _dl_out_of_memory;
     _dl_rtld_di_serinfo; _dl_starting_up; _dl_tls_setup;
     _rtld_global; _rtld_global_ro;
 
diff --git a/elf/dl-addr.c b/elf/dl-addr.c
index 91cc443..a533466 100644
--- a/elf/dl-addr.c
+++ b/elf/dl-addr.c
@@ -130,18 +130,14 @@ _dl_addr (const void *address, Dl_info *info,
   /* Protect against concurrent loads and unloads.  */
   __rtld_lock_lock_recursive (GL(dl_load_lock));
 
-  /* Find the highest-addressed object that ADDRESS is not below.  */
-  for (Lmid_t ns = 0; ns < GL(dl_nns); ++ns)
-    for (struct link_map *l = GL(dl_ns)[ns]._ns_loaded; l; l = l->l_next)
-      if (addr >= l->l_map_start && addr < l->l_map_end
-  && (l->l_contiguous || _dl_addr_inside_object (l, addr)))
- {
-  determine_info (addr, l, info, mapp, symbolp);
-  result = 1;
-  goto out;
- }
+  struct link_map *l = _dl_find_dso_for_object (addr);
+
+  if (l)
+    {
+      determine_info (addr, l, info, mapp, symbolp);
+      result = 1;
+    }
 
- out:
   __rtld_lock_unlock_recursive (GL(dl_load_lock));
 
   return result;
diff --git a/elf/dl-open.c b/elf/dl-open.c
index 67f7e73..201d95d 100644
--- a/elf/dl-open.c
+++ b/elf/dl-open.c
@@ -165,6 +165,29 @@ add_to_global (struct link_map *new)
   return 0;
 }
 
+/* Search link maps in all namespaces for the DSO that containes the object at
+   address ADDR.  Returns the pointer to the link map of the matching DSO, or
+   NULL if a match is not found.  */
+struct link_map *
+internal_function
+_dl_find_dso_for_object (const ElfW(Addr) addr)
+{
+  struct link_map *l;
+
+  /* Find the highest-addressed object that ADDR is not below.  */
+  for (Lmid_t ns = 0; ns < GL(dl_nns); ++ns)
+    for (l = GL(dl_ns)[ns]._ns_loaded; l != NULL; l = l->l_next)
+      if (addr >= l->l_map_start && addr < l->l_map_end
+  && (l->l_contiguous
+      || _dl_addr_inside_object (l, (ElfW(Addr)) addr)))
+ {
+  assert (ns == l->l_ns);
+  return l;
+ }
+  return NULL;
+}
+rtld_hidden_def (_dl_find_dso_for_object);
+
 static void
 dl_open_worker (void *a)
 {
@@ -194,20 +217,11 @@ dl_open_worker (void *a)
       call_map = GL(dl_ns)[LM_ID_BASE]._ns_loaded;
 #endif
 
-      struct link_map *l;
-      for (Lmid_t ns = 0; ns < GL(dl_nns); ++ns)
- for (l = GL(dl_ns)[ns]._ns_loaded; l != NULL; l = l->l_next)
-  if (caller_dlopen >= (const void *) l->l_map_start
-      && caller_dlopen < (const void *) l->l_map_end
-      && (l->l_contiguous
-  || _dl_addr_inside_object (l, (ElfW(Addr)) caller_dlopen)))
-    {
-      assert (ns == l->l_ns);
-      call_map = l;
-      goto found_caller;
-    }
+      struct link_map *l = _dl_find_dso_for_object ((ElfW(Addr)) caller_dlopen);
+
+      if (l)
+        call_map = l;
 
-    found_caller:
       if (args->nsid == __LM_ID_CALLER)
  {
 #ifndef SHARED
diff --git a/elf/dl-sym.c b/elf/dl-sym.c
index d2b4db7..05de6c1 100644
--- a/elf/dl-sym.c
+++ b/elf/dl-sym.c
@@ -91,20 +91,10 @@ do_sym (void *handle, const char *name, void *who,
   lookup_t result;
   ElfW(Addr) caller = (ElfW(Addr)) who;
 
+  struct link_map *l = _dl_find_dso_for_object (caller);
   /* If the address is not recognized the call comes from the main
      program (we hope).  */
-  struct link_map *match = GL(dl_ns)[LM_ID_BASE]._ns_loaded;
-
-  /* Find the highest-addressed object that CALLER is not below.  */
-  for (Lmid_t ns = 0; ns < GL(dl_nns); ++ns)
-    for (struct link_map *l = GL(dl_ns)[ns]._ns_loaded; l != NULL;
- l = l->l_next)
-      if (caller >= l->l_map_start && caller < l->l_map_end
-  && (l->l_contiguous || _dl_addr_inside_object (l, caller)))
- {
-  match = l;
-  break;
- }
+  struct link_map *match = l ? l : GL(dl_ns)[LM_ID_BASE]._ns_loaded;
 
   if (handle == RTLD_DEFAULT)
     {
diff --git a/sysdeps/generic/ldsodefs.h b/sysdeps/generic/ldsodefs.h
index d6350fa..01a2712 100644
--- a/sysdeps/generic/ldsodefs.h
+++ b/sysdeps/generic/ldsodefs.h
@@ -1006,6 +1006,10 @@ extern int _dl_addr_inside_object (struct link_map *l, const ElfW(Addr) addr)
 /* Show show of an object.  */
 extern void _dl_show_scope (struct link_map *new, int from);
 
+extern struct link_map *_dl_find_dso_for_object (const ElfW(Addr) addr)
+     internal_function;
+rtld_hidden_proto (_dl_find_dso_for_object)
+
 __END_DECLS
 
 #endif /* ldsodefs.h */
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Consolidate object search in DSO in _dl_find_dso_for_object

Carlos O'Donell-2
In reply to this post by Andreas Jaeger-8
On Sun, Feb 17, 2013 at 10:27 AM, Andreas Jaeger <[hidden email]> wrote:
>> Add it to Concensus on the wiki?
> What exactly do you propose? Adding a note that sorting these files is
> obvious?

Yes exactly. Allow anyone to checkin a fix to sort them.

Cheers,
Carlos.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Consolidate object search in DSO in _dl_find_dso_for_object

Carlos O'Donell-2
In reply to this post by Siddhesh Poyarekar-3
On Mon, Feb 18, 2013 at 7:34 AM, Siddhesh Poyarekar <[hidden email]> wrote:
> +/* Search link maps in all namespaces for the DSO that containes the object at
> +   address ADDR.  Returns the pointer to the link map of the matching DSO, or
> +   NULL if a match is not found.  */

Excellent text. Good job.

Cheers,
Carlos.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Consolidate object search in DSO in _dl_find_dso_for_object

Andreas Jaeger-8
In reply to this post by Carlos O'Donell-2
On Monday, February 18, 2013 10:03:56 Carlos O'Donell wrote:
> On Sun, Feb 17, 2013 at 10:27 AM, Andreas Jaeger <[hidden email]> wrote:
> >> Add it to Concensus on the wiki?
> >
> > What exactly do you propose? Adding a note that sorting these files
> > is obvious?
>
> Yes exactly. Allow anyone to checkin a fix to sort them.

OK, done,

Andreas
--
 Andreas Jaeger aj@{suse.com,opensuse.org} Twitter/Identica: jaegerandi
  SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
   GF: Jeff Hawn,Jennifer Guild,Felix Imendörffer,HRB16746 (AG Nürnberg)
    GPG fingerprint = 93A3 365E CE47 B889 DF7F  FED1 389A 563C C272 A126

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Consolidate object search in DSO in _dl_find_dso_for_object

Carlos O'Donell-2
On Tue, Feb 19, 2013 at 3:31 AM, Andreas Jaeger <[hidden email]> wrote:

> On Monday, February 18, 2013 10:03:56 Carlos O'Donell wrote:
>> On Sun, Feb 17, 2013 at 10:27 AM, Andreas Jaeger <[hidden email]> wrote:
>> >> Add it to Concensus on the wiki?
>> >
>> > What exactly do you propose? Adding a note that sorting these files
>> > is obvious?
>>
>> Yes exactly. Allow anyone to checkin a fix to sort them.
>
> OK, done,

Awesome! One more step towards Just Works (tm) :-)

Cheers,
Carlos.