[PATCH] gdb/python: Introduce gdb.lookup_all_static_symbols

[Andrew Burgess]

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