This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [3/10] introduce psymtab users
- From: Jan Kratochvil <jan dot kratochvil at redhat dot com>
- To: Tom Tromey <tromey at redhat dot com>
- Cc: gdb-patches at sourceware dot org
- Date: Thu, 26 Apr 2012 19:27:11 +0200
- Subject: Re: [3/10] introduce psymtab users
- References: <87haw7brks.fsf@fleche.redhat.com>
On Wed, 25 Apr 2012 20:21:23 +0200, Tom Tromey wrote:
[...]
> @@ -142,6 +167,10 @@ struct partial_symtab
>
> unsigned char psymtabs_addrmap_supported;
>
> + /* A flag that is temporarily used when searching psymtabs. */
> +
> + unsigned char searched_flag;
Please make it the enum.
> @@ -760,6 +769,11 @@ lookup_partial_symbol (struct partial_symtab *pst, const char *name,
> static struct symtab *
> psymtab_to_symtab (struct partial_symtab *pst)
> {
> + /* If it is a shared psymtab, find an unshared psymtab that includes
> + it. Any such psymtab will do. */
> + while (pst->users != NULL)
> + pst = pst->users[0];
Currently GDB already has a problem that if it needs to expand symtab A to get
symbol X it already expands also symtabs B, C and D, all of them pretty big
for C++.
With such random choosing of a symtab for expansion this multi-expand
performance hit may get even worse for big C++ programs.
(Not sure if you do not already plan some more general fix for that.)
> @@ -1236,6 +1260,102 @@ map_matching_symbols_psymtab (const char *name, domain_enum namespace,
> }
> }
>
> +/* A convenience enum to give names to some constants used in
> + recursively_search_psymtabs. */
> +
> +enum
> + {
> + PST_NOT_SEARCHED,
> + PST_SEARCHED_AND_FOUND,
> + PST_SEARCHED_AND_NOT_FOUND
> + };
> +
> +/* A helper for expand_symtabs_matching_via_partial that handles
> + searching included psymtabs. */
Describe return value, please.
> +
> +static int
> +recursively_search_psymtabs (struct partial_symtab *ps,
> + struct objfile *objfile,
> + enum search_domain kind,
> + int (*name_matcher) (const char *, void *),
> + void *data)
> +{
> + struct partial_symbol **psym;
> + struct partial_symbol **bound, **gbound, **sbound;
> + int keep_going = 1;
> + int result = PST_SEARCHED_AND_NOT_FOUND;
> + int i;
> +
> + if (ps->searched_flag != PST_NOT_SEARCHED)
> + return ps->searched_flag == PST_SEARCHED_AND_FOUND;
> +
> + /* Recurse into shared psymtabs first, because they may have already
> + been searched, and this could save some time. */
> + for (i = 0; i < ps->number_of_dependencies; ++i)
> + {
> + int r;
> +
> + /* Skip non-shared dependencies, these are handled elsewhere. */
> + if (ps->dependencies[i]->users == NULL)
> + continue;
> +
> + r = recursively_search_psymtabs (ps->dependencies[i],
> + objfile, kind, name_matcher, data);
> + if (r != 0)
> + {
> + ps->searched_flag = PST_SEARCHED_AND_FOUND;
> + return 1;
> + }
> + }
> +
> + gbound = (objfile->global_psymbols.list
> + + ps->globals_offset + ps->n_global_syms);
nitpick: While I do not like it IMO GNU Coding Standard says:
gbound = &objfile->global_psymbols.list[ps->globals_offset + ps->n_global_syms];
But it applies also to the current code.
> + sbound = (objfile->static_psymbols.list
> + + ps->statics_offset + ps->n_static_syms);
> + bound = gbound;
> +
> + /* Go through all of the symbols stored in a partial
> + symtab in one loop. */
> + psym = objfile->global_psymbols.list + ps->globals_offset;
> + while (keep_going)
I do not see why to use the 'keep_going' variable here, just use 'break;'.
But it applies also to the current code.
> + {
> + if (psym >= bound)
> + {
> + if (bound == gbound && ps->n_static_syms != 0)
> + {
> + psym = objfile->static_psymbols.list + ps->statics_offset;
> + bound = sbound;
> + }
> + else
> + keep_going = 0;
> + continue;
> + }
> + else
> + {
> + QUIT;
> +
> + if ((kind == ALL_DOMAIN
> + || (kind == VARIABLES_DOMAIN
> + && SYMBOL_CLASS (*psym) != LOC_TYPEDEF
> + && SYMBOL_CLASS (*psym) != LOC_BLOCK)
> + || (kind == FUNCTIONS_DOMAIN
> + && SYMBOL_CLASS (*psym) == LOC_BLOCK)
> + || (kind == TYPES_DOMAIN
> + && SYMBOL_CLASS (*psym) == LOC_TYPEDEF))
> + && (*name_matcher) (SYMBOL_SEARCH_NAME (*psym), data))
> + {
> + /* Found a match, so notify our caller. */
> + result = PST_SEARCHED_AND_FOUND;
> + keep_going = 0;
> + }
> + }
> + psym++;
> + }
> +
> + ps->searched_flag = result;
> + return result == PST_SEARCHED_AND_FOUND;
> +}
> +
> static void
> expand_symtabs_matching_via_partial
> (struct objfile *objfile,
Thanks,
Jan