From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from rock.gnat.com (rock.gnat.com [205.232.38.15]) by sourceware.org (Postfix) with ESMTP id 4DEB03846077 for ; Thu, 6 May 2021 17:23:42 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 4DEB03846077 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=adacore.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=tromey@adacore.com Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id 2C29956315; Thu, 6 May 2021 13:23:42 -0400 (EDT) X-Virus-Scanned: Debian amavisd-new at gnat.com Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id beJSL-1hCT13; Thu, 6 May 2021 13:23:42 -0400 (EDT) Received: from murgatroyd (97-122-70-176.hlrn.qwest.net [97.122.70.176]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by rock.gnat.com (Postfix) with ESMTPSA id D030656310; Thu, 6 May 2021 13:23:41 -0400 (EDT) From: Tom Tromey To: Tom Tromey Cc: gdb-patches@sourceware.org Subject: Re: [PATCH 1/3] Add virtual destructor to dwarf2_per_cu_data References: <20210503193206.4008066-1-tromey@adacore.com> <20210503193206.4008066-2-tromey@adacore.com> <87r1imsebb.fsf@tromey.com> X-Attribution: Tom Date: Thu, 06 May 2021 11:23:41 -0600 In-Reply-To: <87r1imsebb.fsf@tromey.com> (Tom Tromey's message of "Tue, 04 May 2021 09:34:32 -0600") Message-ID: <87mtt7pyhu.fsf@tromey.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-Spam-Status: No, score=-11.2 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_STATUS, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 06 May 2021 17:23:44 -0000 Tom> I'm thinking that instead I should add a custom deleter for these Tom> objects that knows to cast to signatured_type when appropriate. This Tom> would avoid a vtable pointer just to hold the destructor. Here's the patch. Tom commit 25e4481af0d85f3c22c824323c42bc23438c1cbf Author: Tom Tromey Date: Mon May 3 12:42:58 2021 -0600 Change how dwarf2_per_cu_data is deleted Address sanitizer pointed out that the patch to use 'delete' for dwarf2_per_cu_data introduced a bug -- now it is possible to delete a signatured_type using a pointer to its base class. This patch fixes the problem by introducing a deleter and a unique_ptr specialization. A virtual destructor would be more ordinary here, but it seemed wasteful to add a vtable just for this purpose. If virtual methods are ever needed here, we can revisit this. 2021-05-06 Tom Tromey * dwarf2/read.h (struct dwarf2_per_cu_data_deleter: New. (dwarf2_per_cu_data_up): New typedef. (struct dwarf2_per_bfd) : Change return type. : Use dwarf2_per_cu_data_up. * dwarf2/read.c (dwarf2_per_cu_data::operator()): New function. (dwarf2_per_bfd::allocate_per_cu): Return dwarf2_per_cu_data_up. (create_cu_from_index_list): Likewise. (create_signatured_type_table_from_index) (create_cus_from_debug_names_list, add_type_unit) (read_comp_units_from_section): Update. (dwarf2_find_containing_comp_unit): Change type of all_comp_units. (run_test): Update. diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 187249db47e..78a3dadbdd5 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,18 @@ +2021-05-06 Tom Tromey + + * dwarf2/read.h (struct dwarf2_per_cu_data_deleter: New. + (dwarf2_per_cu_data_up): New typedef. + (struct dwarf2_per_bfd) : Change return type. + : Use dwarf2_per_cu_data_up. + * dwarf2/read.c (dwarf2_per_cu_data::operator()): New function. + (dwarf2_per_bfd::allocate_per_cu): Return dwarf2_per_cu_data_up. + (create_cu_from_index_list): Likewise. + (create_signatured_type_table_from_index) + (create_cus_from_debug_names_list, add_type_unit) + (read_comp_units_from_section): Update. + (dwarf2_find_containing_comp_unit): Change type of all_comp_units. + (run_test): Update. + 2021-05-06 Andrew Burgess * guile/scm-breakpoint.c (bpscm_print_breakpoint_smob): Only print diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c index 5796cf1730b..e1965c193ed 100644 --- a/gdb/dwarf2/read.c +++ b/gdb/dwarf2/read.c @@ -1760,6 +1760,17 @@ dwarf2_queue_item::~dwarf2_queue_item () } } +/* See dwarf2/read.h. */ + +void +dwarf2_per_cu_data_deleter::operator() (dwarf2_per_cu_data *data) +{ + if (data->is_debug_types) + delete static_cast (data); + else + delete data; +} + /* The return type of find_file_and_directory. Note, the enclosed string pointers are only valid while this object is valid. */ @@ -2522,10 +2533,10 @@ dw2_instantiate_symtab (dwarf2_per_cu_data *per_cu, /* See read.h. */ -std::unique_ptr +dwarf2_per_cu_data_up dwarf2_per_bfd::allocate_per_cu () { - std::unique_ptr result (new dwarf2_per_cu_data); + dwarf2_per_cu_data_up result (new dwarf2_per_cu_data); result->per_bfd = this; result->index = m_num_psymtabs++; return result; @@ -2546,13 +2557,13 @@ dwarf2_per_bfd::allocate_signatured_type () /* Return a new dwarf2_per_cu_data allocated on the per-bfd obstack, and constructed with the specified field values. */ -static std::unique_ptr +static dwarf2_per_cu_data_up create_cu_from_index_list (dwarf2_per_bfd *per_bfd, struct dwarf2_section_info *section, int is_dwz, sect_offset sect_off, ULONGEST length) { - std::unique_ptr the_cu = per_bfd->allocate_per_cu (); + dwarf2_per_cu_data_up the_cu = per_bfd->allocate_per_cu (); the_cu->sect_off = sect_off; the_cu->length = length; the_cu->section = section; @@ -2580,7 +2591,7 @@ create_cus_from_index_list (dwarf2_per_bfd *per_bfd, ULONGEST length = extract_unsigned_integer (cu_list + 8, 8, BFD_ENDIAN_LITTLE); cu_list += 2 * 8; - std::unique_ptr per_cu + dwarf2_per_cu_data_up per_cu = create_cu_from_index_list (per_bfd, section, is_dwz, sect_off, length); per_bfd->all_comp_units.push_back (std::move (per_cu)); @@ -2647,7 +2658,7 @@ create_signatured_type_table_from_index slot = htab_find_slot (sig_types_hash.get (), sig_type.get (), INSERT); *slot = sig_type.get (); - per_bfd->all_comp_units.push_back (std::move (sig_type)); + per_bfd->all_comp_units.emplace_back (sig_type.release ()); } per_bfd->signatured_types = std::move (sig_types_hash); @@ -2699,7 +2710,7 @@ create_signatured_type_table_from_debug_names slot = htab_find_slot (sig_types_hash.get (), sig_type.get (), INSERT); *slot = sig_type.get (); - per_objfile->per_bfd->all_comp_units.push_back (std::move (sig_type)); + per_objfile->per_bfd->all_comp_units.emplace_back (sig_type.release ()); } per_objfile->per_bfd->signatured_types = std::move (sig_types_hash); @@ -4930,7 +4941,7 @@ create_cus_from_debug_names_list (dwarf2_per_bfd *per_bfd, of the next CU as end of this CU. We create the CUs here with length 0, and in cutu_reader::cutu_reader we'll fill in the actual length. */ - std::unique_ptr per_cu + dwarf2_per_cu_data_up per_cu = create_cu_from_index_list (per_bfd, §ion, is_dwz, sect_off, 0); per_bfd->all_comp_units.push_back (std::move (per_cu)); @@ -4955,7 +4966,7 @@ create_cus_from_debug_names_list (dwarf2_per_bfd *per_bfd, if (i >= 1) { const ULONGEST length = sect_off_next - sect_off_prev; - std::unique_ptr per_cu + dwarf2_per_cu_data_up per_cu = create_cu_from_index_list (per_bfd, §ion, is_dwz, sect_off_prev, length); per_bfd->all_comp_units.push_back (std::move (per_cu)); @@ -6132,7 +6143,8 @@ add_type_unit (dwarf2_per_objfile *per_objfile, ULONGEST sig, void **slot) per_objfile->resize_symtabs (); - per_objfile->per_bfd->all_comp_units.push_back (std::move (sig_type_holder)); + per_objfile->per_bfd->all_comp_units.emplace_back + (sig_type_holder.release ()); sig_type->signature = sig; sig_type->is_debug_types = 1; if (per_objfile->per_bfd->using_index) @@ -7711,7 +7723,7 @@ read_comp_units_from_section (dwarf2_per_objfile *per_objfile, while (info_ptr < section->buffer + section->size) { - std::unique_ptr this_cu; + dwarf2_per_cu_data_up this_cu; sect_offset sect_off = (sect_offset) (info_ptr - section->buffer); @@ -7732,7 +7744,7 @@ read_comp_units_from_section (dwarf2_per_objfile *per_objfile, signatured_type *sig_ptr = sig_type.get (); sig_type->signature = cu_header.signature; sig_type->type_offset_in_tu = cu_header.type_cu_offset_in_tu; - this_cu = std::move (sig_type); + this_cu.reset (sig_type.release ()); void **slot = htab_find_slot (types_htab.get (), sig_ptr, INSERT); gdb_assert (slot != nullptr); @@ -24581,7 +24593,7 @@ static int dwarf2_find_containing_comp_unit (sect_offset sect_off, unsigned int offset_in_dwz, - const std::vector> &all_comp_units) + const std::vector &all_comp_units) { int low, high; @@ -24647,13 +24659,13 @@ namespace find_containing_comp_unit { static void run_test () { - std::unique_ptr one (new dwarf2_per_cu_data); + dwarf2_per_cu_data_up one (new dwarf2_per_cu_data); dwarf2_per_cu_data *one_ptr = one.get (); - std::unique_ptr two (new dwarf2_per_cu_data); + dwarf2_per_cu_data_up two (new dwarf2_per_cu_data); dwarf2_per_cu_data *two_ptr = two.get (); - std::unique_ptr three (new dwarf2_per_cu_data); + dwarf2_per_cu_data_up three (new dwarf2_per_cu_data); dwarf2_per_cu_data *three_ptr = three.get (); - std::unique_ptr four (new dwarf2_per_cu_data); + dwarf2_per_cu_data_up four (new dwarf2_per_cu_data); dwarf2_per_cu_data *four_ptr = four.get (); one->length = 5; @@ -24666,7 +24678,7 @@ run_test () four->length = 7; four->is_dwz = 1; - std::vector> units; + std::vector units; units.push_back (std::move (one)); units.push_back (std::move (two)); units.push_back (std::move (three)); diff --git a/gdb/dwarf2/read.h b/gdb/dwarf2/read.h index 80f0e3ee13b..756d1934ca0 100644 --- a/gdb/dwarf2/read.h +++ b/gdb/dwarf2/read.h @@ -74,6 +74,20 @@ struct dwarf2_queue_item enum language pretend_language; }; +/* A deleter for dwarf2_per_cu_data that knows to downcast to + signatured_type as appropriate. This approach lets us avoid a + virtual destructor, which saves a bit of space. */ + +struct dwarf2_per_cu_data_deleter +{ + void operator() (dwarf2_per_cu_data *data); +}; + +/* A specialization of unique_ptr for dwarf2_per_cu_data and + subclasses. */ +typedef std::unique_ptr + dwarf2_per_cu_data_up; + /* Some DWARF data can be shared across objfiles who share the same BFD, this data is stored in this object. @@ -103,7 +117,7 @@ struct dwarf2_per_bfd /* A convenience function to allocate a dwarf2_per_cu_data. The returned object has its "index" field set properly. The object is allocated on the dwarf2_per_bfd obstack. */ - std::unique_ptr allocate_per_cu (); + dwarf2_per_cu_data_up allocate_per_cu (); /* A convenience function to allocate a signatured_type. The returned object has its "index" field set properly. The object @@ -154,7 +168,7 @@ struct dwarf2_per_bfd /* Table of all the compilation units. This is used to locate the target compilation unit of a particular reference. */ - std::vector> all_comp_units; + std::vector all_comp_units; /* Table of struct type_unit_group objects. The hash key is the DW_AT_stmt_list value. */