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

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

[Tom de Vries]

On openSUSE Leap 15.6 x86_64, with gcc 7 and test-case
gdb.base/condbreak-multi-context.exp I run into:
...
(gdb) print aaa^M
$3 = <optimized out>^M
(gdb) FAIL: $exp: start_before=true: scenario_1: print aaa
...

This is a regression since commit 86ac8c54623 ("Convert
lookup_symbol_in_objfile").

Likewise in test-cases gdb.cp/m-static.exp and gdb.cp/namespace.exp.

The failure is specific to using Dwarf v4:
- using target board unix/gdb:debug_flags=-gdwarf-5 fixes it
- using target board unix/gdb:debug_flags=-gdwarf-4 on Tumbleweed (with gcc 15
  and Dwarf v5 default) triggers it

The variable we're trying to print, A::aaa is a static const int member:
...
class A : public Base
{
public:
  static const int aaa = 10;
  ...
};
...

With Dwarf v5, we have this DIE:
...
<2><356>: Abbrev Number: 2 (DW_TAG_variable)
    <357>   DW_AT_name        : aaa
    <35c>   DW_AT_linkage_name: _ZN1A3aaaE
    <364>   DW_AT_external    : 1
    <364>   DW_AT_accessibility: 1      (public)
    <364>   DW_AT_declaration : 1
    <364>   DW_AT_const_value : 10
...
and the cooked index contains these corresponding entries:
...
    [45] ((cooked_index_entry *) 0x7facf0004730)
    name:       _ZN1A3aaaE
    canonical:  _ZN1A3aaaE
    qualified:  _ZN1A3aaaE
    DWARF tag:  DW_TAG_variable
    flags:      0x4 [IS_LINKAGE]
    DIE offset: 0x356
    parent:     ((cooked_index_entry *) 0)
    [52] ((cooked_index_entry *) 0x7facf0004700)
    name:       aaa
    canonical:  aaa
    qualified:  A::aaa
    DWARF tag:  DW_TAG_variable
    flags:      0x0 []
    DIE offset: 0x356
    parent:     ((cooked_index_entry *) 0x7facf00046d0) [A]
...

With Dwarf v4, we have instead the following DIE:
...
 <2><350>: Abbrev Number: 3 (DW_TAG_member)
    <351>   DW_AT_name        : aaa
    <35b>   DW_AT_external    : 1
    <35b>   DW_AT_accessibility: 1      (public)
    <35c>   DW_AT_declaration : 1
    <35c>   DW_AT_const_value : 4 byte block: a 0 0 0
...
and there are no corresponding entries.

Fix this by adding an entry:
...
    [47] ((cooked_index_entry *) 0x7f5a24004660)
    name:       aaa
    canonical:  aaa
    qualified:  A::aaa
    DWARF tag:  DW_TAG_variable
    flags:      0x0 []
    DIE offset: 0x350
    parent:     ((cooked_index_entry *) 0x7f5a24004630) [A]
...

The entry is somewhat unusual because the DW_TAG_variable is used instead of
DW_TAG_member, but this approach makes the code handling the cooked index more
regular.

Add a regression test in the form of a dwarf assembly test-case printing the
value of variable A::aaa.

In the test-case, for A::aaa, DW_FORM_flag is used to encode
DW_AT_declaration.  In v1 of this patch that mattered (because of using
has_hardcoded_declaration in abbrev_table::read), but that's no longer the
case.

Nevertheless, also add an A::bbb using FORM_flag_present for
DW_AT_declaration.

Tested on x86_64-linux.

PR symtab/33415
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=33415
---
 gdb/dwarf2/abbrev.c                           | 12 +++
 gdb/dwarf2/cooked-index-shard.c               |  9 ++
 gdb/dwarf2/cooked-indexer.c                   |  3 +-
 .../gdb.dwarf2/static-const-member.exp        | 97 +++++++++++++++++++
 4 files changed, 120 insertions(+), 1 deletion(-)
 create mode 100644 gdb/testsuite/gdb.dwarf2/static-const-member.exp

diff --git a/gdb/dwarf2/abbrev.c b/gdb/dwarf2/abbrev.c
index e3c268e7c47..9f415e0095b 100644
--- a/gdb/dwarf2/abbrev.c
+++ b/gdb/dwarf2/abbrev.c
@@ -119,6 +119,7 @@ abbrev_table::read (struct dwarf2_section_info *section,
       bool has_name = false;
       bool has_linkage_name = false;
       bool has_external = false;
+      bool has_const_value = false;
 
       /* Now read in declarations.  */
       int num_attrs = 0;
@@ -176,6 +177,10 @@ abbrev_table::read (struct dwarf2_section_info *section,
 	      if (is_csize && cur_attr.form == DW_FORM_ref4)
 		sibling_offset = size;
 	      break;
+
+	    case DW_AT_const_value:
+	      has_const_value = true;
+	      break;
 	    }
 
 	  switch (cur_attr.form)
@@ -242,6 +247,13 @@ abbrev_table::read (struct dwarf2_section_info *section,
 	     the correct scope.  */
 	  cur_abbrev->interesting = true;
 	}
+      else if (cur_abbrev->tag == DW_TAG_member && has_const_value
+	       && has_external)
+	{
+	  /* For Dwarf v4, GCC generates a DW_TAG_member for a static const
+	     member.  */
+	  cur_abbrev->interesting = true;
+	}
       else if (has_hardcoded_declaration
 	       && (cur_abbrev->tag != DW_TAG_variable || !has_external))
 	{
diff --git a/gdb/dwarf2/cooked-index-shard.c b/gdb/dwarf2/cooked-index-shard.c
index e440d85e1c9..6c115aa4962 100644
--- a/gdb/dwarf2/cooked-index-shard.c
+++ b/gdb/dwarf2/cooked-index-shard.c
@@ -65,6 +65,15 @@ cooked_index_shard::create (sect_offset die_offset,
   else if (tag_is_type (tag))
     flags |= IS_STATIC;
 
+  if (tag == DW_TAG_member)
+    {
+      /* A cooked index entry generated for a DW_TAG_member should be treated
+	 the same as one generated for a DW_TAG_variable.  Normalize to
+	 DW_TAG_variable, to simplify code dealing with cooked index
+	 entries.  */
+      tag = DW_TAG_variable;
+    }
+
   return new (&m_storage) cooked_index_entry (die_offset, tag, flags,
 					      lang, name, parent_entry,
 					      per_cu);
diff --git a/gdb/dwarf2/cooked-indexer.c b/gdb/dwarf2/cooked-indexer.c
index 913ff77f890..19361b25517 100644
--- a/gdb/dwarf2/cooked-indexer.c
+++ b/gdb/dwarf2/cooked-indexer.c
@@ -290,7 +290,8 @@ cooked_indexer::scan_attributes (dwarf2_per_cu *scanning_per_cu,
      that is ok.  Similarly, we allow an external variable without a
      location; those are resolved via minimal symbols.  */
   if (is_declaration && !for_specification
-      && !(abbrev->tag == DW_TAG_variable && (*flags & IS_STATIC) == 0))
+      && !((abbrev->tag == DW_TAG_variable || abbrev->tag == DW_TAG_member)
+	   && (*flags & IS_STATIC) == 0))
     {
       /* We always want to recurse into some types, but we may not
 	 want to treat them as definitions.  */
diff --git a/gdb/testsuite/gdb.dwarf2/static-const-member.exp b/gdb/testsuite/gdb.dwarf2/static-const-member.exp
new file mode 100644
index 00000000000..dbe7a5b9ae0
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/static-const-member.exp
@@ -0,0 +1,97 @@
+# Copyright 2025 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/>.
+
+load_lib dwarf.exp
+
+# This test can only be run on targets which support DWARF-2 and use gas.
+require dwarf2_support
+
+standard_testfile main.c .S
+
+set asm_file [standard_output_file ${srcfile2}]
+
+if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile}] } {
+    return -1
+}
+
+set int_size [get_sizeof "int" -1]
+
+Dwarf::assemble ${asm_file} {
+    cu {} {
+	DW_TAG_compile_unit {
+	    {DW_AT_language @DW_LANG_C_plus_plus}
+	} {
+	    declare_labels int_label
+
+	    int_label: DW_TAG_base_type {
+		{DW_AT_byte_size $::int_size DW_FORM_udata}
+		{DW_AT_encoding @DW_ATE_signed}
+		{DW_AT_name "int"}
+	    }
+
+	    DW_TAG_class_type {
+		{DW_AT_name "A"}
+	    } {
+		DW_TAG_member {
+		    {DW_AT_name "aaa"}
+		    {DW_AT_type :$int_label}
+		    {DW_AT_external 1 DW_FORM_flag}
+		    {DW_AT_declaration 1 DW_FORM_flag}
+		    {DW_AT_accessibility 1 DW_FORM_data1}
+		    {DW_AT_const_value 10 DW_FORM_data1}
+		}
+		DW_TAG_member {
+		    {DW_AT_name "bbb"}
+		    {DW_AT_type :$int_label}
+		    {DW_AT_external 1 DW_FORM_flag}
+		    {DW_AT_declaration 1 DW_FORM_flag_present}
+		    {DW_AT_accessibility 1 DW_FORM_data1}
+		    {DW_AT_const_value 11 DW_FORM_data1}
+		}
+	    }
+
+	    DW_TAG_subprogram {
+		{MACRO_AT_func { "main" }}
+		{DW_AT_type :${int_label}}
+		{DW_AT_external 1 DW_FORM_flag}
+	    } {
+	    }
+	}
+    }
+}
+
+if { [prepare_for_testing "failed to prepare" $testfile \
+	  [list $asm_file $srcfile] {nodebug}] } {
+    return
+}
+
+# Regression test for PR symtab/33415.  Print the value of A::aaa in:
+#
+#   class A
+#   {
+#     public:
+#      static const int aaa = 10;
+#   };
+#
+# With DWARF 5, we get a DW_TAG_variable, but with DWARF 4, we get a
+# DW_TAG_member instead.
+
+gdb_test \
+    "print A::aaa" \
+    " = 10"
+
+gdb_test \
+    "print A::bbb" \
+    " = 11"

base-commit: 6affec82bd4186aaf2e6733425b1e05870fc0582
-- 
2.51.0

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

[Tom Tromey]

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.  And I'm not sure
has_external is needed?

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?

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.

Tom

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

[Tom Tromey]

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> +    }

Forgot to mention -- we document the various gdb .debug_names
extensions.  This rewriting qualifies as one and should be documented.

Another approach I guess would be to leave the tag alone and update the
matching.

Tom

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

[Tom de Vries]

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

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

[Tom de Vries]

On 9/19/25 7:31 PM, Tom Tromey wrote:
> 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> +    }
> 
> Forgot to mention -- we document the various gdb .debug_names
> extensions.  This rewriting qualifies as one and should be documented.
> 
> Another approach I guess would be to leave the tag alone and update the
> matching.

I've gone for the latter option.  I didn't realize this would be user 
visible, so it seems easier to just handle the DW_TAG_member.

Thanks,
- Tom