This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [patch] Use mmap instead of obstack_alloc for dwarf debug sections.
- From: Tom Tromey <tromey at redhat dot com>
- To: Paul Pluzhnikov <ppluzhnikov at google dot com>
- Cc: gdb-patches at sources dot redhat dot com
- Date: Tue, 16 Jun 2009 13:19:11 -0600
- Subject: Re: [patch] Use mmap instead of obstack_alloc for dwarf debug sections.
- References: <20090527001157.934BD76BC0@localhost> <m3iqjmt4bn.fsf@fleche.redhat.com> <8ac60eac0905280956v79d9a84apad9a4370212283b9@mail.gmail.com> <m3vdniw74z.fsf@fleche.redhat.com> <8ac60eac0906101839t4d3978fyc1c6d3b3e2eccb6e@mail.gmail.com> <8ac60eac0906101842y2d2fc9fco331cb4336d9508d0@mail.gmail.com>
- Reply-to: tromey at redhat dot com
>>>>> "Paul" == Paul Pluzhnikov <ppluzhnikov@google.com> writes:
Paul> Sorry, attached the bfd patch instead of the gdb one :-(
Paul> Here is the right one.
Thanks. One point of order -- if you re-send a patch, re-send the
ChangeLog entry too. Not a big deal, but it makes things simpler when
reviewing. Thanks.
I saw in your original post that this patch helps on
memory-constrained machines; your example was 2G of debug info on a 4G
machine. I just want to make sure that this doesn't hurt other cases;
I don't expect it would, but it is good to cross the t's...
I like how this patch cleans up some globals in dwarf2read.c.
I have a few comments, mostly nits, but one real one.
Paul> +struct dwarf2_section_info
Paul> +{
Paul> + asection *asection;
Paul> + gdb_byte *buffer;
Paul> + bfd_size_type size;
Paul> + int was_mmaped;
I would spell this "was_mmapped" (two "p"s).
Paul> +zlib_decompress_section (struct objfile *objfile, asection *sectp,
Paul> + gdb_byte **outbuf, bfd_size_type *outsize)
[...]
Paul> + bfd_size_type compressed_size = bfd_get_section_size (sectp);
Paul> + gdb_byte *compressed_buffer = xmalloc (compressed_size);
Paul> + bfd_size_type uncompressed_size;
Paul> + gdb_byte *uncompressed_buffer;
Paul> + z_stream strm;
Paul> + int rc;
Paul> + int header_size = 12;
Paul> +
Paul> + if (bfd_seek (abfd, sectp->filepos, SEEK_SET) != 0
Paul> + || bfd_bread (compressed_buffer, compressed_size, abfd) != compressed_size)
Paul> + error (_("Dwarf Error: Can't read DWARF data from '%s'"),
Paul> + bfd_get_filename (abfd));
This isn't your problem -- but I noticed that this will leak
compressed_buffer if there is an error. This code needs to use a
cleanup.
I'll fix this after your patch goes in.
Paul> +static void
Paul> +dwarf2_read_section (struct objfile *objfile, struct dwarf2_section_info *info)
[...]
Paul> +#ifdef HAVE_MMAP
[...]
Paul> + {
Paul> + info->was_mmaped = 1;
Paul> + info->buffer = retbuf + (sectp->filepos & (pagesize - 1)) ;
Paul> + return;
Paul> + }
[...]
Paul> + /* When debugging .o files, we may need to apply relocations; see
Paul> + http://sourceware.org/ml/gdb-patches/2002-04/msg00136.html .
Paul> + We never compress sections in .o files, so we only need to
Paul> + try this when the section is not compressed. */
Paul> + retbuf = symfile_relocate_debug_section (abfd, sectp, buf);
If we're using mmap then we seem to skip this relocation step.
This seems like a bug.
IIUC, this is an unusual case. So perhaps one fix would be to
introduce a new symfile_debug_section_needs_relocate_p, then protect
the mmap branch with that.
Paul> +static void
Paul> +munmap_section_buffer (struct dwarf2_section_info *info)
[...]
Paul> + gdb_assert (munmap ((void *)map_begin, map_length) == 0);
Space after the ")" of the cast to void *.
Tom