From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from barracuda.ebox.ca (barracuda.ebox.ca [96.127.255.19]) by sourceware.org (Postfix) with ESMTPS id 1D3EB3861828 for ; Tue, 30 Mar 2021 15:43:38 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 1D3EB3861828 X-ASG-Debug-ID: 1617119016-0c856e6cd5ddda30001-fS2M51 Received: from smtp.ebox.ca (smtp.ebox.ca [96.127.255.82]) by barracuda.ebox.ca with ESMTP id SVEHjOB1HkqLBL3B (version=TLSv1 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Tue, 30 Mar 2021 11:43:36 -0400 (EDT) X-Barracuda-Envelope-From: simon.marchi@polymtl.ca X-Barracuda-RBL-Trusted-Forwarder: 96.127.255.82 Received: from simark.localdomain (192-222-157-6.qc.cable.ebox.net [192.222.157.6]) by smtp.ebox.ca (Postfix) with ESMTP id A0D67441B21; Tue, 30 Mar 2021 11:43:36 -0400 (EDT) From: Simon Marchi X-Barracuda-RBL-IP: 192.222.157.6 X-Barracuda-Effective-Source-IP: 192-222-157-6.qc.cable.ebox.net[192.222.157.6] X-Barracuda-Apparent-Source-IP: 192.222.157.6 To: gdb-patches@sourceware.org Subject: [PATCH v2] gdb/dwarf: disable per-BFD resource sharing for -readnow objfiles Date: Tue, 30 Mar 2021 11:43:36 -0400 X-ASG-Orig-Subj: [PATCH v2] gdb/dwarf: disable per-BFD resource sharing for -readnow objfiles Message-Id: <20210330154336.1676067-1-simon.marchi@polymtl.ca> X-Mailer: git-send-email 2.30.1 In-Reply-To: <20210315200340.2918476-1-simon.marchi@polymtl.ca> References: <20210315200340.2918476-1-simon.marchi@polymtl.ca> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Barracuda-Connect: smtp.ebox.ca[96.127.255.82] X-Barracuda-Start-Time: 1617119016 X-Barracuda-Encrypted: DHE-RSA-AES256-SHA X-Barracuda-URL: https://96.127.255.19:443/cgi-mod/mark.cgi X-Barracuda-BRTS-Status: 1 X-Virus-Scanned: by bsmtpd at ebox.ca X-Barracuda-Scan-Msg-Size: 10148 X-Barracuda-Spam-Score: 0.50 X-Barracuda-Spam-Status: No, SCORE=0.50 using global scores of TAG_LEVEL=1000.0 QUARANTINE_LEVEL=1000.0 KILL_LEVEL=8.0 tests=BSF_RULE7568M X-Barracuda-Spam-Report: Code version 3.2, rules version 3.2.3.88895 Rule breakdown below pts rule name description ---- ---------------------- -------------------------------------------------- 0.50 BSF_RULE7568M Custom Rule 7568M X-Spam-Status: No, score=-20.6 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_QUARANTINE, KAM_DMARC_STATUS, KAM_SHORT, RCVD_IN_DNSWL_LOW, SPF_HELO_NONE, SPF_SOFTFAIL, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 30 Mar 2021 15:43:40 -0000 New in v2: - Disable sharing only for -readnow objfiles, not all objfiles. As described in PR 27541, we hit an internal error when loading a binary the standard way and then loading it with the -readnow option: $ ./gdb -nx -q --data-directory=data-directory ~/a.out -ex "set confirm off" -ex "file -readnow ~/a.out" Reading symbols from /home/simark/a.out... Reading symbols from ~/a.out... /home/simark/src/binutils-gdb/gdb/dwarf2/read.c:8098: internal-error: void create_all_comp_units(dwarf2_per_objfile*): Assertion `per_objfile->per_bfd->all_comp_units.empty ()' failed. This is a recurring problem that exposes a design issue in the DWARF per-BFD sharing feature. Things work well when loading a binary with the same method (with/without index, with/without readnow) twice in a row. But they don't work so well when loading a binary with different methods. See this previous fix, for example: efb763a5ea35 ("gdb: check for partial symtab presence in dwarf2_initialize_objfile") That one handled the case where the first load is normal (uses partial symbols) and the second load uses an index. The problem is that when loading an objfile with a method A, we create a dwarf2_per_bfd and some dwarf2_per_cu_data and initialize them with the data belonging to that method. When loading another obfile sharing the same BFD but with a different method B, it's not clear how to re-use the dwarf2_per_bfd/dwarf2_per_cu_data previously created, because they contain the data specific to method A. I think the most sensible fix would be to not share a dwarf2_per_bfd between two objfiles loaded with different methods. That means that two objfiles sharing the same BFD and loaded the same way would share a dwarf2_per_bfd. Two objfiles sharing the same BFD but loaded with different methods would use two different dwarf2_per_bfd structures. However, this isn't a trivial change. So to fix the known issue quickly (including in the gdb 10 branch), this patch just disables all dwarf2_per_bfd sharing for objfiles using READNOW. Generalize the gdb.base/index-cache-load-twice.exp test to test all the possible combinations of loading a file with partial symtabs, index and readnow. Move it to gdb.dwarf2, since it really exercises features of the DWARF reader. gdb/ChangeLog: * dwarf2/read.c (dwarf2_has_info): Don't share dwarf2_per_bfd with objfiles using READNOW. gdb/testsuite/ChangeLog: * gdb.base/index-cache-load-twice.exp: Remove. * gdb.base/index-cache-load-twice.c: Remove. * gdb.dwarf2/per-bfd-sharing.exp: New. * gdb.dwarf2/per-bfd-sharing.c: New. Change-Id: I9ffcf1e136f3e75242f70e4e58e4ba1fd3083389 --- gdb/dwarf2/read.c | 11 ++- .../gdb.base/index-cache-load-twice.exp | 42 --------- .../per-bfd-sharing.c} | 8 +- gdb/testsuite/gdb.dwarf2/per-bfd-sharing.exp | 93 +++++++++++++++++++ 4 files changed, 108 insertions(+), 46 deletions(-) delete mode 100644 gdb/testsuite/gdb.base/index-cache-load-twice.exp rename gdb/testsuite/{gdb.base/index-cache-load-twice.c => gdb.dwarf2/per-bfd-sharing.c} (91%) create mode 100644 gdb/testsuite/gdb.dwarf2/per-bfd-sharing.exp diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c index 1b98b758c350..24183014cf4b 100644 --- a/gdb/dwarf2/read.c +++ b/gdb/dwarf2/read.c @@ -1951,9 +1951,14 @@ dwarf2_has_info (struct objfile *objfile, { dwarf2_per_bfd *per_bfd; - /* We can share a "dwarf2_per_bfd" with other objfiles if the BFD - doesn't require relocations. */ - if (!gdb_bfd_requires_relocations (objfile->obfd)) + /* We can share a "dwarf2_per_bfd" with other objfiles if the + BFD doesn't require relocations. + + We don't share with objfiles for which -readnow was requested, + because it would complicate things when loading the same BFD with + -readnow and then without -readnow. */ + if (!gdb_bfd_requires_relocations (objfile->obfd) + && (objfile->flags & OBJF_READNOW) == 0) { /* See if one has been created for this BFD yet. */ per_bfd = dwarf2_per_bfd_bfd_data_key.get (objfile->obfd); diff --git a/gdb/testsuite/gdb.base/index-cache-load-twice.exp b/gdb/testsuite/gdb.base/index-cache-load-twice.exp deleted file mode 100644 index f442527d81f8..000000000000 --- a/gdb/testsuite/gdb.base/index-cache-load-twice.exp +++ /dev/null @@ -1,42 +0,0 @@ -# Copyright 2020-2021 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 . - -# This test checks that loading a file twice with the index cache enabled does -# not crash. - -standard_testfile - -lassign [remote_exec host mktemp -d] ret cache_dir - -# The output of mktemp contains an end of line, remove it. -set cache_dir [string trimright $cache_dir \r\n] - -if { $ret != 0 } { - fail "couldn't create temporary cache dir" - return -} - -save_vars { GDBFLAGS } { - set GDBFLAGS "$GDBFLAGS -iex \"set index-cache directory $cache_dir\"" - set GDBFLAGS "$GDBFLAGS -iex \"set index-cache on\"" - - if { [prepare_for_testing "failed to prepare" $testfile $srcfile \ - {debug additional_flags=-Wl,--build-id}] } { - return - } - - # This file command would generate an internal error. - gdb_file_cmd [standard_output_file $testfile] -} diff --git a/gdb/testsuite/gdb.base/index-cache-load-twice.c b/gdb/testsuite/gdb.dwarf2/per-bfd-sharing.c similarity index 91% rename from gdb/testsuite/gdb.base/index-cache-load-twice.c rename to gdb/testsuite/gdb.dwarf2/per-bfd-sharing.c index 76c0dea3f1de..455ea3d54074 100644 --- a/gdb/testsuite/gdb.base/index-cache-load-twice.c +++ b/gdb/testsuite/gdb.dwarf2/per-bfd-sharing.c @@ -15,8 +15,14 @@ You should have received a copy of the GNU General Public License along with this program. If not, see . */ +static +int foo (int a, int b) +{ + return a + b; +} + int main (void) { - return 0; + return foo (2, 3); } diff --git a/gdb/testsuite/gdb.dwarf2/per-bfd-sharing.exp b/gdb/testsuite/gdb.dwarf2/per-bfd-sharing.exp new file mode 100644 index 000000000000..22ab91f8f65e --- /dev/null +++ b/gdb/testsuite/gdb.dwarf2/per-bfd-sharing.exp @@ -0,0 +1,93 @@ +# Copyright 2020-2021 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 . + +# This test checks that loading a file with different methods (partial symtabs, +# index, readnow) does not crash. + +standard_testfile + +lassign [remote_exec host mktemp -d] ret cache_dir + +# The output of mktemp contains an end of line, remove it. +set cache_dir [string trimright $cache_dir \r\n] + +if { $ret != 0 } { + fail "couldn't create temporary cache dir" + return +} + +verbose -log "Index cache dir: $cache_dir" + +if { [build_executable "failed to prepare" $testfile $srcfile \ + {debug additional_flags=-Wl,--build-id}] == -1 } { + return +} + +# Populate the index-cache. +with_test_prefix "populate index cache" { + clean_restart + + gdb_test_no_output "set index-cache directory $cache_dir" \ + "set index-cache directory" + gdb_test_no_output "set index-cache on" + gdb_test "file $binfile" "Reading symbols from .*" "file" +} + +proc load_binary { method } { + global binfile + global hex + + if { $method == "standard" } { + gdb_test "file $binfile" "Reading symbols from .*" "file" + } elseif { $method == "index" } { + gdb_test_no_output "set index-cache on" + gdb_test "file $binfile" "Reading symbols from .*" "file index" + gdb_test_no_output "set index-cache off" + } elseif { $method == "readnow" } { + gdb_test "file -readnow $binfile" \ + "Reading symbols from .*Expanding full symbols from .*" \ + "file readnow" + } else { + error "unknown method" + } + + # Print a static function: seeing it and its signature confirms GDB + # sees some symbols. + gdb_test "print foo" " = {int \\(int, int\\)} $hex " +} + +set methods {standard index readnow} + +foreach_with_prefix first $methods { + foreach_with_prefix second $methods { + foreach_with_prefix third $methods { + # Start with a clean GDB. + clean_restart + + # Set the index cache dir, but don't enable the index-cache, it will + # be enabled only when needed, when loading a file with the "index" + # method. + gdb_test_no_output "set index-cache directory $cache_dir" \ + "set index-cache directory" + + # Avoid GDB asking whether we really want to load a new binary. + gdb_test_no_output "set confirm off" + + with_test_prefix "load first" { load_binary $first } + with_test_prefix "load second" { load_binary $second } + with_test_prefix "load third" { load_binary $third } + } + } +} -- 2.30.1