This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH v2 4/8] Search global block from basic_lookup_symbol_nonlocal
* Tom Tromey <tromey@adacore.com> [2019-08-01 11:04:08 -0600]:
> This changes basic_lookup_symbol_nonlocal to look in the global block
> of the passed-in block. If no block was passed in, it reverts to the
> previous behavior.
>
> This change is needed to ensure that 'FILENAME'::NAME lookups work
> properly. As debugging Pedro's test case showed, this was not working
> properly in the case where multiple identical names could be found
> (the one situation where this feature is truly needed :-).
>
> This also removes some old comments from basic_lookup_symbol_nonlocal
> that no longer apply once this patch goes in.
So I guess the tests for this are going to be in the
gdb.base/print-file-var.exp changes that are part of patch #8. It
would be great if the commit message could mention this - it just
makes life easier later on.
I wonder if we need to update other *_lookup_symbol_nonlocal functions
in a similar way? For example can the C tests be compiled as C++,
which should cause GDB to use cp_lookup_symbol_nonlocal.
Looking at both basic_lookup_symbol_nonlocal and
cp_lookup_symbol_nonlocal, I wonder if your fix could be moved into
lookup_global_symbol? And just have 'block_global_block (block)'
checked before the search of all global blocks?
Some other languages provide their own *_lookup_symbol_nonlocal, I
don't know if these would also need fixing.
Thanks,
Andrew
>
> gdb/ChangeLog
> 2019-08-01 Tom Tromey <tromey@adacore.com>
>
> * symtab.c (basic_lookup_symbol_nonlocal): Search global block.
> Remove old comments.
> ---
> gdb/ChangeLog | 5 +++++
> gdb/symtab.c | 44 ++++++++++++++++----------------------------
> 2 files changed, 21 insertions(+), 28 deletions(-)
>
> diff --git a/gdb/symtab.c b/gdb/symtab.c
> index 0ff212e0d97..b8f33509c09 100644
> --- a/gdb/symtab.c
> +++ b/gdb/symtab.c
> @@ -2417,34 +2417,6 @@ basic_lookup_symbol_nonlocal (const struct language_defn *langdef,
> {
> struct block_symbol result;
>
> - /* NOTE: carlton/2003-05-19: The comments below were written when
> - this (or what turned into this) was part of lookup_symbol_aux;
> - I'm much less worried about these questions now, since these
> - decisions have turned out well, but I leave these comments here
> - for posterity. */
> -
> - /* NOTE: carlton/2002-12-05: There is a question as to whether or
> - not it would be appropriate to search the current global block
> - here as well. (That's what this code used to do before the
> - is_a_field_of_this check was moved up.) On the one hand, it's
> - redundant with the lookup in all objfiles search that happens
> - next. On the other hand, if decode_line_1 is passed an argument
> - like filename:var, then the user presumably wants 'var' to be
> - searched for in filename. On the third hand, there shouldn't be
> - multiple global variables all of which are named 'var', and it's
> - not like decode_line_1 has ever restricted its search to only
> - global variables in a single filename. All in all, only
> - searching the static block here seems best: it's correct and it's
> - cleanest. */
> -
> - /* NOTE: carlton/2002-12-05: There's also a possible performance
> - issue here: if you usually search for global symbols in the
> - current file, then it would be slightly better to search the
> - current global block before searching all the symtabs. But there
> - are other factors that have a much greater effect on performance
> - than that one, so I don't think we should worry about that for
> - now. */
> -
> /* NOTE: dje/2014-10-26: The lookup in all objfiles search could skip
> the current objfile. Searching the current objfile first is useful
> for both matching user expectations as well as performance. */
> @@ -2453,6 +2425,22 @@ basic_lookup_symbol_nonlocal (const struct language_defn *langdef,
> if (result.symbol != NULL)
> return result;
>
> + /* If a block was passed in, we want to search the corresponding
> + global block now. This yields "more expected" behavior, and is
> + needed to support 'FILENAME'::VARIABLE lookups. */
> + const struct block *global_block = block_global_block (block);
> + if (global_block != nullptr)
> + {
> + result.symbol = lookup_symbol_in_block (name,
> + symbol_name_match_type::FULL,
> + global_block, domain);
> + if (result.symbol != nullptr)
> + {
> + result.block = global_block;
> + return result;
> + }
> + }
> +
> /* If we didn't find a definition for a builtin type in the static block,
> search for it now. This is actually the right thing to do and can be
> a massive performance win. E.g., when debugging a program with lots of
> --
> 2.20.1
>