[PING][PATCH] [gdb/symtab] Keep track of all parents for cooked index

[Tom de Vries <tdevries@suse.de>]

On 10/24/24 13:23, Tom de Vries wrote:
> [ This is a resubmission of v2 of "[gdb/symtab] Keep track of all parents for
> cooked index" [1], this time with a DWARF assembly test-case.
> 
> The test-case is based on the DWARF info generated for test-case
> gdb.cp/breakpoint-locs.exp with target board cc-with-dwz.
> 
> The difference is that the test-case does not contain
> DW_TAG_inlined_subroutine, which makes it possible to submit this patch
> independently from the patch series.
> 
> A consequence of this difference is that the relevant DIEs in the PU are not
> named, and consequently have no entries in the cooked index, which meant that
> the original fix didn't work, so the fix has been updated to handle this case
> as well. ]
> 
> Consider the new DWARF assembly test-case gdb.dwarf2/dwz-2.exp.
> 
> With readnow, we have:
> ...
> $ gdb -q -batch -readnow outputs/gdb.dwarf2/dwz-2/dwz-2 \
>      -ex "print ns::foo" \
>      -ex "print ns::bar"
> $1 = {int (void)} 0x4101ac <ns::foo()>
> $2 = {int (void)} 0x4101b4 <ns::bar()>
> ...
> but with the cooked index we have either:
> ...
> $1 = {int (void)} 0x4101ac <ns::foo()>
> No symbol "bar" in namespace "ns".
> ...
> or:
> ...
> No symbol "foo" in namespace "ns".
> $1 = {int (void)} 0x4101b4 <ns::bar()>
> ...
> 
> The problem is that both the entries for foo and bar don't have the correct
> parent:
> ...
> $ gdb -q -batch outputs/gdb.dwarf2/dwz-2/dwz-2 -ex "maint print objfiles" \
>    | egrep "qualified:.*(foo|bar)$"
>      qualified:  foo
>      qualified:  bar
> ...
> so gdb ends up expanding the first CU that contains namespace ns, which is the
> CU that won the import race for the PU, which is why we're getting either
> ns::foo or ns::bar.
> 
> So why are the foo and bar entries not getting the correct parent?
> 
> The DWARF representation of ns::foo and ns::bar is as follows: we have the foo
> DIE (in CU1):
> ...
>   <1><42>: Abbrev Number: 3 (DW_TAG_subprogram)
>      <43>   DW_AT_specification: <0x2a>
>      <47>   DW_AT_name        : foo
>      <4b>   DW_AT_low_pc      : 0x4101ac
>      <53>   DW_AT_high_pc     : 0x4101b4
>      <5b>   DW_AT_linkage_name: _ZN2ns3fooEv
> ...
> and the bar DIE (in CU2):
> ...
>   <1><7b>: Abbrev Number: 3 (DW_TAG_subprogram)
>      <7c>   DW_AT_specification: <0x2a>
>      <80>   DW_AT_name        : bar
>      <84>   DW_AT_low_pc      : 0x4101b4
>      <8c>   DW_AT_high_pc     : 0x4101bc
>      <94>   DW_AT_linkage_name: _ZN2ns3barEv
> ...
> both referring to this DIE (in the PU):
> ...
>   <1><2a>: Abbrev Number: 5 (DW_TAG_subprogram)
>      <2b>   DW_AT_specification: <0x23>
> ...
> which refers to this DIE (also in the PU):
> ...
>   <1><1f>: Abbrev Number: 3 (DW_TAG_namespace)
>      <20>   DW_AT_name        : ns
>   <2><23>: Abbrev Number: 4 (DW_TAG_subprogram)
>      <24>   DW_AT_type        : <0x18>
>      <28>   DW_AT_external    : 1
> ...
> 
> When processing both the foo and bar DIEs, finding the parent is deferred, but
> when the deferred parents are resolved there is no entry at 0x2a in the parent
> map:
> ...
> map start:
>    0x0000000000000000 0x0
>    0x0000000000000020 0x31f497c0 (0x1f: ns)
>    0x000000000000002a 0x0
> ...
> 
> Fix this by adding an entry at 0x2a in the parent map, such that we have
> instead:
> ...
> map start:
>    0x0000000000000000 0x0
>    0x0000000000000020 0x321117c0 (0x1f: ns)
>    0x000000000000002b 0x0
>    0x0000000000000042 0x321117c0 (0x1f: ns)
>    0x0000000000000043 0x0
>    0x000000000000007b 0x321117c0 (0x1f: ns)
>    0x000000000000007c 0x0
> ...
> and:
> ...
> $ gdb -q -batch outputs/gdb.dwarf2/dwz-2/dwz-2 -ex "maint print objfiles" \
>    | egrep "qualified:.*(foo|bar)$"
>      qualified:  ns::foo
>      qualified:  ns::bar
> ...
> and:
> ...
> $ gdb -q -batch outputs/gdb.dwarf2/dwz-2/dwz-2 \
>      -ex "print ns::foo" \
>      -ex "print ns::bar"
> $1 = {int (void)} 0x4101ac <ns::foo()>
> $2 = {int (void)} 0x4101b4 <ns::bar()>
> ...
> 
> Tested on aarch64-linux.
> 

Ping.

Thanks,
- Tom

> PR symtab/32299
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32299
> 
> [1] https://sourceware.org/pipermail/gdb-patches/2023-December/205056.html
> ---
>   gdb/dwarf2/read.c                  |   8 ++
>   gdb/testsuite/gdb.dwarf2/dwz-2.c   |  37 +++++++++
>   gdb/testsuite/gdb.dwarf2/dwz-2.exp | 116 +++++++++++++++++++++++++++++
>   3 files changed, 161 insertions(+)
>   create mode 100644 gdb/testsuite/gdb.dwarf2/dwz-2.c
>   create mode 100644 gdb/testsuite/gdb.dwarf2/dwz-2.exp
> 
> diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
> index 6ac6f7c6a39..cacfab7e6d5 100644
> --- a/gdb/dwarf2/read.c
> +++ b/gdb/dwarf2/read.c
> @@ -16601,6 +16601,14 @@ cooked_indexer::index_dies (cutu_reader *reader,
>   				      this_parent_entry, m_per_cu);
>   	}
>   
> +      if (this_parent_entry != nullptr)
> +	{
> +	  parent_map::addr_type addr
> +	    = parent_map::form_addr (this_die,
> +				     reader->cu->per_cu->is_dwz);
> +	  m_die_range_map->add_entry (addr, addr, this_parent_entry);
> +	}
> +
>         if (linkage_name != nullptr)
>   	{
>   	  /* We only want this to be "main" if it has a linkage name
> diff --git a/gdb/testsuite/gdb.dwarf2/dwz-2.c b/gdb/testsuite/gdb.dwarf2/dwz-2.c
> new file mode 100644
> index 00000000000..7b3236e980a
> --- /dev/null
> +++ b/gdb/testsuite/gdb.dwarf2/dwz-2.c
> @@ -0,0 +1,37 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> +   Copyright 2024 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 <http://www.gnu.org/licenses/>.  */
> +
> +int
> +foo (void)
> +{
> +  asm ("foo_label: .globl foo_label");
> +  return 1;
> +}
> +
> +int
> +bar (void)
> +{
> +  asm ("bar_label: .globl bar_label");
> +  return 2;
> +}
> +
> +int
> +main()
> +{
> +  asm ("main_label: .globl main_label");
> +  return 0;
> +}
> diff --git a/gdb/testsuite/gdb.dwarf2/dwz-2.exp b/gdb/testsuite/gdb.dwarf2/dwz-2.exp
> new file mode 100644
> index 00000000000..17508926340
> --- /dev/null
> +++ b/gdb/testsuite/gdb.dwarf2/dwz-2.exp
> @@ -0,0 +1,116 @@
> +# Copyright 2024 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 <http://www.gnu.org/licenses/>.
> +
> +# Check that the cooked index gets the correct parents for some entries with
> +# the parents in a partial unit.
> +#
> +# The structure is similar to what's generated for test-case
> +# gdb.cp/breakpoint-locs.exp with gcc and target board cc-with-dwz, only in
> +# that case we have:
> +#   DW_TAG_inlined_subroutine -ao-> decl DW_TAG_subprogram -s->
> +#     (DW_TAG_namespace::DW_TAG_class_type::decl DW_TAG_subprogram)
> +# and here we use:
> +#   DW_TAG_subprogram -s-> decl DW_TAG_subprogram -s->
> +#     (DW_TAG_namespace::decl DW_TAG_subprogram)
> +
> +load_lib dwarf.exp
> +
> +# This test can only be run on targets which support DWARF-2 and use gas.
> +require dwarf2_support
> +
> +standard_testfile .c dwz.S
> +
> +# Create the DWARF.
> +set asm_file [standard_output_file $srcfile2]
> +Dwarf::assemble $asm_file {
> +    get_func_info foo
> +    get_func_info bar
> +    get_func_info main
> +
> +    declare_labels partial_label int_label decl1 decl2
> +
> +    cu {} {
> +	partial_label: partial_unit {} {
> +	    int_label: base_type {
> +		{name int}
> +		{byte_size 4 sdata}
> +		{encoding @DW_ATE_signed}
> +	    }
> +
> +	    DW_TAG_namespace {
> +		{DW_AT_name ns}
> +	    } {
> +		decl1: subprogram {
> +		    {type :$int_label}
> +		    {external 1 flag}
> +		}
> +	    }
> +
> +	    decl2: subprogram {
> +		{DW_AT_specification %$decl1}
> +	    }
> +	}
> +    }
> +
> +    cu {} {
> +	compile_unit {{language @DW_LANG_C_plus_plus}} {
> +	    imported_unit {
> +		{import $partial_label ref_addr}
> +	    }
> +
> +	    subprogram {
> +		{DW_AT_specification %$decl2}
> +		{DW_AT_name foo}
> +		{DW_AT_low_pc $foo_start DW_FORM_addr}
> +		{DW_AT_high_pc $foo_end DW_FORM_addr}
> +		{DW_AT_linkage_name _ZN2ns3fooEv}
> +	    }
> +	}
> +    }
> +
> +    cu {} {
> +	compile_unit {{language @DW_LANG_C_plus_plus}} {
> +	    imported_unit {
> +		{import $partial_label ref_addr}
> +	    }
> +
> +	    subprogram {
> +		{DW_AT_specification %$decl2}
> +		{DW_AT_name bar}
> +		{DW_AT_low_pc $bar_start DW_FORM_addr}
> +		{DW_AT_high_pc $bar_end DW_FORM_addr}
> +		{DW_AT_linkage_name _ZN2ns3barEv}
> +	    }
> +
> +	    subprogram {
> +		{DW_AT_name main}
> +		{DW_AT_low_pc $main_start DW_FORM_addr}
> +		{DW_AT_high_pc $main_end DW_FORM_addr}
> +	    }
> +	}
> +    }
> +}
> +
> +if { [prepare_for_testing "failed to prepare" $testfile \
> +	  [list $asm_file $srcfile] {nodebug}] } {
> +    return -1
> +}
> +
> +# Regression test for PR32299.  These should both pass, but before the fix
> +# only one of them passed.
> +gdb_test "p ns::foo" \
> +    [string_to_regexp " <ns::foo()>"]
> +gdb_test "p ns::bar" \
> +    [string_to_regexp " <ns::bar()>"]
> 
> base-commit: 86b26b453f65404b29ce035a2eb3d62671aa0612