From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 20212 invoked by alias); 16 Jun 2009 19:19:25 -0000 Received: (qmail 20199 invoked by uid 22791); 16 Jun 2009 19:19:22 -0000 X-SWARE-Spam-Status: No, hits=-2.3 required=5.0 tests=AWL,BAYES_00,SPF_HELO_PASS,SPF_PASS X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (66.187.233.31) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 16 Jun 2009 19:19:14 +0000 Received: from int-mx1.corp.redhat.com (int-mx1.corp.redhat.com [172.16.52.254]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id n5GJJCEp019530 for ; Tue, 16 Jun 2009 15:19:12 -0400 Received: from ns3.rdu.redhat.com (ns3.rdu.redhat.com [10.11.255.199]) by int-mx1.corp.redhat.com (8.13.1/8.13.1) with ESMTP id n5GJJCAP029206 for ; Tue, 16 Jun 2009 15:19:12 -0400 Received: from opsy.redhat.com (vpn-13-48.rdu.redhat.com [10.11.13.48]) by ns3.rdu.redhat.com (8.13.8/8.13.8) with ESMTP id n5GJJBmU007197; Tue, 16 Jun 2009 15:19:12 -0400 Received: by opsy.redhat.com (Postfix, from userid 500) id 56AF43784BE; Tue, 16 Jun 2009 13:19:11 -0600 (MDT) To: Paul Pluzhnikov Cc: gdb-patches@sources.redhat.com Subject: Re: [patch] Use mmap instead of obstack_alloc for dwarf debug sections. References: <20090527001157.934BD76BC0@localhost> <8ac60eac0905280956v79d9a84apad9a4370212283b9@mail.gmail.com> <8ac60eac0906101839t4d3978fyc1c6d3b3e2eccb6e@mail.gmail.com> <8ac60eac0906101842y2d2fc9fco331cb4336d9508d0@mail.gmail.com> From: Tom Tromey Reply-To: tromey@redhat.com Date: Tue, 16 Jun 2009 19:19:00 -0000 In-Reply-To: <8ac60eac0906101842y2d2fc9fco331cb4336d9508d0@mail.gmail.com> (Paul Pluzhnikov's message of "Wed\, 10 Jun 2009 18\:42\:47 -0700") Message-ID: User-Agent: Gnus/5.11 (Gnus v5.11) Emacs/22.2 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2009-06/txt/msg00409.txt.bz2 >>>>> "Paul" == Paul Pluzhnikov 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