gdb crash: double free with free_objfile

gdb crash: double free with free_objfile

[Mathieu Lacage]

Hi,

I have been trying to find a fix to avoid a gdb crash but I am not
sure which direction to follow so, hints would be welcome.

1) the problem

My inferior reports to gdb a linkmap which contains two entries with
the same name (they have different start and end addresses). gdb sees
both entries, creates corresponding duplicate entries in his copy of
the inferior's and then initializes the objfile field of the two
so_list entries to point to the same struct objfile. Later on, when
these entries disappear from the inferior's linkmap, gdb attempts to
call free_objfile twice on this shared struct objfile from
update_solib_list (solib.c).

2) what could be done

I assume that gdb should be able to deal with this case without
crashing so, I looked at 2 options:
  a. make the duplicate entries point to private duplicates of struct objfile *
  b. make the duplicate entries point to a shared struct objfile and
add refcounting or some means to detect when the struct objfile is
shared.

I could implement a. by removing the for loop at the top of
symbol_add_stub but I suspect that there is a good reason for its
presence and that removing it would create some waste of memory in
other cases so, it's probably not a good idea to do this.

I am not totally sure that b. is correct to do. i.e., I _think_ (but I
am not sure) that the content of struct objfile is not dependent upon
the base address of the corresponding binary so I _think_ that it
should be safe to reuse the same one between two so_list entries
loaded at different base addresses. Am I wrong ? If so, my only option
is a but I the proposed 'fix' described above is probably not good. Is
there anyone with a better suggestion ?

Mathieu

--
Mathieu Lacage <mathieu.lacage@gmail.com>

Re: gdb crash: double free with free_objfile

[Tom Tromey]

>>>>> "Mathieu" == Mathieu Lacage <mathieu.lacage@gmail.com> writes:

Mathieu> a. make the duplicate entries point to private duplicates of
Mathieu> struct objfile *

Mathieu> b. make the duplicate entries point to a shared struct objfile and
Mathieu> add refcounting or some means to detect when the struct objfile is
Mathieu> shared.

Mathieu> I could implement a. by removing the for loop at the top of
Mathieu> symbol_add_stub but I suspect that there is a good reason for its
Mathieu> presence and that removing it would create some waste of memory in
Mathieu> other cases so, it's probably not a good idea to do this.

Mathieu> I am not totally sure that b. is correct to do. i.e., I _think_ (but I
Mathieu> am not sure) that the content of struct objfile is not dependent upon
Mathieu> the base address of the corresponding binary so I _think_ that it
Mathieu> should be safe to reuse the same one between two so_list entries
Mathieu> loaded at different base addresses. Am I wrong ? If so, my only option
Mathieu> is a but I the proposed 'fix' described above is probably not good. Is
Mathieu> there anyone with a better suggestion ?

Nope, right now objfile has the base address baked in to many things,
e.g. symbol addresses or psymtabs_addrmap.  I think we'd like to change
this, so that we can share objfiles between inferiors, but nobody has
tried to do that yet.  I don't know how hard this would be.

Your quickest route is probably a form of (a) -- change symbol_add_stub
to check both the file name and the base address.

Tom

Re: gdb crash: double free with free_objfile

[Mathieu Lacage]

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

hi,

On Fri, Feb 5, 2010 at 5:39 PM, Tom Tromey <tromey@redhat.com> wrote:

> Nope, right now objfile has the base address baked in to many things,
> e.g. symbol addresses or psymtabs_addrmap.  I think we'd like to change
> this, so that we can share objfiles between inferiors, but nobody has
> tried to do that yet.  I don't know how hard this would be.
>
> Your quickest route is probably a form of (a) -- change symbol_add_stub
> to check both the file name and the base address.

It took me a long time to get back to this issue because I was busy
with a couple of other things but the attached patch which was
generated against CVS HEAD fixes the problem for me (i.e., avoids the
segfault in gdb and gets me readable backtraces). Is there something
else I could do to help get this issue fixed in mainline ?

Mathieu
-- 
Mathieu Lacage <mathieu.lacage@gmail.com>

[-- Attachment #2: solib.patch --]
[-- Type: application/octet-stream, Size: 1335 bytes --]

? objdir
? solib.patch
Index: gdb/objfiles.h
===================================================================
RCS file: /cvs/src/src/gdb/objfiles.h,v
retrieving revision 1.72
diff -u -r1.72 objfiles.h
--- gdb/objfiles.h	14 Apr 2010 17:26:11 -0000	1.72
+++ gdb/objfiles.h	23 Jun 2010 15:14:16 -0000
@@ -193,6 +193,8 @@
 
     char *name;
 
+    CORE_ADDR addr_low;
+
     /* Some flag bits for this objfile. */
 
     unsigned short flags;
Index: gdb/solib.c
===================================================================
RCS file: /cvs/src/src/gdb/solib.c,v
retrieving revision 1.140
diff -u -r1.140 solib.c
--- gdb/solib.c	16 May 2010 23:49:58 -0000	1.140
+++ gdb/solib.c	23 Jun 2010 15:14:16 -0000
@@ -634,11 +634,11 @@
       TRY_CATCH (e, RETURN_MASK_ERROR)
 	{
 	  struct section_addr_info *sap;
-
 	  /* Have we already loaded this shared object?  */
 	  ALL_OBJFILES (so->objfile)
 	    {
-	      if (strcmp (so->objfile->name, so->so_name) == 0)
+	      if (strcmp (so->objfile->name, so->so_name) == 0
+		  && so->objfile->addr_low == so->addr_low)
 		break;
 	    }
 	  if (so->objfile != NULL)
@@ -648,6 +648,7 @@
 							    so->sections_end);
 	  so->objfile = symbol_file_add_from_bfd (so->abfd,
 						  flags, sap, OBJF_SHARED);
+	  so->objfile->addr_low = so->addr_low;
 	  free_section_addr_info (sap);
 	}
 

Re: gdb crash: double free with free_objfile

[Mathieu Lacage]

I attached a reduced testcase and this patch to:
http://sourceware.org/bugzilla/show_bug.cgi?id=11766

enjoy !
Mathieu

On Wed, Jun 23, 2010 at 5:15 PM, Mathieu Lacage
<mathieu.lacage@gmail.com> wrote:
> hi,
>
> On Fri, Feb 5, 2010 at 5:39 PM, Tom Tromey <tromey@redhat.com> wrote:
>
>> Nope, right now objfile has the base address baked in to many things,
>> e.g. symbol addresses or psymtabs_addrmap.  I think we'd like to change
>> this, so that we can share objfiles between inferiors, but nobody has
>> tried to do that yet.  I don't know how hard this would be.
>>
>> Your quickest route is probably a form of (a) -- change symbol_add_stub
>> to check both the file name and the base address.
>
> It took me a long time to get back to this issue because I was busy
> with a couple of other things but the attached patch which was
> generated against CVS HEAD fixes the problem for me (i.e., avoids the
> segfault in gdb and gets me readable backtraces). Is there something
> else I could do to help get this issue fixed in mainline ?
>
> Mathieu
> --
> Mathieu Lacage <mathieu.lacage@gmail.com>
>



-- 
Mathieu Lacage <mathieu.lacage@gmail.com>