[5/5,gdb/symtab] Fix gdb.dwarf2/enum-type-c++.exp with cc-with-debug-types

  When running test-case gdb.dwarf2/enum-type-c++.exp with target board
cc-with-debug-types, we run into:
...
(gdb) FAIL: gdb.dwarf2/enum-type-c++.exp: val1 has a parent
...
because val1 has no parent:
...
    [31] ((cooked_index_entry *) 0x7efedc002e90)
    name:       val1
    canonical:  val1
    qualified:  val1
    DWARF tag:  DW_TAG_enumerator
    flags:      0x0 []
    DIE offset: 0xef
    parent:     ((cooked_index_entry *) 0)

  ...

    [37] ((cooked_index_entry *) 0x38ffd280)
    name:       val1
    canonical:  val1
    qualified:  val1
    DWARF tag:  DW_TAG_enumerator
    flags:      0x0 []
    DIE offset: 0xef
    parent:     ((cooked_index_entry *) 0)
...

There are two entries, which seems to be an inefficiency, but for now let's
focus on the correctness issue.

The debug info for val1 looks like this:
...
 <1><cb>: Abbrev Number: 2 (DW_TAG_namespace)
    <cc>   DW_AT_name        : ns
    <cf>   DW_AT_declaration : 1
 <2><d3>: Abbrev Number: 12 (DW_TAG_class_type)
    <d4>   DW_AT_name        : A
    <d6>   DW_AT_declaration : 1
 <3><d6>: Abbrev Number: 13 (DW_TAG_enumeration_type)
    <db>   DW_AT_declaration : 1
 <1><dd>: Abbrev Number: 14 (DW_TAG_enumeration_type)
    <e7>   DW_AT_specification: <0xd6>
 <2><ef>: Abbrev Number: 5 (DW_TAG_enumerator)
    <f0>   DW_AT_name        : val1
    <f4>   DW_AT_const_value : 1
...

While indexing DIE 0xdd and scanning its attributes, the specification
attribute is encountered.

A bit later, we use it to get the parent entry:
...
16278                  *parent_entry = m_die_range_map->find (addr);
...
but fail to find an entry because there is none for 0xd6.  There's none either
for DIEs 0xd3 and 0xcb.

When indexing DIE 0xcb, it is found uninteresting, and the DIE and its
children are skipped.

Fixing that and a few more issues gets us these entries:
...
    [35] ((cooked_index_entry *) 0x7fa540005280)
    name:       ns
    canonical:  ns
    qualified:  ns
    DWARF tag:  DW_TAG_namespace
    flags:      0x8 [IS_TYPE_DECLARATION]
    DIE offset: 0xcb
    parent:     ((cooked_index_entry *) 0)

  ...

    [41] ((cooked_index_entry *) 0x277b3340)
    name:       A
    canonical:  A
    qualified:  ns::A
    DWARF tag:  DW_TAG_class_type
    flags:      0x8 [IS_TYPE_DECLARATION]
    DIE offset: 0xd3
    parent:     ((cooked_index_entry *) 0x277b3310) [ns]

  ...

   [33] ((cooked_index_entry *) 0x7fa5400052e0)
    name:       (anonymous enum)
    canonical:  (anonymous enum)
    qualified:  ns::A::(anonymous enum)
    DWARF tag:  DW_TAG_enumeration_type
    flags:      0x8 [IS_TYPE_DECLARATION]
    DIE offset: 0xd6
    parent:     ((cooked_index_entry *) 0x7fa5400052b0) [A]
...

That still doesn't get us the correct parent for the val1 entry, but this fixes it:
...
-		  recurse_parent_entry = parent_entry;
+		  recurse_parent_entry = this_parent_entry;
...
and we get:
...
    [37] ((cooked_index_entry *) 0x7fa540005310)
    name:       val1
    canonical:  val1
    qualified:  ns::A::val1
    DWARF tag:  DW_TAG_enumerator
    flags:      0x0 []
    DIE offset: 0xef
    parent:     ((cooked_index_entry *) 0x7fa5400052b0) [A]
...

Tested on x86_64-linux.
---
 gdb/dwarf2/abbrev.c |  3 ++-
 gdb/dwarf2/read.c   | 16 +++++++++++-----
 2 files changed, 13 insertions(+), 6 deletions(-)
  

>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:

Tom> The debug info for val1 looks like this:
Tom> ...
Tom>  <1><cb>: Abbrev Number: 2 (DW_TAG_namespace)
Tom>     <cc>   DW_AT_name        : ns
Tom>     <cf>   DW_AT_declaration : 1
Tom>  <2><d3>: Abbrev Number: 12 (DW_TAG_class_type)
Tom>     <d4>   DW_AT_name        : A
Tom>     <d6>   DW_AT_declaration : 1
Tom>  <3><d6>: Abbrev Number: 13 (DW_TAG_enumeration_type)
Tom>     <db>   DW_AT_declaration : 1
Tom>  <1><dd>: Abbrev Number: 14 (DW_TAG_enumeration_type)
Tom>     <e7>   DW_AT_specification: <0xd6>
Tom>  <2><ef>: Abbrev Number: 5 (DW_TAG_enumerator)
Tom>     <f0>   DW_AT_name        : val1
Tom>     <f4>   DW_AT_const_value : 1
Tom> ...

Tom> That still doesn't get us the correct parent for the val1 entry, but this fixes it:
Tom> ...
Tom> -		  recurse_parent_entry = parent_entry;
Tom> +		  recurse_parent_entry = this_parent_entry;

I find this hard to reason about, but does this patch still work if the
type is an "enum class" instead?  The difficulty in this code is
normally that enum and enum class have different ideas of where the
enumerators themselves should be scoped.

Tom> +      if (abbrev->tag == DW_TAG_enumeration_type && *name == nullptr
Tom> +	  && (*flags & IS_TYPE_DECLARATION))
Tom> +	*name = "(anonymous enum)";

I don't think anonymous enums should be given a name or be entered into
the symbol table.

thanks,
Tom
  

On 9/13/24 21:51, Tom Tromey wrote:
>>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
> 
> Tom> The debug info for val1 looks like this:
> Tom> ...
> Tom>  <1><cb>: Abbrev Number: 2 (DW_TAG_namespace)
> Tom>     <cc>   DW_AT_name        : ns
> Tom>     <cf>   DW_AT_declaration : 1
> Tom>  <2><d3>: Abbrev Number: 12 (DW_TAG_class_type)
> Tom>     <d4>   DW_AT_name        : A
> Tom>     <d6>   DW_AT_declaration : 1
> Tom>  <3><d6>: Abbrev Number: 13 (DW_TAG_enumeration_type)
> Tom>     <db>   DW_AT_declaration : 1
> Tom>  <1><dd>: Abbrev Number: 14 (DW_TAG_enumeration_type)
> Tom>     <e7>   DW_AT_specification: <0xd6>
> Tom>  <2><ef>: Abbrev Number: 5 (DW_TAG_enumerator)
> Tom>     <f0>   DW_AT_name        : val1
> Tom>     <f4>   DW_AT_const_value : 1
> Tom> ...
> 
> Tom> That still doesn't get us the correct parent for the val1 entry, but this fixes it:
> Tom> ...
> Tom> -		  recurse_parent_entry = parent_entry;
> Tom> +		  recurse_parent_entry = this_parent_entry;
> 
> I find this hard to reason about, but does this patch still work if the
> type is an "enum class" instead?  The difficulty in this code is
> normally that enum and enum class have different ideas of where the
> enumerators themselves should be scoped.
> 

The test-case also contains an enum class, and the dwarf generated there is:
...
  <1><25>: Abbrev Number: 2 (DW_TAG_namespace)
     <26>   DW_AT_name        : ns
     <29>   DW_AT_declaration : 1
  <2><2d>: Abbrev Number: 3 (DW_TAG_enumeration_type)
     <2e>   DW_AT_name        : ec
     <31>   DW_AT_type        : <0x51>
     <35>   DW_AT_declaration : 1
  <2><35>: Abbrev Number: 0
  <1><36>: Abbrev Number: 4 (DW_TAG_enumeration_type)
     <37>   DW_AT_name        : ec
     <3a>   DW_AT_enum_class  : 1
     <3a>   DW_AT_encoding    : 5        (signed)
     <3b>   DW_AT_byte_size   : 4
     <3c>   DW_AT_type        : <0x51>
     <42>   DW_AT_specification: <0x2d>
  <2><4a>: Abbrev Number: 5 (DW_TAG_enumerator)
     <4b>   DW_AT_name        : val2
     <4f>   DW_AT_const_value : 2
  <1><51>: Abbrev Number: 6 (DW_TAG_base_type)
     <52>   DW_AT_byte_size   : 4
     <53>   DW_AT_encoding    : 5        (signed)
     <54>   DW_AT_name        : int
...
so we are exercising it.

Showing the propose change in a larger scope:
...
                 if (is_enum_class)
                   {
                     gdb_assert (this_entry != nullptr);
                     recurse_parent = this_entry;
                   }
                 else if (defer != 0)
                   recurse_parent = defer;
                 else
                   recurse_parent = this_parent_entry;
...
it's clear that enum class is handled separately.

> Tom> +      if (abbrev->tag == DW_TAG_enumeration_type && *name == nullptr
> Tom> +	  && (*flags & IS_TYPE_DECLARATION))
> Tom> +	*name = "(anonymous enum)";
> 
> I don't think anonymous enums should be given a name or be entered into
> the symbol table.

I've managed to implement this, and consequently dropped a fair bit of 
the patch.  I'm not sure why I thought I needed this.

V2 submitted here ( 
https://sourceware.org/pipermail/gdb-patches/2024-September/211957.html ).

Thanks,
- Tom