From 4a636814934b4403baeffbf29af44ed3a3cb2962 Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Thu, 22 Oct 2020 10:27:39 -0400 Subject: [PATCH] gdb/dwarf: fix reading subprogram with DW_AT_specification (PR gdb/26693) Fix a regression introduced by commit 7188ed02d2a7 ("Replace dwarf2_per_cu_data::cu backlink with per-objfile map"). This patch targets both master and gdb-10-branch, since this is a regression from GDB 9. Analysis -------- The DWARF generated by the included test case looks like: 0x0000000b: DW_TAG_compile_unit DW_AT_language [DW_FORM_sdata] (4) 0x0000000d: DW_TAG_base_type DW_AT_name [DW_FORM_string] ("int") DW_AT_byte_size [DW_FORM_data1] (0x04) DW_AT_encoding [DW_FORM_sdata] (5) 0x00000014: DW_TAG_subprogram DW_AT_name [DW_FORM_string] ("apply") 0x0000001b: DW_TAG_subprogram DW_AT_specification [DW_FORM_ref4] (0x00000014 "apply") DW_AT_low_pc [DW_FORM_addr] (0x0000000000001234) DW_AT_high_pc [DW_FORM_data8] (0x0000000000000020) 0x00000030: DW_TAG_template_type_parameter DW_AT_name [DW_FORM_string] ("T") DW_AT_type [DW_FORM_ref4] (0x0000000d "int") 0x00000037: NULL 0x00000038: NULL Simply loading the file in GDB makes it crash: $ ./gdb -nx --data-directory=data-directory testsuite/outputs/gdb.dwarf2/pr26693/pr26693 [1] 15188 abort (core dumped) ./gdb -nx --data-directory=data-directory The crash happens here, where htab (a dwarf2_cu::die_hash field) is unexpectedly NULL while generating partial symbols: #0 0x000055555fa28188 in htab_find_with_hash (htab=0x0, element=0x7fffffffbfa0, hash=27) at /home/simark/src/binutils-gdb/libiberty/hashtab.c:591 #1 0x000055555cb4eb2e in follow_die_offset (sect_off=(unknown: 27), offset_in_dwz=0, ref_cu=0x7fffffffc110) at /home/simark/src/binutils-gdb/gdb/dwarf2/read.c:22951 #2 0x000055555cb4edfb in follow_die_ref (src_die=0x0, attr=0x7fffffffc130, ref_cu=0x7fffffffc110) at /home/simark/src/binutils-gdb/gdb/dwarf2/read.c:22968 #3 0x000055555caa48c5 in partial_die_full_name (pdi=0x621000157e70, cu=0x615000023f80) at /home/simark/src/binutils-gdb/gdb/dwarf2/read.c:8441 #4 0x000055555caa4d79 in add_partial_symbol (pdi=0x621000157e70, cu=0x615000023f80) at /home/simark/src/binutils-gdb/gdb/dwarf2/read.c:8469 #5 0x000055555caa7d8c in add_partial_subprogram (pdi=0x621000157e70, lowpc=0x7fffffffc5c0, highpc=0x7fffffffc5e0, set_addrmap=1, cu=0x615000023f80) at /home/simark/src/binutils-gdb/gdb/dwarf2/read.c:8737 #6 0x000055555caa265c in scan_partial_symbols (first_die=0x621000157e00, lowpc=0x7fffffffc5c0, highpc=0x7fffffffc5e0, set_addrmap=1, cu=0x615000023f80) at /home/simark/src/binutils-gdb/gdb/dwarf2/read.c:8230 #7 0x000055555ca98e3f in process_psymtab_comp_unit_reader (reader=0x7fffffffc6b0, info_ptr=0x60600009650d "\003int", comp_unit_die=0x621000157d10, pretend_language=language_minimal) at /home/simark/src/binutils-gdb/gdb/dwarf2/read.c:7614 #8 0x000055555ca9aa2c in process_psymtab_comp_unit (this_cu=0x621000155510, per_objfile=0x613000009f80, want_partial_unit=false, pretend_language=language_minimal) at /home/simark/src/binutils-gdb/gdb/dwarf2/read.c:7712 #9 0x000055555caa051a in dwarf2_build_psymtabs_hard (per_objfile=0x613000009f80) at /home/simark/src/binutils-gdb/gdb/dwarf2/read.c:8073 The special thing about this DWARF is that the subprogram at 0x1b is a template specialization described with DW_AT_specification, and has no DW_AT_name in itself. To compute the name of this subprogram, partial_die_full_name needs to load the full DIE for this partial DIE. The name is generated from the templated function name and the actual tempalate parameter values of the specialization. To load the full DIE, partial_die_full_name creates a dummy DWARF attribute of form DW_FORM_ref_addr that points to our subprogram's DIE, and calls follow_die_ref on it. This eventually causes load_full_comp_unit to be called for the exact same CU we are currently making partial symbols for: #0 load_full_comp_unit (this_cu=0x621000155510, per_objfile=0x613000009f80, skip_partial=false, pretend_language=language_minimal) at /home/simark/src/binutils-gdb/gdb/dwarf2/read.c:9238 #1 0x000055555cb4e943 in follow_die_offset (sect_off=(unknown: 27), offset_in_dwz=0, ref_cu=0x7fffffffc110) at /home/simark/src/binutils-gdb/gdb/dwarf2/read.c:22942 #2 0x000055555cb4edfb in follow_die_ref (src_die=0x0, attr=0x7fffffffc130, ref_cu=0x7fffffffc110) at /home/simark/src/binutils-gdb/gdb/dwarf2/read.c:22968 #3 0x000055555caa48c5 in partial_die_full_name (pdi=0x621000157e70, cu=0x615000023f80) at /home/simark/src/binutils-gdb/gdb/dwarf2/read.c:8441 #4 0x000055555caa4d79 in add_partial_symbol (pdi=0x621000157e70, cu=0x615000023f80) at /home/simark/src/binutils-gdb/gdb/dwarf2/read.c:8469 #5 0x000055555caa7d8c in add_partial_subprogram (pdi=0x621000157e70, lowpc=0x7fffffffc5c0, highpc=0x7fffffffc5e0, set_addrmap=1, cu=0x615000023f80) at /home/simark/src/binutils-gdb/gdb/dwarf2/read.c:8737 #6 0x000055555caa265c in scan_partial_symbols (first_die=0x621000157e00, lowpc=0x7fffffffc5c0, highpc=0x7fffffffc5e0, set_addrmap=1, cu=0x615000023f80) at /home/simark/src/binutils-gdb/gdb/dwarf2/read.c:8230 #7 0x000055555ca98e3f in process_psymtab_comp_unit_reader (reader=0x7fffffffc6b0, info_ptr=0x60600009650d "\003int", comp_unit_die=0x621000157d10, pretend_language=language_minimal) at /home/simark/src/binutils-gdb/gdb/dwarf2/read.c:7614 #8 0x000055555ca9aa2c in process_psymtab_comp_unit (this_cu=0x621000155510, per_objfile=0x613000009f80, want_partial_unit=false, pretend_language=language_minimal) at /home/simark/src/binutils-gdb/gdb/dwarf2/read.c:7712 #9 0x000055555caa051a in dwarf2_build_psymtabs_hard (per_objfile=0x613000009f80) at /home/simark/src/binutils-gdb/gdb/dwarf2/read.c:8073 load_full_comp_unit creates a cutu_reader for the CU. Since a dwarf2_cu object already exists for the CU, load_full_comp_unit is expected to find it and pass it to cutu_reader, so that cutu_reader doesn't create a new dwarf2_cu for the CU. And this is the difference between before and after the regression. Before commit 7188ed02d2a7, the dwarf2_per_cu_data -> dwarf2_cu link was a simple pointer in dwarf2_per_cu_data. This pointer was set up when starting the read the partial symbols. So it was already available at that point where load_full_comp_unit gets called. Post-7188ed02d2a7, this link is per-objfile, kept in the dwarf2_per_objfile::m_dwarf2_cus hash map. The entry is only put in the hash map once the partial symbols have been successfully read, when cutu_reader::keep is called. Therefore, it is _not_ set at the point load_full_comp_unit is called. As a consequence, a new dwarf2_cu object gets created and initialized by load_full_comp_unit (including initializing that dwarf2_cu::die_hash field). Meanwhile, the dwarf2_cu object created and used by the callers up the stack does not get initialized for full symbol reading, and the dwarf2_cu::die_hash field stays unexpectedly NULL. Solution -------- Since the caller of load_full_comp_unit knows about the existing dwarf2_cu object for the CU we are reading (the one load_full_comp_unit is expected to find), we can simply make it pass it down, instead of having load_full_comp_unit look up the per-objfile map. load_full_comp_unit therefore gets a new `existing_cu` parameter. All other callers get updated to pass `per_objfile->get_cu (per_cu)`, so the behavior shouldn't change for them, compared to the current HEAD. A test is added, which is the bare minimum to reproduce the issue. Notes ----- The original problem was reproduced by downloading https://github.com/oneapi-src/oneTBB/releases/download/v2020.3/tbb-2020.3-lin.tgz and loading libtbb.so in GDB. This code was compiled with the Intel C/C++ compiler. I was not able to reproduce the issue using GCC, I think because GCC puts a DW_AT_name in the specialized subprogram, so there's no need for partial_die_full_name to load the full DIE of the subprogram, and the faulty code doesn't execute. gdb/ChangeLog: PR gdb/26693 * dwarf2/read.c (load_full_comp_unit): Add existing_cu parameter. (load_cu): Pass existing CU. (process_imported_unit_die): Likewise. (follow_die_offset): Likewise. gdb/testsuite/ChangeLog: PR gdb/26693 * gdb.dwarf2/template-specification-full-name.exp: New test. Change-Id: I57c8042f96c45f15797a3848e4d384181c56bb44 --- gdb/ChangeLog | 9 +++ gdb/dwarf2/read.c | 22 ++++-- gdb/testsuite/ChangeLog | 5 ++ .../template-specification-full-name.exp | 77 +++++++++++++++++++ 4 files changed, 107 insertions(+), 6 deletions(-) create mode 100644 gdb/testsuite/gdb.dwarf2/template-specification-full-name.exp diff --git a/gdb/ChangeLog b/gdb/ChangeLog index a976442f9a4..bda88839b8d 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,12 @@ +2020-10-22 Simon Marchi + + PR gdb/26693 + * dwarf2/read.c (load_full_comp_unit): Add existing_cu + parameter. + (load_cu): Pass existing CU. + (process_imported_unit_die): Likewise. + (follow_die_offset): Likewise. + 2020-10-22 Luis Machado * corelow.c (core_target::xfer_partial): Also check for an empty diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c index 37409c5c3fc..436a116dc80 100644 --- a/gdb/dwarf2/read.c +++ b/gdb/dwarf2/read.c @@ -1606,6 +1606,7 @@ static int create_all_type_units (dwarf2_per_objfile *per_objfile); static void load_full_comp_unit (dwarf2_per_cu_data *per_cu, dwarf2_per_objfile *per_objfile, + dwarf2_cu *existing_cu, bool skip_partial, enum language pretend_language); @@ -2385,7 +2386,8 @@ load_cu (dwarf2_per_cu_data *per_cu, dwarf2_per_objfile *per_objfile, if (per_cu->is_debug_types) load_full_type_unit (per_cu, per_objfile); else - load_full_comp_unit (per_cu, per_objfile, skip_partial, language_minimal); + load_full_comp_unit (per_cu, per_objfile, per_objfile->get_cu (per_cu), + skip_partial, language_minimal); dwarf2_cu *cu = per_objfile->get_cu (per_cu); if (cu == nullptr) @@ -9226,17 +9228,22 @@ die_eq (const void *item_lhs, const void *item_rhs) return die_lhs->sect_off == die_rhs->sect_off; } -/* Load the DIEs associated with PER_CU into memory. */ +/* Load the DIEs associated with PER_CU into memory. + + In some cases, the caller, while reading partial symbols, will need to load + the full symbols for the CU for some reason. It will already have a + dwarf2_cu object for THIS_CU and pass it as EXISTING_CU, so it can be re-used + rather than creating a new one. */ static void load_full_comp_unit (dwarf2_per_cu_data *this_cu, dwarf2_per_objfile *per_objfile, + dwarf2_cu *existing_cu, bool skip_partial, enum language pretend_language) { gdb_assert (! this_cu->is_debug_types); - dwarf2_cu *existing_cu = per_objfile->get_cu (this_cu); cutu_reader reader (this_cu, per_objfile, NULL, existing_cu, skip_partial); if (reader.dummy_p) return; @@ -10102,7 +10109,8 @@ process_imported_unit_die (struct die_info *die, struct dwarf2_cu *cu) /* If necessary, add it to the queue and load its DIEs. */ if (maybe_queue_comp_unit (cu, per_cu, per_objfile, cu->language)) - load_full_comp_unit (per_cu, per_objfile, false, cu->language); + load_full_comp_unit (per_cu, per_objfile, per_objfile->get_cu (per_cu), + false, cu->language); cu->per_cu->imported_symtabs_push (per_cu); } @@ -22932,7 +22940,8 @@ follow_die_offset (sect_offset sect_off, int offset_in_dwz, /* If necessary, add it to the queue and load its DIEs. */ if (maybe_queue_comp_unit (cu, per_cu, per_objfile, cu->language)) - load_full_comp_unit (per_cu, per_objfile, false, cu->language); + load_full_comp_unit (per_cu, per_objfile, per_objfile->get_cu (per_cu), + false, cu->language); target_cu = per_objfile->get_cu (per_cu); } @@ -22940,7 +22949,8 @@ follow_die_offset (sect_offset sect_off, int offset_in_dwz, { /* We're loading full DIEs during partial symbol reading. */ gdb_assert (per_objfile->per_bfd->reading_partial_symbols); - load_full_comp_unit (cu->per_cu, per_objfile, false, language_minimal); + load_full_comp_unit (cu->per_cu, per_objfile, cu, false, + language_minimal); } *ref_cu = target_cu; diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index f6084b892b6..f75c7327863 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,3 +1,8 @@ +2020-10-22 Simon Marchi + + PR gdb/26693 + * gdb.dwarf2/template-specification-full-name.exp: New test. + 2020-10-22 Luis Machado * gdb.base/msym-bp-shl.exp (test_break): Adjust pattern to not diff --git a/gdb/testsuite/gdb.dwarf2/template-specification-full-name.exp b/gdb/testsuite/gdb.dwarf2/template-specification-full-name.exp new file mode 100644 index 00000000000..259bdf56ad5 --- /dev/null +++ b/gdb/testsuite/gdb.dwarf2/template-specification-full-name.exp @@ -0,0 +1,77 @@ +# Copyright 2020 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 . + +# Test to reproduce the crash described in PR 26693. The following DWARF was +# crashing GDB while loading partial symbols: a DW_TAG_subprogram with a +# DW_AT_specification (pointing to another subprogram), without a DW_AT_name +# and with a template parameter (DW_TAG_template_type_parameter). More +# precisely, the crash was happening when trying to compute the full name of the +# subprogram. + +load_lib dwarf.exp + +if {![dwarf2_support]} { + return 0 +} + +standard_testfile main.c .S + +set asm_file [standard_output_file $srcfile2] +Dwarf::assemble $asm_file { + cu {} { + DW_TAG_compile_unit { + {DW_AT_language @DW_LANG_C_plus_plus} + } { + declare_labels templated_subprogram int + + int: DW_TAG_base_type { + {DW_AT_name "int"} + {DW_AT_byte_size 4 DW_FORM_data1} + {DW_AT_encoding @DW_ATE_signed} + } + + # The templated subprogram. + templated_subprogram: DW_TAG_subprogram { + {DW_AT_name "apply"} + } + + # The template specialization. + # + # The low and high PC are phony: we just need an address range that + # is valid in the program, so we use the main function's range. + DW_TAG_subprogram { + {DW_AT_specification :$templated_subprogram} + {MACRO_AT_range main} + } { + DW_TAG_template_type_param { + {DW_AT_name "T"} + {DW_AT_type :$int DW_FORM_ref4} + } + } + } + } +} + +if { [prepare_for_testing "failed to prepare" ${testfile} \ + [list $srcfile $asm_file] {nodebug}] } { + return -1 +} + +if ![runto_main] { + return -1 +} + +# Just a sanity check to make sure GDB slurped the symbols correctly. +gdb_test "print apply" " = {void \\(void\\)} $hex \\(\\)>" -- 2.30.2