From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.efficios.com (mail.efficios.com [167.114.26.124]) by sourceware.org (Postfix) with ESMTPS id CCD79385DC00 for ; Tue, 2 Jun 2020 21:27:49 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org CCD79385DC00 Received: from localhost (localhost [127.0.0.1]) by mail.efficios.com (Postfix) with ESMTP id 5CD912B4C8E; Tue, 2 Jun 2020 17:27:49 -0400 (EDT) Received: from mail.efficios.com ([127.0.0.1]) by localhost (mail03.efficios.com [127.0.0.1]) (amavisd-new, port 10032) with ESMTP id 7Ne9oFLVRZvK; Tue, 2 Jun 2020 17:27:48 -0400 (EDT) Received: from localhost (localhost [127.0.0.1]) by mail.efficios.com (Postfix) with ESMTP id 95A952B4C8D; Tue, 2 Jun 2020 17:27:48 -0400 (EDT) DKIM-Filter: OpenDKIM Filter v2.10.3 mail.efficios.com 95A952B4C8D X-Virus-Scanned: amavisd-new at efficios.com Received: from mail.efficios.com ([127.0.0.1]) by localhost (mail03.efficios.com [127.0.0.1]) (amavisd-new, port 10026) with ESMTP id RkcUHKE5nqQ8; Tue, 2 Jun 2020 17:27:48 -0400 (EDT) Received: from [172.16.0.95] (192-222-181-218.qc.cable.ebox.net [192.222.181.218]) by mail.efficios.com (Postfix) with ESMTPSA id 59F292B4D07; Tue, 2 Jun 2020 17:27:48 -0400 (EDT) Subject: Re: [PATCH v2 00/42] Share DWARF partial symtabs between objfiles To: Tom de Vries , Simon Marchi , Pedro Alves , Tom Tromey , Simon Marchi via Gdb-patches References: <20200512210913.5593-1-simon.marchi@efficios.com> <875zcn1xwp.fsf@tromey.com> <5032927f-877b-100b-c3a7-55c78d3a7d7a@redhat.com> <058f6a56-4690-cfd8-ede4-a9bbafc85e1a@efficios.com> <28656eeb-c0b3-c601-f05c-14cfe5493a7e@redhat.com> <63f4d428-b0ec-26de-0265-31002b61f4dd@simark.ca> From: Simon Marchi Message-ID: Date: Tue, 2 Jun 2020 17:27:47 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.8.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: tl Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-13.8 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, SPF_HELO_NONE, SPF_PASS, 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, 02 Jun 2020 21:27:51 -0000 On 2020-05-31 10:22 a.m., Tom de Vries wrote: > On 31-05-2020 06:16, Simon Marchi wrote: >> See this proof of concept fix, which sets per_bfd->partial_symtabs in >> dwarf2_read_gdb_index (when parsing the first objfile), and reads it back >> in dwarf2_initialize_objfile (when parsing the second objfile). > > I've tested this patch with the test-suite file-twice hack and target > board cc-with-gdb-index, and it fixes all regressions. > > Also, I've tested it with my ada .gdb_index patch series, and also there > it fixes the regressions. > > Thanks, > - Tom > Ok, thanks for testing. Here's a somewhat more complete patch. >From 0b342d014459b86027aa9c877233caccac8d9446 Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Sun, 31 May 2020 00:10:33 -0400 Subject: [PATCH] gdb: really share partial symtabs when using .gdb_index or .debug_names Fix/follow-up to commit 17ee85fc2a ("Share DWARF partial symtabs"). In the non-index case, where GDB builds partial symbols from scratch, two objfiles around the same BFD correctly share partial symtabs. The first objfile, which has to do all the work, saves a reference to the created partial symtabs in the shared per_bfd object (at the end of dwarf2_build_psymtabs). The second objfile, when it reaches dwarf2_build_psymtabs, sees that there are already partial symtabs built for this BFD and just uses it. However, that commit missed implementing the same sharing for cases where GDB uses .gdb_index or .debug_names to build the partial symtabs. This patch fixes it by having the first objfile to use the BFD set per_bfd->partial_symtabs at the end of dwarf2_read_gdb_index / dwarf2_read_debug_names. For the subsequent objfiles using that BFD, the partial symtabs are then picked up in dwarf2_initialize_objfile. This patch adds a test that mimics how the issue was originally triggered: 1. Load the test file twice, such that the second objfile re-uses the per_bfd object created for the first objfile. 2. Run to some point where in the backtrace there is a frame for a function that's in a CU that's not yet read in. 3. Check that this frame's information is complete in the "backtrace" output. Step 2 requires an address -> symbol lookup which uses the addrmap at objfile->partial_symtabs->psymtabs_addrmap. If the objfile->partial_symtabs link is not properly setup (as is the case before this patch), the symbol for that frame won't be found and we'll get a frame with incomplete information. The test fails without the fix when using boards "cc-with-gdb-index" and "cc-with-debug-names". gdb/ChangeLog: * dwarf2/read.c (dwarf2_read_gdb_index): Save partial_symtabs in the per_bfd object. (dwarf2_read_debug_names): Likewise. (dwarf2_initialize_objfile): Use partial_symtabs from per_bfd object when re-using a per_bfd object with an index. gdb/testsuite/ChangeLog: * gdb.dwarf2/share-psymtabs-bt.exp: New file. * gdb.dwarf2/share-psymtabs-bt.c: New file. * gdb.dwarf2/share-psymtabs-bt-2.c: New file. Change-Id: Ibb26210e2dfc03b80ba9fa56b875ba4cc58c0352 --- gdb/dwarf2/read.c | 56 +++++++++++-------- gdb/testsuite/gdb.base/share-psymtabs-bt-2.c | 24 +++++++++ gdb/testsuite/gdb.base/share-psymtabs-bt.c | 29 ++++++++++ gdb/testsuite/gdb.base/share-psymtabs-bt.exp | 57 ++++++++++++++++++++ 4 files changed, 144 insertions(+), 22 deletions(-) create mode 100644 gdb/testsuite/gdb.base/share-psymtabs-bt-2.c create mode 100644 gdb/testsuite/gdb.base/share-psymtabs-bt.c create mode 100644 gdb/testsuite/gdb.base/share-psymtabs-bt.exp diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c index e6566f9649f8..f04bdd1ce58e 100644 --- a/gdb/dwarf2/read.c +++ b/gdb/dwarf2/read.c @@ -3068,9 +3068,10 @@ dwarf2_read_gdb_index offset_type cu_list_elements, types_list_elements, dwz_list_elements = 0; struct dwz_file *dwz; struct objfile *objfile = per_objfile->objfile; + dwarf2_per_bfd *per_bfd = per_objfile->per_bfd; gdb::array_view main_index_contents - = get_gdb_index_contents (objfile, per_objfile->per_bfd); + = get_gdb_index_contents (objfile, per_bfd); if (main_index_contents.empty ()) return 0; @@ -3089,7 +3090,7 @@ dwarf2_read_gdb_index /* If there is a .dwz file, read it so we can get its CU list as well. */ - dwz = dwarf2_get_dwz_file (per_objfile->per_bfd); + dwz = dwarf2_get_dwz_file (per_bfd); if (dwz != NULL) { struct mapped_index dwz_map; @@ -3114,29 +3115,33 @@ dwarf2_read_gdb_index } } - create_cus_from_index (per_objfile->per_bfd, cu_list, cu_list_elements, - dwz_list, dwz_list_elements); + create_cus_from_index (per_bfd, cu_list, cu_list_elements, dwz_list, + dwz_list_elements); if (types_list_elements) { /* We can only handle a single .debug_types when we have an index. */ - if (per_objfile->per_bfd->types.size () != 1) + if (per_bfd->types.size () != 1) return 0; - dwarf2_section_info *section = &per_objfile->per_bfd->types[0]; + dwarf2_section_info *section = &per_bfd->types[0]; - create_signatured_type_table_from_index (per_objfile->per_bfd, - section, types_list, + create_signatured_type_table_from_index (per_bfd, section, types_list, types_list_elements); } create_addrmap_from_index (per_objfile, map.get ()); - per_objfile->per_bfd->index_table = std::move (map); - per_objfile->per_bfd->using_index = 1; - per_objfile->per_bfd->quick_file_names_table = - create_quick_file_names_table (per_objfile->per_bfd->all_comp_units.size ()); + per_bfd->index_table = std::move (map); + per_bfd->using_index = 1; + per_bfd->quick_file_names_table = + create_quick_file_names_table (per_bfd->all_comp_units.size ()); + + /* Save partial symtabs in the per_bfd object, for the benefit of subsequent + objfiles using the same BFD. */ + gdb_assert (per_bfd->partial_symtabs == nullptr); + per_bfd->partial_symtabs = objfile->partial_symtabs; return 1; } @@ -5205,6 +5210,7 @@ dwarf2_read_debug_names (dwarf2_per_objfile *per_objfile) std::unique_ptr map (new mapped_debug_names); mapped_debug_names dwz_map; struct objfile *objfile = per_objfile->objfile; + dwarf2_per_bfd *per_bfd = per_objfile->per_bfd; if (!read_debug_names_from_section (objfile, objfile_name (objfile), &per_objfile->per_bfd->debug_names, *map)) @@ -5216,7 +5222,7 @@ dwarf2_read_debug_names (dwarf2_per_objfile *per_objfile) /* If there is a .dwz file, read it so we can get its CU list as well. */ - dwz_file *dwz = dwarf2_get_dwz_file (per_objfile->per_bfd); + dwz_file *dwz = dwarf2_get_dwz_file (per_bfd); if (dwz != NULL) { if (!read_debug_names_from_section (objfile, @@ -5229,29 +5235,33 @@ dwarf2_read_debug_names (dwarf2_per_objfile *per_objfile) } } - create_cus_from_debug_names (per_objfile->per_bfd, *map, dwz_map); + create_cus_from_debug_names (per_bfd, *map, dwz_map); if (map->tu_count != 0) { /* We can only handle a single .debug_types when we have an index. */ - if (per_objfile->per_bfd->types.size () != 1) + if (per_bfd->types.size () != 1) return false; - dwarf2_section_info *section = &per_objfile->per_bfd->types[0]; + dwarf2_section_info *section = &per_bfd->types[0]; create_signatured_type_table_from_debug_names - (per_objfile, *map, section, &per_objfile->per_bfd->abbrev); + (per_objfile, *map, section, &per_bfd->abbrev); } - create_addrmap_from_aranges (per_objfile, - &per_objfile->per_bfd->debug_aranges); + create_addrmap_from_aranges (per_objfile, &per_bfd->debug_aranges); - per_objfile->per_bfd->debug_names_table = std::move (map); - per_objfile->per_bfd->using_index = 1; - per_objfile->per_bfd->quick_file_names_table = + per_bfd->debug_names_table = std::move (map); + per_bfd->using_index = 1; + per_bfd->quick_file_names_table = create_quick_file_names_table (per_objfile->per_bfd->all_comp_units.size ()); + /* Save partial symtabs in the per_bfd object, for the benefit of subsequent + objfiles using the same BFD. */ + gdb_assert (per_bfd->partial_symtabs == nullptr); + per_bfd->partial_symtabs = objfile->partial_symtabs; + return true; } @@ -5972,6 +5982,7 @@ dwarf2_initialize_objfile (struct objfile *objfile, dw_index_kind *index_kind) if (per_bfd->debug_names_table != nullptr) { *index_kind = dw_index_kind::DEBUG_NAMES; + per_objfile->objfile->partial_symtabs = per_bfd->partial_symtabs; per_objfile->resize_symtabs (); return true; } @@ -5981,6 +5992,7 @@ dwarf2_initialize_objfile (struct objfile *objfile, dw_index_kind *index_kind) if (per_bfd->index_table != nullptr) { *index_kind = dw_index_kind::GDB_INDEX; + per_objfile->objfile->partial_symtabs = per_bfd->partial_symtabs; per_objfile->resize_symtabs (); return true; } diff --git a/gdb/testsuite/gdb.base/share-psymtabs-bt-2.c b/gdb/testsuite/gdb.base/share-psymtabs-bt-2.c new file mode 100644 index 000000000000..c713eb22693c --- /dev/null +++ b/gdb/testsuite/gdb.base/share-psymtabs-bt-2.c @@ -0,0 +1,24 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2020 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 . */ + +void bar (int x); + +void +foo (int x) +{ + bar (x); +} diff --git a/gdb/testsuite/gdb.base/share-psymtabs-bt.c b/gdb/testsuite/gdb.base/share-psymtabs-bt.c new file mode 100644 index 000000000000..97ad3457905b --- /dev/null +++ b/gdb/testsuite/gdb.base/share-psymtabs-bt.c @@ -0,0 +1,29 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2020 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 . */ + +void +bar (int x) +{} + +void foo (int x); + +int +main (void) +{ + foo (12345); + return 0; +} diff --git a/gdb/testsuite/gdb.base/share-psymtabs-bt.exp b/gdb/testsuite/gdb.base/share-psymtabs-bt.exp new file mode 100644 index 000000000000..f4ca58e30b96 --- /dev/null +++ b/gdb/testsuite/gdb.base/share-psymtabs-bt.exp @@ -0,0 +1,57 @@ +# Copyright 2020 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 . + +# Test that a backtrace is shown correctly for an objfile that uses partial +# symtabs created by another objfile sharing the same BFD. +# +# It mimics how a bug with psymtab sharing was initially found: +# +# 1. Load the test file twice, such that the second objfile re-uses the +# per_bfd object created for the first objfile. +# 2. Run to some point where in the backtrace there is a frame for a +# function that's in a CU that's not yet read in. +# 3. Check that this frame's information is complete in the "backtrace" +# output. + +standard_testfile share-psymtabs-bt.c share-psymtabs-bt-2.c + +set sources [list \ + "${srcdir}/${subdir}/${srcfile}" \ + "${srcdir}/${subdir}/${srcfile2}" \ +] + +if { [gdb_compile $sources $binfile executable {debug}] != "" } { + untested "failed to compile" + return -1 +} + +clean_restart $binfile + +# Load $binfile a second time. The second created objfile will re-use the +# partial symtabs created by the first one. +if { [gdb_file_cmd $binfile] != 0 } { + fail "file command failed" + return -1 +} + +gdb_breakpoint "bar" +if { ![runto "bar"] } { + fail "failed to run to bar" + return -1 +} + +# A buggy GDB would fail to find the full symbol associated to this frame's +# address, so would just show "foo ()" (from the minimal symbol). +gdb_test "bt" "foo \\(x=12345\\).*" -- 2.26.2