From patchwork Mon Jan 29 01:15:04 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Marchi X-Patchwork-Id: 25656 Received: (qmail 101966 invoked by alias); 29 Jan 2018 01:15:28 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 101780 invoked by uid 89); 29 Jan 2018 01:15:09 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, SPF_HELO_PASS, SPF_PASS, T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy=6526 X-HELO: simark.ca Received: from simark.ca (HELO simark.ca) (158.69.221.121) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 29 Jan 2018 01:15:06 +0000 Received: from [10.0.0.11] (192-222-251-162.qc.cable.ebox.net [192.222.251.162]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id E81CE1E513; Sun, 28 Jan 2018 20:15:04 -0500 (EST) Subject: Re: [PATCH 4/7] Class-fy partial_die_info To: Yao Qi , gdb-patches@sourceware.org References: <1516873114-7449-1-git-send-email-yao.qi@linaro.org> <1516873114-7449-5-git-send-email-yao.qi@linaro.org> From: Simon Marchi Message-ID: Date: Sun, 28 Jan 2018 20:15:04 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.2 MIME-Version: 1.0 In-Reply-To: <1516873114-7449-5-git-send-email-yao.qi@linaro.org> On 2018-01-25 04:38 AM, Yao Qi wrote: > This patch is to class-fy partial_die_info. Two things special here, > > - disable assignment operator, but keep copy ctor, which is used in > load_partial_dies, > - have a private ctor which is only used by dwarf2_cu::find_partial_die, > I don't want other code use it, so make it private, Hi Yao, From what I understand, the only reason to have that private constructor is to construct a temporary partial_die_info object used to search in the htab, is that right? If so, converting that htab_t to an std::unordered_map would remove the need for all this, since you don't need to construct an object to search it. See the diff below that applies on top of this patch. It's not thoroughly tested and I am not sure of the validity of the per_cu->cu->partial_dies.empty () call in find_partial_die, but I think it should work. Plus, it adds some type-safety, which I am a big fan of. But otherwise, the patch is fine with me. Simon From dacaf6028e921efeb8c7420db7f663e5b4d7e8f1 Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Sun, 28 Jan 2018 19:47:01 -0500 Subject: [PATCH] use std::unordered_map --- gdb/dwarf2read.c | 115 +++++++++++++------------------------------------------ 1 file changed, 26 insertions(+), 89 deletions(-) diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c index 8996f49bae..330ca87a1d 100644 --- a/gdb/dwarf2read.c +++ b/gdb/dwarf2read.c @@ -652,6 +652,8 @@ struct delayed_method_info struct die_info *die; }; +struct partial_die_info; + /* Internal state when decoding a particular compilation unit. */ struct dwarf2_cu { @@ -686,9 +688,9 @@ struct dwarf2_cu distinguish these in buildsym.c. */ struct pending **list_in_scope = nullptr; - /* Hash table holding all the loaded partial DIEs - with partial_die->offset.SECT_OFF as hash. */ - htab_t partial_dies = nullptr; + /* Hash map holding all the loaded partial DIEs + with their section offset as the key. */ + std::unordered_map partial_dies; /* Storage for things with the same lifetime as this read-in compilation unit, including partial DIEs. */ @@ -1493,36 +1495,6 @@ struct partial_die_info struct partial_die_info *die_parent = nullptr; struct partial_die_info *die_child = nullptr; struct partial_die_info *die_sibling = nullptr; - - friend struct partial_die_info * - dwarf2_cu::find_partial_die (sect_offset sect_off); - - private: - /* Only need to do look up in dwarf2_cu::find_partial_die. */ - partial_die_info (sect_offset sect_off) - : partial_die_info (sect_off, DW_TAG_padding, 0) - { - } - - partial_die_info (sect_offset sect_off_, enum dwarf_tag tag_, - int has_children_) - : sect_off (sect_off_), tag (tag_), has_children (has_children_) - { - is_external = 0; - is_declaration = 0; - has_type = 0; - has_specification = 0; - has_pc_info = 0; - may_be_inlined = 0; - main_subprogram = 0; - scope_set = 0; - has_byte_size = 0; - has_const_value = 0; - has_template_arguments = 0; - fixup_called = 0; - is_dwz = 0; - spec_is_dwz = 0; - } }; /* This data structure holds the information of an abbrev. */ @@ -2180,10 +2152,6 @@ static const gdb_byte *skip_one_die (const struct die_reader_specs *reader, const gdb_byte *info_ptr, struct abbrev_info *abbrev); -static hashval_t partial_die_hash (const void *item); - -static int partial_die_eq (const void *item_lhs, const void *item_rhs); - static struct dwarf2_per_cu_data *dwarf2_find_containing_comp_unit (sect_offset sect_off, unsigned int offset_in_dwz, struct dwarf2_per_objfile *dwarf2_per_objfile); @@ -18259,14 +18227,7 @@ load_partial_dies (const struct die_reader_specs *reader, if (cu->per_cu->load_all_dies) load_all = 1; - cu->partial_dies - = htab_create_alloc_ex (cu->header.length / 12, - partial_die_hash, - partial_die_eq, - NULL, - &cu->comp_unit_obstack, - hashtab_obstack_allocate, - dummy_obstack_deallocate); + cu->partial_dies.clear (); while (1) { @@ -18459,14 +18420,7 @@ load_partial_dies (const struct die_reader_specs *reader, || abbrev->tag == DW_TAG_variable || abbrev->tag == DW_TAG_namespace || part_die->is_declaration) - { - void **slot; - - slot = htab_find_slot_with_hash (cu->partial_dies, part_die, - to_underlying (part_die->sect_off), - INSERT); - *slot = part_die; - } + cu->partial_dies[part_die->sect_off] = part_die; /* For some DIEs we want to follow their children (if any). For C we have no reason to follow the children of structures; for other @@ -18512,8 +18466,22 @@ load_partial_dies (const struct die_reader_specs *reader, partial_die_info::partial_die_info (sect_offset sect_off_, struct abbrev_info *abbrev) - : partial_die_info (sect_off_, abbrev->tag, abbrev->has_children) -{ + : sect_off (sect_off_), tag (abbrev->tag), has_children (abbrev->has_children) +{ + is_external = 0; + is_declaration = 0; + has_type = 0; + has_specification = 0; + has_pc_info = 0; + may_be_inlined = 0; + main_subprogram = 0; + scope_set = 0; + has_byte_size = 0; + has_const_value = 0; + has_template_arguments = 0; + fixup_called = 0; + is_dwz = 0; + spec_is_dwz = 0; } /* Read a minimal amount of information into the minimal die structure. */ @@ -18735,14 +18703,9 @@ read_partial_die (const struct die_reader_specs *reader, struct partial_die_info * dwarf2_cu::find_partial_die (sect_offset sect_off) { - struct partial_die_info *lookup_die = NULL; - struct partial_die_info part_die (sect_off); - - lookup_die = ((struct partial_die_info *) - htab_find_with_hash (partial_dies, &part_die, - to_underlying (sect_off))); + auto it = partial_dies.find (sect_off); - return lookup_die; + return it != partial_dies.end () ? it->second : nullptr; } /* Find a partial DIE at OFFSET, which may or may not be in CU, @@ -18782,7 +18745,7 @@ find_partial_die (sect_offset sect_off, int offset_in_dwz, struct dwarf2_cu *cu) per_cu = dwarf2_find_containing_comp_unit (sect_off, offset_in_dwz, dwarf2_per_objfile); - if (per_cu->cu == NULL || per_cu->cu->partial_dies == NULL) + if (per_cu->cu == NULL || per_cu->cu->partial_dies.empty ()) load_partial_comp_unit (per_cu); per_cu->cu->last_used = 0; @@ -25483,32 +25446,6 @@ dwarf2_clear_marks (struct dwarf2_per_cu_data *per_cu) } } -/* Trivial hash function for partial_die_info: the hash value of a DIE - is its offset in .debug_info for this objfile. */ - -static hashval_t -partial_die_hash (const void *item) -{ - const struct partial_die_info *part_die - = (const struct partial_die_info *) item; - - return to_underlying (part_die->sect_off); -} - -/* Trivial comparison function for partial_die_info structures: two DIEs - are equal if they have the same offset. */ - -static int -partial_die_eq (const void *item_lhs, const void *item_rhs) -{ - const struct partial_die_info *part_die_lhs - = (const struct partial_die_info *) item_lhs; - const struct partial_die_info *part_die_rhs - = (const struct partial_die_info *) item_rhs; - - return part_die_lhs->sect_off == part_die_rhs->sect_off; -} - static struct cmd_list_element *set_dwarf_cmdlist; static struct cmd_list_element *show_dwarf_cmdlist;