Re: [PATCH v2] gdb/dwarf: dump cooked index contents in cooked_index_functions::dump

Andrew Burgess via Gdb-patches
2023-01-31T09:22:29Z

Simon Marchi <simon.marchi@polymtl.ca> writes:

>> 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.

I think this would be a good improvement. Especially the "No debugging
symbols found..." message now being printed, is, I think, a good thing.

Thanks,
Andrew

>
> Simon
>
> From ef55dbecacc7ec5f5e1cea82fc731b21e078783e Mon Sep 17 00:00:00 2001
> From: Simon Marchi <simon.marchi@polymtl.ca>
> 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