[patch] Assertion for a regression by gdb_bfd_unref

[patch] Assertion for a regression by gdb_bfd_unref

[Jan Kratochvil]

Hi,

since
	Re: Solibs and objfile BFD ownership ownership
	http://sourceware.org/ml/gdb/2009-08/msg00020.html
	http://sourceware.org/ml/gdb-cvs/2009-08/msg00013.html

there is a regression:

CFLAGS=-g ./configure --build=x86_64-unknown-linux-gnu --host=x86_64-unknown-linux-gnu --target=powerpc64-unknown-linux-gnu --enable-targets=i386-unknown-go32
make
echo 'main(){}' >1.c;gcc -o 1 1.c -g;./gdb/gdb -nx -ex 'set confirm no' -ex 'file ./1' -ex 'b main' -ex file
GNU gdb (GDB) 6.8.50.20090809-cvs
Copyright (C) 2009 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "--host=x86_64-unknown-linux-gnu --target=powerpc64-unknown-linux-gnu".
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>.
Breakpoint 1 at 0x400480: file 1.c, line 1.
Don't know how to run.  Try "help target".
objfiles.c:1029: internal-error: gdb_bfd_unref: Assertion `p_refcount == NULL || *p_refcount == 1 || *p_refcount == 2' failed.
A problem internal to GDB has been detected,
further debugging may prove unreliable.
Aborted

With the attached patch the reason is:

symfile.c:1642: internal-error: symfile_bfd_open: Assertion `bfd_usrdata (sym_bfd) == NULL' failed.

(found it while using --enable-targets=all)

Going to post now patches to binutils for bfd/ to fix the bfd_usrdata
initialization.


Thanks,
Jan


gdb/
2009-08-09  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* objfiles.c (gdb_bfd_unref): Use the bfd_usrdata accessor.
	* solib.c (symbol_add_stub): Likewise.
	* symfile.c (symfile_bfd_open): Assert BFD_USRDATA is NULL.

--- a/gdb/objfiles.c
+++ b/gdb/objfiles.c
@@ -1022,7 +1022,7 @@ gdb_bfd_unref (struct bfd *abfd)
   if (abfd == NULL)
     return;
 
-  p_refcount = abfd->usrdata;
+  p_refcount = bfd_usrdata (abfd);
 
   /* Valid range for p_refcount: NULL (single owner), or a pointer
      to int counter, which has a value of 1 (single owner) or 2 (shared).  */
@@ -1035,7 +1035,7 @@ gdb_bfd_unref (struct bfd *abfd)
 	return;
     }
   xfree (p_refcount);
-  abfd->usrdata = NULL;  /* Paranoia.  */
+  bfd_usrdata (abfd) = NULL;  /* Paranoia.  */
 
   name = bfd_get_filename (abfd);
   if (!bfd_close (abfd))
--- a/gdb/solib.c
+++ b/gdb/solib.c
@@ -459,7 +459,7 @@ symbol_add_stub (struct so_list *so, int flags)
   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.  */
-  so->abfd->usrdata = p_refcount;
+  bfd_usrdata (so->abfd) = p_refcount;
 
   free_section_addr_info (sap);
 
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -1638,6 +1638,9 @@ symfile_bfd_open (char *name)
 	     bfd_errmsg (bfd_get_error ()));
     }
 
+  /* bfd_usrdata exists for applications and libbfd must not touch it.
+  gdb_assert (bfd_usrdata (sym_bfd) == NULL);
+
   return sym_bfd;
 }
 

Re: [patch] Assertion for a regression by gdb_bfd_unref

[Paul Pluzhnikov]

On Sun, Aug 9, 2009 at 2:37 PM, Jan Kratochvil<jan.kratochvil@redhat.com> wrote:

> Going to post now patches to binutils for bfd/ to fix the bfd_usrdata
> initialization.

I thought I'd checked that it *is* initialized. In bfd/opncls.c:

/* Return a new BFD.  All BFD's are allocated through this routine.  */
bfd *
_bfd_new_bfd (void)
...
  nbfd->usrdata = NULL;

Confused. Ah! bfd uses usrdata internally despite declaring them to be
for application use:

coff-stgo32.c:#define bfd_coff_go32stub bfd_usrdata
coff-stgo32.c:  bfd_coff_go32stub (abfd) = (PTR) bfd_alloc (abfd,
(bfd_size_type) STUBSIZE);

How naughty!


>        * objfiles.c (gdb_bfd_unref): Use the bfd_usrdata accessor.
>        * solib.c (symbol_add_stub): Likewise.
>        * symfile.c (symfile_bfd_open): Assert BFD_USRDATA is NULL.

Thanks for fixing this.

-- 
Paul Pluzhnikov

Re: [patch] Assertion for a regression by gdb_bfd_unref

[Tom Tromey]

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

Jan> 2009-08-09  Jan Kratochvil  <jan.kratochvil@redhat.com>
Jan> 	* objfiles.c (gdb_bfd_unref): Use the bfd_usrdata accessor.
Jan> 	* solib.c (symbol_add_stub): Likewise.
Jan> 	* symfile.c (symfile_bfd_open): Assert BFD_USRDATA is NULL.

This is ok, thanks.

Tom

Re: [patch] Assertion for a regression by gdb_bfd_unref

[Jan Kratochvil]

On Mon, 10 Aug 2009 22:10:21 +0200, Tom Tromey wrote:
> >>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:
> 
> Jan> 2009-08-09  Jan Kratochvil  <jan.kratochvil@redhat.com>
> Jan> 	* objfiles.c (gdb_bfd_unref): Use the bfd_usrdata accessor.
> Jan> 	* solib.c (symbol_add_stub): Likewise.
> Jan> 	* symfile.c (symfile_bfd_open): Assert BFD_USRDATA is NULL.
> 
> This is ok, thanks.

Checked-in:
	http://sourceware.org/ml/gdb-cvs/2009-08/msg00055.html

The binutils go32-target patches are now also checked-in:
	http://sourceware.org/ml/binutils/2009-08/threads.html#00155


Thanks,
Jan


--- src/gdb/ChangeLog	2009/08/10 20:14:13	1.10794
+++ src/gdb/ChangeLog	2009/08/10 22:09:22	1.10795
@@ -1,3 +1,9 @@
+2009-08-10  Jan Kratochvil  <jan.kratochvil@redhat.com>
+
+	* objfiles.c (gdb_bfd_unref): Use the bfd_usrdata accessor.
+	* solib.c (symbol_add_stub): Likewise.
+	* symfile.c (symfile_bfd_open): Assert BFD_USRDATA is NULL.
+
 2009-08-10  Paul Pluzhnikov  <ppluzhnikov@google.com>
 
 	* objfiles.c (qsort_cmp): Remove assert.
--- src/gdb/objfiles.c	2009/08/10 20:14:14	1.90
+++ src/gdb/objfiles.c	2009/08/10 22:09:22	1.91
@@ -1070,7 +1070,7 @@
   if (abfd == NULL)
     return;
 
-  p_refcount = abfd->usrdata;
+  p_refcount = bfd_usrdata (abfd);
 
   /* Valid range for p_refcount: NULL (single owner), or a pointer
      to int counter, which has a value of 1 (single owner) or 2 (shared).  */
@@ -1083,7 +1083,7 @@
 	return;
     }
   xfree (p_refcount);
-  abfd->usrdata = NULL;  /* Paranoia.  */
+  bfd_usrdata (abfd) = NULL;  /* Paranoia.  */
 
   name = bfd_get_filename (abfd);
   if (!bfd_close (abfd))
--- src/gdb/solib.c	2009/08/04 18:46:05	1.123
+++ src/gdb/solib.c	2009/08/10 22:09:22	1.124
@@ -459,7 +459,7 @@
   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.  */
-  so->abfd->usrdata = p_refcount;
+  bfd_usrdata (so->abfd) = p_refcount;
 
   free_section_addr_info (sap);
 
--- src/gdb/symfile.c	2009/08/03 17:00:34	1.240
+++ src/gdb/symfile.c	2009/08/10 22:09:22	1.241
@@ -1638,6 +1638,9 @@
 	     bfd_errmsg (bfd_get_error ()));
     }
 
+  /* bfd_usrdata exists for applications and libbfd must not touch it.  */
+  gdb_assert (bfd_usrdata (sym_bfd) == NULL);
+
   return sym_bfd;
 }
 

Re: [patch] Assertion for a regression by gdb_bfd_unref

[Paul Pluzhnikov]

On Mon, Aug 10, 2009 at 3:11 PM, Jan
Kratochvil<jan.kratochvil@redhat.com> wrote:

> The binutils go32-target patches are now also checked-in:
>        http://sourceware.org/ml/binutils/2009-08/threads.html#00155

AFAICT, above patch will not fix the GDB assert.
Ah, you mean this one:
  http://sourceware.org/ml/binutils/2009-08/msg00154.html

Thanks,
-- 
Paul Pluzhnikov

Re: [patch] Assertion for a regression by gdb_bfd_unref

[Jan Kratochvil]

On Tue, 11 Aug 2009 00:23:01 +0200, Paul Pluzhnikov wrote:
> On Mon, Aug 10, 2009 at 3:11 PM, Jan
> Kratochvil<jan.kratochvil@redhat.com> wrote:
> 
> > The binutils go32-target patches are now also checked-in:
> >        http://sourceware.org/ml/binutils/2009-08/threads.html#00155
> 
> AFAICT, above patch will not fix the GDB assert.
> Ah, you mean this one:
>   http://sourceware.org/ml/binutils/2009-08/msg00154.html

I did mean the URL reference as for the whole patchset of [0/4]...[4/4].
You are right the GDB-specific patch was that [3/4] one.


Thanks for no OBJF_KEEPBFD,
Jan