Fix crash in DWARF indexer
Checks
Context |
Check |
Description |
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 |
success
|
Build passed
|
linaro-tcwg-bot/tcwg_gdb_build--master-arm |
success
|
Build passed
|
linaro-tcwg-bot/tcwg_gdb_check--master-arm |
success
|
Test passed
|
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 |
fail
|
Patch failed to apply
|
Commit Message
Iain pointed out a crash in the DWARF indexer when run on a certain D
program. The DWARF in this case has a nameless enum class; this
causes an assertion failure.
This patch arranges to simply ignore such types. The fact that an
enum class is nameless in this case appears to be a compiler bug.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32518
---
gdb/dwarf2/read.c | 35 ++++++------
gdb/testsuite/gdb.dwarf2/nameless-enum.exp | 62 ++++++++++++++++++++++
2 files changed, 82 insertions(+), 15 deletions(-)
create mode 100644 gdb/testsuite/gdb.dwarf2/nameless-enum.exp
Comments
On 1/6/25 21:40, Tom Tromey wrote:
> Iain pointed out a crash in the DWARF indexer when run on a certain D
> program. The DWARF in this case has a nameless enum class; this
> causes an assertion failure.
>
> This patch arranges to simply ignore such types. The fact that an
> enum class is nameless in this case appears to be a compiler bug.
>
Hi Tom,
thanks for fixing this.
I've:
- applied the patch,
- ran the test-case and verified that it failed, then
- rebuild and ran the test-case again, and
- verified that it passes.
Approved-By: Tom de Vries <tdevries@suse.de>
I've got two nits, one in read.c and one in nameless-enum.exp.
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32518
> ---
> gdb/dwarf2/read.c | 35 ++++++------
> gdb/testsuite/gdb.dwarf2/nameless-enum.exp | 62 ++++++++++++++++++++++
> 2 files changed, 82 insertions(+), 15 deletions(-)
> create mode 100644 gdb/testsuite/gdb.dwarf2/nameless-enum.exp
>
> diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
> index a008f0ee88b..034bba01ad6 100644
> --- a/gdb/dwarf2/read.c
> +++ b/gdb/dwarf2/read.c
> @@ -16814,22 +16814,27 @@ cooked_indexer::index_dies (cutu_reader *reader,
> 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. */
> - {
> - std::variant<const cooked_index_entry *,
> - parent_map::addr_type> recurse_parent;
> - 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;
> + names into our own parent scope.
>
> - info_ptr = recurse (reader, info_ptr, recurse_parent, fully);
> - }
> + Some versions of gdc could emit an "enum class"
> + without a name, which is nonsensical. These are
> + skipped. */
> + if (!is_enum_class || this_entry != nullptr)
> + {
> + std::variant<const cooked_index_entry *,
> + parent_map::addr_type> recurse_parent;
> + 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;
> +
> + info_ptr = recurse (reader, info_ptr, recurse_parent, fully);
> + }
> continue;
>
> case DW_TAG_module:
I think that indenting the code makes it harder to read.
Also, I think that the comment describing an exception is better added
in an if-clause matching the exception condition, rather than appended
to the earlier comment.
But these may be a personal preferences.
Anyway, FWIW I prefer something like this:
...
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index a008f0ee88b..3392dfa1aa2 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -16818,11 +16818,14 @@ cooked_indexer::index_dies (cutu_reader *reader,
{
std::variant<const cooked_index_entry *,
parent_map::addr_type> recurse_parent;
- if (is_enum_class)
+ if (is_enum_class && this_entry == nullptr)
{
- gdb_assert (this_entry != nullptr);
- recurse_parent = this_entry;
+ /* Some versions of gdc could emit an "enum class"
+ without a name, which is nonsensical. Skip this
+ DIE. */
}
+ else if (is_enum_class)
+ recurse_parent = this_entry;
else if (defer != 0)
recurse_parent = defer;
else
...
or:
...
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index a008f0ee88b..b512ac84c5c 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -16809,6 +16809,14 @@ cooked_indexer::index_dies (cutu_reader *reader,
break;
case DW_TAG_enumeration_type:
+ if (is_enum_class && this_entry == nullptr)
+ {
+ /* Some versions of gdc could emit an "enum class"
+ without a name, which is nonsensical. Skip this
+ DIE. */
+ continue;
+ }
+
/* 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
@@ -16819,10 +16827,7 @@ cooked_indexer::index_dies (cutu_reader *reader,
std::variant<const cooked_index_entry *,
parent_map::addr_type> recurse_parent;
if (is_enum_class)
- {
- gdb_assert (this_entry != nullptr);
- recurse_parent = this_entry;
- }
+ recurse_parent = this_entry;
else if (defer != 0)
recurse_parent = defer;
else
...
> diff --git a/gdb/testsuite/gdb.dwarf2/nameless-enum.exp b/gdb/testsuite/gdb.dwarf2/nameless-enum.exp
> new file mode 100644
> index 00000000000..c2eda994984
> --- /dev/null
> +++ b/gdb/testsuite/gdb.dwarf2/nameless-enum.exp
> @@ -0,0 +1,62 @@
> +# 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/>.
> +
> +# Test a nameless "enum class". This is nonsensical but previously
> +# made gdb crash.
> +
> +load_lib dwarf.exp
> +require dwarf2_support
> +
> +standard_testfile main.c nameless-enum.S
> +
Using nameless-enum.S is not necessary, we can just use:
...
standard_testfile main.c .S
...
Thanks,
- Tom
> +set asm_file [standard_output_file $srcfile2]
> +Dwarf::assemble $asm_file {
> + global srcfile
> +
> + cu {} {
> + DW_TAG_compile_unit {
> + {DW_AT_language @DW_LANG_D}
> + {DW_AT_name $srcfile}
> + {DW_AT_comp_dir /tmp}
> + } {
> + declare_labels integer_label
> +
> + 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_type :$integer_label}
> + {DW_AT_enum_class 1 DW_FORM_flag}
> + } {
> + DW_TAG_enumerator {
> + {DW_AT_name VALUE}
> + {DW_AT_const_value 17 DW_FORM_sdata}
> + }
> + }
> + }
> + }
> +}
> +
> +if {[prepare_for_testing "failed to prepare" ${testfile} \
> + [list $srcfile $asm_file] {nodebug}]} {
> + return -1
> +}
> +
> +# The bug was a crash, so just do anything here to verify gdb is still
> +# alive.
> +gdb_test "print 23" " = 23"
>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
Tom> I think that indenting the code makes it harder to read.
...
>> +standard_testfile main.c nameless-enum.S
Tom> Using nameless-enum.S is not necessary, we can just use:
Tom> ...
Tom> standard_testfile main.c .S
Tom> ...
I made these changes.
Tom
>>>>> "Tom" == Tom Tromey <tromey@adacore.com> writes:
Tom> Iain pointed out a crash in the DWARF indexer when run on a certain D
Tom> program. The DWARF in this case has a nameless enum class; this
Tom> causes an assertion failure.
Tom> This patch arranges to simply ignore such types. The fact that an
Tom> enum class is nameless in this case appears to be a compiler bug.
Tom> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32518
I'm also going to merge this one to gdb 16.
It may be a regression; and in any case the fix is very simple.
Tom
@@ -16814,22 +16814,27 @@ cooked_indexer::index_dies (cutu_reader *reader,
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. */
- {
- std::variant<const cooked_index_entry *,
- parent_map::addr_type> recurse_parent;
- 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;
+ names into our own parent scope.
- info_ptr = recurse (reader, info_ptr, recurse_parent, fully);
- }
+ Some versions of gdc could emit an "enum class"
+ without a name, which is nonsensical. These are
+ skipped. */
+ if (!is_enum_class || this_entry != nullptr)
+ {
+ std::variant<const cooked_index_entry *,
+ parent_map::addr_type> recurse_parent;
+ 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;
+
+ info_ptr = recurse (reader, info_ptr, recurse_parent, fully);
+ }
continue;
case DW_TAG_module:
new file mode 100644
@@ -0,0 +1,62 @@
+# 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/>.
+
+# Test a nameless "enum class". This is nonsensical but previously
+# made gdb crash.
+
+load_lib dwarf.exp
+require dwarf2_support
+
+standard_testfile main.c nameless-enum.S
+
+set asm_file [standard_output_file $srcfile2]
+Dwarf::assemble $asm_file {
+ global srcfile
+
+ cu {} {
+ DW_TAG_compile_unit {
+ {DW_AT_language @DW_LANG_D}
+ {DW_AT_name $srcfile}
+ {DW_AT_comp_dir /tmp}
+ } {
+ declare_labels integer_label
+
+ 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_type :$integer_label}
+ {DW_AT_enum_class 1 DW_FORM_flag}
+ } {
+ DW_TAG_enumerator {
+ {DW_AT_name VALUE}
+ {DW_AT_const_value 17 DW_FORM_sdata}
+ }
+ }
+ }
+ }
+}
+
+if {[prepare_for_testing "failed to prepare" ${testfile} \
+ [list $srcfile $asm_file] {nodebug}]} {
+ return -1
+}
+
+# The bug was a crash, so just do anything here to verify gdb is still
+# alive.
+gdb_test "print 23" " = 23"