From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp.polymtl.ca (smtp.polymtl.ca [132.207.4.11]) by sourceware.org (Postfix) with ESMTPS id F2E273858D1E for ; Tue, 31 Jan 2023 03:06:53 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org F2E273858D1E Authentication-Results: sourceware.org; dmarc=pass (p=quarantine dis=none) header.from=polymtl.ca Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=polymtl.ca Received: from simark.ca (simark.ca [158.69.221.121]) (authenticated bits=0) by smtp.polymtl.ca (8.14.7/8.14.7) with ESMTP id 30V36ja7003028 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Mon, 30 Jan 2023 22:06:50 -0500 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp.polymtl.ca 30V36ja7003028 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=polymtl.ca; s=default; t=1675134411; bh=+Bo0J8jI8ZxwcOfacza1hPYi0LlNPjuTXrXELCJ6XEE=; h=Date:Subject:To:References:From:In-Reply-To:From; b=pXeANIcC4xSdy4YXpell7uImfyflGmRK4ExbiyFgd2zORoY4b8x0czMwPTwh35yrG O0nw0ZHUA3ujmW4AqgIe99T72GyKAccHSJ0iwyTuR6FrmmW7zLzweMWvs2kgKNscTH u7oYuJE3PcLvP/6wlIEAgmMn+SZtz3nyOigO2vEc= Received: from [10.0.0.11] (unknown [217.28.27.60]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 97E5E1E0D3; Mon, 30 Jan 2023 22:06:45 -0500 (EST) Message-ID: <03ff1644-df3d-2e6b-f2aa-49767abc580b@polymtl.ca> Date: Mon, 30 Jan 2023 22:06:44 -0500 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.6.1 Subject: Re: [PATCH v2] gdb/dwarf: dump cooked index contents in cooked_index_functions::dump Content-Language: en-US To: Andrew Burgess , Simon Marchi , gdb-patches@sourceware.org References: <20230130160337.55890-1-simon.marchi@efficios.com> <87a61zvh7a.fsf@redhat.com> From: Simon Marchi In-Reply-To: <87a61zvh7a.fsf@redhat.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Poly-FromMTA: (simark.ca [158.69.221.121]) at Tue, 31 Jan 2023 03:06:45 +0000 X-Spam-Status: No, score=-3037.9 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,NICE_REPLY_A,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_PASS,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: > With this patch I'm seeing this failure: > > > (gdb) PASS: gdb.dwarf2/dw2-error.exp: file dw2-error > maint print objfiles /tmp/build/gdb/testsuite/outputs/gdb.dwarf2/dw2-error/dw2-error > > Object file /tmp/build/gdb/testsuite/outputs/gdb.dwarf2/dw2-error/dw2-error: Objfile at 0x40dada0, bfd at 0x426bf70, 50 minsyms > > Cooked index in use: > > ../../src/gdb/../gdbsupport/gdb-checked-static-cast.h:58: internal-error: checked_static_cast: Assertion `result != nullptr' failed. Ah, sorry about that. This is because dwarf2_build_psymtabs_hard throws, because of a bad DWARF header (this is the point of the test), so no index ever gets installed in index_table. The obvious fix, to fix the test quickly, would be to add a nullptr check in cooked_index_functions::dump. However, the result is weird: (gdb) maintenance print objfiles Object file /home/simark/build/binutils-gdb/gdb/testsuite/outputs/gdb.dwarf2/dw2-error/dw2-error: Objfile at 0x614000007240, bfd at 0x6120000dbdc0, 24 minsyms Cooked index in use: (gdb) It says the cooked index is in use, but there's nothing, as we never actually managed to build a cooked index. Maybe we should consider that an invalid state, and remove the cooked_index_functions instance from the objfile's quick symbol functions, if failing to read the psymtabs. Of course, this state (cooked_index_functions in the objfile's qf + per_objfile->index_table == nullptr) exists before objfile::require_partial_symbols has been called on the objfile, for quick symbol functions that can read partial symbols lazily (i.e. just the DWARF cooked index at this time), as we haven't tried to read partial symbols yet. But after we called objfile::require_partial_symbols, I don't think it's a state that makes sense. See the patch below for a quick prototype. quick_symbol_functions::read_partial_symbols now returns a bool, and if it returns false (failure), objfile::require_partial_symbols removes that quick symbol instance from the qf list. The result is: (gdb) file testsuite/outputs/gdb.dwarf2/dw2-error/dw2-error Reading symbols from testsuite/outputs/gdb.dwarf2/dw2-error/dw2-error... Dwarf Error: wrong version in compilation unit header (is 153, should be 2, 3, 4 or 5) [in module /home/simark/build/binutils-gdb/gdb/testsuite/outputs/gdb.dwarf2/dw2-error/dw2-error] (No debugging symbols found in testsuite/outputs/gdb.dwarf2/dw2-error/dw2-error) The "No debugging symbols" message now appears. It wouldn't appear, because objfile_has_symbols would return true, which is kind of a lie. When the cooked index qf is removed, it returns false. And the problematic command that crashes the test now shows: (gdb) maintenance print objfiles Object file /home/simark/build/binutils-gdb/gdb/testsuite/outputs/gdb.dwarf2/dw2-error/dw2-error: Objfile at 0x614000007240, bfd at 0x6120000dbdc0, 24 minsyms (gdb) Which is the same that would be shown if the objfile didn't have DWARF info at all in the first place. Please let me know if you think this approach makes sense, and if I'll finish and polish the patch. Simon >From ef55dbecacc7ec5f5e1cea82fc731b21e078783e Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Mon, 30 Jan 2023 21:38:28 -0500 Subject: [PATCH] fix Change-Id: Ic6b741324cca13239ac84c5dbef474534ad08870 --- gdb/dwarf2/read.c | 14 ++++++++++---- gdb/quick-symbol.h | 8 ++++++-- gdb/symfile-debug.c | 17 +++++++++++++---- 3 files changed, 29 insertions(+), 10 deletions(-) diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c index e8a3e359adaa..118cbd450668 100644 --- a/gdb/dwarf2/read.c +++ b/gdb/dwarf2/read.c @@ -5422,13 +5422,13 @@ dwarf2_initialize_objfile (struct objfile *objfile) /* Build a partial symbol table. */ -static void +static bool dwarf2_build_psymtabs (struct objfile *objfile) { dwarf2_per_objfile *per_objfile = get_dwarf2_per_objfile (objfile); if (per_objfile->per_bfd->index_table != nullptr) - return; + return true; try { @@ -5436,10 +5436,14 @@ dwarf2_build_psymtabs (struct objfile *objfile) /* (maybe) store an index in the cache. */ global_index_cache.store (per_objfile); + + return true; } catch (const gdb_exception_error &except) { exception_print (gdb_stderr, except); + + return false; } } @@ -18622,10 +18626,12 @@ struct cooked_index_functions : public dwarf2_base_index_functions return true; } - void read_partial_symbols (struct objfile *objfile) override + bool read_partial_symbols (struct objfile *objfile) override { if (dwarf2_has_info (objfile, nullptr)) - dwarf2_build_psymtabs (objfile); + return dwarf2_build_psymtabs (objfile); + + return true; } }; diff --git a/gdb/quick-symbol.h b/gdb/quick-symbol.h index a7fea2ccb494..41a34ec6e200 100644 --- a/gdb/quick-symbol.h +++ b/gdb/quick-symbol.h @@ -221,9 +221,13 @@ struct quick_symbol_functions } /* Read the partial symbols for OBJFILE. This will only ever be - called if can_lazily_read_symbols returns true. */ - virtual void read_partial_symbols (struct objfile *objfile) + called if can_lazily_read_symbols returns true. + + Return true on success. Return false on failure, in which case the + quick_symbol_functions instance is removed from the objfile's QF list. */ + virtual bool read_partial_symbols (struct objfile *objfile) { + return true; } }; diff --git a/gdb/symfile-debug.c b/gdb/symfile-debug.c index efc6bcdf2bdf..f244ec0d5d6e 100644 --- a/gdb/symfile-debug.c +++ b/gdb/symfile-debug.c @@ -313,7 +313,7 @@ objfile::dump () gdb_printf (gdb_stdlog, "qf->dump (%s)\n", objfile_debug_name (this)); - for (const auto &iter : qf) + for (const auto &iter : this->qf_require_partial_symbols ()) iter->dump (this); } @@ -528,9 +528,10 @@ objfile::require_partial_symbols (bool verbose) flags |= OBJF_PSYMTABS_READ; bool printed = false; - for (const auto &iter : qf) + for (auto iter = qf.begin (), prev = qf.before_begin (); + iter != qf.end (); ) { - if (iter->can_lazily_read_symbols ()) + if ((*iter)->can_lazily_read_symbols ()) { if (verbose && !printed) { @@ -538,7 +539,15 @@ objfile::require_partial_symbols (bool verbose) objfile_name (this)); printed = true; } - iter->read_partial_symbols (this); + + bool success = (*iter)->read_partial_symbols (this); + if (success) + { + ++iter; + ++prev; + } + else + iter = qf.erase_after (prev); } } if (printed && !objfile_has_symbols (this)) base-commit: 9c6e6c8f4b07a51a47c10a84e10a938b64b65fd5 -- 2.39.0