[PATCH] gdb/symtab: pass program space to lookup_symtab and iterate_over_symtabs
Keith Seitz
keiths@redhat.com
Fri Sep 27 17:06:51 GMT 2024
Hi, Simon,
On 8/12/24 10:46 AM, Simon Marchi wrote:
> Make the current program space references bubble up.
Thank you for the patch. I think this is a nice little
cleanup.
> In collect_symtabs_from_filename, remove the calls to
> set_current_program_space and just pass the relevant pspaces.
> This appears safe to do, because nothing in the `collector` callback
> cares about the current pspace.
Yeah, symtab_collector is really little more than an unordered_map.
AFAICT it has always been this way, and those set_program_space()
calls have been there from the introduction of this function in 2011.
That is, I agree: it (the collector callback) has never (obviously, at
least) relied on program spaces.
So, LGTM.
Reviewed-by: Keith Seitz <keiths@redhat.com>
Keith
>
> Change-Id: I00a7ed484bfbe5264f01a6abf0d33b51de373cbb
> ---
> gdb/ada-exp.y | 2 +-
> gdb/c-exp.y | 6 ++----
> gdb/linespec.c | 8 ++------
> gdb/m2-exp.y | 3 ++-
> gdb/mi/mi-cmd-disas.c | 3 ++-
> gdb/mi/mi-symbol-cmds.c | 2 +-
> gdb/p-exp.y | 4 ++--
> gdb/symtab.c | 41 +++++++++++++++--------------------------
> gdb/symtab.h | 14 ++++++++++----
> 9 files changed, 37 insertions(+), 46 deletions(-)
>
> diff --git a/gdb/ada-exp.y b/gdb/ada-exp.y
> index cdfaa3af05e0..dfd4eba54f6a 100644
> --- a/gdb/ada-exp.y
> +++ b/gdb/ada-exp.y
> @@ -1477,7 +1477,7 @@ block_lookup (const struct block *context, const char *raw_name)
>
> if (context == NULL
> && (syms.empty () || syms[0].symbol->aclass () != LOC_BLOCK))
> - symtab = lookup_symtab (name);
> + symtab = lookup_symtab (current_program_space, name);
> else
> symtab = NULL;
>
> diff --git a/gdb/c-exp.y b/gdb/c-exp.y
> index ca411dc0f5b0..645cb59b8781 100644
> --- a/gdb/c-exp.y
> +++ b/gdb/c-exp.y
> @@ -3087,10 +3087,8 @@ classify_name (struct parser_state *par_state, const struct block *block,
> || is_quoted_name)
> {
> /* See if it's a file name. */
> - struct symtab *symtab;
> -
> - symtab = lookup_symtab (copy.c_str ());
> - if (symtab)
> + if (auto symtab = lookup_symtab (current_program_space, copy.c_str ());
> + symtab != nullptr)
> {
> yylval.bval
> = symtab->compunit ()->blockvector ()->static_block ();
> diff --git a/gdb/linespec.c b/gdb/linespec.c
> index ee418e3a5070..2326ccd0d732 100644
> --- a/gdb/linespec.c
> +++ b/gdb/linespec.c
> @@ -3692,15 +3692,11 @@ collect_symtabs_from_filename (const char *file,
> if (pspace->executing_startup)
> continue;
>
> - set_current_program_space (pspace);
> - iterate_over_symtabs (file, collector);
> + iterate_over_symtabs (pspace, file, collector);
> }
> }
> else
> - {
> - set_current_program_space (search_pspace);
> - iterate_over_symtabs (file, collector);
> - }
> + iterate_over_symtabs (search_pspace, file, collector);
>
> return collector.release_symtabs ();
> }
> diff --git a/gdb/m2-exp.y b/gdb/m2-exp.y
> index 28005e1a7000..c12767533a83 100644
> --- a/gdb/m2-exp.y
> +++ b/gdb/m2-exp.y
> @@ -918,8 +918,9 @@ yylex (void)
> std::string tmp = copy_name (yylval.sval);
> struct symbol *sym;
>
> - if (lookup_symtab (tmp.c_str ()))
> + if (lookup_symtab (current_program_space, tmp.c_str ()) != nullptr)
> return BLOCKNAME;
> +
> sym = lookup_symbol (tmp.c_str (), pstate->expression_context_block,
> SEARCH_VFT, 0).symbol;
> if (sym && sym->aclass () == LOC_BLOCK)
> diff --git a/gdb/mi/mi-cmd-disas.c b/gdb/mi/mi-cmd-disas.c
> index 99b2ae418172..a311e25050cb 100644
> --- a/gdb/mi/mi-cmd-disas.c
> +++ b/gdb/mi/mi-cmd-disas.c
> @@ -18,6 +18,7 @@
> along with this program. If not, see <http://www.gnu.org/licenses/>. */
>
> #include "arch-utils.h"
> +#include "progspace.h"
> #include "target.h"
> #include "value.h"
> #include "mi-cmds.h"
> @@ -245,7 +246,7 @@ mi_cmd_disassemble (const char *command, const char *const *argv, int argc)
>
> if (line_seen && file_seen)
> {
> - s = lookup_symtab (file_string);
> + s = lookup_symtab (current_program_space, file_string);
> if (s == NULL)
> error (_("-data-disassemble: Invalid filename."));
> if (!find_line_pc (s, line_num, &start))
> diff --git a/gdb/mi/mi-symbol-cmds.c b/gdb/mi/mi-symbol-cmds.c
> index e90055f6011c..e4d890f3f36f 100644
> --- a/gdb/mi/mi-symbol-cmds.c
> +++ b/gdb/mi/mi-symbol-cmds.c
> @@ -41,7 +41,7 @@ mi_cmd_symbol_list_lines (const char *command, const char *const *argv,
> error (_("-symbol-list-lines: Usage: SOURCE_FILENAME"));
>
> filename = argv[0];
> - s = lookup_symtab (filename);
> + s = lookup_symtab (current_program_space, filename);
>
> if (s == NULL)
> error (_("-symbol-list-lines: Unknown source file name."));
> diff --git a/gdb/p-exp.y b/gdb/p-exp.y
> index ad7cd58196f7..938d3cf20240 100644
> --- a/gdb/p-exp.y
> +++ b/gdb/p-exp.y
> @@ -614,7 +614,7 @@ block : BLOCKNAME
> {
> std::string copy = copy_name ($1.stoken);
> struct symtab *tem =
> - lookup_symtab (copy.c_str ());
> + lookup_symtab (current_program_space, copy.c_str ());
> if (tem)
> $$ = (tem->compunit ()->blockvector ()
> ->static_block ());
> @@ -1520,7 +1520,7 @@ yylex (void)
> no psymtabs (coff, xcoff, or some future change to blow away the
> psymtabs once once symbols are read). */
> if ((sym && sym->aclass () == LOC_BLOCK)
> - || lookup_symtab (tmp.c_str ()))
> + || lookup_symtab (current_program_space, tmp.c_str ()))
> {
> yylval.ssym.sym.symbol = sym;
> yylval.ssym.sym.block = NULL;
> diff --git a/gdb/symtab.c b/gdb/symtab.c
> index 9b3562eeb303..45d377d3a495 100644
> --- a/gdb/symtab.c
> +++ b/gdb/symtab.c
> @@ -700,15 +700,10 @@ iterate_over_some_symtabs (const char *name,
> return false;
> }
>
> -/* Check for a symtab of a specific name; first in symtabs, then in
> - psymtabs. *If* there is no '/' in the name, a match after a '/'
> - in the symtab filename will also work.
> -
> - Calls CALLBACK with each symtab that is found. If CALLBACK returns
> - true, the search stops. */
> +/* See symtab.h. */
>
> void
> -iterate_over_symtabs (const char *name,
> +iterate_over_symtabs (program_space *pspace, const char *name,
> gdb::function_view<bool (symtab *)> callback)
> {
> gdb::unique_xmalloc_ptr<char> real_path;
> @@ -721,34 +716,28 @@ iterate_over_symtabs (const char *name,
> gdb_assert (IS_ABSOLUTE_PATH (real_path.get ()));
> }
>
> - for (objfile *objfile : current_program_space->objfiles ())
> - {
> - if (iterate_over_some_symtabs (name, real_path.get (),
> - objfile->compunit_symtabs, NULL,
> - callback))
> + for (objfile *objfile : pspace->objfiles ())
> + if (iterate_over_some_symtabs (name, real_path.get (),
> + objfile->compunit_symtabs, nullptr,
> + callback))
> return;
> - }
>
> /* Same search rules as above apply here, but now we look thru the
> psymtabs. */
> -
> - for (objfile *objfile : current_program_space->objfiles ())
> - {
> - if (objfile->map_symtabs_matching_filename (name, real_path.get (),
> - callback))
> - return;
> - }
> + for (objfile *objfile : pspace->objfiles ())
> + if (objfile->map_symtabs_matching_filename (name, real_path.get (),
> + callback))
> + return;
> }
>
> -/* A wrapper for iterate_over_symtabs that returns the first matching
> - symtab, or NULL. */
> +/* See symtab.h. */
>
> -struct symtab *
> -lookup_symtab (const char *name)
> +symtab *
> +lookup_symtab (program_space *pspace, const char *name)
> {
> struct symtab *result = NULL;
>
> - iterate_over_symtabs (name, [&] (symtab *symtab)
> + iterate_over_symtabs (pspace, name, [&] (symtab *symtab)
> {
> result = symtab;
> return true;
> @@ -6294,7 +6283,7 @@ collect_file_symbol_completion_matches (completion_tracker &tracker,
>
> /* Go through symtabs for SRCFILE and check the externs and statics
> for symbols which match. */
> - iterate_over_symtabs (srcfile, [&] (symtab *s)
> + iterate_over_symtabs (current_program_space, srcfile, [&] (symtab *s)
> {
> add_symtab_completions (s->compunit (),
> tracker, mode, lookup_name,
> diff --git a/gdb/symtab.h b/gdb/symtab.h
> index 4197a3a164c5..eecccc6f7e1e 100644
> --- a/gdb/symtab.h
> +++ b/gdb/symtab.h
> @@ -2099,9 +2099,9 @@ extern const char multiple_symbols_cancel[];
>
> const char *multiple_symbols_select_mode (void);
>
> -/* lookup a symbol table by source file name. */
> +/* Lookup a symbol table in PSPACE by source file name. */
>
> -extern struct symtab *lookup_symtab (const char *);
> +extern symtab *lookup_symtab (program_space *pspace, const char *name);
>
> /* An object of this type is passed as the 'is_a_field_of_this'
> argument to lookup_symbol and lookup_symbol_in_language. */
> @@ -2818,9 +2818,15 @@ bool iterate_over_some_symtabs (const char *name,
> struct compunit_symtab *after_last,
> gdb::function_view<bool (symtab *)> callback);
>
> -void iterate_over_symtabs (const char *name,
> - gdb::function_view<bool (symtab *)> callback);
> +/* Check in PSPACE for a symtab of a specific name; first in symtabs, then in
> + psymtabs. *If* there is no '/' in the name, a match after a '/' in the
> + symtab filename will also work.
> +
> + Call CALLBACK with each symtab that is found. If CALLBACK returns
> + true, the search stops. */
>
> +void iterate_over_symtabs (program_space *pspace, const char *name,
> + gdb::function_view<bool (symtab *)> callback);
>
> std::vector<CORE_ADDR> find_pcs_for_symtab_line
> (struct symtab *symtab, int line, const linetable_entry **best_entry);
>
> base-commit: 3fc9d4e2e5a90158f1506d84b3486da1c3529177
More information about the Gdb-patches
mailing list