[pushed] Reorder/reindent dw2_expand_symtabs_matching & friends (Re: [PATCH 26/40] Optimize .gdb_index symbol name searching)

Pedro Alves
2017-11-08T16:16:38Z

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 <palves@redhat.com>
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 <palves@redhat.com>

* 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 <palves@redhat.com>

+ * 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 <palves@redhat.com>
+
* 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<expand_symtabs_file_matcher_ftype> file_matcher,
- gdb::function_view<expand_symtabs_exp_notify_ftype> 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<expand_symtabs_symbol_matcher_ftype> symbol_matcher,
- enum search_domain kind,
- gdb::function_view<void (offset_type)> on_match);
-
-static void
-dw2_expand_symtabs_matching
- (struct objfile *objfile,
- gdb::function_view<expand_symtabs_file_matcher_ftype> file_matcher,
- const lookup_name_info &lookup_name,
- gdb::function_view<expand_symtabs_symbol_matcher_ftype> symbol_matcher,
- gdb::function_view<expand_symtabs_exp_notify_ftype> 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<expand_symtabs_file_matcher_ftype> file_matcher,
+ const lookup_name_info &lookup_name,
+ gdb::function_view<expand_symtabs_symbol_matcher_ftype> symbol_matcher,
+ gdb::function_view<expand_symtabs_exp_notify_ftype> 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
--
2.5.5