From patchwork Tue May 16 23:11:05 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Doug Gilmore X-Patchwork-Id: 20472 Received: (qmail 65244 invoked by alias); 16 May 2017 23:11:32 -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 65211 invoked by uid 89); 16 May 2017 23:11:29 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-25.2 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=luis, Luis 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; Tue, 16 May 2017 23:11:27 +0000 Received: from HHMAIL01.hh.imgtec.org (unknown [10.100.10.19]) by Forcepoint Email with ESMTPS id 163814B00F17; Wed, 17 May 2017 00:11:23 +0100 (IST) Received: from BAMAIL02.ba.imgtec.org (10.20.40.28) by HHMAIL01.hh.imgtec.org (10.100.10.19) with Microsoft SMTP Server (TLS) id 14.3.294.0; Wed, 17 May 2017 00:11:27 +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; Tue, 16 May 2017 16:11:05 -0700 Subject: Re: [PATCH] Fix PR 21337 (v3): segfault when re-reading symbols with remote debugging. To: Simon Marchi References: <20511c76-c816-d31d-5144-749eac9fc470@imgtec.com> <3c5ce0a0-72e5-4460-5555-ad2214866260@imgtec.com> <5c494cc147f71dd8246572aa0b815c9f@polymtl.ca> <7e9595026acbfd2f1a7bff321fa255e1@polymtl.ca> <5b5cc0a61e434a3406cbb25c16b8a550@polymtl.ca> <28fcce08-cab6-1e63-80d7-1d61c688cc10@imgtec.com> <09492e58ce0daf1efee14636bc5312cc@polymtl.ca> CC: Luis Machado , From: Doug Gilmore Message-ID: <5c33519c-6345-3cc8-2fa8-054a3b80bfb6@imgtec.com> Date: Tue, 16 May 2017 16:11:05 -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: <09492e58ce0daf1efee14636bc5312cc@polymtl.ca> On 05/11/2017 08:29 PM, Simon Marchi wrote: > Hi Doug, > > On 2017-05-10 19:26, Doug Gilmore wrote: >> The basic issue is that section data referenced through an objfile >> .... > > Thanks for the commit log. > ... > > Thanks for adding a test case! > > Simon Hi Simon, I attached an updated patch, which also fails with --target_board=native-extended-gdbserver, though I haven't come up with a test for --target_board=native-gdbserver, so the test is basically just a NOOP in that situation. I think I resolved all of the issues. Does this look OK? 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 PR gdb/21337. * symfile.c (reread_symbols): Call objfiles_changed just before read_symbols. gdb/testsuite 2017-??-?? Doug Gilmore PR gdb/21337 * gdb.base/reread-readsym.exp: New file. * gdb.base/reread-readsym.c: New file. --- gdb/symfile.c | 20 ++++++++-- gdb/testsuite/gdb.base/reread-readsym.c | 22 +++++++++++ gdb/testsuite/gdb.base/reread-readsym.exp | 61 +++++++++++++++++++++++++++++++ 3 files changed, 100 insertions(+), 3 deletions(-) create mode 100644 gdb/testsuite/gdb.base/reread-readsym.c create mode 100644 gdb/testsuite/gdb.base/reread-readsym.exp 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/reread-readsym.c b/gdb/testsuite/gdb.base/reread-readsym.c new file mode 100644 index 0000000..2fee696 --- /dev/null +++ b/gdb/testsuite/gdb.base/reread-readsym.c @@ -0,0 +1,22 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 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 . */ + +int +main (void) +{ + return 0; +} diff --git a/gdb/testsuite/gdb.base/reread-readsym.exp b/gdb/testsuite/gdb.base/reread-readsym.exp new file mode 100644 index 0000000..6cdb541 --- /dev/null +++ b/gdb/testsuite/gdb.base/reread-readsym.exp @@ -0,0 +1,61 @@ +# Copyright 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 . + +standard_testfile + +set gdbfile [standard_output_file ${testfile}.gdb] + +# Test rereading executable. See PR gdb/21337. + +proc generate_cmd_file {gdbfile binfile} { + set ofd [open $gdbfile w] + + puts $ofd "file ${binfile}" + puts $ofd "shell sleep 1; touch ${binfile}" + puts $ofd "run" + puts $ofd "file" + puts $ofd "file ${binfile}" + puts $ofd "shell sleep 1; touch ${binfile}" + puts $ofd "run" + puts $ofd "file" + puts $ofd "file ${binfile}" + puts $ofd "shell sleep 1; touch ${binfile}" + puts $ofd "run" + puts $ofd "file" + puts $ofd "p \"source-command-completed\"" + close $ofd +} + +if [use_gdb_stub] { + return 0 +} + +if {[prepare_for_testing "failed to prepare" $testfile $srcfile debug]} { + return -1 +} + +# Start with a fresh gdb. +clean_restart ${testfile} + +# 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. +generate_cmd_file $gdbfile $binfile + +gdb_test "source $gdbfile" ".*source-command-completed.*" \ + "source $testfile.gdb" +# Sometimes the failure only occurs on the second invocation. +gdb_test "source $gdbfile" ".*source-command-completed.*" \ + "source $testfile.gdb"