[patch] solib do not add ldd if in libc

[patch] solib do not add ldd if in libc

[Aleksandar Ristovski]

[-- Attachment #1: Type: text/plain, Size: 532 bytes --]

Hello,

On systems (QNX) that have ldd residing in libc, 
svr4_current_sos will add libc twice. Once while walking the 
linkmap and second time from r_ldsomap.

This patch adds check if ldsolib has already been added and 
if so, prevents adding duplicate entry.


Tested on linux - i386, no regressions; however, it would be 
good if someone could test for regressions on Solaris.


Thanks,

-- 
Aleksandar Ristovski
QNX Software Systems



* solib-svr4.c (svr4_current_sos): Do not add dynamic linker 
to solist
if already added.


[-- Attachment #2: solib-svr4-ldsomap-20090707.diff --]
[-- Type: text/x-patch, Size: 899 bytes --]

Index: gdb/solib-svr4.c
===================================================================
RCS file: /cvs/src/src/gdb/solib-svr4.c,v
retrieving revision 1.104
diff -u -p -r1.104 solib-svr4.c
--- gdb/solib-svr4.c	2 Jul 2009 17:25:58 -0000	1.104
+++ gdb/solib-svr4.c	7 Jul 2009 16:04:04 -0000
@@ -1125,7 +1125,20 @@ svr4_current_sos (void)
 	 symbol information for the dynamic linker is quite crucial
 	 for skipping dynamic linker resolver code.  */
       if (lm == 0 && ldsomap == 0)
-	lm = ldsomap = solib_svr4_r_ldsomap (info);
+	{
+	  struct so_list *so = head;
+
+	  lm = ldsomap = solib_svr4_r_ldsomap (info);
+
+	  /* On some other systems, dynamic linker resides in libc.
+	     Make sure we do not add duplicated entry for it.  */
+	  while (so)
+	    {
+	      if (so->lm_info->lm_addr == lm)
+		lm = ldsomap = 0;
+	      break;
+	    }
+	}
 
       discard_cleanups (old_chain);
     }

Re: [patch] solib do not add ldd if in libc

[Aleksandar Ristovski]

[-- Attachment #1: Type: text/plain, Size: 622 bytes --]

Aleksandar Ristovski wrote:
> Hello,
> 
> On systems (QNX) that have ldd residing in libc, svr4_current_sos will 
> add libc twice. Once while walking the linkmap and second time from 
> r_ldsomap.
> 
> This patch adds check if ldsolib has already been added and if so, 
> prevents adding duplicate entry.
> 
> 
> Tested on linux - i386, no regressions; however, it would be good if 
> someone could test for regressions on Solaris.
> 

This time with the right patch.



-- 
Aleksandar Ristovski
QNX Software Systems


ChangeLog:
* solib-svr4.c (svr4_current_sos): Do not add dynamic linker 
to solist
if already added.


[-- Attachment #2: solib-svr4-ldsomap-20090707.diff --]
[-- Type: text/x-patch, Size: 931 bytes --]

Index: gdb/solib-svr4.c
===================================================================
RCS file: /cvs/src/src/gdb/solib-svr4.c,v
retrieving revision 1.104
diff -u -p -r1.104 solib-svr4.c
--- gdb/solib-svr4.c	2 Jul 2009 17:25:58 -0000	1.104
+++ gdb/solib-svr4.c	7 Jul 2009 17:41:49 -0000
@@ -1125,7 +1125,23 @@ svr4_current_sos (void)
 	 symbol information for the dynamic linker is quite crucial
 	 for skipping dynamic linker resolver code.  */
       if (lm == 0 && ldsomap == 0)
-	lm = ldsomap = solib_svr4_r_ldsomap (info);
+	{
+	  struct so_list *so = head;
+
+	  lm = ldsomap = solib_svr4_r_ldsomap (info);
+
+	  /* On some other systems, dynamic linker resides in libc.
+	     Make sure we do not add duplicated entry for it.  */
+	  while (so)
+	    {
+	      if (so->lm_info->lm_addr == lm)
+		{
+		  lm = ldsomap = 0;
+		  break;
+		}
+	      so = so->next;
+	    }
+	}
 
       discard_cleanups (old_chain);
     }

Re: [patch] solib do not add ldd if in libc

[Aleksandar Ristovski]

Pedro, I am copying your reply here since my reply to myself 
contains better while loop which I actually wanted you to 
see. (and I'm replying to you below).

> Aleksandar Ristovski wrote:
>> Hello,
>>
>> On systems (QNX) that have ldd residing in libc, svr4_current_sos will 
>> add libc twice. Once while walking the linkmap and second time from 
>> r_ldsomap.
>>
>> This patch adds check if ldsolib has already been added and if so, 
>> prevents adding duplicate entry.
>>
>>
>> Tested on linux - i386, no regressions; however, it would be good if 
>> someone could test for regressions on Solaris.
>>
> 
> This time with the right patch.
> 
> 
> 

Pedro Alves wrote:
 > On Tuesday 07 July 2009 17:11:21, Aleksandar Ristovski wrote:
 >
 > [This is not a formal review, just a quick note: ] did 
you post the
 > patch you intended to post?  That while loop looks wrong.
 >
 > I note that solib_svr4_r_ldsomap's describing comment 
seems to
 > indicate that a fix there would fit better with its design.
 >
 > /* Find the link map for the dynamic linker (if it is not 
in the
 >    normal list of loaded shared objects).  */
 >
 > static CORE_ADDR
 > solib_svr4_r_ldsomap (struct svr4_info *info)
 > {
 >

Our r_debug is version 2 and we do have r_ldsomap member. We 
set it to our dynamic linker which coincides with libc. My 
patch loops through the list of so-s and looks for already 
added object; if found, then resets ldsomap to prevent outer 
loop from doing another iteration.

I, unfortunately, can not verify that it doesn't break 
solaris... I think it doesn't, but I couldn't test it 
(AFAIK, solaris has dynamic linker in a separate shared 
library and so it should not be found in the so list, but I 
don't have Solaris to verify this).


Thanks,

-- 
Aleksandar Ristovski
QNX Software Systems

Re: [patch] solib do not add ldd if in libc

[Daniel Jacobowitz]

On Tue, Jul 21, 2009 at 11:11:47AM -0400, Aleksandar Ristovski wrote:
> I, unfortunately, can not verify that it doesn't break solaris... I
> think it doesn't, but I couldn't test it (AFAIK, solaris has dynamic
> linker in a separate shared library and so it should not be found in
> the so list, but I don't have Solaris to verify this).

You'll break Linux with prelinking; lm_addr is an offset, so on a
prelinked system everything ought to load with offset zero.

Also, by clearing both lm and ldsomap, you cause this check to run
over and over again instead of just once.  IIRC you can leave ldsomap
set.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [patch] solib do not add ldd if in libc

[Pedro Alves]

On Tuesday 07 July 2009 17:11:21, Aleksandar Ristovski wrote:
>        if (lm == 0 && ldsomap == 0)
> -       lm = ldsomap = solib_svr4_r_ldsomap (info);
> +       {
> +         struct so_list *so = head;
> +
> +         lm = ldsomap = solib_svr4_r_ldsomap (info);
> +
> +         /* On some other systems, dynamic linker resides in libc.
> +            Make sure we do not add duplicated entry for it.  */
> +         while (so)
> +           {
> +             if (so->lm_info->lm_addr == lm)
> +               lm = ldsomap = 0;
> +             break;
> +           }
> +       }

[This is not a formal review, just a quick note: ] did you post the
patch you intended to post?  That while loop looks wrong.

I note that solib_svr4_r_ldsomap's describing comment seems to
indicate that a fix there would fit better with its design.

/* Find the link map for the dynamic linker (if it is not in the
   normal list of loaded shared objects).  */

static CORE_ADDR
solib_svr4_r_ldsomap (struct svr4_info *info)
{

-- 
Pedro Alves