Re: [PATCH 07/11] gdb: split iterate_over_minimal_symbols in for_each_minimal_symbol and find_minimal_symbol

[Andrew Burgess <aburgess@redhat.com>]

Simon Marchi <simon.marchi@efficios.com> writes:

> From: Simon Marchi <simon.marchi@polymtl.ca>

Just 'in' -> 'into' in the subject line again.  Otherwise:

Approved-By: Andrew Burgess <aburgess@redhat.com>

Thanks,
Andrew


>
> Based on the same rationale as the previous patches, split
> iterate_over_minimal_symbols in two.
>
> Implement for_each_minimal_symbol using find_minimal_symbol, since that
> one is really not trivial.
>
> Change-Id: Ie02e67278359454f7aa583200ec68d2f429f7ebe
> ---
>  gdb/linespec.c | 35 ++++++++++++++++-------------------
>  gdb/minsyms.c  | 25 ++++++++++++++++++++-----
>  gdb/minsyms.h  | 37 +++++++++++++++++++++++++++++--------
>  gdb/symtab.c   | 18 +++++++++---------
>  4 files changed, 74 insertions(+), 41 deletions(-)
>
> diff --git a/gdb/linespec.c b/gdb/linespec.c
> index 4ea6d597a093..4170d4ecf2c4 100644
> --- a/gdb/linespec.c
> +++ b/gdb/linespec.c
> @@ -4150,33 +4150,30 @@ search_minsyms_for_name (struct collect_info *info,
>  	  set_current_program_space (pspace);
>  
>  	  for (objfile &objfile : pspace->objfiles ())
> -	    {
> -	      iterate_over_minimal_symbols (&objfile, name,
> -					    [&] (struct minimal_symbol *msym)
> -					    {
> -					      add_minsym (msym, &objfile, nullptr,
> -							  info->state->list_mode,
> -							  &minsyms);
> -					      return false;
> -					    });
> -	    }
> +	    for_each_minimal_symbol (&objfile, name,
> +				     [&] (minimal_symbol *msym)
> +				       {
> +					 add_minsym (msym, &objfile, nullptr,
> +						     info->state->list_mode,
> +						     &minsyms);
> +				       });
>  	}
>      }
>    else
>      {
> -      program_space *pspace = symtab->compunit ()->objfile ()->pspace ();
> +      objfile &objfile = *symtab->compunit ()->objfile ();
> +      program_space *pspace = objfile.pspace ();
>  
>        if (search_pspace == NULL || pspace == search_pspace)
>  	{
>  	  set_current_program_space (pspace);
> -	  iterate_over_minimal_symbols
> -	    (symtab->compunit ()->objfile (), name,
> -	     [&] (struct minimal_symbol *msym)
> -	       {
> -		 add_minsym (msym, symtab->compunit ()->objfile (), symtab,
> -			     info->state->list_mode, &minsyms);
> -		 return false;
> -	       });
> +	  for_each_minimal_symbol (&objfile, name,
> +				   [&] (minimal_symbol *msym)
> +				     {
> +				       add_minsym (msym, &objfile, symtab,
> +						   info->state->list_mode,
> +						   &minsyms);
> +				     });
>  	}
>      }
>  
> diff --git a/gdb/minsyms.c b/gdb/minsyms.c
> index 0d6ea53aecf6..4eafc7890478 100644
> --- a/gdb/minsyms.c
> +++ b/gdb/minsyms.c
> @@ -496,9 +496,22 @@ linkage_name_str (const lookup_name_info &lookup_name)
>  /* See minsyms.h.  */
>  
>  void
> -iterate_over_minimal_symbols
> -    (struct objfile *objf, const lookup_name_info &lookup_name,
> -     gdb::function_view<bool (struct minimal_symbol *)> callback)
> +for_each_minimal_symbol (struct objfile *objf, const lookup_name_info &name,
> +			 for_each_minimal_symbol_callback_ftype callback)
> +{
> +  find_minimal_symbol (objf, name,
> +		       [&] (struct minimal_symbol *msym)
> +			 {
> +			   callback (msym);
> +			   return false;
> +			 });
> +}
> +
> +/* See minsyms.h.  */
> +
> +minimal_symbol *
> +find_minimal_symbol (struct objfile *objf, const lookup_name_info &lookup_name,
> +		     find_minimal_symbol_callback_ftype callback)
>  {
>    /* The first pass is over the ordinary hash table.  */
>      {
> @@ -515,7 +528,7 @@ iterate_over_minimal_symbols
>  	{
>  	  if (mangled_cmp (iter->linkage_name (), name) == 0)
>  	    if (callback (iter))
> -	      return;
> +	      return iter;
>  	}
>      }
>  
> @@ -539,8 +552,10 @@ iterate_over_minimal_symbols
>  	   iter = iter->demangled_hash_next)
>  	if (name_match (iter->search_name (), lookup_name, NULL))
>  	  if (callback (iter))
> -	    return;
> +	    return iter;
>      }
> +
> +  return nullptr;
>  }
>  
>  /* See minsyms.h.  */
> diff --git a/gdb/minsyms.h b/gdb/minsyms.h
> index 8f38cc7137fc..c44d5b20aec8 100644
> --- a/gdb/minsyms.h
> +++ b/gdb/minsyms.h
> @@ -20,6 +20,7 @@
>  #ifndef GDB_MINSYMS_H
>  #define GDB_MINSYMS_H
>  
> +#include "gdbsupport/function-view.h"
>  #include <deque>
>  
>  struct program_space;
> @@ -279,16 +280,36 @@ bound_minimal_symbol lookup_minimal_symbol_by_pc_section
>  
>  bound_minimal_symbol lookup_minimal_symbol_by_pc (CORE_ADDR);
>  
> -/* Iterate over all the minimal symbols in the objfile OBJF which
> -   match NAME.  Both the ordinary and demangled names of each symbol
> -   are considered.  The caller is responsible for canonicalizing NAME,
> -   should that need to be done.
> +/* Callback type for function for_each_minimal_symbol.  */
>  
> -   For each matching symbol, CALLBACK is called with the symbol.  */
> +using for_each_minimal_symbol_callback_ftype
> +  = gdb::function_view<void (struct minimal_symbol *)>;
>  
> -void iterate_over_minimal_symbols
> -    (struct objfile *objf, const lookup_name_info &name,
> -     gdb::function_view<bool (struct minimal_symbol *)> callback);
> +/* Call CALLBACK for all minimal symbols in objfile OBJF which match NAME.
> +
> +   Both the ordinary and demangled names of each symbol are considered.  The
> +   caller is responsible for canonicalizing NAME, should that need to be
> +   done.  */
> +
> +void for_each_minimal_symbol (struct objfile *objf,
> +			      const lookup_name_info &name,
> +			      for_each_minimal_symbol_callback_ftype callback);
> +
> +/* Callback type for function find_minimal_symbol.  */
> +
> +using find_minimal_symbol_callback_ftype
> +  = gdb::function_view<bool (struct minimal_symbol *)>;
> +
> +/* Find the first minimal symbol for objfile OBJF which matches NAME and for
> +   which CALLBACK returns true.
> +
> +   Both the ordinary and demangled names of each symbol are considered.  The
> +   caller is responsible for canonicalizing NAME, should that need to be
> +   done.  */
> +
> +minimal_symbol *find_minimal_symbol
> +  (struct objfile *objf, const lookup_name_info &name,
> +   find_minimal_symbol_callback_ftype callback);
>  
>  /* Compute the upper bound of MINSYM.  The upper bound is the last
>     address thought to be part of the symbol.  If the symbol has a
> diff --git a/gdb/symtab.c b/gdb/symtab.c
> index d22dd81a246a..5a39081dfc54 100644
> --- a/gdb/symtab.c
> +++ b/gdb/symtab.c
> @@ -5782,32 +5782,32 @@ find_gnu_ifunc (const symbol *sym)
>    struct objfile *objfile = sym->objfile ();
>  
>    CORE_ADDR address = sym->value_block ()->entry_pc ();
> -  minimal_symbol *ifunc = NULL;
> -
> -  iterate_over_minimal_symbols (objfile, lookup_name,
> -				[&] (minimal_symbol *minsym)
> +  minimal_symbol *ifunc
> +    = find_minimal_symbol (objfile, lookup_name,
> +			   [&] (minimal_symbol *minsym)
>      {
>        if (minsym->type () == mst_text_gnu_ifunc
>  	  || minsym->type () == mst_data_gnu_ifunc)
>  	{
>  	  CORE_ADDR msym_addr = minsym->value_address (objfile);
> +
>  	  if (minsym->type () == mst_data_gnu_ifunc)
>  	    {
>  	      struct gdbarch *gdbarch = objfile->arch ();
>  	      msym_addr = gdbarch_convert_from_func_ptr_addr
>  		(gdbarch, msym_addr, current_inferior ()->top_target ());
>  	    }
> +
>  	  if (msym_addr == address)
> -	    {
> -	      ifunc = minsym;
> -	      return true;
> -	    }
> +	    return true;
>  	}
> +
>        return false;
>      });
>  
> -  if (ifunc != NULL)
> +  if (ifunc != nullptr)
>      return {ifunc, objfile};
> +
>    return {};
>  }
>  
> -- 
> 2.53.0