This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
RE: [PATCH] Merge lookup_global_symbol and lookup_static_symbol
- From: "Aktemur, Tankut Baris" <tankut dot baris dot aktemur at intel dot com>
- To: Christian Biesinger <cbiesinger at google dot com>
- Cc: "gdb-patches at sourceware dot org" <gdb-patches at sourceware dot org>
- Date: Fri, 2 Aug 2019 05:49:25 +0000
- Subject: RE: [PATCH] Merge lookup_global_symbol and lookup_static_symbol
- References: <20190801212516.81494-1-cbiesinger@google.com>
* On Thursday, August 1, 2019 11:25 PM, Christian Biesinger wrote:
>
> These functions are extremely similar; make them a single function
> with a block_enum argument. Cleanup only; no behavior change.
>
> gdb/ChangeLog:
>
> 2019-08-01 Christian Biesinger <cbiesinger@google.com>
>
> * cp-namespace.c (cp_basic_lookup_symbol): Update function call.
> (cp_lookup_bare_symbol): Likewise.
> (cp_search_static_and_baseclasses): Likewise.
> (cp_lookup_nested_symbol_1): Likewise.
> * d-namespace.c (d_lookup_symbol): Likewise.
> (find_symbol_in_baseclass): Likewise.
> (d_lookup_nested_symbol): Likewise.
> * guile/scm-symbol.c (gdbscm_lookup_global_symbol): Likewise.
> * python/py-symbol.c (gdbpy_lookup_global_symbol): Likewise.
> (gdbpy_lookup_static_symbol): Likewise.
> * rust-lang.c (rust_lookup_symbol_nonlocal): Likewise.
> * symtab.c (lookup_symbol_aux): Likewise.
> (basic_lookup_symbol_nonlocal): Likewise.
> (lookup_static_symbol): Remove.
> (struct global_sym_lookup_data): Add block_enum member.
> (lookup_symbol_global_iterator_cb): Pass block_index instead of
> GLOBAL_BLOCK to lookup_symbol_in_objfile.
> (lookup_global_symbol): Pass block_index instead of GLOBAL_BLOCK
> to cache and lookup_data, and clear block if block_index is not
> GLOBAL_BLOCK.
> * symtab.h (lookup_static_symbol): Remove.
> (lookup_global_symbol): Add block_enum argument.
Would it be a cleaner approach to merge the common behavior in a static
function in symtab.c, and make the existing lookup_static_symbol and
lookup_global_symbol functions wrappers on that new static function?
This way, no change would be necessary in the users. I also think
"lookup_static_symbol (arg1, arg2)" is easier to read than
"lookup_global_symbol (arg1, nullptr, STATIC_BLOCK, arg2)"; and
"lookup_global_symbol (arg1, arg2, arg3)" is easier than
"lookup_global_symbol (arg1, arg2, GLOBAL_BLOCK, arg3)".
Regards,
-Baris
> ---
> gdb/cp-namespace.c | 12 ++++++---
> gdb/d-namespace.c | 11 +++++---
> gdb/guile/scm-symbol.c | 3 ++-
> gdb/python/py-symbol.c | 6 +++--
> gdb/rust-lang.c | 2 +-
> gdb/symtab.c | 59 ++++++++++++------------------------------
> gdb/symtab.h | 12 +++------
> 7 files changed, 43 insertions(+), 62 deletions(-)
>
> diff --git a/gdb/cp-namespace.c b/gdb/cp-namespace.c
> index 5b352d1d77..14967ab975 100644
> --- a/gdb/cp-namespace.c
> +++ b/gdb/cp-namespace.c
> @@ -149,7 +149,7 @@ cp_basic_lookup_symbol (const char *name, const struct block *block,
> }
> }
> else
> - sym = lookup_global_symbol (name, block, domain);
> + sym = lookup_global_symbol (name, block, GLOBAL_BLOCK, domain);
>
> return sym;
> }
> @@ -202,7 +202,7 @@ cp_lookup_bare_symbol (const struct language_defn *langdef,
> return sym;
> }
>
> - sym = lookup_global_symbol (name, block, domain);
> + sym = lookup_global_symbol (name, block, GLOBAL_BLOCK, domain);
> if (sym.symbol != NULL)
> return sym;
>
> @@ -270,7 +270,10 @@ cp_search_static_and_baseclasses (const char *name,
> block_symbol scope_sym = lookup_symbol_in_static_block (scope.c_str (),
> block, VAR_DOMAIN);
> if (scope_sym.symbol == NULL)
> - scope_sym = lookup_global_symbol (scope.c_str (), block, VAR_DOMAIN);
> + {
> + scope_sym = lookup_global_symbol (scope.c_str (), block, GLOBAL_BLOCK,
> + VAR_DOMAIN);
> + }
> if (scope_sym.symbol == NULL)
> return {};
>
> @@ -881,7 +884,8 @@ cp_lookup_nested_symbol_1 (struct type *container_type,
> which we just searched. */
> if (!is_in_anonymous)
> {
> - sym = lookup_static_symbol (concatenated_name, domain);
> + sym = lookup_global_symbol (concatenated_name, nullptr, STATIC_BLOCK,
> + domain);
> if (sym.symbol != NULL)
> return sym;
> }
> diff --git a/gdb/d-namespace.c b/gdb/d-namespace.c
> index 768ba23f01..52c766d6d7 100644
> --- a/gdb/d-namespace.c
> +++ b/gdb/d-namespace.c
> @@ -102,7 +102,7 @@ d_lookup_symbol (const struct language_defn *langdef,
> return sym;
> }
>
> - sym = lookup_global_symbol (name, block, domain);
> + sym = lookup_global_symbol (name, block, GLOBAL_BLOCK, domain);
>
> if (sym.symbol != NULL)
> return sym;
> @@ -146,7 +146,8 @@ d_lookup_symbol (const struct language_defn *langdef,
>
> /* Lookup a class named CLASSNAME. If none is found, there is nothing
> more that can be done. */
> - class_sym = lookup_global_symbol (classname.c_str (), block, domain);
> + class_sym = lookup_global_symbol (classname.c_str (), block, GLOBAL_BLOCK,
> + domain);
> if (class_sym.symbol == NULL)
> return {};
>
> @@ -276,7 +277,8 @@ find_symbol_in_baseclass (struct type *parent_type, const char *name,
> /* Nope. We now have to search all static blocks in all objfiles,
> even if block != NULL, because there's no guarantees as to which
> symtab the symbol we want is in. */
> - sym = lookup_static_symbol (concatenated_name.c_str (), VAR_DOMAIN);
> + sym = lookup_global_symbol (concatenated_name.c_str (), nullptr,
> + STATIC_BLOCK, VAR_DOMAIN);
> if (sym.symbol != NULL)
> break;
>
> @@ -336,7 +338,8 @@ d_lookup_nested_symbol (struct type *parent_type,
> xsnprintf (concatenated_name, size, "%s.%s",
> parent_name, nested_name);
>
> - sym = lookup_static_symbol (concatenated_name, VAR_DOMAIN);
> + sym = lookup_global_symbol (concatenated_name, nullptr, STATIC_BLOCK,
> + VAR_DOMAIN);
> if (sym.symbol != NULL)
> return sym;
>
> diff --git a/gdb/guile/scm-symbol.c b/gdb/guile/scm-symbol.c
> index 7b44b56581..36f327e519 100644
> --- a/gdb/guile/scm-symbol.c
> +++ b/gdb/guile/scm-symbol.c
> @@ -660,7 +660,8 @@ gdbscm_lookup_global_symbol (SCM name_scm, SCM rest)
>
> try
> {
> - symbol = lookup_global_symbol (name, NULL, (domain_enum) domain).symbol;
> + symbol = lookup_global_symbol (name, NULL, GLOBAL_BLOCK,
> + (domain_enum) domain).symbol;
> }
> catch (const gdb_exception &ex)
> {
> diff --git a/gdb/python/py-symbol.c b/gdb/python/py-symbol.c
> index 2b10e21d87..b16e66f557 100644
> --- a/gdb/python/py-symbol.c
> +++ b/gdb/python/py-symbol.c
> @@ -449,7 +449,8 @@ gdbpy_lookup_global_symbol (PyObject *self, PyObject *args, PyObject *kw)
>
> try
> {
> - symbol = lookup_global_symbol (name, NULL, (domain_enum) domain).symbol;
> + symbol = lookup_global_symbol (name, NULL, GLOBAL_BLOCK,
> + (domain_enum) domain).symbol;
> }
> catch (const gdb_exception &except)
> {
> @@ -489,7 +490,8 @@ gdbpy_lookup_static_symbol (PyObject *self, PyObject *args, PyObject *kw)
>
> try
> {
> - symbol = lookup_static_symbol (name, (domain_enum) domain).symbol;
> + symbol = lookup_global_symbol (name, NULL, STATIC_BLOCK,
> + (domain_enum) domain).symbol;
> }
> catch (const gdb_exception &except)
> {
> diff --git a/gdb/rust-lang.c b/gdb/rust-lang.c
> index 79f13311cd..a623d061e5 100644
> --- a/gdb/rust-lang.c
> +++ b/gdb/rust-lang.c
> @@ -2077,7 +2077,7 @@ rust_lookup_symbol_nonlocal (const struct language_defn *langdef,
> {
> result = lookup_symbol_in_static_block (name, block, domain);
> if (result.symbol == NULL)
> - result = lookup_global_symbol (name, block, domain);
> + result = lookup_global_symbol (name, block, GLOBAL_BLOCK, domain);
> }
> return result;
> }
> diff --git a/gdb/symtab.c b/gdb/symtab.c
> index 87a0c8e4da..dbdf7dbfc6 100644
> --- a/gdb/symtab.c
> +++ b/gdb/symtab.c
> @@ -2102,7 +2102,7 @@ lookup_symbol_aux (const char *name, symbol_name_match_type match_type,
> /* Now search all static file-level symbols. Not strictly correct,
> but more useful than an error. */
>
> - result = lookup_static_symbol (name, domain);
> + result = lookup_global_symbol (name, nullptr, STATIC_BLOCK, domain);
> if (symbol_lookup_debug)
> {
> fprintf_unfiltered (gdb_stdlog, "lookup_symbol_aux (...) = %s\n",
> @@ -2474,7 +2474,7 @@ basic_lookup_symbol_nonlocal (const struct language_defn *langdef,
> return result;
> }
>
> - return lookup_global_symbol (name, block, domain);
> + return lookup_global_symbol (name, block, GLOBAL_BLOCK, domain);
> }
>
> /* See symtab.h. */
> @@ -2566,44 +2566,6 @@ lookup_symbol_in_objfile (struct objfile *objfile, enum block_enum block_index,
> return result;
> }
>
> -/* See symtab.h. */
> -
> -struct block_symbol
> -lookup_static_symbol (const char *name, const domain_enum domain)
> -{
> - struct symbol_cache *cache = get_symbol_cache (current_program_space);
> - struct block_symbol result;
> - struct block_symbol_cache *bsc;
> - struct symbol_cache_slot *slot;
> -
> - /* Lookup in STATIC_BLOCK is not current-objfile-dependent, so just pass
> - NULL for OBJFILE_CONTEXT. */
> - result = symbol_cache_lookup (cache, NULL, STATIC_BLOCK, name, domain,
> - &bsc, &slot);
> - if (result.symbol != NULL)
> - {
> - if (SYMBOL_LOOKUP_FAILED_P (result))
> - return {};
> - return result;
> - }
> -
> - for (objfile *objfile : current_program_space->objfiles ())
> - {
> - result = lookup_symbol_in_objfile (objfile, STATIC_BLOCK, name, domain);
> - if (result.symbol != NULL)
> - {
> - /* Still pass NULL for OBJFILE_CONTEXT here. */
> - symbol_cache_mark_found (bsc, slot, NULL, result.symbol,
> - result.block);
> - return result;
> - }
> - }
> -
> - /* Still pass NULL for OBJFILE_CONTEXT here. */
> - symbol_cache_mark_not_found (bsc, slot, NULL, name, domain);
> - return {};
> -}
> -
> /* Private data to be used with lookup_symbol_global_iterator_cb. */
>
> struct global_sym_lookup_data
> @@ -2614,6 +2576,9 @@ struct global_sym_lookup_data
> /* The domain to use for our search. */
> domain_enum domain;
>
> + /* The block index in which to search */
> + enum block_enum block_index;
> +
> /* The field where the callback should store the symbol if found.
> It should be initialized to {NULL, NULL} before the search is started. */
> struct block_symbol result;
> @@ -2634,7 +2599,7 @@ lookup_symbol_global_iterator_cb (struct objfile *objfile,
> gdb_assert (data->result.symbol == NULL
> && data->result.block == NULL);
>
> - data->result = lookup_symbol_in_objfile (objfile, GLOBAL_BLOCK,
> + data->result = lookup_symbol_in_objfile (objfile, data->block_index,
> data->name, data->domain);
>
> /* If we found a match, tell the iterator to stop. Otherwise,
> @@ -2647,6 +2612,7 @@ lookup_symbol_global_iterator_cb (struct objfile *objfile,
> struct block_symbol
> lookup_global_symbol (const char *name,
> const struct block *block,
> + enum block_enum block_index,
> const domain_enum domain)
> {
> struct symbol_cache *cache = get_symbol_cache (current_program_space);
> @@ -2656,11 +2622,19 @@ lookup_global_symbol (const char *name,
> struct block_symbol_cache *bsc;
> struct symbol_cache_slot *slot;
>
> + gdb_assert (block_index == GLOBAL_BLOCK || block_index == STATIC_BLOCK);
> +
> + /* We only use the block when looking in the GLOBAL_BLOCK.
> + TODO: Wouldn't it be even more important to use the block for static
> + symbol lookups? */
> + if (block_index != GLOBAL_BLOCK)
> + block = nullptr;
> +
> objfile = lookup_objfile_from_block (block);
>
> /* First see if we can find the symbol in the cache.
> This works because we use the current objfile to qualify the lookup. */
> - result = symbol_cache_lookup (cache, objfile, GLOBAL_BLOCK, name, domain,
> + result = symbol_cache_lookup (cache, objfile, block_index, name, domain,
> &bsc, &slot);
> if (result.symbol != NULL)
> {
> @@ -2679,6 +2653,7 @@ lookup_global_symbol (const char *name,
> memset (&lookup_data, 0, sizeof (lookup_data));
> lookup_data.name = name;
> lookup_data.domain = domain;
> + lookup_data.block_index = block_index;
> gdbarch_iterate_over_objfiles_in_search_order
> (objfile != NULL ? get_objfile_arch (objfile) : target_gdbarch (),
> lookup_symbol_global_iterator_cb, &lookup_data, objfile);
> diff --git a/gdb/symtab.h b/gdb/symtab.h
> index 9880ecc4c5..5601a28350 100644
> --- a/gdb/symtab.h
> +++ b/gdb/symtab.h
> @@ -1617,15 +1617,10 @@ extern struct block_symbol
> const struct block *block,
> const domain_enum domain);
>
> -/* Search all static file-level symbols for NAME from DOMAIN.
> - Upon success fixes up the symbol's section if necessary. */
> -
> -extern struct block_symbol lookup_static_symbol (const char *name,
> - const domain_enum domain);
> -
> -/* Lookup a symbol in all files' global blocks.
> +/* Lookup a symbol in all files' global or static blocks.
>
> - If BLOCK is non-NULL then it is used for two things:
> + If BLOCK_INDEX is GLOBAL_BLOCK and BLOCK is non-NULL then it is used for
> + two things:
> 1) If a target-specific lookup routine for libraries exists, then use the
> routine for the objfile of BLOCK, and
> 2) The objfile of BLOCK is used to assist in determining the search order
> @@ -1637,6 +1632,7 @@ extern struct block_symbol lookup_static_symbol (const char *name,
> extern struct block_symbol
> lookup_global_symbol (const char *name,
> const struct block *block,
> + enum block_enum block_index,
> const domain_enum domain);
>
> /* Lookup a symbol in block BLOCK.
> --
> 2.22.0.770.g0f2c4a37fd-goog
Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Gary Kershaw
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928