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

Andrew Burgess
2019-11-01T11:53:12Z

* Christian Biesinger <cbiesinger@google.com> [2019-10-28 00:07:17 -0500]:

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

This is not correct, both lookup_static_symbol AND
lookup_static_symbols BOTH only lookup file level static globals.

[ SIDE-ISSUE : The name for both of these is obviously a possible
sauce of confusion, maybe before this function makes it into a
release we should consider renaming it/them? How about
lookup_static_global(s) ? ]

Here's a demo session using the above 3 patches, the test file is:

static int global_v1 = 1;
static int global_v2 = 2;

int
use_them ()
{
return global_v1 + global_v2;
}

int
main ()
{
static int global_v2 = 3;
static int global_v3 = 4;

return use_them () + global_v2 + global_v3;
}

And my GDB session:

(gdb) start
Temporary breakpoint 1 at 0x40049a: file test.c, line 16.
Starting program: /projects/gdb/tmp/static-syms/test

Temporary breakpoint 1, main () at test.c:16
16 return use_them () + global_v2 + global_v3;
(gdb) python print (gdb.lookup_static_symbol ('global_v1').value ())
1
(gdb) python print (gdb.lookup_static_symbol ('global_v2').value ())
2
(gdb) python print (gdb.lookup_static_symbol ('global_v3').value ())
Traceback (most recent call last):
File "<string>", line 1, in <module>
AttributeError: 'NoneType' object has no attribute 'value'
Error while executing Python code.
(gdb)

Notice that despite being inside main, we see the file level
'global_v2' and completely miss the 'global_v3'. This is inline with
the original behaviour of this function before I started messing with
it.

One source of confusion may have been that I added a call to
lookup_symbol_in_static_block, this doesn't find a static symbol in a
block, or the first static symbol between block and the global scope,
but instead finds the static scope for a block and looks for a
matching symbol in that block.

The other source of confusion was my talking about 'print
symbol_name'. You are correct that in the above test if I did 'print
global_v2' I would see the static within main, so in that sense what
I've implemented is _not_ like 'print symbol_name'. The only
comparison I meant to draw with 'print symbol_name' was that if I do
try to access 'global_v1' while in main I know I'll get the global_v1
from _this_ file, not a global_v1 from some other file.

With the new 3rd patch a user can now from Python say, if I was in
some other block, which static global would I see, which I think for a
scripting interface is helpful. Then with the second patch the user
can also find all static globals. However, anything you can find with
lookup_static_symbol will _always_ be in the list returned by
lookup_static_symbols.

Hope this clears things up,

Thanks,
Andrew

>
> 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" \