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 0738D398B170 for ; Wed, 17 Jun 2020 18:50:39 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 0738D398B170 Received: from localhost (localhost [127.0.0.1]) by mail.efficios.com (Postfix) with ESMTP id C67092C4B6A; Wed, 17 Jun 2020 14:50:38 -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 UVMVthHHdhGV; Wed, 17 Jun 2020 14:50:38 -0400 (EDT) Received: from localhost (localhost [127.0.0.1]) by mail.efficios.com (Postfix) with ESMTP id 3AF8D2C4DB3; Wed, 17 Jun 2020 14:50:38 -0400 (EDT) DKIM-Filter: OpenDKIM Filter v2.10.3 mail.efficios.com 3AF8D2C4DB3 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 E2nAOJKMNsXF; Wed, 17 Jun 2020 14:50:38 -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 EC4D92C4DB2; Wed, 17 Jun 2020 14:50:37 -0400 (EDT) Subject: Re: [PATCH] gdb: check for partial symtab presence in dwarf2_initialize_objfile To: Tom de Vries , gdb-patches@sourceware.org Cc: Tom Tromey References: <20200612214454.1896242-1-simon.marchi@efficios.com> <01c28a5e-7b2b-5292-5e3d-c5880f8153b8@suse.de> From: Simon Marchi Message-ID: <88d6b630-5117-cfcc-bcb9-f1ed9c5e7a3b@efficios.com> Date: Wed, 17 Jun 2020 14:50:36 -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: <01c28a5e-7b2b-5292-5e3d-c5880f8153b8@suse.de> Content-Type: text/plain; charset=utf-8 Content-Language: tl Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-13.5 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: Wed, 17 Jun 2020 18:50:41 -0000 On 2020-06-17 12:22 p.m., Tom de Vries wrote: > On 6/12/20 11:44 PM, Simon Marchi wrote: >> This patch fixes an internal error that is triggered when loading the >> same binary twice with the index-cache on: >> >> $ ./gdb -q -nx --data-directory=data-directory >> (gdb) set index-cache on >> (gdb) shell mktemp -d >> /tmp/tmp.BLgouVoPq4 >> (gdb) set index-cache directory /tmp/tmp.BLgouVoPq4 >> (gdb) file a.out >> Reading symbols from a.out... >> (gdb) file a.out >> Load new symbol table from "a.out"? (y or n) y >> Reading symbols from a.out... >> /home/smarchi/src/binutils-gdb/gdb/dwarf2/read.c:2540: internal-error: void create_cus_from_index(dwarf2_per_bfd*, const gdb_byte*, offset_type, const gdb_byte*, offset_type): Assertion `per_bfd->all_comp_units.empty ()' failed. >> A problem internal to GDB has been detected, >> further debugging may prove unreliable. >> Quit this debugging session? (y or n) >> >> This is what happens: >> >> 1. We load the binary the first time, partial symtabs are created, >> per_bfd->all_comp_units is filled from those. >> 2. Because index-cache is on, we also generate an index in the cache. >> 3. We load the binary a second time, in dwarf2_initialize_objfile we >> check: was an index already loaded for this BFD? No, so we try to >> read the index and fill the per-bfd using it. We do find an index, >> it's in the cache. >> 4. The function create_cus_from_index asserts (rightfully) that >> per_cu->all_comp_units is empty, and the assertion fails. >> >> This assertion verifies that we are not reading an index for a BFD for >> which we have already built partial symtabs or read another index. >> >> The index-cache gives a situation that isn't currently accounted for: a >> BFD for which we have built the partial symtabs the first time, but has >> an index the second time. >> >> This patch addresses it by checking for the presence of partial symtabs >> in dwarf2_initialize_objfile. If there are, we don't try reading the >> index. >> >> gdb/ChangeLog: >> >> * dwarf2/read.c (dwarf2_initialize_objfile): Check for presence >> of partial symtabs. >> >> gdb/testsuite/ChangeLog: >> >> * gdb.base/index-cache-load-twice.c: New. >> * gdb.base/index-cache-load-twice.exp: New. >> >> Change-Id: Ie05474c44823fcdff852b73170dd28dfd66cb6a2 >> --- >> gdb/dwarf2/read.c | 8 ++++ >> .../gdb.base/index-cache-load-twice.c | 22 ++++++++++ >> .../gdb.base/index-cache-load-twice.exp | 42 +++++++++++++++++++ >> 3 files changed, 72 insertions(+) >> create mode 100644 gdb/testsuite/gdb.base/index-cache-load-twice.c >> create mode 100644 gdb/testsuite/gdb.base/index-cache-load-twice.exp >> >> diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c >> index e3073fe43ce3..a6b74ae4da6b 100644 >> --- a/gdb/dwarf2/read.c >> +++ b/gdb/dwarf2/read.c >> @@ -6027,6 +6027,14 @@ dwarf2_initialize_objfile (struct objfile *objfile, dw_index_kind *index_kind) >> return true; >> } >> >> + /* There might already be partial symtabs built for this BFD. This happens >> + when loading the same binary twice with the index-cache enabled. If so, >> + don't try to read an index. The objfile / per_objfile initialization will >> + be completed in dwarf2_build_psymtabs, in the standard partial symtabs >> + code path. */ >> + if (per_bfd->partial_symtabs != nullptr) >> + return false; >> + >> if (dwarf2_read_debug_names (per_objfile)) >> { >> *index_kind = dw_index_kind::DEBUG_NAMES; >> diff --git a/gdb/testsuite/gdb.base/index-cache-load-twice.c b/gdb/testsuite/gdb.base/index-cache-load-twice.c >> new file mode 100644 >> index 000000000000..9d7b2f1a4c28 >> --- /dev/null >> +++ b/gdb/testsuite/gdb.base/index-cache-load-twice.c >> @@ -0,0 +1,22 @@ >> +/* 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 . */ >> + >> +int >> +main (void) >> +{ >> + return 0; >> +} > > I wonder if this is generic enough to call main.c, and then perhaps we > can start reusing that one. I'd probably call it empty.c. But yeah if there are multiple tests using a trivial source file like this, I'm not against re-using the same. I won't do it as part of this patch though, it can be done separately. >> +# The output of mktemp contains an end of line, remote it. > > remote -> remove. Fixed. >> +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] >> +} >> > > LGTM otherwise. Thanks, I'll push it. Simon