gdb/python: Introduce gdb.lookup_all_static_symbols

Message ID 20191023191354.GH4962@embecosm.com
State New, archived
Headers

Commit Message

Andrew Burgess Oct. 23, 2019, 7:13 p.m. UTC
  * 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?

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
  

Comments

Simon Marchi Oct. 24, 2019, 3:11 a.m. UTC | #1
On 2019-10-23 3:13 p.m., Andrew Burgess wrote:
>>> 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?
> 
> Thanks,
> Andrew

I'm confused, I don't gdb.lookup_static_symbol(s) only look through
static symbols, as in file-level static variables?

Simon
  
Andrew Burgess Oct. 24, 2019, 4:53 p.m. UTC | #2
* Simon Marchi <simark@simark.ca> [2019-10-23 23:11:30 -0400]:

> On 2019-10-23 3:13 p.m., Andrew Burgess wrote:
> >>> 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?
> > 
> > Thanks,
> > Andrew
> 
> I'm confused, I don't gdb.lookup_static_symbol(s) only look through
> static symbols, as in file-level static variables?

So, before patch 3 the situation was:

  gdb.lookup_static_symbol

  Return the file level static symbol matching a given name.  Will
  return the static from the current file first, before searching
  through all other files.  This matches the CLI behaviour for if you
  just typed: 'print var_name' (with var_name being file-level
  static).

  gdb.lookup_static_symbols

  Returns all of the file level static symbols matching a given name,
  the order corresponds to the order of the compilation units as found
  by GDB (I guess).  In general a user shouldn't read any meaning into
  the order, just that these are all file level statics with that
  name.

The last patch in the series modifies the first of these to take an
optional block parameter.  GDB will then find the file level static
that would be seen from that block.

This allows the user to ask the question, what file-level static would
I see from over there, without actually having to be "over there".

This is demonstrated in the testsuite changes where I lookup a block
based on a $pc from some other function in a different file, I can
then query the file-level static that would be visible from that
function.

Is this super useful? I don't know, I think this is what Christian was
suggesting and adding the feature was both easy to implement and easy
to test, so I figured why not.

Thanks,
Andrew
  
Terekhov, Mikhail via Gdb-patches Oct. 28, 2019, 5:07 a.m. UTC | #3
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" \
  
Andrew Burgess Nov. 1, 2019, 11:53 a.m. UTC | #4
* 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" \
  
Simon Marchi Nov. 1, 2019, 1:55 p.m. UTC | #5
On 2019-11-01 7:53 a.m., Andrew Burgess wrote:
>> 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) ? ]

I would at least make sure the naming is consistent with gdb.lookup_global_symbol,
so have the _symbol(s) suffix.  So, lookup_static_global_symbol(s).

I don't think that lookup_static_symbols is particularly confusing.  As a user, I
don't think I would expect it to return static symbols from local scopes.

I think what makes lookup_static_symbol ambiguous is that we're considering making
it take a block as parameter.  Then, I agree, a user could think that it will find
static symbols from local scopes.

But AFAIK, we're considering passing a block to lookup_static_symbol only because
of a current shortcoming in the API, that we don't have an object type
representing a compilation unit.  If lookup_static_symbol accepted a compilation
unit object instead, then I don't think there would be a similar confusion
(especially that it is singular, and returns a single symbol).

Would it be an option to not add lookup_static_symbol for now, and instead work
on having a type representing compilation units, and then think about introducing
lookup_static_symbol again?  Christian, can you achieve what you wanted using
lookup_static_symbols plus some filtering instead?

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

Ok, that's the behavior I expected (to not find static local symbols), thanks
for clearing that up.

Simon
  
Andrew Burgess Nov. 4, 2019, 5:12 p.m. UTC | #6
* Simon Marchi <simark@simark.ca> [2019-11-01 09:55:07 -0400]:

> On 2019-11-01 7:53 a.m., Andrew Burgess wrote:
> >> 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) ? ]
> 
> I would at least make sure the naming is consistent with gdb.lookup_global_symbol,
> so have the _symbol(s) suffix.  So, lookup_static_global_symbol(s).
> 
> I don't think that lookup_static_symbols is particularly confusing.  As a user, I
> don't think I would expect it to return static symbols from local scopes.
> 
> I think what makes lookup_static_symbol ambiguous is that we're considering making
> it take a block as parameter.  Then, I agree, a user could think that it will find
> static symbols from local scopes.
> 
> But AFAIK, we're considering passing a block to lookup_static_symbol only because
> of a current shortcoming in the API, that we don't have an object type
> representing a compilation unit.  If lookup_static_symbol accepted a compilation
> unit object instead, then I don't think there would be a similar confusion
> (especially that it is singular, and returns a single symbol).
> 
> Would it be an option to not add lookup_static_symbol for now, and instead work
> on having a type representing compilation units, and then think about introducing
> lookup_static_symbol again?  Christian, can you achieve what you wanted using
> lookup_static_symbols plus some filtering instead?

How about I merged patches #1 and #2, but drop #3 for now?  This would
give:

  lookup_static_symbol - returns the global static symbol for the
      current compilation unit, or whatever else GDB can find if
      there's nothing in the current CU.  This is basically inline
      with what we get from 'print symbol_name' at the CLI (if we
      limit symbol_name to just global static things).

AND,

  lookup_static_symbols - returns the list of all global static
      symbols.

We no longer have a block parameter passed to 'lookup_static_symbol',
so hopefully and confusion can be avoided.  If/when we later add a
Python wrapper for CU we can _extend_ the API for lookup_static_symbol
to also take a CU and so provide the "look over there" type behaviour
in a clearer way.

Would everyone be happy with this?

Thanks,
Andrew




> 
> > 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.
> 
> Ok, that's the behavior I expected (to not find static local symbols), thanks
> for clearing that up.
> 
> Simon
  
Simon Marchi Nov. 4, 2019, 6:28 p.m. UTC | #7
On 2019-11-04 12:12 p.m., Andrew Burgess wrote:
> How about I merged patches #1 and #2, but drop #3 for now?  This would
> give:
> 
>   lookup_static_symbol - returns the global static symbol for the
>       current compilation unit, or whatever else GDB can find if
>       there's nothing in the current CU.  This is basically inline
>       with what we get from 'print symbol_name' at the CLI (if we
>       limit symbol_name to just global static things).
> 
> AND,
> 
>   lookup_static_symbols - returns the list of all global static
>       symbols.
> 
> We no longer have a block parameter passed to 'lookup_static_symbol',
> so hopefully and confusion can be avoided.  If/when we later add a
> Python wrapper for CU we can _extend_ the API for lookup_static_symbol
> to also take a CU and so provide the "look over there" type behaviour
> in a clearer way.
> 
> Would everyone be happy with this?

I certainly would, I think that brings us back to the same place we were before
it was suggested to add the block parameter, doesn't it?

But it's Christian who initially wanted to add these functions, he probably had
a particular use case in mind.  It would be up to him to say if that covers that
use case or not.

Simon
  
Terekhov, Mikhail via Gdb-patches Nov. 9, 2019, 6:23 a.m. UTC | #8
On Mon, Nov 4, 2019 at 12:28 PM Simon Marchi <simark@simark.ca> wrote:
>
> On 2019-11-04 12:12 p.m., Andrew Burgess wrote:
> > How about I merged patches #1 and #2, but drop #3 for now?  This would
> > give:
> >
> >   lookup_static_symbol - returns the global static symbol for the
> >       current compilation unit, or whatever else GDB can find if
> >       there's nothing in the current CU.  This is basically inline
> >       with what we get from 'print symbol_name' at the CLI (if we
> >       limit symbol_name to just global static things).
> >
> > AND,
> >
> >   lookup_static_symbols - returns the list of all global static
> >       symbols.
> >
> > We no longer have a block parameter passed to 'lookup_static_symbol',
> > so hopefully and confusion can be avoided.  If/when we later add a
> > Python wrapper for CU we can _extend_ the API for lookup_static_symbol
> > to also take a CU and so provide the "look over there" type behaviour
> > in a clearer way.
> >
> > Would everyone be happy with this?
>
> I certainly would, I think that brings us back to the same place we were before
> it was suggested to add the block parameter, doesn't it?

Sorry for the late reply & for confusing things -- yes, after reading
through all the discussions I think that's the best way forward.

My personal use case was & is still addressed, I just wanted a way to
find variables like these:
https://cs.chromium.org/chromium/src/content/renderer/render_frame_impl.cc?q=g_frame_map&sq=package:chromium&g=0&l=332
(^ in an anonymous namespace)

ideally also:
https://cs.chromium.org/chromium/src/ui/accessibility/ax_tree_manager_map.cc?type=cs&sq=package:chromium&g=0&l=17
(static variable in a function)
But I do have a fallback to gdb.lookup_symbol, so that one works too,
though less efficiently. Even with a CU API, this might be difficult
(maybe? depending how easy it is to find a specific function from a
CU)

Christian
  
Andrew Burgess Nov. 10, 2019, 10:27 p.m. UTC | #9
* Christian Biesinger <cbiesinger@google.com> [2019-11-09 00:23:00 -0600]:

> On Mon, Nov 4, 2019 at 12:28 PM Simon Marchi <simark@simark.ca> wrote:
> >
> > On 2019-11-04 12:12 p.m., Andrew Burgess wrote:
> > > How about I merged patches #1 and #2, but drop #3 for now?  This would
> > > give:
> > >
> > >   lookup_static_symbol - returns the global static symbol for the
> > >       current compilation unit, or whatever else GDB can find if
> > >       there's nothing in the current CU.  This is basically inline
> > >       with what we get from 'print symbol_name' at the CLI (if we
> > >       limit symbol_name to just global static things).
> > >
> > > AND,
> > >
> > >   lookup_static_symbols - returns the list of all global static
> > >       symbols.
> > >
> > > We no longer have a block parameter passed to 'lookup_static_symbol',
> > > so hopefully and confusion can be avoided.  If/when we later add a
> > > Python wrapper for CU we can _extend_ the API for lookup_static_symbol
> > > to also take a CU and so provide the "look over there" type behaviour
> > > in a clearer way.
> > >
> > > Would everyone be happy with this?
> >
> > I certainly would, I think that brings us back to the same place we were before
> > it was suggested to add the block parameter, doesn't it?
> 
> Sorry for the late reply & for confusing things -- yes, after reading
> through all the discussions I think that's the best way forward.

Thanks for the feedback.

I've now pushed patches #1 and #2.

Thanks,
Andrew
  
Terekhov, Mikhail via Gdb-patches Nov. 10, 2019, 10:36 p.m. UTC | #10
On Sun, Nov 10, 2019 at 4:27 PM Andrew Burgess
<andrew.burgess@embecosm.com> wrote:
>
> * Christian Biesinger <cbiesinger@google.com> [2019-11-09 00:23:00 -0600]:
>
> > On Mon, Nov 4, 2019 at 12:28 PM Simon Marchi <simark@simark.ca> wrote:
> > >
> > > On 2019-11-04 12:12 p.m., Andrew Burgess wrote:
> > > > How about I merged patches #1 and #2, but drop #3 for now?  This would
> > > > give:
> > > >
> > > >   lookup_static_symbol - returns the global static symbol for the
> > > >       current compilation unit, or whatever else GDB can find if
> > > >       there's nothing in the current CU.  This is basically inline
> > > >       with what we get from 'print symbol_name' at the CLI (if we
> > > >       limit symbol_name to just global static things).
> > > >
> > > > AND,
> > > >
> > > >   lookup_static_symbols - returns the list of all global static
> > > >       symbols.
> > > >
> > > > We no longer have a block parameter passed to 'lookup_static_symbol',
> > > > so hopefully and confusion can be avoided.  If/when we later add a
> > > > Python wrapper for CU we can _extend_ the API for lookup_static_symbol
> > > > to also take a CU and so provide the "look over there" type behaviour
> > > > in a clearer way.
> > > >
> > > > Would everyone be happy with this?
> > >
> > > I certainly would, I think that brings us back to the same place we were before
> > > it was suggested to add the block parameter, doesn't it?
> >
> > Sorry for the late reply & for confusing things -- yes, after reading
> > through all the discussions I think that's the best way forward.
>
> Thanks for the feedback.
>
> I've now pushed patches #1 and #2.

Thanks! By the way, should Objfile also get a lookup_static_symbols
method now, similar to Objfile.lookup_static_symbol?

(I don't personally have a use-case for this, but figured I should ask)

Christian
  
Andrew Burgess Nov. 11, 2019, 4:27 p.m. UTC | #11
* Christian Biesinger <cbiesinger@google.com> [2019-11-10 16:36:52 -0600]:

> On Sun, Nov 10, 2019 at 4:27 PM Andrew Burgess
> <andrew.burgess@embecosm.com> wrote:
> >
> > * Christian Biesinger <cbiesinger@google.com> [2019-11-09 00:23:00 -0600]:
> >
> > > On Mon, Nov 4, 2019 at 12:28 PM Simon Marchi <simark@simark.ca> wrote:
> > > >
> > > > On 2019-11-04 12:12 p.m., Andrew Burgess wrote:
> > > > > How about I merged patches #1 and #2, but drop #3 for now?  This would
> > > > > give:
> > > > >
> > > > >   lookup_static_symbol - returns the global static symbol for the
> > > > >       current compilation unit, or whatever else GDB can find if
> > > > >       there's nothing in the current CU.  This is basically inline
> > > > >       with what we get from 'print symbol_name' at the CLI (if we
> > > > >       limit symbol_name to just global static things).
> > > > >
> > > > > AND,
> > > > >
> > > > >   lookup_static_symbols - returns the list of all global static
> > > > >       symbols.
> > > > >
> > > > > We no longer have a block parameter passed to 'lookup_static_symbol',
> > > > > so hopefully and confusion can be avoided.  If/when we later add a
> > > > > Python wrapper for CU we can _extend_ the API for lookup_static_symbol
> > > > > to also take a CU and so provide the "look over there" type behaviour
> > > > > in a clearer way.
> > > > >
> > > > > Would everyone be happy with this?
> > > >
> > > > I certainly would, I think that brings us back to the same place we were before
> > > > it was suggested to add the block parameter, doesn't it?
> > >
> > > Sorry for the late reply & for confusing things -- yes, after reading
> > > through all the discussions I think that's the best way forward.
> >
> > Thanks for the feedback.
> >
> > I've now pushed patches #1 and #2.
> 
> Thanks! By the way, should Objfile also get a lookup_static_symbols
> method now, similar to Objfile.lookup_static_symbol?
> 
> (I don't personally have a use-case for this, but figured I should
> ask)

I don't think that's a crazy idea, but I don't plan to write one right
now.

My interest in this area started when I was auditing how the GDB
function lookup_static_function (the one in symtab.c) was used, and I
felt that the way it was used in the python code wasn't quite right.
I don't have any specific use cases to address.

We can always add extra API methods later if/when there's a need for
them without breaking backward compatibility, so I'm happy to leave
this for now.

Thanks,
Andrew
  
Terekhov, Mikhail via Gdb-patches Nov. 11, 2019, 4:30 p.m. UTC | #12
On Mon, Nov 11, 2019 at 8:27 AM Andrew Burgess
<andrew.burgess@embecosm.com> wrote:
> > Thanks! By the way, should Objfile also get a lookup_static_symbols
> > method now, similar to Objfile.lookup_static_symbol?
> >
> > (I don't personally have a use-case for this, but figured I should
> > ask)
>
> I don't think that's a crazy idea, but I don't plan to write one right
> now.
>
> My interest in this area started when I was auditing how the GDB
> function lookup_static_function (the one in symtab.c) was used, and I
> felt that the way it was used in the python code wasn't quite right.
> I don't have any specific use cases to address.
>
> We can always add extra API methods later if/when there's a need for
> them without breaking backward compatibility, so I'm happy to leave
> this for now.

OK, sounds good.

Christian
  

Patch

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