[PATCH] Merge lookup_global_symbol and lookup_static_symbol

[PATCH] Merge lookup_global_symbol and lookup_static_symbol

[Christian Biesinger via gdb-patches]

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.
---
 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

RE: [PATCH] Merge lookup_global_symbol and lookup_static_symbol

[Aktemur, Tankut Baris]

* 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

[PATCH] Factor out the common code in lookup_{static,global}_symbol

[Christian Biesinger via gdb-patches]

Thanks for the suggestion, that does seem better.

The two functions are extremely similar; this factors out their code into
a shared _internal function.

gdb/ChangeLog:

2019-08-02  Christian Biesinger  <cbiesinger@google.com>

	* symtab.c (lookup_static_symbol): Call the new function (and move
	it down to be next to lookup_global_symbol).
	(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_internal): New function.
	(lookup_global_symbol): Call new function.
---
 gdb/symtab.c | 81 +++++++++++++++++++++++-----------------------------
 1 file changed, 36 insertions(+), 45 deletions(-)

diff --git a/gdb/symtab.c b/gdb/symtab.c
index 87a0c8e4da..55df92f3e0 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -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,
@@ -2642,12 +2607,14 @@ lookup_symbol_global_iterator_cb (struct objfile *objfile,
   return (data->result.symbol != NULL);
 }
 
-/* See symtab.h.  */
-
+/* This function contains the common code of lookup_{global,static}_symbol.
+   BLOCK is only used if BLOCK_INDEX is GLOBAL_SCOPE, in which case it is
+   used to retrieve the objfile to start the lookup in.  */
 struct block_symbol
-lookup_global_symbol (const char *name,
-		      const struct block *block,
-		      const domain_enum domain)
+lookup_global_symbol_internal (const char *name,
+			       enum block_enum block_index,
+			       const struct block *block,
+			       const domain_enum domain)
 {
   struct symbol_cache *cache = get_symbol_cache (current_program_space);
   struct block_symbol result;
@@ -2656,11 +2623,14 @@ 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);
+  gdb_assert (block == nullptr || block_index == GLOBAL_BLOCK);
+
   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)
     {
@@ -2678,6 +2648,7 @@ lookup_global_symbol (const char *name,
     {
       memset (&lookup_data, 0, sizeof (lookup_data));
       lookup_data.name = name;
+      lookup_data.block_index = block_index;
       lookup_data.domain = domain;
       gdbarch_iterate_over_objfiles_in_search_order
 	(objfile != NULL ? get_objfile_arch (objfile) : target_gdbarch (),
@@ -2693,6 +2664,26 @@ lookup_global_symbol (const char *name,
   return result;
 }
 
+/* See symtab.h.  */
+
+struct block_symbol
+lookup_static_symbol (const char *name, const domain_enum domain)
+{
+  /* TODO: Should static symbol lookup start with a block as well, so we can
+     prefer a static symbol in the current CU?  */
+  return lookup_global_symbol_internal (name, STATIC_BLOCK, nullptr, domain);
+}
+
+/* See symtab.h.  */
+
+struct block_symbol
+lookup_global_symbol (const char *name,
+		      const struct block *block,
+		      const domain_enum domain)
+{
+  return lookup_global_symbol_internal (name, GLOBAL_BLOCK, block, domain);
+}
+
 int
 symbol_matches_domain (enum language symbol_language,
 		       domain_enum symbol_domain,
-- 
2.22.0.770.g0f2c4a37fd-goog

Re: [PATCH] Factor out the common code in lookup_{static,global}_symbol

[Simon Marchi]

On 2019-08-02 9:04 p.m., Christian Biesinger via gdb-patches wrote:
> Thanks for the suggestion, that does seem better.
> 
> The two functions are extremely similar; this factors out their code into
> a shared _internal function.

I am reasonably convinced that this is correct.  lookup_static_symbol currently
iterates objfiles by simply iterating the linked list.  Now it will use
gdbarch_iterate_over_objfiles_in_search_order, with current_objfile == NULL.  The
only gdbarch that implements it is Windows (windows_iterate_over_objfiles_in_search_order)
and with current_objfile == NULL, it goes back to iterating the objfiles linked list directly.

So there shouldn't be any functional change.

> gdb/ChangeLog:
> 
> 2019-08-02  Christian Biesinger  <cbiesinger@google.com>
> 
> 	* symtab.c (lookup_static_symbol): Call the new function (and move
> 	it down to be next to lookup_global_symbol).
> 	(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_internal): New function.
> 	(lookup_global_symbol): Call new function.
> ---
>  gdb/symtab.c | 81 +++++++++++++++++++++++-----------------------------
>  1 file changed, 36 insertions(+), 45 deletions(-)
> 
> diff --git a/gdb/symtab.c b/gdb/symtab.c
> index 87a0c8e4da..55df92f3e0 100644
> --- a/gdb/symtab.c
> +++ b/gdb/symtab.c
> @@ -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,
> @@ -2642,12 +2607,14 @@ lookup_symbol_global_iterator_cb (struct objfile *objfile,
>    return (data->result.symbol != NULL);
>  }
>  
> -/* See symtab.h.  */
> -
> +/* This function contains the common code of lookup_{global,static}_symbol.
> +   BLOCK is only used if BLOCK_INDEX is GLOBAL_SCOPE, in which case it is
> +   used to retrieve the objfile to start the lookup in.  */
>  struct block_symbol
> -lookup_global_symbol (const char *name,
> -		      const struct block *block,
> -		      const domain_enum domain)
> +lookup_global_symbol_internal (const char *name,
> +			       enum block_enum block_index,
> +			       const struct block *block,
> +			       const domain_enum domain)

Make this function static.  And to be pedant, add a newline between the doc and function name.

>  {
>    struct symbol_cache *cache = get_symbol_cache (current_program_space);
>    struct block_symbol result;
> @@ -2656,11 +2623,14 @@ 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);
> +  gdb_assert (block == nullptr || block_index == GLOBAL_BLOCK);
> +
>    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)
>      {
> @@ -2678,6 +2648,7 @@ lookup_global_symbol (const char *name,
>      {
>        memset (&lookup_data, 0, sizeof (lookup_data));
>        lookup_data.name = name;
> +      lookup_data.block_index = block_index;
>        lookup_data.domain = domain;
>        gdbarch_iterate_over_objfiles_in_search_order
>  	(objfile != NULL ? get_objfile_arch (objfile) : target_gdbarch (),
> @@ -2693,6 +2664,26 @@ lookup_global_symbol (const char *name,
>    return result;
>  }
>  
> +/* See symtab.h.  */
> +
> +struct block_symbol
> +lookup_static_symbol (const char *name, const domain_enum domain)
> +{
> +  /* TODO: Should static symbol lookup start with a block as well, so we can
> +     prefer a static symbol in the current CU?  */

That's a good question.  So one of the things using lookup_static_symbol is the
gdb.lookup_static_symbol Python function you recently added.  Right now, it is not
context sensitive.  For example, here I have two files with each a static variable
`allo`, one with value 22 and the other with value 33:

    (gdb) python print(gdb.lookup_static_symbol('allo').value())
    22
    (gdb) print allo
    $1 = 22
    (gdb) s
    fonction () at test2.c:6
    6	    printf("allo = %d\n", allo);
    (gdb) python print(gdb.lookup_static_symbol('allo').value())
    22
    (gdb) print allo
    $2 = 33

As you can see, gdb.lookup_static_symbol will always find the same symbol,
regardless of the current context, unlike `print`, which ends up using lookup_symbol_aux.
Should functions like gdb.lookup_static_symbol implicitly prioritize the current context
(e.g. current CU)?  I think something like that makes sense for the command line of GDB,
used interactively.  But for an API, is it strange to get different results when calling
gdb.lookup_static_symbol with the same parameters at different times?

Simon

[PATCH v2] Factor out the common code in lookup_{static,global}_symbol

[Christian Biesinger via gdb-patches]

The two functions are extremely similar; this factors out their code into
a shared _internal function.

gdb/ChangeLog:

2019-08-02  Christian Biesinger  <cbiesinger@google.com>

	* symtab.c (lookup_static_symbol): Call the new function (and move
	it down to be next to lookup_global_symbol).
	(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_internal): New function.
	(lookup_global_symbol): Call new function.
---
 gdb/symtab.c | 82 ++++++++++++++++++++++++----------------------------
 1 file changed, 37 insertions(+), 45 deletions(-)

diff --git a/gdb/symtab.c b/gdb/symtab.c
index 87a0c8e4da..1a5a6bf20e 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -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,
@@ -2642,12 +2607,15 @@ lookup_symbol_global_iterator_cb (struct objfile *objfile,
   return (data->result.symbol != NULL);
 }
 
-/* See symtab.h.  */
+/* This function contains the common code of lookup_{global,static}_symbol.
+   BLOCK is only used if BLOCK_INDEX is GLOBAL_SCOPE, in which case it is
+   used to retrieve the objfile to start the lookup in.  */
 
-struct block_symbol
-lookup_global_symbol (const char *name,
-		      const struct block *block,
-		      const domain_enum domain)
+static struct block_symbol
+lookup_global_symbol_internal (const char *name,
+			       enum block_enum block_index,
+			       const struct block *block,
+			       const domain_enum domain)
 {
   struct symbol_cache *cache = get_symbol_cache (current_program_space);
   struct block_symbol result;
@@ -2656,11 +2624,14 @@ 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);
+  gdb_assert (block == nullptr || block_index == GLOBAL_BLOCK);
+
   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)
     {
@@ -2678,6 +2649,7 @@ lookup_global_symbol (const char *name,
     {
       memset (&lookup_data, 0, sizeof (lookup_data));
       lookup_data.name = name;
+      lookup_data.block_index = block_index;
       lookup_data.domain = domain;
       gdbarch_iterate_over_objfiles_in_search_order
 	(objfile != NULL ? get_objfile_arch (objfile) : target_gdbarch (),
@@ -2693,6 +2665,26 @@ lookup_global_symbol (const char *name,
   return result;
 }
 
+/* See symtab.h.  */
+
+struct block_symbol
+lookup_static_symbol (const char *name, const domain_enum domain)
+{
+  /* TODO: Should static symbol lookup start with a block as well, so we can
+     prefer a static symbol in the current CU?  */
+  return lookup_global_symbol_internal (name, STATIC_BLOCK, nullptr, domain);
+}
+
+/* See symtab.h.  */
+
+struct block_symbol
+lookup_global_symbol (const char *name,
+		      const struct block *block,
+		      const domain_enum domain)
+{
+  return lookup_global_symbol_internal (name, GLOBAL_BLOCK, block, domain);
+}
+
 int
 symbol_matches_domain (enum language symbol_language,
 		       domain_enum symbol_domain,
-- 
2.22.0.770.g0f2c4a37fd-goog

Re: [PATCH] Factor out the common code in lookup_{static,global}_symbol

[Christian Biesinger via gdb-patches]

On Sat, Aug 3, 2019 at 6:41 PM Simon Marchi <simark@simark.ca> wrote:
>
> On 2019-08-02 9:04 p.m., Christian Biesinger via gdb-patches wrote:
> > Thanks for the suggestion, that does seem better.
> >
> > The two functions are extremely similar; this factors out their code into
> > a shared _internal function.
>
> I am reasonably convinced that this is correct.  lookup_static_symbol currently
> iterates objfiles by simply iterating the linked list.  Now it will use
> gdbarch_iterate_over_objfiles_in_search_order, with current_objfile == NULL.  The
> only gdbarch that implements it is Windows (windows_iterate_over_objfiles_in_search_order)
> and with current_objfile == NULL, it goes back to iterating the objfiles linked list directly.
>
> So there shouldn't be any functional change.

Yes, that was my thinking too.

> > gdb/ChangeLog:
> >
> > 2019-08-02  Christian Biesinger  <cbiesinger@google.com>
> >
> >       * symtab.c (lookup_static_symbol): Call the new function (and move
> >       it down to be next to lookup_global_symbol).
> >       (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_internal): New function.
> >       (lookup_global_symbol): Call new function.
> > ---
> >  gdb/symtab.c | 81 +++++++++++++++++++++++-----------------------------
> >  1 file changed, 36 insertions(+), 45 deletions(-)
> >
> > diff --git a/gdb/symtab.c b/gdb/symtab.c
> > index 87a0c8e4da..55df92f3e0 100644
> > --- a/gdb/symtab.c
> > +++ b/gdb/symtab.c
> > @@ -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,
> > @@ -2642,12 +2607,14 @@ lookup_symbol_global_iterator_cb (struct objfile *objfile,
> >    return (data->result.symbol != NULL);
> >  }
> >
> > -/* See symtab.h.  */
> > -
> > +/* This function contains the common code of lookup_{global,static}_symbol.
> > +   BLOCK is only used if BLOCK_INDEX is GLOBAL_SCOPE, in which case it is
> > +   used to retrieve the objfile to start the lookup in.  */
> >  struct block_symbol
> > -lookup_global_symbol (const char *name,
> > -                   const struct block *block,
> > -                   const domain_enum domain)
> > +lookup_global_symbol_internal (const char *name,
> > +                            enum block_enum block_index,
> > +                            const struct block *block,
> > +                            const domain_enum domain)
>
> Make this function static.  And to be pedant, add a newline between the doc and function name.

Oops, thanks. Will send a new patch in a moment.

> >  {
> >    struct symbol_cache *cache = get_symbol_cache (current_program_space);
> >    struct block_symbol result;
> > @@ -2656,11 +2623,14 @@ 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);
> > +  gdb_assert (block == nullptr || block_index == GLOBAL_BLOCK);
> > +
> >    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)
> >      {
> > @@ -2678,6 +2648,7 @@ lookup_global_symbol (const char *name,
> >      {
> >        memset (&lookup_data, 0, sizeof (lookup_data));
> >        lookup_data.name = name;
> > +      lookup_data.block_index = block_index;
> >        lookup_data.domain = domain;
> >        gdbarch_iterate_over_objfiles_in_search_order
> >       (objfile != NULL ? get_objfile_arch (objfile) : target_gdbarch (),
> > @@ -2693,6 +2664,26 @@ lookup_global_symbol (const char *name,
> >    return result;
> >  }
> >
> > +/* See symtab.h.  */
> > +
> > +struct block_symbol
> > +lookup_static_symbol (const char *name, const domain_enum domain)
> > +{
> > +  /* TODO: Should static symbol lookup start with a block as well, so we can
> > +     prefer a static symbol in the current CU?  */
>
> That's a good question.  So one of the things using lookup_static_symbol is the
> gdb.lookup_static_symbol Python function you recently added.  Right now, it is not
> context sensitive.  For example, here I have two files with each a static variable
> `allo`, one with value 22 and the other with value 33:
>
>     (gdb) python print(gdb.lookup_static_symbol('allo').value())
>     22
>     (gdb) print allo
>     $1 = 22
>     (gdb) s
>     fonction () at test2.c:6
>     6       printf("allo = %d\n", allo);
>     (gdb) python print(gdb.lookup_static_symbol('allo').value())
>     22
>     (gdb) print allo
>     $2 = 33
>
> As you can see, gdb.lookup_static_symbol will always find the same symbol,
> regardless of the current context, unlike `print`, which ends up using lookup_symbol_aux.
> Should functions like gdb.lookup_static_symbol implicitly prioritize the current context
> (e.g. current CU)?  I think something like that makes sense for the command line of GDB,
> used interactively.  But for an API, is it strange to get different results when calling
> gdb.lookup_static_symbol with the same parameters at different times?

Yeah, I agree that the python one should not be context-sensitive. But
take, for example, the call to lookup_static_symbol at the end of
lookup_symbol_aux -- shouldn't that take the context into account? For
the one in cp_lookup_nested_symbol_1 I can't quite tell if it makes a
difference, though I think the code there only looks in the current CU
and probably should prefer the current objfile for static symbols as
well. Etc.

Christian

Re: [PATCH] Factor out the common code in lookup_{static,global}_symbol

[Christian Biesinger via gdb-patches]

On Mon, Aug 5, 2019 at 10:32 AM Christian Biesinger
<cbiesinger@google.com> wrote:
>
> On Sat, Aug 3, 2019 at 6:41 PM Simon Marchi <simark@simark.ca> wrote:
> >
> > On 2019-08-02 9:04 p.m., Christian Biesinger via gdb-patches wrote:
> > > Thanks for the suggestion, that does seem better.
> > >
> > > The two functions are extremely similar; this factors out their code into
> > > a shared _internal function.
> >
> > I am reasonably convinced that this is correct.  lookup_static_symbol currently
> > iterates objfiles by simply iterating the linked list.  Now it will use
> > gdbarch_iterate_over_objfiles_in_search_order, with current_objfile == NULL.  The
> > only gdbarch that implements it is Windows (windows_iterate_over_objfiles_in_search_order)
> > and with current_objfile == NULL, it goes back to iterating the objfiles linked list directly.
> >
> > So there shouldn't be any functional change.
>
> Yes, that was my thinking too.
>
> > > gdb/ChangeLog:
> > >
> > > 2019-08-02  Christian Biesinger  <cbiesinger@google.com>
> > >
> > >       * symtab.c (lookup_static_symbol): Call the new function (and move
> > >       it down to be next to lookup_global_symbol).
> > >       (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_internal): New function.
> > >       (lookup_global_symbol): Call new function.
> > > ---
> > >  gdb/symtab.c | 81 +++++++++++++++++++++++-----------------------------
> > >  1 file changed, 36 insertions(+), 45 deletions(-)
> > >
> > > diff --git a/gdb/symtab.c b/gdb/symtab.c
> > > index 87a0c8e4da..55df92f3e0 100644
> > > --- a/gdb/symtab.c
> > > +++ b/gdb/symtab.c
> > > @@ -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,
> > > @@ -2642,12 +2607,14 @@ lookup_symbol_global_iterator_cb (struct objfile *objfile,
> > >    return (data->result.symbol != NULL);
> > >  }
> > >
> > > -/* See symtab.h.  */
> > > -
> > > +/* This function contains the common code of lookup_{global,static}_symbol.
> > > +   BLOCK is only used if BLOCK_INDEX is GLOBAL_SCOPE, in which case it is
> > > +   used to retrieve the objfile to start the lookup in.  */
> > >  struct block_symbol
> > > -lookup_global_symbol (const char *name,
> > > -                   const struct block *block,
> > > -                   const domain_enum domain)
> > > +lookup_global_symbol_internal (const char *name,
> > > +                            enum block_enum block_index,
> > > +                            const struct block *block,
> > > +                            const domain_enum domain)
> >
> > Make this function static.  And to be pedant, add a newline between the doc and function name.
>
> Oops, thanks. Will send a new patch in a moment.
>
> > >  {
> > >    struct symbol_cache *cache = get_symbol_cache (current_program_space);
> > >    struct block_symbol result;
> > > @@ -2656,11 +2623,14 @@ 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);
> > > +  gdb_assert (block == nullptr || block_index == GLOBAL_BLOCK);
> > > +
> > >    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)
> > >      {
> > > @@ -2678,6 +2648,7 @@ lookup_global_symbol (const char *name,
> > >      {
> > >        memset (&lookup_data, 0, sizeof (lookup_data));
> > >        lookup_data.name = name;
> > > +      lookup_data.block_index = block_index;
> > >        lookup_data.domain = domain;
> > >        gdbarch_iterate_over_objfiles_in_search_order
> > >       (objfile != NULL ? get_objfile_arch (objfile) : target_gdbarch (),
> > > @@ -2693,6 +2664,26 @@ lookup_global_symbol (const char *name,
> > >    return result;
> > >  }
> > >
> > > +/* See symtab.h.  */
> > > +
> > > +struct block_symbol
> > > +lookup_static_symbol (const char *name, const domain_enum domain)
> > > +{
> > > +  /* TODO: Should static symbol lookup start with a block as well, so we can
> > > +     prefer a static symbol in the current CU?  */
> >
> > That's a good question.  So one of the things using lookup_static_symbol is the
> > gdb.lookup_static_symbol Python function you recently added.  Right now, it is not
> > context sensitive.  For example, here I have two files with each a static variable
> > `allo`, one with value 22 and the other with value 33:
> >
> >     (gdb) python print(gdb.lookup_static_symbol('allo').value())
> >     22
> >     (gdb) print allo
> >     $1 = 22
> >     (gdb) s
> >     fonction () at test2.c:6
> >     6       printf("allo = %d\n", allo);
> >     (gdb) python print(gdb.lookup_static_symbol('allo').value())
> >     22
> >     (gdb) print allo
> >     $2 = 33
> >
> > As you can see, gdb.lookup_static_symbol will always find the same symbol,
> > regardless of the current context, unlike `print`, which ends up using lookup_symbol_aux.
> > Should functions like gdb.lookup_static_symbol implicitly prioritize the current context
> > (e.g. current CU)?  I think something like that makes sense for the command line of GDB,
> > used interactively.  But for an API, is it strange to get different results when calling
> > gdb.lookup_static_symbol with the same parameters at different times?
>
> Yeah, I agree that the python one should not be context-sensitive. But
> take, for example, the call to lookup_static_symbol at the end of
> lookup_symbol_aux -- shouldn't that take the context into account? For
> the one in cp_lookup_nested_symbol_1 I can't quite tell if it makes a
> difference, though I think the code there only looks in the current CU
> and probably should prefer the current objfile for static symbols as
> well. Etc.

BTW, if that's the only thing holding up this patch, I'm happy to
remove the comment.

Christian

[PING] [PATCH v2] Factor out the common code in lookup_{static,global}_symbol

[Christian Biesinger via gdb-patches]

On Mon, Aug 5, 2019 at 10:33 AM Christian Biesinger
<cbiesinger@google.com> wrote:
>
> The two functions are extremely similar; this factors out their code into
> a shared _internal function.
>
> gdb/ChangeLog:
>
> 2019-08-02  Christian Biesinger  <cbiesinger@google.com>
>
>         * symtab.c (lookup_static_symbol): Call the new function (and move
>         it down to be next to lookup_global_symbol).
>         (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_internal): New function.
>         (lookup_global_symbol): Call new function.
> ---
>  gdb/symtab.c | 82 ++++++++++++++++++++++++----------------------------
>  1 file changed, 37 insertions(+), 45 deletions(-)
>
> diff --git a/gdb/symtab.c b/gdb/symtab.c
> index 87a0c8e4da..1a5a6bf20e 100644
> --- a/gdb/symtab.c
> +++ b/gdb/symtab.c
> @@ -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,
> @@ -2642,12 +2607,15 @@ lookup_symbol_global_iterator_cb (struct objfile *objfile,
>    return (data->result.symbol != NULL);
>  }
>
> -/* See symtab.h.  */
> +/* This function contains the common code of lookup_{global,static}_symbol.
> +   BLOCK is only used if BLOCK_INDEX is GLOBAL_SCOPE, in which case it is
> +   used to retrieve the objfile to start the lookup in.  */
>
> -struct block_symbol
> -lookup_global_symbol (const char *name,
> -                     const struct block *block,
> -                     const domain_enum domain)
> +static struct block_symbol
> +lookup_global_symbol_internal (const char *name,
> +                              enum block_enum block_index,
> +                              const struct block *block,
> +                              const domain_enum domain)
>  {
>    struct symbol_cache *cache = get_symbol_cache (current_program_space);
>    struct block_symbol result;
> @@ -2656,11 +2624,14 @@ 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);
> +  gdb_assert (block == nullptr || block_index == GLOBAL_BLOCK);
> +
>    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)
>      {
> @@ -2678,6 +2649,7 @@ lookup_global_symbol (const char *name,
>      {
>        memset (&lookup_data, 0, sizeof (lookup_data));
>        lookup_data.name = name;
> +      lookup_data.block_index = block_index;
>        lookup_data.domain = domain;
>        gdbarch_iterate_over_objfiles_in_search_order
>         (objfile != NULL ? get_objfile_arch (objfile) : target_gdbarch (),
> @@ -2693,6 +2665,26 @@ lookup_global_symbol (const char *name,
>    return result;
>  }
>
> +/* See symtab.h.  */
> +
> +struct block_symbol
> +lookup_static_symbol (const char *name, const domain_enum domain)
> +{
> +  /* TODO: Should static symbol lookup start with a block as well, so we can
> +     prefer a static symbol in the current CU?  */
> +  return lookup_global_symbol_internal (name, STATIC_BLOCK, nullptr, domain);
> +}
> +
> +/* See symtab.h.  */
> +
> +struct block_symbol
> +lookup_global_symbol (const char *name,
> +                     const struct block *block,
> +                     const domain_enum domain)
> +{
> +  return lookup_global_symbol_internal (name, GLOBAL_BLOCK, block, domain);
> +}
> +
>  int
>  symbol_matches_domain (enum language symbol_language,
>                        domain_enum symbol_domain,
> --
> 2.22.0.770.g0f2c4a37fd-goog
>

Re: [PATCH v2] Factor out the common code in lookup_{static,global}_symbol

[Simon Marchi]

On 2019-08-05 11:33 a.m., Christian Biesinger via gdb-patches wrote:
> @@ -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 */

Period and two spaces:

  /* The block index in which to search.  */

> -struct block_symbol
> -lookup_global_symbol (const char *name,
> -		      const struct block *block,
> -		      const domain_enum domain)
> +static struct block_symbol
> +lookup_global_symbol_internal (const char *name,
> +			       enum block_enum block_index,
> +			       const struct block *block,
> +			       const domain_enum domain)

I'd rename that to lookup_symbol_internal, since it's used both in lookup_global_symbol and
lookup_static_symbol.  If you think it's too general (the function doesn't look up local symbols),
Maybe "lookup_global_or_static_symbol"?  And global_sym_lookup_data should be renamed accordingly.

And since BLOCK is only used for looking up an objfile in the global case, I would suggest
making this function accept the objfile instead of a block.  Otherwise, it could be confused
as "lookup symbol in this block".  Also, if some code already has the objfile handy, they can
pass it down, in which case we avoid the cost of lookup_objfile_from_block.

> +/* See symtab.h.  */
> +
> +struct block_symbol
> +lookup_static_symbol (const char *name, const domain_enum domain)
> +{
> +  /* TODO: Should static symbol lookup start with a block as well, so we can
> +     prefer a static symbol in the current CU?  */
> +  return lookup_global_symbol_internal (name, STATIC_BLOCK, nullptr, domain);

I'd say that we can add another argument when the need comes, it's not a stable API.

Simon

Re: [PATCH v2] Factor out the common code in lookup_{static,global}_symbol

[Christian Biesinger via gdb-patches]

On Sun, Aug 25, 2019 at 9:20 PM Simon Marchi <simark@simark.ca> wrote:
>
> On 2019-08-05 11:33 a.m., Christian Biesinger via gdb-patches wrote:
> > @@ -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 */
>
> Period and two spaces:
>
>   /* The block index in which to search.  */

Done

> > -struct block_symbol
> > -lookup_global_symbol (const char *name,
> > -                   const struct block *block,
> > -                   const domain_enum domain)
> > +static struct block_symbol
> > +lookup_global_symbol_internal (const char *name,
> > +                            enum block_enum block_index,
> > +                            const struct block *block,
> > +                            const domain_enum domain)
>
> I'd rename that to lookup_symbol_internal, since it's used both in lookup_global_symbol and
> lookup_static_symbol.  If you think it's too general (the function doesn't look up local symbols),
> Maybe "lookup_global_or_static_symbol"?  And global_sym_lookup_data should be renamed accordingly.
>
> And since BLOCK is only used for looking up an objfile in the global case, I would suggest
> making this function accept the objfile instead of a block.  Otherwise, it could be confused
> as "lookup symbol in this block".  Also, if some code already has the objfile handy, they can
> pass it down, in which case we avoid the cost of lookup_objfile_from_block.

Done

> > +/* See symtab.h.  */
> > +
> > +struct block_symbol
> > +lookup_static_symbol (const char *name, const domain_enum domain)
> > +{
> > +  /* TODO: Should static symbol lookup start with a block as well, so we can
> > +     prefer a static symbol in the current CU?  */
> > +  return lookup_global_symbol_internal (name, STATIC_BLOCK, nullptr, domain);
>
> I'd say that we can add another argument when the need comes, it's not a stable API.

Removed

[PATCH] Factor out the common code in lookup_{static,global}_symbol

[Christian Biesinger via gdb-patches]

The two functions are extremely similar; this factors out their code into
a shared _internal function.

gdb/ChangeLog:

2019-08-02  Christian Biesinger  <cbiesinger@google.com>

	* symtab.c (lookup_static_symbol): Call the new function (and move
	it down to be next to lookup_global_symbol).
	(struct global_sym_lookup_data): Add block_enum member and rename to...
	(struct global_or_static_sym_lookup_data): ...this.
	(lookup_symbol_global_iterator_cb): Pass block_index instead of
	GLOBAL_BLOCK to lookup_symbol_in_objfile and rename to...
	(lookup_symbol_global_or_static_iterator_cb): ...this.
	(lookup_global_or_static_symbol): New function.
	(lookup_global_symbol): Call new function.
---
 gdb/symtab.c | 96 +++++++++++++++++++++++-----------------------------
 1 file changed, 42 insertions(+), 54 deletions(-)

diff --git a/gdb/symtab.c b/gdb/symtab.c
index d85c77b4ce..5ad6456e03 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -2566,47 +2566,9 @@ 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
+struct global_or_static_sym_lookup_data
 {
   /* The name of the symbol we are searching for.  */
   const char *name;
@@ -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;
@@ -2625,16 +2590,16 @@ struct global_sym_lookup_data
    which in reality is a pointer to struct global_sym_lookup_data.  */
 
 static int
-lookup_symbol_global_iterator_cb (struct objfile *objfile,
-				  void *cb_data)
+lookup_symbol_global_or_static_iterator_cb (struct objfile *objfile,
+					    void *cb_data)
 {
-  struct global_sym_lookup_data *data =
-    (struct global_sym_lookup_data *) cb_data;
+  struct global_or_static_sym_lookup_data *data =
+    (struct global_or_static_sym_lookup_data *) cb_data;
 
   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,
@@ -2642,25 +2607,28 @@ lookup_symbol_global_iterator_cb (struct objfile *objfile,
   return (data->result.symbol != NULL);
 }
 
-/* See symtab.h.  */
+/* This function contains the common code of lookup_{global,static}_symbol.
+   BLOCK is only used if BLOCK_INDEX is GLOBAL_SCOPE, in which case it is
+   used to retrieve the objfile to start the lookup in.  */
 
-struct block_symbol
-lookup_global_symbol (const char *name,
-		      const struct block *block,
-		      const domain_enum domain)
+static struct block_symbol
+lookup_global_or_static_symbol (const char *name,
+				enum block_enum block_index,
+				struct objfile *objfile,
+				const domain_enum domain)
 {
   struct symbol_cache *cache = get_symbol_cache (current_program_space);
   struct block_symbol result;
-  struct objfile *objfile;
-  struct global_sym_lookup_data lookup_data;
+  struct global_or_static_sym_lookup_data lookup_data;
   struct block_symbol_cache *bsc;
   struct symbol_cache_slot *slot;
 
-  objfile = lookup_objfile_from_block (block);
+  gdb_assert (block_index == GLOBAL_BLOCK || block_index == STATIC_BLOCK);
+  gdb_assert (objfile == nullptr || block_index == GLOBAL_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)
     {
@@ -2678,10 +2646,11 @@ lookup_global_symbol (const char *name,
     {
       memset (&lookup_data, 0, sizeof (lookup_data));
       lookup_data.name = name;
+      lookup_data.block_index = block_index;
       lookup_data.domain = domain;
       gdbarch_iterate_over_objfiles_in_search_order
 	(objfile != NULL ? get_objfile_arch (objfile) : target_gdbarch (),
-	 lookup_symbol_global_iterator_cb, &lookup_data, objfile);
+	 lookup_symbol_global_or_static_iterator_cb, &lookup_data, objfile);
       result = lookup_data.result;
     }
 
@@ -2693,6 +2662,25 @@ lookup_global_symbol (const char *name,
   return result;
 }
 
+/* See symtab.h.  */
+
+struct block_symbol
+lookup_static_symbol (const char *name, const domain_enum domain)
+{
+  return lookup_global_or_static_symbol (name, STATIC_BLOCK, nullptr, domain);
+}
+
+/* See symtab.h.  */
+
+struct block_symbol
+lookup_global_symbol (const char *name,
+		      const struct block *block,
+		      const domain_enum domain)
+{
+  struct objfile *objfile = lookup_objfile_from_block (block);
+  return lookup_global_or_static_symbol (name, GLOBAL_BLOCK, objfile, domain);
+}
+
 int
 symbol_matches_domain (enum language symbol_language,
 		       domain_enum symbol_domain,
-- 
2.23.0.187.g17f5b7556c-goog

Re: [PATCH] Factor out the common code in lookup_{static,global}_symbol

[Simon Marchi]

On 2019-08-26 2:29 p.m., Christian Biesinger via gdb-patches wrote:
> The two functions are extremely similar; this factors out their code into
> a shared _internal function.
> 
> gdb/ChangeLog:
> 
> 2019-08-02  Christian Biesinger  <cbiesinger@google.com>
> 
> 	* symtab.c (lookup_static_symbol): Call the new function (and move
> 	it down to be next to lookup_global_symbol).
> 	(struct global_sym_lookup_data): Add block_enum member and rename to...
> 	(struct global_or_static_sym_lookup_data): ...this.
> 	(lookup_symbol_global_iterator_cb): Pass block_index instead of
> 	GLOBAL_BLOCK to lookup_symbol_in_objfile and rename to...
> 	(lookup_symbol_global_or_static_iterator_cb): ...this.
> 	(lookup_global_or_static_symbol): New function.
> 	(lookup_global_symbol): Call new function.
> ---
>  gdb/symtab.c | 96 +++++++++++++++++++++++-----------------------------
>  1 file changed, 42 insertions(+), 54 deletions(-)
> 
> diff --git a/gdb/symtab.c b/gdb/symtab.c
> index d85c77b4ce..5ad6456e03 100644
> --- a/gdb/symtab.c
> +++ b/gdb/symtab.c
> @@ -2566,47 +2566,9 @@ 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
> +struct global_or_static_sym_lookup_data
>  {
>    /* The name of the symbol we are searching for.  */
>    const char *name;
> @@ -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;
> @@ -2625,16 +2590,16 @@ struct global_sym_lookup_data
>     which in reality is a pointer to struct global_sym_lookup_data.  */
>  
>  static int
> -lookup_symbol_global_iterator_cb (struct objfile *objfile,
> -				  void *cb_data)
> +lookup_symbol_global_or_static_iterator_cb (struct objfile *objfile,
> +					    void *cb_data)
>  {

The comment above this function talks about GLOBAL_BLOCK and global_sym_lookup_data,
so it needs to be updated.

> -  struct global_sym_lookup_data *data =
> -    (struct global_sym_lookup_data *) cb_data;
> +  struct global_or_static_sym_lookup_data *data =
> +    (struct global_or_static_sym_lookup_data *) cb_data;
>  
>    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,
> @@ -2642,25 +2607,28 @@ lookup_symbol_global_iterator_cb (struct objfile *objfile,
>    return (data->result.symbol != NULL);
>  }
>  
> -/* See symtab.h.  */
> +/* This function contains the common code of lookup_{global,static}_symbol.
> +   BLOCK is only used if BLOCK_INDEX is GLOBAL_SCOPE, in which case it is
> +   used to retrieve the objfile to start the lookup in.  */

This comment needs to be updated to match the code.

LGTM with those fixed.

Thanks,

Simon

[PATCH] Factor out the common code in lookup_{static,global}_symbol

[Christian Biesinger via gdb-patches]

[Thanks, pushing this patch now, which has the two comments fixed.]

The two functions are extremely similar; this factors out their code into
a shared _internal function.

gdb/ChangeLog:

2019-08-26  Christian Biesinger  <cbiesinger@google.com>

	* symtab.c (lookup_static_symbol): Call the new function (and move
	it down to be next to lookup_global_symbol).
	(struct global_sym_lookup_data): Add block_enum member and rename to...
	(struct global_or_static_sym_lookup_data): ...this.
	(lookup_symbol_global_iterator_cb): Pass block_index instead of
	GLOBAL_BLOCK to lookup_symbol_in_objfile and rename to...
	(lookup_symbol_global_or_static_iterator_cb): ...this.
	(lookup_global_or_static_symbol): New function.
	(lookup_global_symbol): Call new function.
---
 gdb/symtab.c | 102 +++++++++++++++++++++++----------------------------
 1 file changed, 45 insertions(+), 57 deletions(-)

diff --git a/gdb/symtab.c b/gdb/symtab.c
index d85c77b4ce..787ecfe33b 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -2566,47 +2566,9 @@ 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
+struct global_or_static_sym_lookup_data
 {
   /* The name of the symbol we are searching for.  */
   const char *name;
@@ -2614,27 +2576,30 @@ 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;
 };
 
 /* A callback function for gdbarch_iterate_over_objfiles_in_search_order.
-   It searches by name for a symbol in the GLOBAL_BLOCK of the given
-   OBJFILE.  The arguments for the search are passed via CB_DATA,
-   which in reality is a pointer to struct global_sym_lookup_data.  */
+   It searches by name for a symbol in the block given by BLOCK_INDEX of the
+   given OBJFILE.  The arguments for the search are passed via CB_DATA, which
+   in reality is a pointer to struct global_or_static_sym_lookup_data.  */
 
 static int
-lookup_symbol_global_iterator_cb (struct objfile *objfile,
-				  void *cb_data)
+lookup_symbol_global_or_static_iterator_cb (struct objfile *objfile,
+					    void *cb_data)
 {
-  struct global_sym_lookup_data *data =
-    (struct global_sym_lookup_data *) cb_data;
+  struct global_or_static_sym_lookup_data *data =
+    (struct global_or_static_sym_lookup_data *) cb_data;
 
   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,
@@ -2642,25 +2607,28 @@ lookup_symbol_global_iterator_cb (struct objfile *objfile,
   return (data->result.symbol != NULL);
 }
 
-/* See symtab.h.  */
+/* This function contains the common code of lookup_{global,static}_symbol.
+   OBJFILE is only used if BLOCK_INDEX is GLOBAL_SCOPE, in which case it is
+   the objfile to start the lookup in.  */
 
-struct block_symbol
-lookup_global_symbol (const char *name,
-		      const struct block *block,
-		      const domain_enum domain)
+static struct block_symbol
+lookup_global_or_static_symbol (const char *name,
+				enum block_enum block_index,
+				struct objfile *objfile,
+				const domain_enum domain)
 {
   struct symbol_cache *cache = get_symbol_cache (current_program_space);
   struct block_symbol result;
-  struct objfile *objfile;
-  struct global_sym_lookup_data lookup_data;
+  struct global_or_static_sym_lookup_data lookup_data;
   struct block_symbol_cache *bsc;
   struct symbol_cache_slot *slot;
 
-  objfile = lookup_objfile_from_block (block);
+  gdb_assert (block_index == GLOBAL_BLOCK || block_index == STATIC_BLOCK);
+  gdb_assert (objfile == nullptr || block_index == GLOBAL_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)
     {
@@ -2678,10 +2646,11 @@ lookup_global_symbol (const char *name,
     {
       memset (&lookup_data, 0, sizeof (lookup_data));
       lookup_data.name = name;
+      lookup_data.block_index = block_index;
       lookup_data.domain = domain;
       gdbarch_iterate_over_objfiles_in_search_order
 	(objfile != NULL ? get_objfile_arch (objfile) : target_gdbarch (),
-	 lookup_symbol_global_iterator_cb, &lookup_data, objfile);
+	 lookup_symbol_global_or_static_iterator_cb, &lookup_data, objfile);
       result = lookup_data.result;
     }
 
@@ -2693,6 +2662,25 @@ lookup_global_symbol (const char *name,
   return result;
 }
 
+/* See symtab.h.  */
+
+struct block_symbol
+lookup_static_symbol (const char *name, const domain_enum domain)
+{
+  return lookup_global_or_static_symbol (name, STATIC_BLOCK, nullptr, domain);
+}
+
+/* See symtab.h.  */
+
+struct block_symbol
+lookup_global_symbol (const char *name,
+		      const struct block *block,
+		      const domain_enum domain)
+{
+  struct objfile *objfile = lookup_objfile_from_block (block);
+  return lookup_global_or_static_symbol (name, GLOBAL_BLOCK, objfile, domain);
+}
+
 int
 symbol_matches_domain (enum language symbol_language,
 		       domain_enum symbol_domain,
-- 
2.23.0.187.g17f5b7556c-goog