Fwd: Solibs and objfile BFD ownership

[Paul Pluzhnikov]

Redirect to gdb-patches@


---------- Forwarded message ----------
From: Paul Pluzhnikov <ppluzhnikov@google.com>
Date: Mon, Aug 17, 2009 at 3:44 PM
Subject: Re: Solibs and objfile BFD ownership
To: Paul Pluzhnikov <ppluzhnikov@google.com>, tromey@redhat.com,
gdb@sourceware.org


On Tue, Aug 4, 2009 at 11:47 AM, Paul Pluzhnikov<ppluzhnikov@google.com> wrote:

> So committed. Thanks,

I seem to have hit a rough patch with my patches :-(

The BFD refcounting patch from 2009-08-04 causes GDB to crash when I attach
to a process with many solibs, then (while GDB is reading solib symbols)
change my mind about attaching and hit Control-C, then 'run'.

This is happening because in symbol_add_stub refcount may not be set:

  so->objfile = symbol_file_add_from_bfd (so->abfd, flags, sap, OBJF_SHARED);

---> QUIT could be executed deep inside symbol_file_add_from_bfd, and
---> bfd_userdata below is never set.

  p_refcount = xmalloc (sizeof (*p_refcount));
  *p_refcount = 2;  /* Both solib and objfile refer to this abfd.  */
  bfd_usrdata (so->abfd) = p_refcount;

Later, we re-enter symbol_add_stub, and this:

  ALL_OBJFILES (so->objfile)
    {
      if (strcmp (so->objfile->name, so->so_name) == 0)
        return;

re-connects the so with the objfile, but never sets the bfd_usrdata.

Later still (during execution of 'run'), we go through clear_solib and
objfile_purge_solibs, and the latter crashes trying to bfd_close the abfd
which has already been bfd_close()d by the former.

Here is a proposed patch. Tested on Linux/x86_64 with no new failures.

Thanks,
--
Paul Pluzhnikov

2009-08-17  Paul Pluzhnikov  <ppluzhnikov@google.com>

       * solib.c (set_ref_count): New function.
       (symbol_add_stub): Call it.
-------------- next part --------------
Index: solib.c
===================================================================
RCS file: /cvs/src/src/gdb/solib.c,v
retrieving revision 1.124
diff -u -p -u -r1.124 solib.c
--- solib.c	10 Aug 2009 22:09:22 -0000	1.124
+++ solib.c	17 Aug 2009 22:27:07 -0000
@@ -440,26 +440,53 @@ master_so_list (void)
   return so_list_head;
 }
 
+/* Set reference count on ABFD to COUNT.  */
+
+static void
+set_ref_count (struct bfd *abfd, int count)
+{
+  int *p_refcount = (int *) xmalloc (sizeof (*p_refcount));
+  *p_refcount = count;
+
+  gdb_assert (bfd_usrdata (abfd) == NULL);
+  bfd_usrdata (abfd) = p_refcount;
+}
+
 static void
 symbol_add_stub (struct so_list *so, int flags)
 {
   struct section_addr_info *sap;
-  int *p_refcount;
+  struct objfile *objfile;
 
   /* Have we already loaded this shared object?  */
-  ALL_OBJFILES (so->objfile)
+  ALL_OBJFILES (objfile)
     {
-      if (strcmp (so->objfile->name, so->so_name) == 0)
-	return;
+      if (strcmp (objfile->name, so->so_name) == 0)
+	{
+	  if (objfile != so->objfile)
+	    {
+	      /* This could happen when symbol_file_add_from_bfd
+		 below is interrupted.  */
+
+	      gdb_assert (so->objfile == NULL);
+	      gdb_assert (bfd_usrdata (so->abfd) == NULL);
+
+	      so->objfile = objfile;  /* Reconnect.  */
+
+	      /* Both solib and objfile refer to this abfd.  */
+	      set_ref_count (so->abfd, 2);
+	    }
+	  return;
+	}
     }
 
   sap = build_section_addr_info_from_section_table (so->sections,
                                                     so->sections_end);
 
   so->objfile = symbol_file_add_from_bfd (so->abfd, flags, sap, OBJF_SHARED);
-  p_refcount = xmalloc (sizeof (*p_refcount));
-  *p_refcount = 2;  /* Both solib and objfile refer to this abfd.  */
-  bfd_usrdata (so->abfd) = p_refcount;
+
+  /* Both solib and objfile refer to this abfd.  */
+  set_ref_count (so->abfd, 2);
 
   free_section_addr_info (sap);