* [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).