[PING] [PATCH v2] [gdb/symtab] Fix segfault on invalid debug info
Tom de Vries
tdevries@suse.de
Mon Sep 23 08:00:27 GMT 2024
On 9/4/24 09:24, Tom de Vries wrote:
> While looking at PR symtab/31478 (a problem in the cooked indexer with invalid
> dwarf) it occurred to me that I could trigger a similar problem using:
> ...
> Compilation Unit @ offset 0xb2:
> Length: 0x1f (32-bit)
> Version: 4
> Abbrev Offset: 0x6c
> Pointer Size: 8
> <0><bd>: Abbrev Number: 1 (DW_TAG_compile_unit)
> <be> DW_AT_language : 2 (non-ANSI C)
> <1><bf>: Abbrev Number: 2 (DW_TAG_subprogram)
> <c0> DW_AT_low_pc : 0x4004a7
> <c8> DW_AT_high_pc : 0x4004b2
> <d0> DW_AT_specification: <0xd5>
> <1><d4>: Abbrev Number: 0
> Compilation Unit @ offset 0xd5:
> Length: 0x7 (32-bit)
> Version: 4
> Abbrev Offset: 0x7f
> Pointer Size: 8
> ...
> and indeed I get:
> ...
> $ gdb -q -batch outputs/gdb.dwarf2/dw2-inter-cu-error-2/dw2-inter-cu-error-2
>
> Fatal signal: Segmentation fault
> ...
>
> The problem is that we're calling prepare_one_comp_unit with cu == nullptr and
> comp_unit_die == nullptr here in cooked_indexer::ensure_cu_exists:
> ...
> cutu_reader new_reader (per_cu, per_objfile, nullptr, nullptr, false,
> m_index_storage->get_abbrev_cache ());
>
> prepare_one_comp_unit (new_reader.cu, new_reader.comp_unit_die,
> language_minimal);
> ...
>
> Fix this by bailing out for various types of dummy CUs:
> ...
> if (new_reader.dummy_p || new_reader.comp_unit_die == nullptr
> || !new_reader.comp_unit_die->has_children)
> return nullptr;
> ...
>
> Also make sure in scan_attributes that this triggers a dwarf error:
> ...
> $ gdb -q -batch dw2-inter-cu-error-2
> DWARF Error: cannot follow reference to DIE at 0xd5 \
> [in module dw2-inter-cu-error-2]
> ...
>
> With target board readnow, the test-case triggers an assertion failure in
> follow_die_offset, so fix this by throwing the same dwarf error.
>
> While we're at it, make the other check for dummy CUs in
> cooked_indexer::ensure_cu_exists more robust by adding an intermediate test
> for comp_unit_die:
> ...
> - if (result->dummy_p || !result->comp_unit_die->has_children)
> + if (result->dummy_p || result->comp_unit_die == nullptr
> + || !result->comp_unit_die->has_children)
> return nullptr;
> ...
>
Ping.
Thanks,
- Tom
> Tested on x86_64-linux.
> ---
> gdb/dwarf2/read.c | 92 +++++++++++--------
> .../gdb.dwarf2/dw2-inter-cu-error-2.exp | 51 ++++++++++
> 2 files changed, 104 insertions(+), 39 deletions(-)
> create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-inter-cu-error-2.exp
>
> diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
> index 769ca91facc..3b6445d4dd5 100644
> --- a/gdb/dwarf2/read.c
> +++ b/gdb/dwarf2/read.c
> @@ -16063,6 +16063,10 @@ cooked_indexer::ensure_cu_exists (cutu_reader *reader,
> cutu_reader new_reader (per_cu, per_objfile, nullptr, nullptr, false,
> m_index_storage->get_abbrev_cache ());
>
> + if (new_reader.dummy_p || new_reader.comp_unit_die == nullptr
> + || !new_reader.comp_unit_die->has_children)
> + return nullptr;
> +
> prepare_one_comp_unit (new_reader.cu, new_reader.comp_unit_die,
> language_minimal);
> std::unique_ptr<cutu_reader> copy
> @@ -16070,7 +16074,8 @@ cooked_indexer::ensure_cu_exists (cutu_reader *reader,
> result = m_index_storage->preserve (std::move (copy));
> }
>
> - if (result->dummy_p || !result->comp_unit_die->has_children)
> + if (result->dummy_p || result->comp_unit_die == nullptr
> + || !result->comp_unit_die->has_children)
> return nullptr;
>
> if (for_scanning)
> @@ -16256,49 +16261,53 @@ cooked_indexer::scan_attributes (dwarf2_per_cu_data *scanning_per_cu,
> cutu_reader *new_reader
> = ensure_cu_exists (reader, reader->cu->per_objfile, origin_offset,
> origin_is_dwz, false);
> - if (new_reader != nullptr)
> - {
> - const gdb_byte *new_info_ptr = (new_reader->buffer
> - + to_underlying (origin_offset));
> + if (new_reader == nullptr)
> + error (_(DWARF_ERROR_PREFIX
> + "cannot follow reference to DIE at %s"
> + " [in module %s]"),
> + sect_offset_str (origin_offset),
> + bfd_get_filename (reader->abfd));
> +
> + const gdb_byte *new_info_ptr = (new_reader->buffer
> + + to_underlying (origin_offset));
>
> - if (*parent_entry == nullptr)
> - {
> - /* We only perform immediate lookups of parents for DIEs
> - from earlier in this CU. This avoids any problem
> - with a NULL result when when we see a reference to a
> - DIE in another CU that we may or may not have
> - imported locally. */
> - parent_map::addr_type addr
> - = parent_map::form_addr (origin_offset, origin_is_dwz);
> - if (new_reader->cu != reader->cu || new_info_ptr > watermark_ptr)
> - *maybe_defer = addr;
> - else
> - *parent_entry = m_die_range_map->find (addr);
> - }
> + if (*parent_entry == nullptr)
> + {
> + /* We only perform immediate lookups of parents for DIEs
> + from earlier in this CU. This avoids any problem
> + with a NULL result when when we see a reference to a
> + DIE in another CU that we may or may not have
> + imported locally. */
> + parent_map::addr_type addr
> + = parent_map::form_addr (origin_offset, origin_is_dwz);
> + if (new_reader->cu != reader->cu || new_info_ptr > watermark_ptr)
> + *maybe_defer = addr;
> + else
> + *parent_entry = m_die_range_map->find (addr);
> + }
>
> - unsigned int bytes_read;
> - const abbrev_info *new_abbrev = peek_die_abbrev (*new_reader,
> - new_info_ptr,
> - &bytes_read);
> + unsigned int bytes_read;
> + const abbrev_info *new_abbrev = peek_die_abbrev (*new_reader,
> + new_info_ptr,
> + &bytes_read);
>
> - if (new_abbrev == nullptr)
> - error (_(DWARF_ERROR_PREFIX
> - "Unexpected null DIE at offset %s [in module %s]"),
> - sect_offset_str (origin_offset),
> - bfd_get_filename (new_reader->abfd));
> + if (new_abbrev == nullptr)
> + error (_(DWARF_ERROR_PREFIX
> + "Unexpected null DIE at offset %s [in module %s]"),
> + sect_offset_str (origin_offset),
> + bfd_get_filename (new_reader->abfd));
>
> - new_info_ptr += bytes_read;
> + new_info_ptr += bytes_read;
>
> - if (new_reader->cu == reader->cu && new_info_ptr == watermark_ptr)
> - {
> - /* Self-reference, we're done. */
> - }
> - else
> - scan_attributes (scanning_per_cu, new_reader, new_info_ptr,
> - new_info_ptr, new_abbrev, name, linkage_name,
> - flags, nullptr, parent_entry, maybe_defer,
> - is_enum_class, true);
> + if (new_reader->cu == reader->cu && new_info_ptr == watermark_ptr)
> + {
> + /* Self-reference, we're done. */
> }
> + else
> + scan_attributes (scanning_per_cu, new_reader, new_info_ptr,
> + new_info_ptr, new_abbrev, name, linkage_name,
> + flags, nullptr, parent_entry, maybe_defer,
> + is_enum_class, true);
> }
>
> if (!for_specification)
> @@ -20353,7 +20362,12 @@ follow_die_offset (sect_offset sect_off, int offset_in_dwz,
> false, cu->lang ());
>
> target_cu = per_objfile->get_cu (per_cu);
> - gdb_assert (target_cu != nullptr);
> + if (target_cu == nullptr)
> + error (_(DWARF_ERROR_PREFIX
> + "cannot follow reference to DIE at %s"
> + " [in module %s]"),
> + sect_offset_str (sect_off),
> + objfile_name (per_objfile->objfile));
> }
> else if (cu->dies == NULL)
> {
> diff --git a/gdb/testsuite/gdb.dwarf2/dw2-inter-cu-error-2.exp b/gdb/testsuite/gdb.dwarf2/dw2-inter-cu-error-2.exp
> new file mode 100644
> index 00000000000..585fd54ed15
> --- /dev/null
> +++ b/gdb/testsuite/gdb.dwarf2/dw2-inter-cu-error-2.exp
> @@ -0,0 +1,51 @@
> +# Copyright 2024 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/>.
> +
> +# Check that GDB doesn't crash on invalid dwarf, specifically an inter-CU
> +# reference pointing to a dummy CU.
> +
> +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
> +
> +# Create the DWARF.
> +set asm_file [standard_output_file $srcfile2]
> +Dwarf::assemble $asm_file {
> + declare_labels label1
> +
> + cu {} {
> + compile_unit {{language @DW_LANG_C}} {
> + subprogram {
> + {MACRO_AT_range { main }}
> + {DW_AT_specification %$label1}
> + }
> + }
> + }
> +
> + label1: cu {} {
> + }
> +}
> +
> +if [prepare_for_testing "failed to prepare" $testfile \
> + [list $asm_file $srcfile] {nodebug}] {
> + return -1
> +}
> +
> +gdb_assert \
> + { [regexp "DWARF Error: cannot follow reference" $gdb_file_cmd_msg] } \
> + "Error message"
>
> base-commit: b281480b26f1f27dc83ad3271fb2eaf9736dc3e0
More information about the Gdb-patches
mailing list