Re: [PATCH v2] [gdb/symtab] Fix DW_TAG_member regression

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

On 9/19/25 7:30 PM, Tom Tromey wrote:
> Tom> +      else if (cur_abbrev->tag == DW_TAG_member && has_const_value
> Tom> +	       && has_external)
> 
> I'm a little surprised by this test.
> 
> I guess I would assume we'd see this same problem for a static class
> variable even if it doesn't have an initializer.

For something like this:
...
	       class A {
		 static const int aaa;
	       };
	       const int A::aaa = 11;
...
we get instead:
...

	       <2><28>: Abbrev Number: 3 (DW_TAG_member)
		  <29>	 DW_AT_name	   : aaa
		  <34>	 DW_AT_external	   : 1
		  <34>	 DW_AT_declaration : 1
	       <1><41>: Abbrev Number: 6 (DW_TAG_variable)
		  <42>	 DW_AT_specification: <0x28>
		  <48>	 DW_AT_linkage_name: _ZN1A3aaaE
		  <4c>	 DW_AT_location	   : 9 byte block: DW_OP_addr: 0
...

I did add a variant in the dwarf assembly test-case using DW_AT_location 
instead of DW_AT_const_value, but that doesn't seem to be handled by the 
rest of gdb.

> And I'm not sure
> has_external is needed?
> 

I did find an example of this triggering without external:
...
	       <4><27bd192>: Abbrev Number: 279 (DW_TAG_member)
		  <27bd194>   DW_AT_name	: __stored_locally
		  <27bd19e>   DW_AT_accessibility: 2  (protected)
		  <27bd19f>   DW_AT_declaration : 1
		  <27bd19f>   DW_AT_const_value : 1 byte block: 1
...
for 
std::_Function_base::_Base_manager<get_compile_file_tempdir()::<lambda()>::__stored_locally: 
from the gdb binary.

I also added a variant in the dwarf assembly test-case without 
DW_AT_external, but again that doesn't seem to be handled by the rest of 
gdb.

> I would have expected something like "has const value or has a location"
> instead.  Could
> 
> Tom> +	{
> Tom> +	  /* For Dwarf v4, GCC generates a DW_TAG_member for a static const
> Tom> +	     member.  */
> 
> ... but OTOH if this is the only case where DW_TAG_member is generated
> then it seems fine.  Though I still don't understand the "external" bit.
> 
> So if this is correct could you add some explanation for it?
> 

Done, in a v3 ( 
https://sourceware.org/pipermail/gdb-patches/2025-November/222477.html ).

So, I think the current code is correct, in the sense that it fixes the 
entire regression.

We could add support for the DW_AT_location case, but without evidence 
that a compiler generates this, I'm not sure that's worth it.

We could also add support for the no DW_AT_external case, since there is 
such evidence.  But I think that's better done in a separate patch, 
since it's not part of the regression and requires broader changes,

> Tom> --- a/gdb/dwarf2/cooked-index-shard.c
> Tom> +++ b/gdb/dwarf2/cooked-index-shard.c
> Tom> @@ -65,6 +65,15 @@ cooked_index_shard::create (sect_offset die_offset,
> Tom>    else if (tag_is_type (tag))
> Tom>      flags |= IS_STATIC;
>   
> Tom> +  if (tag == DW_TAG_member)
> Tom> +    {
> Tom> +      /* A cooked index entry generated for a DW_TAG_member should be treated
> Tom> +	 the same as one generated for a DW_TAG_variable.  Normalize to
> Tom> +	 DW_TAG_variable, to simplify code dealing with cooked index
> Tom> +	 entries.  */
> Tom> +      tag = DW_TAG_variable;
> Tom> +    }
> 
> I think this should be in the scanner and not here.
I've reverted back to handling DW_TAG_member, so this bit of code is gone.

Thanks,
- Tom