Factor out the common code in lookup_{static,global}_symbol
Commit Message
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(-)
Comments
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
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
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
@@ -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,