Re: [PATCH] gdb/python: Introduce gdb.lookup_all_static_symbols

Christian Biesinger via gdb-patches
2019-10-28T05:08:01Z

On Wed, Oct 23, 2019 at 2:13 PM Andrew Burgess
<andrew.burgess@embecosm.com> wrote:
>
> * Christian Biesinger <cbiesinger@google.com> [2019-10-21 17:36:37 -0500]:
>
> > On Tue, Oct 15, 2019 at 2:28 PM Simon Marchi <simark@simark.ca> wrote:
> > >
> > > On 2019-10-15 12:46 p.m., Andrew Burgess wrote:
> > > > If gdb.lookup_static_symbol is going to return a single symbol then it
> > > > makes sense (I think) for it to return a context sensitive choice of
> > > > symbol, that is the static symbol that would be visible to the program
> > > > at that point.
> > >
> > > I remember discussing this with Christian, and I didn't have a strong option. But
> > > now, I tend to agree with Andrew.
> > >
> > > I see two use cases here:
> > >
> > > 1. I want to get all static symbols named `foo`. In which case, I'd use the
> > > function Andrew proposes in this patch.
> > > 2. I want to get the static symbol named `foo` that's visible from a certain
> > > point, perhaps a given block or where my program is currently stopped at.
> > > Ideally, we would have a "CompilationUnit" object type in Python, such that
> > > I could use
> > >
> > > block.compunit.lookup_static_symbol('foo')
> > >
> > > or
> > >
> > > gdb.current_compunit().lookup_static_symbol('foo')
> >
> > So technically, those don't give you "the static symbol named `foo`
> > that's visible from [this] point", because there could be static
> > variable in the function.
> >
> > Since we already have block objects, should this new function
> > optionally take a block to start the lookup in?
>
> When you say "new function", I assume you mean
> 'gdb.lookup_static_symbol' not 'gdb.lookup_static_symbols', as the
> second one always returns all symbols.
>
> Anyway, I took a stab at adding a block parameter to the first of
> these functions - is this what you were thinking?

I'm sorry, I think I was a bit confused when I wrote that. Here's what
I was thinking about -- with your change to make lookup_static_symbol
produce the same result as "print var", it will also find block-level
statics. However, lookup_static_symbols only looks at file-level
static variables, which means that lookup_static_symbol can find a
variable that's not in lookup_static_symbols, which seems confusing to
users. My suggestion to give it a block to start in didn't really make
sense, I think, but it would be good to think about that a bit more?

Christian

>
> Thanks,
> Andrew
>
> ---
>
> commit 62d488878b467dd83a9abffc6dc3c67c2da82e85
> Author: Andrew Burgess <andrew.burgess@embecosm.com>
> Date: Wed Oct 23 15:08:42 2019 +0100
>
> gdb/python: New block parameter for gdb.lookup_static_symbol
>
> Adds a new block parameter to gdb.lookup_static_symbol. Currently
> gdb.lookup_static_symbol first looks in the global static scope of the
> current block for a matching symbol, and if non is found it then looks
> in all other global static scopes.
>
> The new block parameter for gdb.lookup_static_symbol allows the user
> to search the global static scope for some other block rather than the
> current block if they so desire.
>
> If the block parameter is no given, or is given as None, then GDB will
> search using the current block first (this matches the CLI behaviour).
>
> I added the block parameter to the middle of the parameter list, so
> the parameter order is 'symbol block domain', this matches the order
> of the other symbol lookup function that takes a block 'lookup_symbol'
> which also has the order 'symbol block domain'. I think changing the
> API like this is fine as the 'gdb.lookup_static_symbol' has not been
> in any previous GDB release, so there should be no existing user
> scripts that use this function.
>
> The documentation for gdb.lookup_static_symbol has been reordered so
> that the block parameter can be explained early on, and I've added
> some tests.
>
> gdb/ChangeLog:
>
> * python/py-symbol.c (gdbpy_lookup_static_symbol): Accept and
> handle a new block parameter.
>
> gdb/testsuite/ChangeLog:
>
> * gdb.python/py-symbol.exp: Add tests for passing a block
> parameter to gdb.lookup_static_symbol.
>
> gdb/doc/ChangeLog:
>
> * python.texi (Symbols In Python): Rewrite documentation for
> gdb.lookup_static_symbol, including adding a description of the
> new block parameter.
>
> Change-Id: I132a7f0934ba029c9f32058e9341a6c7cb975606
>
> diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
> index a30c8ae4f03..fe984a06b98 100644
> --- a/gdb/doc/python.texi
> +++ b/gdb/doc/python.texi
> @@ -4853,31 +4853,41 @@
> @end defun
>
> @findex gdb.lookup_static_symbol
> -@defun gdb.lookup_static_symbol (name @r{[}, domain@r{]})
> -This function searches for a global symbol with static linkage by name.
> -The search scope can be restricted to by the domain argument.
> +@defun gdb.lookup_static_symbol (name @r{[}, block @r{[}, domain@r{]]})
> +This function searches for a global symbol with static linkage by
> +name.
>
> @var{name} is the name of the symbol. It must be a string.
> -The optional @var{domain} argument restricts the search to the domain type.
> -The @var{domain} argument must be a domain constant defined in the @code{gdb}
> -module and described later in this chapter.
> -
> -The result is a @code{gdb.Symbol} object or @code{None} if the symbol
> -is not found.
> -
> -Note that this function will not find function-scoped static variables. To look
> -up such variables, iterate over the variables of the function's
> -@code{gdb.Block} and check that @code{block.addr_class} is
> -@code{gdb.SYMBOL_LOC_STATIC}.
>
> There can be multiple global symbols with static linkage with the same
> name. This function will only return the first matching symbol that
> it finds. Which symbol is found depends on where @value{GDBN} is
> currently stopped, as @value{GDBN} will first search for matching
> -symbols in the current object file, and then search all other object
> -files. If the application is not yet running then @value{GDBN} will
> -search all object files in the order they appear in the debug
> -information.
> +symbols in the global static scope of the current object file, and
> +then search all other object files. This corresponds to the behaviour
> +you would see from the CLI if you tried to print the symbol by name.
> +
> +If the application is not yet running then @value{GDBN} will search
> +all object files in the order they appear in the debug information.
> +
> +The optional @var{domain} argument restricts the search to the domain
> +type. The @var{domain} argument must be a domain constant defined in
> +the @code{gdb} module and described later in this chapter.
> +
> +The optional @var{block} argument can be used to change which global
> +static scope @value{GDBN} will search first. If @var{block} is not
> +given, or is passed the value @code{None} then @value{GDBN} will first
> +search the global static scope corresponding to the current block. If
> +@var{block} is passed a @code{gdb.Block} then @value{GDBN} will first
> +search the global static scope corresponding to the supplied block.
> +
> +The result is a @code{gdb.Symbol} object or @code{None} if the symbol
> +is not found.
> +
> +Note that this function will not find function-scoped static
> +variables. To look up such variables, iterate over the variables of
> +the function's @code{gdb.Block} and check that @code{block.addr_class}
> +is @code{gdb.SYMBOL_LOC_STATIC}.
> @end defun
>
> @findex gdb.lookup_global_symbol
> diff --git a/gdb/python/py-symbol.c b/gdb/python/py-symbol.c
> index 647c54b0a5b..24b3f6f4376 100644
> --- a/gdb/python/py-symbol.c
> +++ b/gdb/python/py-symbol.c
> @@ -473,19 +473,20 @@ gdbpy_lookup_global_symbol (PyObject *self, PyObject *args, PyObject *kw)
> }
>
> /* Implementation of
> - gdb.lookup_static_symbol (name [, domain]) -> symbol or None. */
> + gdb.lookup_static_symbol (name [, block] [, domain]) -> symbol or None. */
>
> PyObject *
> gdbpy_lookup_static_symbol (PyObject *self, PyObject *args, PyObject *kw)
> {
> const char *name;
> int domain = VAR_DOMAIN;
> - static const char *keywords[] = { "name", "domain", NULL };
> + static const char *keywords[] = { "name", "block", "domain", NULL };
> struct symbol *symbol = NULL;
> + PyObject *block_obj = NULL;
> PyObject *sym_obj;
>
> - if (!gdb_PyArg_ParseTupleAndKeywords (args, kw, "s|i", keywords, &name,
> - &domain))
> + if (!gdb_PyArg_ParseTupleAndKeywords (args, kw, "s|O|i", keywords, &name,
> + &block_obj, &domain))
> return NULL;
>
> /* In order to find static symbols associated with the "current" object
> @@ -493,16 +494,22 @@ gdbpy_lookup_static_symbol (PyObject *self, PyObject *args, PyObject *kw)
> we can acquire a current block. If this fails however, then we still
> want to search all static symbols, so don't throw an exception just
> yet. */
> - const struct block *block = NULL;
> - try
> - {
> - struct frame_info *selected_frame
> - = get_selected_frame (_("No frame selected."));
> - block = get_frame_block (selected_frame, NULL);
> - }
> - catch (const gdb_exception &except)
> + const struct block *block = nullptr;
> + if (block_obj)
> + block = block_object_to_block (block_obj);
> +
> + if (block == nullptr)
> {
> - /* Nothing. */
> + try
> + {
> + struct frame_info *selected_frame
> + = get_selected_frame (_("No frame selected."));
> + block = get_frame_block (selected_frame, NULL);
> + }
> + catch (const gdb_exception &except)
> + {
> + /* Nothing. */
> + }
> }
>
> try
> diff --git a/gdb/testsuite/gdb.python/py-symbol.exp b/gdb/testsuite/gdb.python/py-symbol.exp
> index ea41297f54f..0ffe07df5a8 100644
> --- a/gdb/testsuite/gdb.python/py-symbol.exp
> +++ b/gdb/testsuite/gdb.python/py-symbol.exp
> @@ -87,6 +87,10 @@ if ![runto_main] then {
>
> global hex decimal
>
> +# Grab the value of $pc, we'll use this later.
> +set pc_val_file_1 [get_integer_valueof "\$pc" 0 \
> + "get value of \$pc in main file"]
> +
> gdb_breakpoint [gdb_get_line_number "Block break here."]
> gdb_continue_to_breakpoint "Block break here."
> gdb_py_test_silent_cmd "python frame = gdb.selected_frame()" "Get Frame" 0
> @@ -116,7 +120,18 @@ gdb_test "python print (func.addr_class == gdb.SYMBOL_LOC_BLOCK)" "True" "test f
> gdb_breakpoint "function_in_other_file"
> gdb_continue_to_breakpoint "function_in_other_file"
> gdb_test "python print (gdb.lookup_static_symbol ('rr').value ())" "99" \
> - "print value of rr from other file"
> + "print value of rr from other file, don't pass a block value"
> +gdb_test "python print (gdb.lookup_static_symbol ('rr', None).value ())" "99" \
> + "print value of rr from other file, pass block value of None"
> +
> +# Test gdb.lookup_static_symbol passing a block parameter
> +set pc_val_file_2 [get_integer_valueof "\$pc" 0 \
> + "get \$pc in other file"]
> +gdb_test "python print (gdb.lookup_static_symbol ('rr', gdb.current_progspace ().block_for_pc (${pc_val_file_2})).value ())" "99" \
> + "print value of rr from other file, pass block for other file"
> +gdb_test "python print (gdb.lookup_static_symbol ('rr', gdb.current_progspace ().block_for_pc (${pc_val_file_1})).value ())" "42" \
> + "print value of rr from other file, pass block for main file"
> +
> gdb_test "python print (gdb.lookup_static_symbols ('rr')\[0\].value ())" "99" \
> "print value of gdb.lookup_static_symbols ('rr')\[0\], from the other file"
> gdb_test "python print (gdb.lookup_static_symbols ('rr')\[1\].value ())" "42" \
> @@ -131,7 +146,14 @@ gdb_py_test_silent_cmd "python frame = gdb.selected_frame()" "Get Frame" 0
> # Check that we find the static sybol local to this file over the
> # static symbol from the second source file.
> gdb_test "python print (gdb.lookup_static_symbol ('rr').value ())" "42" \
> - "print value of rr from main file"
> + "print value of rr from main file, passing no block parameter"
> +gdb_test "python print (gdb.lookup_static_symbol ('rr', None).value ())" "42" \
> + "print value of rr from main file, passing None block parameter"
> +gdb_test "python print (gdb.lookup_static_symbol ('rr', gdb.current_progspace ().block_for_pc (${pc_val_file_2})).value ())" "99" \
> + "print value of rr from main file, pass block for other file"
> +gdb_test "python print (gdb.lookup_static_symbol ('rr', gdb.current_progspace ().block_for_pc (${pc_val_file_1})).value ())" "42" \
> + "print value of rr from main file, pass block for main file"
> +
> gdb_test "python print (gdb.lookup_static_symbols ('rr')\[0\].value ())" "99" \
> "print value of gdb.lookup_static_symbols ('rr')\[0\], from the main file"
> gdb_test "python print (gdb.lookup_static_symbols ('rr')\[1\].value ())" "42" \