* [patch] .gdb_index: Do not crash on NOBITS
@ 2010-09-08 19:40 Jan Kratochvil
2010-09-08 23:19 ` Tom Tromey
0 siblings, 1 reply; 9+ messages in thread
From: Jan Kratochvil @ 2010-09-08 19:40 UTC (permalink / raw)
To: gdb-patches
Hi,
elfutils-0.148 still do not contain patch of its GIT
804e9ca4d644e64a6125307cbf0a0b89477d7611 where the .gdb_index section has been
also split into the separate debug info file.
Due to it binaries split using elfutils-0.148 contain
[38] .gdb_index NOBITS 0000000000000000 0000338c
instead of expected
[28] .gdb_index PROGBITS 0000000000000000 0000211c
and due to it GDB while reading the file can error() by:
Reading symbols from x.debug...Dwarf Error: Can't read DWARF data from 'x.debug'
which should not be fatal but due to some other bugs therein it can crash GDB.
The wrong separate debug info file is for example:
http://kojipkgs.fedoraproject.org/packages/glibc/2.12.90/10/x86_64/glibc-debuginfo-2.12.90-10.x86_64.rpm
/usr/lib/debug/lib64/libutil-2.12.90.so.debug
OK to check-in?
It does not attempt to use .gdb_index from the main binary, it will just
disable .gdb_index usage on these binaries.
Thanks,
Jan
gdb/
2010-09-08 Jan Kratochvil <jan.kratochvil@redhat.com>
* dwarf2read.c (dwarf2_read_index): Return on no SEC_HAS_CONTENTS.
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -1904,6 +1904,13 @@ dwarf2_read_index (struct objfile *objfile)
if (dwarf2_per_objfile->gdb_index.asection == NULL
|| dwarf2_per_objfile->gdb_index.size == 0)
return 0;
+
+ /* Older elfutils strip versions could keep the section in the main
+ executable while splitting it for the separate debug info file. */
+ if ((bfd_get_file_flags (dwarf2_per_objfile->gdb_index.asection)
+ & SEC_HAS_CONTENTS) == 0)
+ return 0;
+
dwarf2_read_section (objfile, &dwarf2_per_objfile->gdb_index);
addr = dwarf2_per_objfile->gdb_index.buffer;
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [patch] .gdb_index: Do not crash on NOBITS
2010-09-08 19:40 [patch] .gdb_index: Do not crash on NOBITS Jan Kratochvil
@ 2010-09-08 23:19 ` Tom Tromey
2010-09-08 23:36 ` Jan Kratochvil
2010-09-09 14:05 ` [patch] Fix ELF stale reference [Re: [patch] .gdb_index: Do not crash on NOBITS] Jan Kratochvil
0 siblings, 2 replies; 9+ messages in thread
From: Tom Tromey @ 2010-09-08 23:19 UTC (permalink / raw)
To: Jan Kratochvil; +Cc: gdb-patches
>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:
Jan> and due to it GDB while reading the file can error() by:
Jan> Reading symbols from x.debug...Dwarf Error: Can't read DWARF data from 'x.debug'
Jan> which should not be fatal but due to some other bugs therein it can
Jan> crash GDB.
I am curious about these other bugs.
Jan> OK to check-in?
Jan> It does not attempt to use .gdb_index from the main binary, it will just
Jan> disable .gdb_index usage on these binaries.
This is ok. Thank you.
Tom
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [patch] .gdb_index: Do not crash on NOBITS
2010-09-08 23:19 ` Tom Tromey
@ 2010-09-08 23:36 ` Jan Kratochvil
2010-09-09 14:05 ` [patch] Fix ELF stale reference [Re: [patch] .gdb_index: Do not crash on NOBITS] Jan Kratochvil
1 sibling, 0 replies; 9+ messages in thread
From: Jan Kratochvil @ 2010-09-08 23:36 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
On Wed, 08 Sep 2010 21:40:12 +0200, Tom Tromey wrote:
> >>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:
>
> Jan> and due to it GDB while reading the file can error() by:
> Jan> Reading symbols from x.debug...Dwarf Error: Can't read DWARF data from 'x.debug'
> Jan> which should not be fatal but due to some other bugs therein it can
> Jan> crash GDB.
>
> I am curious about these other bugs.
Going to check it.
> This is ok. Thank you.
Checked-in:
http://sourceware.org/ml/gdb-cvs/2010-09/msg00062.html
Thanks,
Jan
^ permalink raw reply [flat|nested] 9+ messages in thread
* [patch] Fix ELF stale reference [Re: [patch] .gdb_index: Do not crash on NOBITS]
2010-09-08 23:19 ` Tom Tromey
2010-09-08 23:36 ` Jan Kratochvil
@ 2010-09-09 14:05 ` Jan Kratochvil
2010-09-09 16:01 ` Doug Evans
1 sibling, 1 reply; 9+ messages in thread
From: Jan Kratochvil @ 2010-09-09 14:05 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
On Wed, 08 Sep 2010 21:40:12 +0200, Tom Tromey wrote:
> >>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:
>
> Jan> which should not be fatal but due to some other bugs therein it can
> Jan> crash GDB.
>
> I am curious about these other bugs.
+ /* Memory gets permanently referenced from ABFD after
+ bfd_get_synthetic_symtab so it must not get freed before ABFD gets.
+ It happens only in the case when elf_slurp_reloc_table sees
+ asection->relocation NULL. Determining which section is asection is
+ done by _bfd_elf_get_synthetic_symtab which is all a bfd
+ implementation detail, though. */
That is from:
#0 in elf_slurp_reloc_table_from_section (abfd, asect, rel_hdr, reloc_count=1170, relents, symbols, dynamic=1) at elfcode.h:1482
#1 in bfd_elf64_slurp_reloc_table (abfd, asect, symbols, dynamic=1) at elfcode.h:1563
#2 in _bfd_elf_get_synthetic_symtab (abfd, symcount=0, syms, dynsymcount=1792, dynsyms, ret) at elf.c:9269
#3 in elf_symfile_read (objfile, symfile_flags=6) at elfread.c:809
Where
elfcode.h:elf_slurp_reloc_table_from_section
contains
ps = symbols + ELF_R_SYM (rela.r_info) - 1;
relent->sym_ptr_ptr = ps;
`symbols' here is elf_symfile_read's `dyn_symbol_table'. `dyn_symbol_table'
got immediately xfree'd but the freed memory remained referenced by
asect->relocation (containing the RELENT memory above, stored there by
elf_slurp_reloc_table).
asect->relocation probably does not get used if ABFD is not being read-in the
second time, which happens only if OBJFILE is being created the second time,
which happens due to the error call in the previous mail.
I was curious there elf_symfile_read uses 0 for COPY_NAMES in a similar case:
elf_symtab_read (objfile, ST_REGULAR, symcount, symbol_table, 0);
where SYMBOL_TABLE is also immediately xfreed. But that seems to be correct as
elf_slurp_symbol_table uses
symbase = (elf_symbol_type *) bfd_zalloc (abfd, amt);
for the content where later elfread.c's SYMBOL_TABLE points to. Only the
pointers get xfreed which is OK.
No regressions on {x86_64,x86_64-m32,i686}-fedora14snapshot-linux-gnu.
Thanks,
Jan
gdb/
2010-09-09 Jan Kratochvil <jan.kratochvil@redhat.com>
Fix stale memory references.
* elfread.c: Include libbfd.h.
(elf_symfile_read): Replace xmalloc by bfd_alloc, drop xfree, new
comment.
--- a/gdb/elfread.c
+++ b/gdb/elfread.c
@@ -37,6 +37,7 @@
#include "complaints.h"
#include "demangle.h"
#include "psympriv.h"
+#include "libbfd.h"
extern void _initialize_elfread (void);
@@ -792,8 +793,14 @@ elf_symfile_read (struct objfile *objfile, int symfile_flags)
if (storage_needed > 0)
{
- dyn_symbol_table = (asymbol **) xmalloc (storage_needed);
- make_cleanup (xfree, dyn_symbol_table);
+ /* Memory gets permanently referenced from ABFD after
+ bfd_get_synthetic_symtab so it must not get freed before ABFD gets.
+ It happens only in the case when elf_slurp_reloc_table sees
+ asection->relocation NULL. Determining which section is asection is
+ done by _bfd_elf_get_synthetic_symtab which is all a bfd
+ implementation detail, though. */
+
+ dyn_symbol_table = bfd_alloc (abfd, storage_needed);
dynsymcount = bfd_canonicalize_dynamic_symtab (objfile->obfd,
dyn_symbol_table);
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [patch] Fix ELF stale reference [Re: [patch] .gdb_index: Do not crash on NOBITS]
2010-09-09 14:05 ` [patch] Fix ELF stale reference [Re: [patch] .gdb_index: Do not crash on NOBITS] Jan Kratochvil
@ 2010-09-09 16:01 ` Doug Evans
2010-09-09 16:11 ` Jan Kratochvil
0 siblings, 1 reply; 9+ messages in thread
From: Doug Evans @ 2010-09-09 16:01 UTC (permalink / raw)
To: Jan Kratochvil; +Cc: Tom Tromey, gdb-patches
On Thu, Sep 9, 2010 at 2:05 AM, Jan Kratochvil
<jan.kratochvil@redhat.com> wrote:
>
> gdb/
> 2010-09-09 Jan Kratochvil <jan.kratochvil@redhat.com>
>
> Fix stale memory references.
> * elfread.c: Include libbfd.h.
> (elf_symfile_read): Replace xmalloc by bfd_alloc, drop xfree, new
> comment.
>
> --- a/gdb/elfread.c
> +++ b/gdb/elfread.c
> @@ -37,6 +37,7 @@
> #include "complaints.h"
> #include "demangle.h"
> #include "psympriv.h"
> +#include "libbfd.h"
Apologies for nitpicking but this raises an issue I'd like to understand better.
I thought libbfd.h was an internal bfd header.
[I know I've wanted to use it at least once and been told "No." :-)]
>
> extern void _initialize_elfread (void);
>
> @@ -792,8 +793,14 @@ elf_symfile_read (struct objfile *objfile, int symfile_flags)
>
> if (storage_needed > 0)
> {
> - dyn_symbol_table = (asymbol **) xmalloc (storage_needed);
> - make_cleanup (xfree, dyn_symbol_table);
> + /* Memory gets permanently referenced from ABFD after
> + bfd_get_synthetic_symtab so it must not get freed before ABFD gets.
> + It happens only in the case when elf_slurp_reloc_table sees
> + asection->relocation NULL. Determining which section is asection is
> + done by _bfd_elf_get_synthetic_symtab which is all a bfd
> + implementation detail, though. */
> +
> + dyn_symbol_table = bfd_alloc (abfd, storage_needed);
> dynsymcount = bfd_canonicalize_dynamic_symtab (objfile->obfd,
> dyn_symbol_table);
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [patch] Fix ELF stale reference [Re: [patch] .gdb_index: Do not crash on NOBITS]
2010-09-09 16:01 ` Doug Evans
@ 2010-09-09 16:11 ` Jan Kratochvil
2010-10-14 16:07 ` [patch] Fix ELF stale reference Jan Kratochvil
0 siblings, 1 reply; 9+ messages in thread
From: Jan Kratochvil @ 2010-09-09 16:11 UTC (permalink / raw)
To: Doug Evans; +Cc: Tom Tromey, gdb-patches
On Thu, 09 Sep 2010 16:51:51 +0200, Doug Evans wrote:
> On Thu, Sep 9, 2010 at 2:05 AM, Jan Kratochvil <jan.kratochvil@redhat.com> wrote:
> > --- a/gdb/elfread.c
> > +++ b/gdb/elfread.c
> > @@ -37,6 +37,7 @@
> > +#include "libbfd.h"
>
> Apologies for nitpicking but this raises an issue I'd like to understand better.
> I thought libbfd.h was an internal bfd header.
> [I know I've wanted to use it at least once and been told "No." :-)]
I have seen
amd64-darwin-tdep.c:#include "libbfd.h"
i386-darwin-tdep.c:#include "libbfd.h"
rs6000-nat.c:#include "libbfd.h" /* For bfd_default_set_arch_mach (FIXME) */
rs6000-tdep.c:#include "libbfd.h" /* for bfd_default_set_arch_mach */
so I considered it legal.
OTOH this patch is not completely clean, it can needlessly allocate
bfd-associated memory and the right fix would probably span into bfd/ IMO.
Thanks,
Jan
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [patch] Fix ELF stale reference
2010-09-09 16:11 ` Jan Kratochvil
@ 2010-10-14 16:07 ` Jan Kratochvil
2010-10-14 17:46 ` Tom Tromey
0 siblings, 1 reply; 9+ messages in thread
From: Jan Kratochvil @ 2010-10-14 16:07 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey, Doug Evans
Hi,
I was debugging https://bugzilla.redhat.com/show_bug.cgi?id=642879 and got to
this fix from a different side. It is in fact a very common GDB crash - due
to CTRL-C hit (to get GDB prompt) in the moment an ELF file is being read in.
Original thread: http://sourceware.org/ml/gdb-patches/2010-09/msg00192.html
On Thu, 09 Sep 2010 16:56:15 +0200, Jan Kratochvil wrote:
> OTOH this patch is not completely clean, it can needlessly allocate
> bfd-associated memory and the right fix would probably span into bfd/ IMO.
While the memory could use for example register_objfile_data_with_cleanup
instead of bfd_alloc so that if errors/CTRL-Cs happen the dynamic symbol table
pointers memory is not allocated twice. Still I would not find it correct as
such memory would be objfile-bound instead of abfd-bound - while being
referenced by abfd.
OK to check-in? Or some bfd/ API improvement should be made?
Thanks,
Jan
gdb/
2010-09-09 Jan Kratochvil <jan.kratochvil@redhat.com>
Fix stale memory references.
* elfread.c: Include libbfd.h.
(elf_symfile_read): Replace xmalloc by bfd_alloc, drop xfree, new
comment.
--- a/gdb/elfread.c
+++ b/gdb/elfread.c
@@ -37,6 +37,7 @@
#include "complaints.h"
#include "demangle.h"
#include "psympriv.h"
+#include "libbfd.h"
extern void _initialize_elfread (void);
@@ -792,8 +793,14 @@ elf_symfile_read (struct objfile *objfile, int symfile_flags)
if (storage_needed > 0)
{
- dyn_symbol_table = (asymbol **) xmalloc (storage_needed);
- make_cleanup (xfree, dyn_symbol_table);
+ /* Memory gets permanently referenced from ABFD after
+ bfd_get_synthetic_symtab so it must not get freed before ABFD gets.
+ It happens only in the case when elf_slurp_reloc_table sees
+ asection->relocation NULL. Determining which section is asection is
+ done by _bfd_elf_get_synthetic_symtab which is all a bfd
+ implementation detail, though. */
+
+ dyn_symbol_table = bfd_alloc (abfd, storage_needed);
dynsymcount = bfd_canonicalize_dynamic_symtab (objfile->obfd,
dyn_symbol_table);
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [patch] Fix ELF stale reference
2010-10-14 16:07 ` [patch] Fix ELF stale reference Jan Kratochvil
@ 2010-10-14 17:46 ` Tom Tromey
2010-11-19 22:49 ` Jan Kratochvil
0 siblings, 1 reply; 9+ messages in thread
From: Tom Tromey @ 2010-10-14 17:46 UTC (permalink / raw)
To: Jan Kratochvil; +Cc: gdb-patches, Doug Evans
>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:
Jan> OK to check-in? Or some bfd/ API improvement should be made?
I think this is probably the cleanest fix.
However, libbfd.h does say right at the top that it shouldn't be used.
I guess we could ask for advice on the binutils list.
Perhaps we could make bfd_alloc a public function.
Or maybe it is ok to bend this rule.
Tom
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [patch] Fix ELF stale reference
2010-10-14 17:46 ` Tom Tromey
@ 2010-11-19 22:49 ` Jan Kratochvil
0 siblings, 0 replies; 9+ messages in thread
From: Jan Kratochvil @ 2010-11-19 22:49 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches, Doug Evans
On Thu, 14 Oct 2010 19:46:13 +0200, Tom Tromey wrote:
> >>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:
>
> Jan> OK to check-in? Or some bfd/ API improvement should be made?
>
> I think this is probably the cleanest fix.
Checked in.
> However, libbfd.h does say right at the top that it shouldn't be used.
>
> I guess we could ask for advice on the binutils list.
This part has been fixed recently in binutils (as you forwarded me):
http://sourceware.org/ml/binutils/2010-10/msg00413.html
So no new libbfd.h include is now needed in GDB.
Regarding the introduced memory leak it seems to be fixable but I have only
filed GDB PR for it now:
http://sourceware.org/bugzilla/show_bug.cgi?id=12243
Thanks,
Jan
http://sourceware.org/ml/gdb-cvs/2010-11/msg00094.html
--- src/gdb/ChangeLog 2010/11/19 18:10:43 1.12319
+++ src/gdb/ChangeLog 2010/11/19 22:30:44 1.12320
@@ -1,4 +1,10 @@
2010-11-19 Jan Kratochvil <jan.kratochvil@redhat.com>
+
+ Fix stale memory references.
+ * elfread.c (elf_symfile_read): Replace xmalloc by bfd_alloc, drop
+ xfree, new comment.
+
+2010-11-19 Jan Kratochvil <jan.kratochvil@redhat.com>
Tom Tromey <tromey@redhat.com>
* Makefile.in (.y.c): Directly create $@ from YLWRAP.
--- src/gdb/elfread.c 2010/10/01 20:26:11 1.99
+++ src/gdb/elfread.c 2010/11/19 22:30:47 1.100
@@ -790,8 +790,14 @@
if (storage_needed > 0)
{
- dyn_symbol_table = (asymbol **) xmalloc (storage_needed);
- make_cleanup (xfree, dyn_symbol_table);
+ /* Memory gets permanently referenced from ABFD after
+ bfd_get_synthetic_symtab so it must not get freed before ABFD gets.
+ It happens only in the case when elf_slurp_reloc_table sees
+ asection->relocation NULL. Determining which section is asection is
+ done by _bfd_elf_get_synthetic_symtab which is all a bfd
+ implementation detail, though. */
+
+ dyn_symbol_table = bfd_alloc (abfd, storage_needed);
dynsymcount = bfd_canonicalize_dynamic_symtab (objfile->obfd,
dyn_symbol_table);
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2010-11-19 22:49 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-09-08 19:40 [patch] .gdb_index: Do not crash on NOBITS Jan Kratochvil
2010-09-08 23:19 ` Tom Tromey
2010-09-08 23:36 ` Jan Kratochvil
2010-09-09 14:05 ` [patch] Fix ELF stale reference [Re: [patch] .gdb_index: Do not crash on NOBITS] Jan Kratochvil
2010-09-09 16:01 ` Doug Evans
2010-09-09 16:11 ` Jan Kratochvil
2010-10-14 16:07 ` [patch] Fix ELF stale reference Jan Kratochvil
2010-10-14 17:46 ` Tom Tromey
2010-11-19 22:49 ` Jan Kratochvil
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).