From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 30835 invoked by alias); 29 Apr 2017 01:42:00 -0000 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 Received: (qmail 30821 invoked by uid 89); 29 Apr 2017 01:41:59 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-24.0 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,SPF_HELO_PASS,SPF_SOFTFAIL autolearn=ham version=3.3.2 spammy=happening, our X-HELO: simark.ca Received: from simark.ca (HELO simark.ca) (158.69.221.121) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sat, 29 Apr 2017 01:41:57 +0000 Received: by simark.ca (Postfix, from userid 33) id 1C6931E4BF; Fri, 28 Apr 2017 21:41:58 -0400 (EDT) To: Doug Gilmore Subject: Re: [PATCH] Fix PR 21337 v2: segfault when re-reading symbols with remote debugging. X-PHP-Originating-Script: 33:rcube.php MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII; format=flowed Content-Transfer-Encoding: 7bit Date: Sat, 29 Apr 2017 01:42:00 -0000 From: Simon Marchi Cc: Luis Machado , gdb-patches@sourceware.org In-Reply-To: References: <20511c76-c816-d31d-5144-749eac9fc470@imgtec.com> <3c5ce0a0-72e5-4460-5555-ad2214866260@imgtec.com> <5c494cc147f71dd8246572aa0b815c9f@polymtl.ca> <7e9595026acbfd2f1a7bff321fa255e1@polymtl.ca> Message-ID: <5b5cc0a61e434a3406cbb25c16b8a550@polymtl.ca> X-Sender: simon.marchi@polymtl.ca User-Agent: Roundcube Webmail/1.2.5 X-IsSubscribed: yes X-SW-Source: 2017-04/txt/msg00817.txt.bz2 On 2017-04-28 19:44, Doug Gilmore wrote: > Hi Simon, > > After thinking about it my comment and code placement wasn't > particularly good. Something along the line's of Luis's change > is better. > > Does Luis's comment address the question you have? > > If so, Luis: Should is it OK we incorporate your changes in the patch? > > I attached a diff for the change. > > Thanks, > > Doug Hi Doug, The comment certainly helps, but in the commit log I'd like to see a more detailed list of events that leads to the crash. Now that I look into it again, I think I understand. The objfile_pspace_info::sections array/vector is a list of obj_section pointers (in C++ we'd probably use an std::vector). That list contains pointers to all the sections from all the objfiles sorted in order of increasing address. They point directly to the sections allocated by the objfile in their obstacks (and accessible through objfile::sections). So when the obstack is freed in reread_symbols, the sorted list contains stale pointers. Is that it? If that's what's happening, then I'm more convinced the fix is right. Is this behaviour caught by a test? If not, could you write one? > > diff --git a/gdb/symfile.c b/gdb/symfile.c > index 846aabe..aff4341 100644 > --- a/gdb/symfile.c > +++ b/gdb/symfile.c > @@ -2628,6 +2628,20 @@ reread_symbols (void) > clear_complaints (&symfile_complaints, 1, 1); > > objfile->flags &= ~OBJF_PSYMTABS_READ; > + > + /* We are about to read new symbols and potentially also DWARF > + information. Some targets may want to pass addresses read from > + DWARF DIE's through an adjustment function before saving them, > like > + MIPS, which may call into "find_pc_section". When called, that > + function will make use of per-objfile program space data. If you are talking about objfile_pspace_info, it's not per-objfile. There's one instance of it per program space, so it's actually "objfiles-related per-program-space data". It contains the sorted list of all sections of all objfiles loaded in the pspace. > + > + Since we discarded our section information above, we have > dangling > + pointers in the per-objfile program space data structure. Force again > + GDB to update the section mapping information by letting it know > + the objfile has changed, making the dangling pointers point to > + correct data again. */ > + objfiles_changed (); > + > read_symbols (objfile, 0); > > if (!objfile_has_symbols (objfile)) > @@ -2660,9 +2674,6 @@ reread_symbols (void) > > if (!new_objfiles.empty ()) > { > - /* Notify objfiles that we've modified objfile sections. */ > - objfiles_changed (); > - > clear_symtab_users (0); > > /* clear_objfile_data for each objfile was called before freeing > it and Thanks, Simon