[gdb/symtab] Revert "Change handling of DW_TAG_enumeration_type in DWARF scanner"

After adding dwarf assembly to test-case gdb.dwarf2/enum-type.exp that adds
this debug info:
...
 <1><11f>: Abbrev Number: 3 (DW_TAG_enumeration_type)
    <120>   DW_AT_specification: <0x130>
 <2><124>: Abbrev Number: 4 (DW_TAG_enumerator)
    <125>   DW_AT_name        : val1
    <12a>   DW_AT_const_value : 1
 <2><12b>: Abbrev Number: 0
 <1><12c>: Abbrev Number: 5 (DW_TAG_namespace)
    <12d>   DW_AT_name        : ns
 <2><130>: Abbrev Number: 6 (DW_TAG_enumeration_type)
    <131>   DW_AT_name        : e
    <133>   DW_AT_type        : <0x118>
    <137>   DW_AT_declaration : 1
...
I run into an assertion failure:
...
(gdb) file enum-type^M
Reading symbols from enum-type...^M
cooked-index.h:214: internal-error: get_parent: \
  Assertion `(flags & IS_PARENT_DEFERRED) == 0' failed.^M
...

This was reported in PR32160 comment 1.

This is a regression since commit 4e417d7bb1 ("Change handling of
DW_TAG_enumeration_type in DWARF scanner").

Fix this by reverting the commit.

[ Also drop the kfails for PR31900 and PR32158, which are regressions by that
same commit. ]

That allows us to look at the output of "maint print objfiles", and for val1
we get an entry without parent:
...
    [27] ((cooked_index_entry *) 0x7fbbb4002ef0)
    name:       val1
    canonical:  val1
    qualified:  val1
    DWARF tag:  DW_TAG_enumerator
    flags:      0x0 []
    DIE offset: 0x124
    parent:     ((cooked_index_entry *) 0)
...
which is incorrect, as noted in that same comment, but an improvement over the
assertion failure, and I don't think that ever worked.  This is to be
addressed in a follow-up patch.

Reverting the commit begs the question: what was it trying to fix in the first
place, and do we need a different fix?  I've investigated this and filed
PR32160 to track this.

My guess is that the commit was based on a misunderstand of what we track
in cooked_indexer::m_die_range_map.

Each DIE has two types of parent DIEs:
- a DIE that is the parent as indicated by the tree structure in which DIEs
  occur, and
- a DIE that represent the parent scope.

In most cases, these two are the same, but some times they're not.

The debug info above demonstrates such a case.  The DIE at 0x11f:
- has a tree-parent: the DIE representing the CU, and
- has a scope-parent: DIE 0x12c representing namespace ns.

In cooked_indexer::m_die_range_map, we track scope-parents, and the commit
tried to add a tree-parent instead.

So, I don't think we need a different fix, and propose we backport the reversal
for gdb 15.2.

Tested on x86_64-linux.

Approved-By: Tom Tromey <tom@tromey.com>

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31900
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32158
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32160
(cherry picked from commit a2860473ef)
This commit is contained in:
Tom de Vries
2024-09-15 15:30:53 +02:00
parent 9ca807bc5f
commit 21528df40e
3 changed files with 44 additions and 9 deletions

View File

@@ -16440,12 +16440,6 @@ cooked_indexer::index_dies (cutu_reader *reader,
flags &= ~IS_STATIC; flags &= ~IS_STATIC;
flags |= parent_entry->flags & IS_STATIC; flags |= parent_entry->flags & IS_STATIC;
} }
/* If the parent is an enum, but not an enum class, then use the
grandparent instead. */
if (this_parent_entry != nullptr
&& this_parent_entry->tag == DW_TAG_enumeration_type
&& !is_enum_class)
this_parent_entry = this_parent_entry->get_parent ();
if (abbrev->tag == DW_TAG_namespace if (abbrev->tag == DW_TAG_namespace
&& m_language == language_cplus && m_language == language_cplus
@@ -16505,7 +16499,15 @@ cooked_indexer::index_dies (cutu_reader *reader,
break; break;
case DW_TAG_enumeration_type: case DW_TAG_enumeration_type:
info_ptr = recurse (reader, info_ptr, this_entry, fully); /* We need to recurse even for an anonymous enumeration.
Which scope we record as the parent scope depends on
whether we're reading an "enum class". If so, we use
the enum itself as the parent, yielding names like
"enum_class::enumerator"; otherwise we inject the
names into our own parent scope. */
info_ptr = recurse (reader, info_ptr,
is_enum_class ? this_entry : parent_entry,
fully);
continue; continue;
case DW_TAG_module: case DW_TAG_module:

View File

@@ -32,7 +32,6 @@ require {string equal [have_index $binfile] ""}
set re_ws "\[ \t\]" set re_ws "\[ \t\]"
# Regression test for PR31900. # Regression test for PR31900.
setup_kfail "gdb/31900" *-*-*
set val1 ns::A::val1 set val1 ns::A::val1
gdb_test_lines "maint print objfiles" \ gdb_test_lines "maint print objfiles" \
"val1 has a parent" \ "val1 has a parent" \
@@ -44,7 +43,6 @@ gdb_test_lines "maint print objfiles" \
gdb_test "print $val1" " = $val1" gdb_test "print $val1" " = $val1"
# Regression test for PR32158. # Regression test for PR32158.
setup_kfail "gdb/32158" *-*-*
set val2 ns::ec::val2 set val2 ns::ec::val2
gdb_test_lines "maint print objfiles" \ gdb_test_lines "maint print objfiles" \
"val2 has correct parent" \ "val2 has correct parent" \

View File

@@ -65,6 +65,41 @@ Dwarf::assemble $asm_file {
} }
} }
} }
cu {} {
DW_TAG_compile_unit {
{DW_AT_language @DW_LANG_C_plus_plus}
{DW_AT_name tmp.c}
{DW_AT_comp_dir /tmp}
} {
declare_labels integer_label forward
integer_label: DW_TAG_base_type {
{DW_AT_byte_size 4 DW_FORM_sdata}
{DW_AT_encoding @DW_ATE_signed}
{DW_AT_name int}
}
DW_TAG_enumeration_type {
{DW_AT_specification :$forward}
} {
DW_TAG_enumerator {
{DW_AT_name val1}
{DW_AT_const_value 1 DW_FORM_sdata}
}
}
DW_TAG_namespace {
{DW_AT_name ns}
} {
forward: DW_TAG_enumeration_type {
{DW_AT_name e}
{DW_AT_type :$integer_label}
{DW_AT_declaration 1 flag}
}
}
}
}
} }
if { [prepare_for_testing "failed to prepare" ${testfile} \ if { [prepare_for_testing "failed to prepare" ${testfile} \