From patchwork Wed Nov 8 16:16:32 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pedro Alves X-Patchwork-Id: 24156 Received: (qmail 47218 invoked by alias); 8 Nov 2017 16:16:38 -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 46174 invoked by uid 89); 8 Nov 2017 16:16:37 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.8 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_STOCKGEN, RP_MATCHES_RCVD, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 08 Nov 2017 16:16:35 +0000 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 6540C356CB for ; Wed, 8 Nov 2017 16:16:34 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 6540C356CB Authentication-Results: ext-mx06.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx06.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=palves@redhat.com Received: from [127.0.0.1] (ovpn04.gateway.prod.ext.ams2.redhat.com [10.39.146.4]) by smtp.corp.redhat.com (Postfix) with ESMTP id 7B0EF64452; Wed, 8 Nov 2017 16:16:33 +0000 (UTC) Subject: [pushed] Reorder/reindent dw2_expand_symtabs_matching & friends (Re: [PATCH 26/40] Optimize .gdb_index symbol name searching) To: Keith Seitz , gdb-patches@sourceware.org References: <1496406158-12663-1-git-send-email-palves@redhat.com> <1496406158-12663-27-git-send-email-palves@redhat.com> From: Pedro Alves Message-ID: Date: Wed, 8 Nov 2017 16:16:32 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: On 11/08/2017 04:14 PM, Pedro Alves wrote: > Right, I should have mentioned -- I'd like to move > the new dw2_expand_symtabs_matching_symbol and dw2_expand_marked_cus > above dw2_expand_symtabs_matching, but I didn't do that here to > make the patch easier to read and maintain. Likewise with the > reindenting dw2_expand_marked_cus where I had marked with: > > /* XXX reindent code below before pushing. */ > > I'll move the code around in the following patch (a new patch), > getting rid of the forward declarations. Since I'm doing that > as a separate patch, it was also easier to leave the > reindentation to that patch too. > Here's what I pushed for that. From 61920122ba93d58cc2e8c78a30475c569c2506fd Mon Sep 17 00:00:00 2001 From: Pedro Alves Date: Wed, 8 Nov 2017 14:22:32 +0000 Subject: [PATCH] Reorder/reindent dw2_expand_symtabs_matching & friends The previous patch had added dw2_expand_symtabs_matching_symbol and dw2_expand_marked_cus forward declarations and did not reindent dw2_expand_marked_cus to avoid moving the code around while changing it at the same time. gdb/ChangeLog: 2017-11-08 Pedro Alves * dwarf2read.c (dw2_expand_marked_cus) (dw2_expand_symtabs_matching_symbol): Remove forward declarations. (dw2_expand_symtabs_matching): Move further below. (dw2_expand_marked_cus): Reindent. --- gdb/ChangeLog | 7 ++ gdb/dwarf2read.c | 334 ++++++++++++++++++++++++++----------------------------- 2 files changed, 165 insertions(+), 176 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index deb1614..9776cf2 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,5 +1,12 @@ 2017-11-08 Pedro Alves + * dwarf2read.c (dw2_expand_marked_cus) + (dw2_expand_symtabs_matching_symbol): Remove forward declarations. + (dw2_expand_symtabs_matching): Move further below. + (dw2_expand_marked_cus): Reindent. + +2017-11-08 Pedro Alves + * dwarf2read.c (byte_swap, MAYBE_SWAP): Move higher up in file. (struct name_component): New. (mapped_index::name_components): New field. diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c index 6f88091..d715082 100644 --- a/gdb/dwarf2read.c +++ b/gdb/dwarf2read.c @@ -4188,123 +4188,6 @@ gdb_index_symbol_name_matcher::matches (const char *symbol_name) return false; } -static void -dw2_expand_marked_cus - (mapped_index &index, offset_type idx, - struct objfile *objfile, - gdb::function_view file_matcher, - gdb::function_view expansion_notify, - search_domain kind); - -static void -dw2_expand_symtabs_matching_symbol - (mapped_index &index, - const lookup_name_info &lookup_name_in, - gdb::function_view symbol_matcher, - enum search_domain kind, - gdb::function_view on_match); - -static void -dw2_expand_symtabs_matching - (struct objfile *objfile, - gdb::function_view file_matcher, - const lookup_name_info &lookup_name, - gdb::function_view symbol_matcher, - gdb::function_view expansion_notify, - enum search_domain kind) -{ - int i; - offset_type iter; - - dw2_setup (objfile); - - /* index_table is NULL if OBJF_READNOW. */ - if (!dwarf2_per_objfile->index_table) - return; - - if (file_matcher != NULL) - { - htab_up visited_found (htab_create_alloc (10, htab_hash_pointer, - htab_eq_pointer, - NULL, xcalloc, xfree)); - htab_up visited_not_found (htab_create_alloc (10, htab_hash_pointer, - htab_eq_pointer, - NULL, xcalloc, xfree)); - - /* The rule is CUs specify all the files, including those used by - any TU, so there's no need to scan TUs here. */ - - for (i = 0; i < dwarf2_per_objfile->n_comp_units; ++i) - { - int j; - struct dwarf2_per_cu_data *per_cu = dw2_get_cu (i); - struct quick_file_names *file_data; - void **slot; - - QUIT; - - per_cu->v.quick->mark = 0; - - /* We only need to look at symtabs not already expanded. */ - if (per_cu->v.quick->compunit_symtab) - continue; - - file_data = dw2_get_file_names (per_cu); - if (file_data == NULL) - continue; - - if (htab_find (visited_not_found.get (), file_data) != NULL) - continue; - else if (htab_find (visited_found.get (), file_data) != NULL) - { - per_cu->v.quick->mark = 1; - continue; - } - - for (j = 0; j < file_data->num_file_names; ++j) - { - const char *this_real_name; - - if (file_matcher (file_data->file_names[j], false)) - { - per_cu->v.quick->mark = 1; - break; - } - - /* Before we invoke realpath, which can get expensive when many - files are involved, do a quick comparison of the basenames. */ - if (!basenames_may_differ - && !file_matcher (lbasename (file_data->file_names[j]), - true)) - continue; - - this_real_name = dw2_get_real_path (objfile, file_data, j); - if (file_matcher (this_real_name, false)) - { - per_cu->v.quick->mark = 1; - break; - } - } - - slot = htab_find_slot (per_cu->v.quick->mark - ? visited_found.get () - : visited_not_found.get (), - file_data, INSERT); - *slot = file_data; - } - } - - mapped_index &index = *dwarf2_per_objfile->index_table; - - dw2_expand_symtabs_matching_symbol (index, lookup_name, - symbol_matcher, - kind, [&] (offset_type idx) - { - dw2_expand_marked_cus (index, idx, objfile, file_matcher, - expansion_notify, kind); - }); -} - /* Helper for dw2_expand_symtabs_matching that works with a mapped_index instead of the containing objfile. This is split to a separate function in order to be able to unit test the @@ -4498,83 +4381,182 @@ dw2_expand_marked_cus offset_type *vec, vec_len, vec_idx; bool global_seen = false; - vec = (offset_type *) (index.constant_pool - + MAYBE_SWAP (index.symbol_table[idx + 1])); - vec_len = MAYBE_SWAP (vec[0]); - for (vec_idx = 0; vec_idx < vec_len; ++vec_idx) + vec = (offset_type *) (index.constant_pool + + MAYBE_SWAP (index.symbol_table[idx + 1])); + vec_len = MAYBE_SWAP (vec[0]); + for (vec_idx = 0; vec_idx < vec_len; ++vec_idx) + { + struct dwarf2_per_cu_data *per_cu; + offset_type cu_index_and_attrs = MAYBE_SWAP (vec[vec_idx + 1]); + /* This value is only valid for index versions >= 7. */ + int is_static = GDB_INDEX_SYMBOL_STATIC_VALUE (cu_index_and_attrs); + gdb_index_symbol_kind symbol_kind = + GDB_INDEX_SYMBOL_KIND_VALUE (cu_index_and_attrs); + int cu_index = GDB_INDEX_CU_VALUE (cu_index_and_attrs); + /* Only check the symbol attributes if they're present. + Indices prior to version 7 don't record them, + and indices >= 7 may elide them for certain symbols + (gold does this). */ + int attrs_valid = + (index.version >= 7 + && symbol_kind != GDB_INDEX_SYMBOL_KIND_NONE); + + /* Work around gold/15646. */ + if (attrs_valid) { - struct dwarf2_per_cu_data *per_cu; - offset_type cu_index_and_attrs = MAYBE_SWAP (vec[vec_idx + 1]); - /* This value is only valid for index versions >= 7. */ - int is_static = GDB_INDEX_SYMBOL_STATIC_VALUE (cu_index_and_attrs); - gdb_index_symbol_kind symbol_kind = - GDB_INDEX_SYMBOL_KIND_VALUE (cu_index_and_attrs); - int cu_index = GDB_INDEX_CU_VALUE (cu_index_and_attrs); - /* Only check the symbol attributes if they're present. - Indices prior to version 7 don't record them, - and indices >= 7 may elide them for certain symbols - (gold does this). */ - int attrs_valid = - (index.version >= 7 - && symbol_kind != GDB_INDEX_SYMBOL_KIND_NONE); + if (!is_static && global_seen) + continue; + if (!is_static) + global_seen = true; + } - /* Work around gold/15646. */ - if (attrs_valid) + /* Only check the symbol's kind if it has one. */ + if (attrs_valid) + { + switch (kind) { - if (!is_static && global_seen) + case VARIABLES_DOMAIN: + if (symbol_kind != GDB_INDEX_SYMBOL_KIND_VARIABLE) + continue; + break; + case FUNCTIONS_DOMAIN: + if (symbol_kind != GDB_INDEX_SYMBOL_KIND_FUNCTION) continue; - if (!is_static) - global_seen = true; + break; + case TYPES_DOMAIN: + if (symbol_kind != GDB_INDEX_SYMBOL_KIND_TYPE) + continue; + break; + default: + break; } + } - /* Only check the symbol's kind if it has one. */ - if (attrs_valid) - { - switch (kind) - { - case VARIABLES_DOMAIN: - if (symbol_kind != GDB_INDEX_SYMBOL_KIND_VARIABLE) - continue; - break; - case FUNCTIONS_DOMAIN: - if (symbol_kind != GDB_INDEX_SYMBOL_KIND_FUNCTION) - continue; - break; - case TYPES_DOMAIN: - if (symbol_kind != GDB_INDEX_SYMBOL_KIND_TYPE) - continue; - break; - default: - break; - } - } + /* Don't crash on bad data. */ + if (cu_index >= (dwarf2_per_objfile->n_comp_units + + dwarf2_per_objfile->n_type_units)) + { + complaint (&symfile_complaints, + _(".gdb_index entry has bad CU index" + " [in module %s]"), objfile_name (objfile)); + continue; + } - /* Don't crash on bad data. */ - if (cu_index >= (dwarf2_per_objfile->n_comp_units - + dwarf2_per_objfile->n_type_units)) + per_cu = dw2_get_cutu (cu_index); + if (file_matcher == NULL || per_cu->v.quick->mark) + { + int symtab_was_null = + (per_cu->v.quick->compunit_symtab == NULL); + + dw2_instantiate_symtab (per_cu); + + if (expansion_notify != NULL + && symtab_was_null + && per_cu->v.quick->compunit_symtab != NULL) + expansion_notify (per_cu->v.quick->compunit_symtab); + } + } +} + +static void +dw2_expand_symtabs_matching + (struct objfile *objfile, + gdb::function_view file_matcher, + const lookup_name_info &lookup_name, + gdb::function_view symbol_matcher, + gdb::function_view expansion_notify, + enum search_domain kind) +{ + int i; + offset_type iter; + + dw2_setup (objfile); + + /* index_table is NULL if OBJF_READNOW. */ + if (!dwarf2_per_objfile->index_table) + return; + + if (file_matcher != NULL) + { + htab_up visited_found (htab_create_alloc (10, htab_hash_pointer, + htab_eq_pointer, + NULL, xcalloc, xfree)); + htab_up visited_not_found (htab_create_alloc (10, htab_hash_pointer, + htab_eq_pointer, + NULL, xcalloc, xfree)); + + /* The rule is CUs specify all the files, including those used by + any TU, so there's no need to scan TUs here. */ + + for (i = 0; i < dwarf2_per_objfile->n_comp_units; ++i) + { + int j; + struct dwarf2_per_cu_data *per_cu = dw2_get_cu (i); + struct quick_file_names *file_data; + void **slot; + + QUIT; + + per_cu->v.quick->mark = 0; + + /* We only need to look at symtabs not already expanded. */ + if (per_cu->v.quick->compunit_symtab) + continue; + + file_data = dw2_get_file_names (per_cu); + if (file_data == NULL) + continue; + + if (htab_find (visited_not_found.get (), file_data) != NULL) + continue; + else if (htab_find (visited_found.get (), file_data) != NULL) { - complaint (&symfile_complaints, - _(".gdb_index entry has bad CU index" - " [in module %s]"), objfile_name (objfile)); + per_cu->v.quick->mark = 1; continue; } - per_cu = dw2_get_cutu (cu_index); - if (file_matcher == NULL || per_cu->v.quick->mark) + for (j = 0; j < file_data->num_file_names; ++j) { - int symtab_was_null = - (per_cu->v.quick->compunit_symtab == NULL); + const char *this_real_name; - dw2_instantiate_symtab (per_cu); + if (file_matcher (file_data->file_names[j], false)) + { + per_cu->v.quick->mark = 1; + break; + } + + /* Before we invoke realpath, which can get expensive when many + files are involved, do a quick comparison of the basenames. */ + if (!basenames_may_differ + && !file_matcher (lbasename (file_data->file_names[j]), + true)) + continue; - if (expansion_notify != NULL - && symtab_was_null - && per_cu->v.quick->compunit_symtab != NULL) + this_real_name = dw2_get_real_path (objfile, file_data, j); + if (file_matcher (this_real_name, false)) { - expansion_notify (per_cu->v.quick->compunit_symtab); + per_cu->v.quick->mark = 1; + break; } } + + slot = htab_find_slot (per_cu->v.quick->mark + ? visited_found.get () + : visited_not_found.get (), + file_data, INSERT); + *slot = file_data; } + } + + mapped_index &index = *dwarf2_per_objfile->index_table; + + dw2_expand_symtabs_matching_symbol (index, lookup_name, + symbol_matcher, + kind, [&] (offset_type idx) + { + dw2_expand_marked_cus (index, idx, objfile, file_matcher, + expansion_notify, kind); + }); } /* A helper for dw2_find_pc_sect_compunit_symtab which finds the most specific