From patchwork Wed May 10 23:26:22 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Doug Gilmore X-Patchwork-Id: 20388 Received: (qmail 16559 invoked by alias); 10 May 2017 23:26:28 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 16542 invoked by uid 89); 10 May 2017 23:26:27 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-25.3 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, RP_MATCHES_RCVD, SPF_PASS autolearn=ham version=3.3.2 spammy=H*r:Thu, puts, D*imgtec.com, doug X-HELO: mailapp01.imgtec.com Received: from mailapp01.imgtec.com (HELO mailapp01.imgtec.com) (195.59.15.196) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 10 May 2017 23:26:25 +0000 Received: from HHMAIL01.hh.imgtec.org (unknown [10.100.10.19]) by Forcepoint Email with ESMTPS id 2D6469F91D5BD; Thu, 11 May 2017 00:26:19 +0100 (IST) Received: from HHMAIL-X.hh.imgtec.org (10.100.10.113) by HHMAIL01.hh.imgtec.org (10.100.10.19) with Microsoft SMTP Server (TLS) id 14.3.294.0; Thu, 11 May 2017 00:26:24 +0100 Received: from BAMAIL02.ba.imgtec.org (10.20.40.28) by HHMAIL-X.hh.imgtec.org (10.100.10.113) with Microsoft SMTP Server (TLS) id 14.3.294.0; Thu, 11 May 2017 00:26:24 +0100 Received: from [10.20.2.42] (10.20.2.42) by bamail02.ba.imgtec.org (10.20.40.28) with Microsoft SMTP Server id 14.3.266.1; Wed, 10 May 2017 16:26:21 -0700 From: Doug Gilmore To: Simon Marchi CC: Luis Machado , Subject: Re: [PATCH] Fix PR 21337 v2: segfault when re-reading symbols with remote debugging. References: <20511c76-c816-d31d-5144-749eac9fc470@imgtec.com> <3c5ce0a0-72e5-4460-5555-ad2214866260@imgtec.com> <5c494cc147f71dd8246572aa0b815c9f@polymtl.ca> <7e9595026acbfd2f1a7bff321fa255e1@polymtl.ca> <5b5cc0a61e434a3406cbb25c16b8a550@polymtl.ca> Message-ID: <28fcce08-cab6-1e63-80d7-1d61c688cc10@imgtec.com> Date: Wed, 10 May 2017 16:26:22 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: On 04/29/2017 10:14 PM, Doug Gilmore wrote: > > >>> ... > >> I'll need to take a look. Last time I tried I it was more difficult > >> to expose the problem on the native build of GDB. > > > > reread_symbols is called when using the run (run_command_1), attach (attach_post_wait which then calls setup_inferior) and load (load_command) commands. So maybe something like this would reproduce it? > > > > - compile test program > > - launch gdb with test program > > - touch test program > > - run > > > > Simon > > > Hi Simon, > > What I meant was that when I previously did tests with the native > build, for some reason the freed data was not being overwritten, or > possible written with the same data, at the time read_symbols was > called in reread_symbols. Thus problem wasn't exposed before the > objfiles_changed is eventually called in reread_symbols. > > I just did a test with a native build of gdb (7.9.1) and the problem > was exposed, so chances are it will be also be exposed with a ToT > build. > > Doug > It tooks some experimentation, but I found a quick, and it appears reliable, way to reproduce the problem natively. I have attached a new patch with commit with a detailed explanation for the commit log entry. OK to apply? Thanks, Doug Fix PR 21337: segfault when re-reading symbols. Fix issue exposed by commit 3e29f34. The basic issue is that section data referenced through an objfile pointer can also be referenced via the program-space data pointer, although via a separate mapping mechanism, which is set up by update_section_map. Thus once section data attached to an objfile pointer is released, the section map associated with the program-space data pointer must be marked dirty to ensure that update_section_map is called to prevent stale data being referenced. For the matter at hand this marking is being done via a call to objfiles_changed. Before commit 3e29f34 objfiles_changed could be called after all of the objfile pointers were processed in reread_symbols since section data references via the program-space data pointer would not occur in the calls of read_symbols performed by reread_symbols. With commit 3e29f34 MIPS target specific calls to find_pc_section were added to the code for DWARF information processing, which is called via read_symbols. Thus in reread_symbols the call to objfiles_changed needs to be called before calling read_symbols, otherwise stale section data can be referenced. Thanks to Luis Machado for providing text for the main comment associated with the change. gdb/ 2017-??-?? Doug Gilmore * symfile.c (reread_symbols): Fix PR 21337. gdb/testsuite 2017-??-?? Doug Gilmore PR gdb/r21337 * gdb.base/pr21337.c: New file. * gdb.base/pr21337.exp: New file. * gdb.base/pr21337.gdb: New file. --- gdb/symfile.c | 20 +++++++++++-- gdb/testsuite/gdb.base/pr21337.c | 4 +++ gdb/testsuite/gdb.base/pr21337.exp | 57 ++++++++++++++++++++++++++++++++++++++ gdb/testsuite/gdb.base/pr21337.gdb | 13 +++++++++ 4 files changed, 91 insertions(+), 3 deletions(-) create mode 100644 gdb/testsuite/gdb.base/pr21337.c create mode 100644 gdb/testsuite/gdb.base/pr21337.exp create mode 100644 gdb/testsuite/gdb.base/pr21337.gdb diff --git a/gdb/symfile.c b/gdb/symfile.c index 846aabe..0492f56 100644 --- a/gdb/symfile.c +++ b/gdb/symfile.c @@ -2576,6 +2576,9 @@ reread_symbols (void) /* Free the obstacks for non-reusable objfiles. */ psymbol_bcache_free (objfile->psymbol_cache); objfile->psymbol_cache = psymbol_bcache_init (); + + /* NB: after this call to obstack_free, objfiles_changed + will need to be called (see discussion below). */ obstack_free (&objfile->objfile_obstack, 0); objfile->sections = NULL; objfile->compunit_symtabs = NULL; @@ -2628,6 +2631,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. + + Since we discarded our section information above, we have dangling + pointers in the per-objfile program space data structure. Force + 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 +2677,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 diff --git a/gdb/testsuite/gdb.base/pr21337.c b/gdb/testsuite/gdb.base/pr21337.c new file mode 100644 index 0000000..f8b643a --- /dev/null +++ b/gdb/testsuite/gdb.base/pr21337.c @@ -0,0 +1,4 @@ +int main() +{ + return 0; +} diff --git a/gdb/testsuite/gdb.base/pr21337.exp b/gdb/testsuite/gdb.base/pr21337.exp new file mode 100644 index 0000000..d7718b4 --- /dev/null +++ b/gdb/testsuite/gdb.base/pr21337.exp @@ -0,0 +1,57 @@ +# Copyright 1998-2017 Free Software Foundation, Inc. + +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +set prototypes 1 + +# build the test case + +set testfile "pr21337" +set srcfile ${testfile}.c +# Cygwin needs $EXEEXT. +set binfile [standard_output_file ${testfile}$EXEEXT] +# set binfile ${testfile} + +set gdbfile [standard_output_file ${testfile}.gdb] + +if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug nowarnings}] != "" } { + untested "failed to compile first testcase" + return -1 +} + +# Using the source command to read commands from a file is important, +# otherwise section data is freed and reallocated using the same +# memory locations and the bug is not exposed. + +set ifd [open "$srcdir/$subdir/${testfile}.gdb" r] +set ofd [open $gdbfile w] +while {[gets $ifd line] >= 0} { + regsub $testfile $line $binfile tmp + puts $ofd $tmp +} +close $ifd +close $ofd + +gdb_start + +if [is_remote target] { + unsupported $test +} else { + gdb_test "source $gdbfile" ".*source-command-completed.*" "source $testfile.gdb" + gdb_test "source $gdbfile" ".*source-command-completed.*" "source $testfile.gdb" +} + +# End of tests. + +return 0 diff --git a/gdb/testsuite/gdb.base/pr21337.gdb b/gdb/testsuite/gdb.base/pr21337.gdb new file mode 100644 index 0000000..42fac26 --- /dev/null +++ b/gdb/testsuite/gdb.base/pr21337.gdb @@ -0,0 +1,13 @@ +file pr21337 +shell sleep 1; touch pr21337 +run +file +file pr21337 +shell sleep 1; touch pr21337 +run +file +file pr21337 +shell sleep 1; touch pr21337 +run +file +p "source-command-completed"