[RFC] Detect loops in the solib chain

[RFC] Detect loops in the solib chain

[Daniel Jacobowitz]

A MontaVista customer had a very interestingly corrupt core file -
there was a stray pointer in the list of loaded shared libraries.  But
it pointed to something which looked enough like a shared library
entry to get by, and the bad entry's l_next pointed back at the
corrupted entry that led to it.  So around and around we went, adding
the same two libraries to the list.  When the solib chain reached
about 2GB, GDB was killed.

The best I could think of was to detect cycles.  It's a linked list
that we're walking, so without cycles we're bounded by the amount of
memory in the debuggee; it's by no means foolproof, but this should
prevent more cases of wandering off into the woods than we do now.

Does this look OK?  Tested on x86_64-linux, no regressions.

-- 
Daniel Jacobowitz
CodeSourcery

2008-07-17  Daniel Jacobowitz  <dan@codesourcery.com>

	* solib-svr4.c (svr4_current_sos): Check for cycles in the list.

Index: solib-svr4.c
===================================================================
RCS file: /cvs/src/src/gdb/solib-svr4.c,v
retrieving revision 1.87
diff -u -p -r1.87 solib-svr4.c
--- solib-svr4.c	3 Jun 2008 12:59:37 -0000	1.87
+++ solib-svr4.c	17 Jul 2008 20:36:23 -0000
@@ -751,7 +751,8 @@ static struct so_list *
 svr4_current_sos (void)
 {
   CORE_ADDR lm;
-  struct so_list *head = 0;
+  int loop_flag = 0;
+  struct so_list *head = 0, *loop_so_list = NULL;
   struct so_list **link_ptr = &head;
   CORE_ADDR ldsomap = 0;
 
@@ -825,6 +826,13 @@ svr4_current_sos (void)
 	      new->next = 0;
 	      *link_ptr = new;
 	      link_ptr = &new->next;
+
+	      /* Advance loop_so_list at half speed.  */
+	      if (loop_so_list == NULL)
+		loop_so_list = new;
+	      else if (loop_flag)
+		loop_so_list = loop_so_list->next;
+	      loop_flag = !loop_flag;
 	    }
 	}
 
@@ -836,6 +844,13 @@ svr4_current_sos (void)
 	lm = ldsomap = solib_svr4_r_ldsomap ();
 
       discard_cleanups (old_chain);
+
+      /* Check for cycles in the list.  */
+      if (loop_so_list && loop_so_list->lm_info->lm_addr == lm)
+	{
+	  warning (_("Corrupt shared library list"));
+	  break;
+	}
     }
 
   if (head == NULL)

Re: [RFC] Detect loops in the solib chain

[Stan Shebs]

Daniel Jacobowitz wrote:
> A MontaVista customer had a very interestingly corrupt core file -
> there was a stray pointer in the list of loaded shared libraries.  But
> it pointed to something which looked enough like a shared library
> entry to get by, and the bad entry's l_next pointed back at the
> corrupted entry that led to it.  So around and around we went, adding
> the same two libraries to the list.  When the solib chain reached
> about 2GB, GDB was killed.
>
> The best I could think of was to detect cycles.  It's a linked list
> that we're walking, so without cycles we're bounded by the amount of
> memory in the debuggee; it's by no means foolproof, but this should
> prevent more cases of wandering off into the woods than we do now.
>
> Does this look OK?  Tested on x86_64-linux, no regressions.
>   
What, no test case? :-) Anyway, it looks OK to me.

(You could make a little script to scribble on the executable...)

Stan

Re: [RFC] Detect loops in the solib chain

[Thiago Jung Bauermann]

On Thu, 2008-07-17 at 16:57 -0400, Daniel Jacobowitz wrote:
> A MontaVista customer had a very interestingly corrupt core file -
> there was a stray pointer in the list of loaded shared libraries.  But
> it pointed to something which looked enough like a shared library
> entry to get by, and the bad entry's l_next pointed back at the
> corrupted entry that led to it.  So around and around we went, adding
> the same two libraries to the list.  When the solib chain reached
> about 2GB, GDB was killed.

For my own education: it looks to me that this customer won the lottery
two times here (one for pointing to a link map fake entry, and the other
for it to be circular). How often can this happen in practice?

Can we expect somone else to have this problem in this millenium? :-)
-- 
[]'s
Thiago Jung Bauermann
Software Engineer
IBM Linux Technology Center

Re: [RFC] Detect loops in the solib chain

[Paul Pluzhnikov]

On Thu, Jul 17, 2008 at 1:57 PM, Daniel Jacobowitz <drow@false.org> wrote:

> The best I could think of was to detect cycles.

This is a double-linked list. You could also check that

  lm->l_next == NULL || lm->l_next->l_prev == lm

-- 
Paul Pluzhnikov

Re: [RFC] Detect loops in the solib chain

[Daniel Jacobowitz]

On Thu, Jul 17, 2008 at 02:17:36PM -0700, Paul Pluzhnikov wrote:
> On Thu, Jul 17, 2008 at 1:57 PM, Daniel Jacobowitz <drow@false.org> wrote:
> 
> > The best I could think of was to detect cycles.
> 
> This is a double-linked list. You could also check that
> 
>   lm->l_next == NULL || lm->l_next->l_prev == lm

Hmm - since this is doubly linked, but not circularly, will this
prevent us ever getting into a cycle?  I hadn't thought of that.

This version seems to work too.

-- 
Daniel Jacobowitz
CodeSourcery

2008-07-17  Daniel Jacobowitz  <dan@codesourcery.com>

	* solib-svr4.c (LM_PREV): New function.
	(IGNORE_FIRST_LINK_MAP_ENTRY): Use it.
	(svr4_current_sos): Check for correct l_prev.

Index: solib-svr4.c
===================================================================
RCS file: /cvs/src/src/gdb/solib-svr4.c,v
retrieving revision 1.87
diff -u -p -r1.87 solib-svr4.c
--- solib-svr4.c	3 Jun 2008 12:59:37 -0000	1.87
+++ solib-svr4.c	17 Jul 2008 21:39:50 -0000
@@ -248,6 +248,15 @@ LM_NEXT (struct so_list *so)
 }
 
 static CORE_ADDR
+LM_PREV (struct so_list *so)
+{
+  struct link_map_offsets *lmo = svr4_fetch_link_map_offsets ();
+
+  return extract_typed_address (so->lm_info->lm + lmo->l_prev_offset,
+				builtin_type_void_data_ptr);
+}
+
+static CORE_ADDR
 LM_NAME (struct so_list *so)
 {
   struct link_map_offsets *lmo = svr4_fetch_link_map_offsets ();
@@ -259,15 +268,12 @@ LM_NAME (struct so_list *so)
 static int
 IGNORE_FIRST_LINK_MAP_ENTRY (struct so_list *so)
 {
-  struct link_map_offsets *lmo = svr4_fetch_link_map_offsets ();
-
   /* Assume that everything is a library if the dynamic loader was loaded
      late by a static executable.  */
   if (bfd_get_section_by_name (exec_bfd, ".dynamic") == NULL)
     return 0;
 
-  return extract_typed_address (so->lm_info->lm + lmo->l_prev_offset,
-				builtin_type_void_data_ptr) == 0;
+  return LM_PREV (so) == 0;
 }
 
 static CORE_ADDR debug_base;	/* Base of dynamic linker structures */
@@ -750,7 +756,7 @@ svr4_default_sos (void)
 static struct so_list *
 svr4_current_sos (void)
 {
-  CORE_ADDR lm;
+  CORE_ADDR lm, prev_lm;
   struct so_list *head = 0;
   struct so_list **link_ptr = &head;
   CORE_ADDR ldsomap = 0;
@@ -766,6 +772,7 @@ svr4_current_sos (void)
 
   /* Walk the inferior's link map list, and build our list of
      `struct so_list' nodes.  */
+  prev_lm = 0;
   lm = solib_svr4_r_map ();
 
   while (lm)
@@ -773,6 +780,7 @@ svr4_current_sos (void)
       struct link_map_offsets *lmo = svr4_fetch_link_map_offsets ();
       struct so_list *new = XZALLOC (struct so_list);
       struct cleanup *old_chain = make_cleanup (xfree, new);
+      CORE_ADDR next_lm;
 
       new->lm_info = xmalloc (sizeof (struct lm_info));
       make_cleanup (xfree, new->lm_info);
@@ -784,14 +792,21 @@ svr4_current_sos (void)
 
       read_memory (lm, new->lm_info->lm, lmo->link_map_size);
 
-      lm = LM_NEXT (new);
+      next_lm = LM_NEXT (new);
+
+      if (prev_lm != 0 && LM_PREV (new) != prev_lm && ldsomap == 0)
+	{
+	  warning (_("Corrupted shared library list"));
+	  free_so (new);
+	  next_lm = 0;
+	}
 
       /* For SVR4 versions, the first entry in the link map is for the
          inferior executable, so we must ignore it.  For some versions of
          SVR4, it has no name.  For others (Solaris 2.3 for example), it
          does have a name, so we can no longer use a missing name to
          decide when to ignore it. */
-      if (IGNORE_FIRST_LINK_MAP_ENTRY (new) && ldsomap == 0)
+      else if (IGNORE_FIRST_LINK_MAP_ENTRY (new) && ldsomap == 0)
 	{
 	  main_lm_addr = new->lm_info->lm_addr;
 	  free_so (new);
@@ -828,6 +843,9 @@ svr4_current_sos (void)
 	    }
 	}
 
+      prev_lm = lm;
+      lm = next_lm;
+
       /* On Solaris, the dynamic linker is not in the normal list of
 	 shared objects, so make sure we pick it up too.  Having
 	 symbol information for the dynamic linker is quite crucial

Re: [RFC] Detect loops in the solib chain

[Daniel Jacobowitz]

On Thu, Jul 17, 2008 at 06:14:59PM -0300, Thiago Jung Bauermann wrote:
> For my own education: it looks to me that this customer won the lottery
> two times here (one for pointing to a link map fake entry, and the other
> for it to be circular). How often can this happen in practice?

Beats me, but my guess is more often than you think - the value you
need for it to be circular is readily available.  For instance
something that shifts the pointer by a word will make us load l_prev
when we wanted l_next.

They definitely did lose the lottery twice though :-)

-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFC] Detect loops in the solib chain

[Paul Pluzhnikov]

On Thu, Jul 17, 2008 at 2:40 PM, Daniel Jacobowitz <drow@false.org> wrote:

> @@ -784,14 +792,21 @@ svr4_current_sos (void)
>
>       read_memory (lm, new->lm_info->lm, lmo->link_map_size);
>
> -      lm = LM_NEXT (new);
> +      next_lm = LM_NEXT (new);
> +
> +      if (prev_lm != 0 && LM_PREV (new) != prev_lm && ldsomap == 0)

I think you want just:

+      if (LM_PREV (new) != prev_lm)

First entry on the list should also be properly terminated, and
ldsomap has nothing to do with whether the list is corrupt or not.


-- 
Paul Pluzhnikov

Re: [RFC] Detect loops in the solib chain

[Daniel Jacobowitz]

On Thu, Jul 17, 2008 at 02:57:03PM -0700, Paul Pluzhnikov wrote:
> On Thu, Jul 17, 2008 at 2:40 PM, Daniel Jacobowitz <drow@false.org> wrote:
> 
> > @@ -784,14 +792,21 @@ svr4_current_sos (void)
> >
> >       read_memory (lm, new->lm_info->lm, lmo->link_map_size);
> >
> > -      lm = LM_NEXT (new);
> > +      next_lm = LM_NEXT (new);
> > +
> > +      if (prev_lm != 0 && LM_PREV (new) != prev_lm && ldsomap == 0)
> 
> I think you want just:
> 
> +      if (LM_PREV (new) != prev_lm)
> 
> First entry on the list should also be properly terminated, and
> ldsomap has nothing to do with whether the list is corrupt or not.

You're right about prev_lm, thanks.  The ldsomap check is necessary,
because that entry may not be on the list (see down below).

-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFC] Detect loops in the solib chain

[Kevin Buettner]

On Thu, 17 Jul 2008 18:09:59 -0400
Daniel Jacobowitz <drow@false.org> wrote:

> On Thu, Jul 17, 2008 at 02:57:03PM -0700, Paul Pluzhnikov wrote:
> > On Thu, Jul 17, 2008 at 2:40 PM, Daniel Jacobowitz <drow@false.org> wrote:
> > 
> > > @@ -784,14 +792,21 @@ svr4_current_sos (void)
> > >
> > >       read_memory (lm, new->lm_info->lm, lmo->link_map_size);
> > >
> > > -      lm = LM_NEXT (new);
> > > +      next_lm = LM_NEXT (new);
> > > +
> > > +      if (prev_lm != 0 && LM_PREV (new) != prev_lm && ldsomap == 0)
> > 
> > I think you want just:
> > 
> > +      if (LM_PREV (new) != prev_lm)
> > 
> > First entry on the list should also be properly terminated, and
> > ldsomap has nothing to do with whether the list is corrupt or not.
> 
> You're right about prev_lm, thanks.  The ldsomap check is necessary,
> because that entry may not be on the list (see down below).

As written, I agree that the ldsomap check is necessary.  Of course,
having this check means that as soon as ldsomap gets set, this
integrity check is effectively disabled.  I think that this will only
matter if the ldsomap entry ends up having a non-zero LM_NEXT.

It seems to me that the ldsomap check could be avoided if you were to
set prev_lm to 0 just after ldsomap gets set.  (It's been a while
since I've checked, but I'm guessing that in the case of a unattached
dynamic linker entry, you'd expect both LM_NEXT and LM_PREV for that
entry to be zero.)

Kevin

Re: [RFC] Detect loops in the solib chain

[Jan Kratochvil]

Hi,

this is a follow-up on my different implementation at:
	[patch] Fix deadlock on looped solib list
	http://sourceware.org/ml/gdb-patches/2010-04/msg00054.html

This thread started by Daniel Jacobowitz has died, pinging it.


On Fri, 18 Jul 2008 00:09:59 +0200, Daniel Jacobowitz wrote:
> On Thu, Jul 17, 2008 at 02:57:03PM -0700, Paul Pluzhnikov wrote:
> > I think you want just:
> > 
> > +      if (LM_PREV (new) != prev_lm)
> > 
> > First entry on the list should also be properly terminated, and
> > ldsomap has nothing to do with whether the list is corrupt or not.
> 
> You're right about prev_lm, thanks.  The ldsomap check is necessary,
> because that entry may not be on the list (see down below).

Done.


On Sat, 19 Jul 2008 01:02:10 +0200, Kevin Buettner wrote:
> It seems to me that the ldsomap check could be avoided if you were to
> set prev_lm to 0 just after ldsomap gets set.  (It's been a while
> since I've checked, but I'm guessing that in the case of a unattached
> dynamic linker entry, you'd expect both LM_NEXT and LM_PREV for that
> entry to be zero.)

Not done.  It has a risk of regression on Solaris, I do not have Solaris
easily available (OK, to boot a VM) and keeping the patch as is just does not
apply the new sanity check on the Solaris specific ld.so single entry list.


OK to check-in?  (included by name for the fnchange.lst entry :-) )

No regressions on {x86_64,x86_64-m32,i686}-fedora12-linux-gnu.


Thanks,
Jan


gdb/
2010-04-03  Daniel Jacobowitz  <dan@codesourcery.com>
	    Paul Pluzhnikov  <ppluzhnikov@google.com>
	    Jan Kratochvil  <jan.kratochvil@redhat.com>

	Fix deadlock on looped list of loaded shared objects.
	* solib-svr4.c (LM_PREV): New function.
	(IGNORE_FIRST_LINK_MAP_ENTRY): Use it.
	(svr4_current_sos): Check for correct l_prev.  New variable prev_lm.
	* config/djgpp/fnchange.lst: Add translation for solib-corrupted.exp.

gdb/testsuite/
2010-04-03  Jan Kratochvil  <jan.kratochvil@redhat.com>

	Fix deadlock on looped list of loaded shared objects.
	* gdb.base/solib-corrupted.exp: New.

--- a/gdb/solib-svr4.c
+++ b/gdb/solib-svr4.c
@@ -272,6 +272,16 @@ LM_NEXT (struct so_list *so)
 }
 
 static CORE_ADDR
+LM_PREV (struct so_list *so)
+{
+  struct link_map_offsets *lmo = svr4_fetch_link_map_offsets ();
+  struct type *ptr_type = builtin_type (target_gdbarch)->builtin_data_ptr;
+
+  return extract_typed_address (so->lm_info->lm + lmo->l_prev_offset,
+				ptr_type);
+}
+
+static CORE_ADDR
 LM_NAME (struct so_list *so)
 {
   struct link_map_offsets *lmo = svr4_fetch_link_map_offsets ();
@@ -284,16 +294,12 @@ LM_NAME (struct so_list *so)
 static int
 IGNORE_FIRST_LINK_MAP_ENTRY (struct so_list *so)
 {
-  struct link_map_offsets *lmo = svr4_fetch_link_map_offsets ();
-  struct type *ptr_type = builtin_type (target_gdbarch)->builtin_data_ptr;
-
   /* Assume that everything is a library if the dynamic loader was loaded
      late by a static executable.  */
   if (exec_bfd && bfd_get_section_by_name (exec_bfd, ".dynamic") == NULL)
     return 0;
 
-  return extract_typed_address (so->lm_info->lm + lmo->l_prev_offset,
-				ptr_type) == 0;
+  return LM_PREV (so) == 0;
 }
 
 /* Per pspace SVR4 specific data.  */
@@ -1101,7 +1107,7 @@ svr4_default_sos (void)
 static struct so_list *
 svr4_current_sos (void)
 {
-  CORE_ADDR lm;
+  CORE_ADDR lm, prev_lm;
   struct so_list *head = 0;
   struct so_list **link_ptr = &head;
   CORE_ADDR ldsomap = 0;
@@ -1120,6 +1126,7 @@ svr4_current_sos (void)
 
   /* Walk the inferior's link map list, and build our list of
      `struct so_list' nodes.  */
+  prev_lm = 0;
   lm = solib_svr4_r_map (info);
 
   while (lm)
@@ -1127,6 +1134,7 @@ svr4_current_sos (void)
       struct link_map_offsets *lmo = svr4_fetch_link_map_offsets ();
       struct so_list *new = XZALLOC (struct so_list);
       struct cleanup *old_chain = make_cleanup (xfree, new);
+      CORE_ADDR next_lm;
 
       new->lm_info = xmalloc (sizeof (struct lm_info));
       make_cleanup (xfree, new->lm_info);
@@ -1138,14 +1146,21 @@ svr4_current_sos (void)
 
       read_memory (lm, new->lm_info->lm, lmo->link_map_size);
 
-      lm = LM_NEXT (new);
+      next_lm = LM_NEXT (new);
+
+      if (LM_PREV (new) != prev_lm && ldsomap == 0)
+	{
+	  warning (_("Corrupted shared library list"));
+	  free_so (new);
+	  next_lm = 0;
+	}
 
       /* For SVR4 versions, the first entry in the link map is for the
          inferior executable, so we must ignore it.  For some versions of
          SVR4, it has no name.  For others (Solaris 2.3 for example), it
          does have a name, so we can no longer use a missing name to
          decide when to ignore it. */
-      if (IGNORE_FIRST_LINK_MAP_ENTRY (new) && ldsomap == 0)
+      else if (IGNORE_FIRST_LINK_MAP_ENTRY (new) && ldsomap == 0)
 	{
 	  info->main_lm_addr = new->lm_info->lm_addr;
 	  free_so (new);
@@ -1182,6 +1197,9 @@ svr4_current_sos (void)
 	    }
 	}
 
+      prev_lm = lm;
+      lm = next_lm;
+
       /* On Solaris, the dynamic linker is not in the normal list of
 	 shared objects, so make sure we pick it up too.  Having
 	 symbol information for the dynamic linker is quite crucial
--- a/gdb/config/djgpp/fnchange.lst
+++ b/gdb/config/djgpp/fnchange.lst
@@ -397,6 +397,7 @@
 @V@/gdb/testsuite/gdb.base/siginfo-obj.c @V@/gdb/testsuite/gdb.base/si-obj.c
 @V@/gdb/testsuite/gdb.base/siginfo-addr.exp @V@/gdb/testsuite/gdb.base/si-addr.exp
 @V@/gdb/testsuite/gdb.base/siginfo-obj.exp @V@/gdb/testsuite/gdb.base/si-obj.exp
+@V@/gdb/testsuite/gdb.base/solib-corrupted.exp @V@/gdb/testsuite/gdb.base/so-crptd.exp
 @V@/gdb/testsuite/gdb.base/solib-disc.c @V@/gdb/testsuite/gdb.base/so-disc.c
 @V@/gdb/testsuite/gdb.base/solib-display-lib.c @V@/gdb/testsuite/gdb.base/so-displib.c
 @V@/gdb/testsuite/gdb.base/solib-display-main.c @V@/gdb/testsuite/gdb.base/so-dispmain.c
--- /dev/null
+++ b/gdb/testsuite/gdb.base/solib-corrupted.exp
@@ -0,0 +1,45 @@
+# Copyright 2010 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+set testfile "solib-corrupted"
+set srcfile start.c
+
+if { [prepare_for_testing ${testfile}.exp ${testfile} ${srcfile}] } {
+    untested ${testfile}.exp
+    return -1
+}
+
+if ![runto_main] {
+    fail "Can't run to main"
+    return
+}
+
+gdb_test "info sharedlibrary" "" "normal list"
+
+# GDB checks there for matching L_PREV.
+set test "make solibs looping"
+gdb_test_multiple "p/x _r_debug->r_map->l_next = _r_debug->r_map" $test {
+    -re "(No symbol \"_r_debug\" in current context\\.|Attempt to extract a component of a value that is not a structure pointer\\.)\r\n$gdb_prompt $" {
+	# glibc debug info is not available and it is too difficult to find and
+	# parse it from this testcase without the gdb supporting functions.
+	xfail "$test (no _r_debug symbol)"
+	untested ${testfile}.exp
+	return
+    }
+    -re " = 0x\[0-9a-f\]+\r\n$gdb_prompt $" {
+	pass $test
+    }
+}
+gdb_test "info sharedlibrary" "warning: Corrupted shared library list\r\n.*" "corrupted list"

Re: [RFC] Detect loops in the solib chain

[Jan Kratochvil]

On Fri, 09 Apr 2010 17:41:23 +0200, Jan Kratochvil wrote:
> On Sat, 19 Jul 2008 01:02:10 +0200, Kevin Buettner wrote:
> > It seems to me that the ldsomap check could be avoided if you were to
> > set prev_lm to 0 just after ldsomap gets set.  (It's been a while
> > since I've checked, but I'm guessing that in the case of a unattached
> > dynamic linker entry, you'd expect both LM_NEXT and LM_PREV for that
> > entry to be zero.)
> 
> Not done.  It has a risk of regression on Solaris, I do not have Solaris
> easily available (OK, to boot a VM) and keeping the patch as is just does not
> apply the new sanity check on the Solaris specific ld.so single entry list.

Done this part this time.  It works during basic test but it is not regression
tested on Solaris as system expect + custom dejagnu produce \r\r\n EOLs
breaking the testsuite completely.


Thanks,
Jan


gdb/
2010-04-09  Daniel Jacobowitz  <dan@codesourcery.com>
	    Paul Pluzhnikov  <ppluzhnikov@google.com>
	    Jan Kratochvil  <jan.kratochvil@redhat.com>

	Fix deadlock on looped list of loaded shared objects.
	* solib-svr4.c (LM_PREV): New function.
	(IGNORE_FIRST_LINK_MAP_ENTRY): Use it.
	(svr4_current_sos): Check for correct l_prev.  New variables prev_lm
	and next_lm.  Clear prev_lm for solib_svr4_r_ldsomap.
	* config/djgpp/fnchange.lst: Add translation for solib-corrupted.exp.

gdb/testsuite/
2010-04-09  Jan Kratochvil  <jan.kratochvil@redhat.com>

	Fix deadlock on looped list of loaded shared objects.
	* gdb.base/solib-corrupted.exp: New.

--- a/gdb/config/djgpp/fnchange.lst
+++ b/gdb/config/djgpp/fnchange.lst
@@ -397,6 +397,7 @@
 @V@/gdb/testsuite/gdb.base/siginfo-obj.c @V@/gdb/testsuite/gdb.base/si-obj.c
 @V@/gdb/testsuite/gdb.base/siginfo-addr.exp @V@/gdb/testsuite/gdb.base/si-addr.exp
 @V@/gdb/testsuite/gdb.base/siginfo-obj.exp @V@/gdb/testsuite/gdb.base/si-obj.exp
+@V@/gdb/testsuite/gdb.base/solib-corrupted.exp @V@/gdb/testsuite/gdb.base/so-crptd.exp
 @V@/gdb/testsuite/gdb.base/solib-disc.c @V@/gdb/testsuite/gdb.base/so-disc.c
 @V@/gdb/testsuite/gdb.base/solib-display-lib.c @V@/gdb/testsuite/gdb.base/so-displib.c
 @V@/gdb/testsuite/gdb.base/solib-display-main.c @V@/gdb/testsuite/gdb.base/so-dispmain.c
--- a/gdb/solib-svr4.c
+++ b/gdb/solib-svr4.c
@@ -272,6 +272,16 @@ LM_NEXT (struct so_list *so)
 }
 
 static CORE_ADDR
+LM_PREV (struct so_list *so)
+{
+  struct link_map_offsets *lmo = svr4_fetch_link_map_offsets ();
+  struct type *ptr_type = builtin_type (target_gdbarch)->builtin_data_ptr;
+
+  return extract_typed_address (so->lm_info->lm + lmo->l_prev_offset,
+				ptr_type);
+}
+
+static CORE_ADDR
 LM_NAME (struct so_list *so)
 {
   struct link_map_offsets *lmo = svr4_fetch_link_map_offsets ();
@@ -284,16 +294,12 @@ LM_NAME (struct so_list *so)
 static int
 IGNORE_FIRST_LINK_MAP_ENTRY (struct so_list *so)
 {
-  struct link_map_offsets *lmo = svr4_fetch_link_map_offsets ();
-  struct type *ptr_type = builtin_type (target_gdbarch)->builtin_data_ptr;
-
   /* Assume that everything is a library if the dynamic loader was loaded
      late by a static executable.  */
   if (exec_bfd && bfd_get_section_by_name (exec_bfd, ".dynamic") == NULL)
     return 0;
 
-  return extract_typed_address (so->lm_info->lm + lmo->l_prev_offset,
-				ptr_type) == 0;
+  return LM_PREV (so) == 0;
 }
 
 /* Per pspace SVR4 specific data.  */
@@ -1101,7 +1107,7 @@ svr4_default_sos (void)
 static struct so_list *
 svr4_current_sos (void)
 {
-  CORE_ADDR lm;
+  CORE_ADDR lm, prev_lm;
   struct so_list *head = 0;
   struct so_list **link_ptr = &head;
   CORE_ADDR ldsomap = 0;
@@ -1120,6 +1126,7 @@ svr4_current_sos (void)
 
   /* Walk the inferior's link map list, and build our list of
      `struct so_list' nodes.  */
+  prev_lm = 0;
   lm = solib_svr4_r_map (info);
 
   while (lm)
@@ -1127,6 +1134,7 @@ svr4_current_sos (void)
       struct link_map_offsets *lmo = svr4_fetch_link_map_offsets ();
       struct so_list *new = XZALLOC (struct so_list);
       struct cleanup *old_chain = make_cleanup (xfree, new);
+      CORE_ADDR next_lm;
 
       new->lm_info = xmalloc (sizeof (struct lm_info));
       make_cleanup (xfree, new->lm_info);
@@ -1138,14 +1146,21 @@ svr4_current_sos (void)
 
       read_memory (lm, new->lm_info->lm, lmo->link_map_size);
 
-      lm = LM_NEXT (new);
+      next_lm = LM_NEXT (new);
+
+      if (LM_PREV (new) != prev_lm)
+	{
+	  warning (_("Corrupted shared library list"));
+	  free_so (new);
+	  next_lm = 0;
+	}
 
       /* For SVR4 versions, the first entry in the link map is for the
          inferior executable, so we must ignore it.  For some versions of
          SVR4, it has no name.  For others (Solaris 2.3 for example), it
          does have a name, so we can no longer use a missing name to
          decide when to ignore it. */
-      if (IGNORE_FIRST_LINK_MAP_ENTRY (new) && ldsomap == 0)
+      else if (IGNORE_FIRST_LINK_MAP_ENTRY (new) && ldsomap == 0)
 	{
 	  info->main_lm_addr = new->lm_info->lm_addr;
 	  free_so (new);
@@ -1182,12 +1197,18 @@ svr4_current_sos (void)
 	    }
 	}
 
+      prev_lm = lm;
+      lm = next_lm;
+
       /* On Solaris, the dynamic linker is not in the normal list of
 	 shared objects, so make sure we pick it up too.  Having
 	 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);
+	{
+	  lm = ldsomap = solib_svr4_r_ldsomap (info);
+	  prev_lm = 0;
+	}
 
       discard_cleanups (old_chain);
     }
--- /dev/null
+++ b/gdb/testsuite/gdb.base/solib-corrupted.exp
@@ -0,0 +1,45 @@
+# Copyright 2010 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+set testfile "solib-corrupted"
+set srcfile start.c
+
+if { [prepare_for_testing ${testfile}.exp ${testfile} ${srcfile}] } {
+    untested ${testfile}.exp
+    return -1
+}
+
+if ![runto_main] {
+    fail "Can't run to main"
+    return
+}
+
+gdb_test "info sharedlibrary" "" "normal list"
+
+# GDB checks there for matching L_PREV.
+set test "make solibs looping"
+gdb_test_multiple "p/x _r_debug->r_map->l_next = _r_debug->r_map" $test {
+    -re "(No symbol \"_r_debug\" in current context\\.|Attempt to extract a component of a value that is not a structure pointer\\.)\r\n$gdb_prompt $" {
+	# glibc debug info is not available and it is too difficult to find and
+	# parse it from this testcase without the gdb supporting functions.
+	xfail "$test (no _r_debug symbol)"
+	untested ${testfile}.exp
+	return
+    }
+    -re " = 0x\[0-9a-f\]+\r\n$gdb_prompt $" {
+	pass $test
+    }
+}
+gdb_test "info sharedlibrary" "warning: Corrupted shared library list\r\n.*" "corrupted list"

Re: [RFC] Detect loops in the solib chain

[Kevin Buettner]

On Fri, 9 Apr 2010 23:02:55 +0200
Jan Kratochvil <jan.kratochvil@redhat.com> wrote:

> 	Fix deadlock on looped list of loaded shared objects.
> 	* solib-svr4.c (LM_PREV): New function.
> 	(IGNORE_FIRST_LINK_MAP_ENTRY): Use it.
> 	(svr4_current_sos): Check for correct l_prev.  New variables prev_lm
> 	and next_lm.  Clear prev_lm for solib_svr4_r_ldsomap.
> 	* config/djgpp/fnchange.lst: Add translation for solib-corrupted.exp.

I reviewed this part of the patch.  It looks okay to me.

Kevin

Re: [RFC] Detect loops in the solib chain

[Pedro Alves]

On Friday 09 April 2010 22:02:55, Jan Kratochvil wrote:
> Done this part this time.  It works during basic test but it is not regression
> tested on Solaris as system expect + custom dejagnu produce \r\r\n EOLs
> breaking the testsuite completely.

Was that OpenSolaris?  I've seen that before too.  Here
is the workaround I was using a while ago:

 http://sourceware.org/ml/gdb/2008-08/msg00144.html

I don't really know what exactly causes this.
I never saw that problem on Solaris (non-Open), but then
again, there we're using a built from source dejagnu, I think.

-- 
Pedro Alves

OpenSolaris dejagnu workaround [Re: [RFC] Detect loops in the solib chain]

[Jan Kratochvil]

On Tue, 13 Apr 2010 01:07:34 +0200, Pedro Alves wrote:
> On Friday 09 April 2010 22:02:55, Jan Kratochvil wrote:
> > Done this part this time.  It works during basic test but it is not regression
> > tested on Solaris as system expect + custom dejagnu produce \r\r\n EOLs
> > breaking the testsuite completely.
> 
> Was that OpenSolaris?  I've seen that before too.

Yes, filed
http://bugs.opensolaris.org/bugdatabase/view_bug.do?bug_id=6942789


> Here is the workaround I was using a while ago:
> 
>  http://sourceware.org/ml/gdb/2008-08/msg00144.html

It works, thanks.


> I never saw that problem on Solaris (non-Open), but then
> again, there we're using a built from source dejagnu, I think.

dejagnu must be from source, haven't found it packaged in OpenSolaris.
expect is there some Sun packaged one but it has the same problem as the GNU
one rebuilt there from sources.


Regards,
Jan

Re: [RFC] Detect loops in the solib chain

[Tom Tromey]

>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:

Jan> 2010-04-09  Daniel Jacobowitz  <dan@codesourcery.com>
Jan> 	    Paul Pluzhnikov  <ppluzhnikov@google.com>
Jan> 	    Jan Kratochvil  <jan.kratochvil@redhat.com>
Jan> 	Fix deadlock on looped list of loaded shared objects.
Jan> 	* solib-svr4.c (LM_PREV): New function.
Jan> 	(IGNORE_FIRST_LINK_MAP_ENTRY): Use it.
Jan> 	(svr4_current_sos): Check for correct l_prev.  New variables prev_lm
Jan> 	and next_lm.  Clear prev_lm for solib_svr4_r_ldsomap.
Jan> 	* config/djgpp/fnchange.lst: Add translation for solib-corrupted.exp.

Kevin already ok'd this part.
(FWIW I agree it is ok.)

Jan> 2010-04-09  Jan Kratochvil  <jan.kratochvil@redhat.com>
Jan> 	Fix deadlock on looped list of loaded shared objects.
Jan> 	* gdb.base/solib-corrupted.exp: New.

Jan> +	# glibc debug info is not available and it is too difficult to find and
Jan> +	# parse it from this testcase without the gdb supporting functions.
Jan> +	xfail "$test (no _r_debug symbol)"
Jan> +	untested ${testfile}.exp
Jan> +	return
Jan> +    }
Jan> +    -re " = 0x\[0-9a-f\]+\r\n$gdb_prompt $" {
Jan> +	pass $test

I think it is preferable to have the test name the same, regardless of
whether it passes or xfails.  Extra info can be logged with verbose.

This is ok with that change.

thanks,
Tom

Re: [RFC] Detect loops in the solib chain

[Jan Kratochvil]

On Fri, 23 Apr 2010 22:09:43 +0200, Tom Tromey wrote:
> >>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:
> Jan> +	# glibc debug info is not available and it is too difficult to find and
> Jan> +	# parse it from this testcase without the gdb supporting functions.
> Jan> +	xfail "$test (no _r_debug symbol)"
> Jan> +	untested ${testfile}.exp
> Jan> +	return
> Jan> +    }
> Jan> +    -re " = 0x\[0-9a-f\]+\r\n$gdb_prompt $" {
> Jan> +	pass $test
> 
> I think it is preferable to have the test name the same, regardless of
> whether it passes or xfails.  Extra info can be logged with verbose.

Changed.  Although there are precedent cases of this, still they are therefore
probably not considered as the preferred ones.  Such as:
	gdb.base/corefile.exp
	set test "accessing mmapped data in core file"
		pass "$test"
		fail "$test (mapping failed at runtime)"
		fail "$test (mapping address not found in core file)"


> This is ok with that change.

Checked in.


Thanks,
Jan


http://sourceware.org/ml/gdb-cvs/2010-04/msg00240.html

--- src/gdb/ChangeLog	2010/04/23 18:09:16	1.11678
+++ src/gdb/ChangeLog	2010/04/23 21:44:19	1.11679
@@ -1,3 +1,14 @@
+2010-04-23  Daniel Jacobowitz  <dan@codesourcery.com>
+	    Paul Pluzhnikov  <ppluzhnikov@google.com>
+	    Jan Kratochvil  <jan.kratochvil@redhat.com>
+
+	Fix deadlock on looped list of loaded shared objects.
+	* solib-svr4.c (LM_PREV): New function.
+	(IGNORE_FIRST_LINK_MAP_ENTRY): Use it.
+	(svr4_current_sos): Check for correct l_prev.  New variables prev_lm
+	and next_lm.  Clear prev_lm for solib_svr4_r_ldsomap.
+	* config/djgpp/fnchange.lst: Add translation for solib-corrupted.exp.
+
 2010-04-23  Doug Evans  <dje@google.com>
 
 	* configure.ac (CONFIG_SRCS): Add py-auto-load.o even if not using
--- src/gdb/solib-svr4.c	2010/03/11 22:07:02	1.130
+++ src/gdb/solib-svr4.c	2010/04/23 21:44:19	1.131
@@ -272,6 +272,16 @@
 }
 
 static CORE_ADDR
+LM_PREV (struct so_list *so)
+{
+  struct link_map_offsets *lmo = svr4_fetch_link_map_offsets ();
+  struct type *ptr_type = builtin_type (target_gdbarch)->builtin_data_ptr;
+
+  return extract_typed_address (so->lm_info->lm + lmo->l_prev_offset,
+				ptr_type);
+}
+
+static CORE_ADDR
 LM_NAME (struct so_list *so)
 {
   struct link_map_offsets *lmo = svr4_fetch_link_map_offsets ();
@@ -284,16 +294,12 @@
 static int
 IGNORE_FIRST_LINK_MAP_ENTRY (struct so_list *so)
 {
-  struct link_map_offsets *lmo = svr4_fetch_link_map_offsets ();
-  struct type *ptr_type = builtin_type (target_gdbarch)->builtin_data_ptr;
-
   /* Assume that everything is a library if the dynamic loader was loaded
      late by a static executable.  */
   if (exec_bfd && bfd_get_section_by_name (exec_bfd, ".dynamic") == NULL)
     return 0;
 
-  return extract_typed_address (so->lm_info->lm + lmo->l_prev_offset,
-				ptr_type) == 0;
+  return LM_PREV (so) == 0;
 }
 
 /* Per pspace SVR4 specific data.  */
@@ -1101,7 +1107,7 @@
 static struct so_list *
 svr4_current_sos (void)
 {
-  CORE_ADDR lm;
+  CORE_ADDR lm, prev_lm;
   struct so_list *head = 0;
   struct so_list **link_ptr = &head;
   CORE_ADDR ldsomap = 0;
@@ -1120,6 +1126,7 @@
 
   /* Walk the inferior's link map list, and build our list of
      `struct so_list' nodes.  */
+  prev_lm = 0;
   lm = solib_svr4_r_map (info);
 
   while (lm)
@@ -1127,6 +1134,7 @@
       struct link_map_offsets *lmo = svr4_fetch_link_map_offsets ();
       struct so_list *new = XZALLOC (struct so_list);
       struct cleanup *old_chain = make_cleanup (xfree, new);
+      CORE_ADDR next_lm;
 
       new->lm_info = xmalloc (sizeof (struct lm_info));
       make_cleanup (xfree, new->lm_info);
@@ -1138,14 +1146,21 @@
 
       read_memory (lm, new->lm_info->lm, lmo->link_map_size);
 
-      lm = LM_NEXT (new);
+      next_lm = LM_NEXT (new);
+
+      if (LM_PREV (new) != prev_lm)
+	{
+	  warning (_("Corrupted shared library list"));
+	  free_so (new);
+	  next_lm = 0;
+	}
 
       /* For SVR4 versions, the first entry in the link map is for the
          inferior executable, so we must ignore it.  For some versions of
          SVR4, it has no name.  For others (Solaris 2.3 for example), it
          does have a name, so we can no longer use a missing name to
          decide when to ignore it. */
-      if (IGNORE_FIRST_LINK_MAP_ENTRY (new) && ldsomap == 0)
+      else if (IGNORE_FIRST_LINK_MAP_ENTRY (new) && ldsomap == 0)
 	{
 	  info->main_lm_addr = new->lm_info->lm_addr;
 	  free_so (new);
@@ -1182,12 +1197,18 @@
 	    }
 	}
 
+      prev_lm = lm;
+      lm = next_lm;
+
       /* On Solaris, the dynamic linker is not in the normal list of
 	 shared objects, so make sure we pick it up too.  Having
 	 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);
+	{
+	  lm = ldsomap = solib_svr4_r_ldsomap (info);
+	  prev_lm = 0;
+	}
 
       discard_cleanups (old_chain);
     }
--- src/gdb/config/djgpp/fnchange.lst	2010/04/09 15:15:05	1.112
+++ src/gdb/config/djgpp/fnchange.lst	2010/04/23 21:44:19	1.113
@@ -397,6 +397,7 @@
 @V@/gdb/testsuite/gdb.base/siginfo-obj.c @V@/gdb/testsuite/gdb.base/si-obj.c
 @V@/gdb/testsuite/gdb.base/siginfo-addr.exp @V@/gdb/testsuite/gdb.base/si-addr.exp
 @V@/gdb/testsuite/gdb.base/siginfo-obj.exp @V@/gdb/testsuite/gdb.base/si-obj.exp
+@V@/gdb/testsuite/gdb.base/solib-corrupted.exp @V@/gdb/testsuite/gdb.base/so-crptd.exp
 @V@/gdb/testsuite/gdb.base/solib-disc.c @V@/gdb/testsuite/gdb.base/so-disc.c
 @V@/gdb/testsuite/gdb.base/solib-display-lib.c @V@/gdb/testsuite/gdb.base/so-displib.c
 @V@/gdb/testsuite/gdb.base/solib-display-main.c @V@/gdb/testsuite/gdb.base/so-dispmain.c
--- src/gdb/testsuite/ChangeLog	2010/04/23 18:03:31	1.2252
+++ src/gdb/testsuite/ChangeLog	2010/04/23 21:44:20	1.2253
@@ -1,3 +1,8 @@
+2010-04-23  Jan Kratochvil  <jan.kratochvil@redhat.com>
+
+	Fix deadlock on looped list of loaded shared objects.
+	* gdb.base/solib-corrupted.exp: New.
+
 2010-04-23  Doug Evans  <dje@google.com>
 
 	* gdb.python/py-section-script.c: New file.
--- src/gdb/testsuite/gdb.base/solib-corrupted.exp
+++ src/gdb/testsuite/gdb.base/solib-corrupted.exp	2010-04-23 21:44:35.962406000 +0000
@@ -0,0 +1,46 @@
+# Copyright 2010 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+set testfile "solib-corrupted"
+set srcfile start.c
+
+if { [prepare_for_testing ${testfile}.exp ${testfile} ${srcfile}] } {
+    untested ${testfile}.exp
+    return -1
+}
+
+if ![runto_main] {
+    fail "Can't run to main"
+    return
+}
+
+gdb_test "info sharedlibrary" "" "normal list"
+
+# GDB checks there for matching L_PREV.
+set test "make solibs looping"
+gdb_test_multiple "p/x _r_debug->r_map->l_next = _r_debug->r_map" $test {
+    -re "(No symbol \"_r_debug\" in current context\\.|Attempt to extract a component of a value that is not a structure pointer\\.)\r\n$gdb_prompt $" {
+	# glibc debug info is not available and it is too difficult to find and
+	# parse it from this testcase without the gdb supporting functions.
+	verbose -log "no _r_debug symbol has been found"
+	xfail $test
+	untested ${testfile}.exp
+	return
+    }
+    -re " = 0x\[0-9a-f\]+\r\n$gdb_prompt $" {
+	pass $test
+    }
+}
+gdb_test "info sharedlibrary" "warning: Corrupted shared library list\r\n.*" "corrupted list"

Re: [RFC] Detect loops in the solib chain

[Ulrich Weigand]

Jan Kratochvil wrote:

> +2010-04-23  Jan Kratochvil  <jan.kratochvil@redhat.com>
> +
> +	Fix deadlock on looped list of loaded shared objects.
> +	* gdb.base/solib-corrupted.exp: New.
> +

Like all solib test cases, this should be skipped on platforms
that do not support shared libraries.

Tested on spu-elf, committed to mainline.

Bye,
Ulrich

ChangeLog:

	* gdb.base/solib-corrupted.exp: Respect skip_shlib_tests.

Index: gdb/testsuite/gdb.base/solib-corrupted.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/solib-corrupted.exp,v
retrieving revision 1.2
diff -u -p -r1.2 solib-corrupted.exp
--- gdb/testsuite/gdb.base/solib-corrupted.exp	1 Jun 2010 21:29:21 -0000	1.2
+++ gdb/testsuite/gdb.base/solib-corrupted.exp	11 Jun 2010 16:43:07 -0000
@@ -13,6 +13,10 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
+if {[skip_shlib_tests]} {
+    return 0
+}
+
 set testfile "solib-corrupted"
 set srcfile start.c
 


-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: [RFC] Detect loops in the solib chain

[Jan Kratochvil]

On Fri, 11 Jun 2010 19:39:11 +0200, Ulrich Weigand wrote:
> Like all solib test cases, this should be skipped on platforms
> that do not support shared libraries.

Aware of it but I somehow missed it in this case, thanks.


Sorry,
Jan