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

[Andrew Burgess <andrew.burgess@embecosm.com>]

* 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

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