gdb/python: Introduce gdb.lookup_all_static_symbols

Message ID 20191015164647.1837-1-andrew.burgess@embecosm.com
State New, archived
Headers

Commit Message

Andrew Burgess Oct. 15, 2019, 4:46 p.m. UTC
  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.

However, if the user of the python API wants to instead get a
consistent set of static symbols, no matter where they stop, then they
have to instead consider all static symbols with a given name - there
could be many.  That is what this new API function offers, it returns
a list (possibly empty) of all static symbols matching a given
name (and optionally a given symbol domain).

gdb/ChangeLog:

	* python/py-symbol.c (gdbpy_lookup_all_static_symbols): New
	function.
	* python/python-internal.h (gdbpy_lookup_all_static_symbols):
	Declare new function.
	* python/python.c (python_GdbMethods): Add
	gdb.lookup_all_static_symbols method.

gdb/testsuite/ChangeLog:

	* gdb.python/py-symbol.exp: Add test for
	gdb.lookup_all_static_symbols.

gdb/doc/ChangeLog:

	* python.texi (Symbols In Python): Add documentation for
	gdb.lookup_all_static_symbols.

Change-Id: I1153b0ae5bcbc43b3dcf139043c7a48bf791e1a3
---
 gdb/ChangeLog                          |  9 ++++++
 gdb/doc/ChangeLog                      |  5 +++
 gdb/doc/python.texi                    | 35 +++++++++++++++++++++
 gdb/python/py-symbol.c                 | 56 ++++++++++++++++++++++++++++++++++
 gdb/python/python-internal.h           |  2 ++
 gdb/python/python.c                    |  4 +++
 gdb/testsuite/ChangeLog                |  5 +++
 gdb/testsuite/gdb.python/py-symbol.exp |  8 +++++
 8 files changed, 124 insertions(+)
  

Comments

Eli Zaretskii Oct. 15, 2019, 5:07 p.m. UTC | #1
> From: Andrew Burgess <andrew.burgess@embecosm.com>
> Cc: Christian Biesinger <cbiesinger@google.com>,	Andrew Burgess <andrew.burgess@embecosm.com>
> Date: Tue, 15 Oct 2019 17:46:47 +0100
> 
> gdb/ChangeLog:
> 
> 	* python/py-symbol.c (gdbpy_lookup_all_static_symbols): New
> 	function.
> 	* python/python-internal.h (gdbpy_lookup_all_static_symbols):
> 	Declare new function.
> 	* python/python.c (python_GdbMethods): Add
> 	gdb.lookup_all_static_symbols method.
> 
> gdb/testsuite/ChangeLog:
> 
> 	* gdb.python/py-symbol.exp: Add test for
> 	gdb.lookup_all_static_symbols.
> 
> gdb/doc/ChangeLog:
> 
> 	* python.texi (Symbols In Python): Add documentation for
> 	gdb.lookup_all_static_symbols.

The python.texi part is OK, but I think this change also needs a NEWS
entry.

Thanks.
  
Simon Marchi Oct. 15, 2019, 7:28 p.m. UTC | #2
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')

If we had that, we wouldn't need gdb.lookup_static_symbol.  However, we don't
have compilation unit objects in Python, so I see gdb.lookup_static_symbol
as a crutch until then.  If gdb.lookup_static_symbol returns in priority the
symbol that's visible from the current context, it partially covers use case #2.

If we have gdb.lookup_all_static_symbols, I think it's not useful to also have
gdb.lookup_static_symbol returning the first element, because it's equivalent
to gdb.lookup_all_static_symbols(...)[0].

> However, if the user of the python API wants to instead get a
> consistent set of static symbols, no matter where they stop, then they
> have to instead consider all static symbols with a given name - there
> could be many.  That is what this new API function offers, it returns
> a list (possibly empty) of all static symbols matching a given
> name (and optionally a given symbol domain).

Bikeshed time: what do you think of naming the new function `lookup_static_symbols`?

> gdb/ChangeLog:
> 
> 	* python/py-symbol.c (gdbpy_lookup_all_static_symbols): New
> 	function.
> 	* python/python-internal.h (gdbpy_lookup_all_static_symbols):
> 	Declare new function.
> 	* python/python.c (python_GdbMethods): Add
> 	gdb.lookup_all_static_symbols method.
> 
> gdb/testsuite/ChangeLog:
> 
> 	* gdb.python/py-symbol.exp: Add test for
> 	gdb.lookup_all_static_symbols.
> 
> gdb/doc/ChangeLog:
> 
> 	* python.texi (Symbols In Python): Add documentation for
> 	gdb.lookup_all_static_symbols.
> 
> Change-Id: I1153b0ae5bcbc43b3dcf139043c7a48bf791e1a3
> ---
>  gdb/ChangeLog                          |  9 ++++++
>  gdb/doc/ChangeLog                      |  5 +++
>  gdb/doc/python.texi                    | 35 +++++++++++++++++++++
>  gdb/python/py-symbol.c                 | 56 ++++++++++++++++++++++++++++++++++
>  gdb/python/python-internal.h           |  2 ++
>  gdb/python/python.c                    |  4 +++
>  gdb/testsuite/ChangeLog                |  5 +++
>  gdb/testsuite/gdb.python/py-symbol.exp |  8 +++++
>  8 files changed, 124 insertions(+)
> 
> diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
> index 0f12de94bba..2126effa12f 100644
> --- a/gdb/doc/python.texi
> +++ b/gdb/doc/python.texi
> @@ -4880,6 +4880,41 @@
>  information.
>  @end defun
>  
> +@findex gdb.lookup_global_symbol
> +@defun gdb.lookup_global_symbol (name @r{[}, domain@r{]})
> +This function searches for a global symbol by name.
> +The search scope can be restricted to by the domain argument.
> +
> +@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.
> +@end defun
> +
> +@findex gdb.lookup_all_static_symbols
> +@defun gdb.lookup_all_static_symbols (name @r{[}, domain@r{]})
> +Similar to @code{gdb.lookup_static_symbol}, this function searches for
> +global symbols with static linkage by name, and optionally restricted
> +by the domain argument.  However, this function returns a list of all
> +matching symbols found, not just the first one.
> +
> +@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 list of @code{gdb.Symbol} objects which could be empty
> +if no matching symbols were 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
> +
>  A @code{gdb.Symbol} object has the following attributes:
>  
>  @defvar Symbol.type
> diff --git a/gdb/python/py-symbol.c b/gdb/python/py-symbol.c
> index ae9aca6c5d0..6e126f9f377 100644
> --- a/gdb/python/py-symbol.c
> +++ b/gdb/python/py-symbol.c
> @@ -534,6 +534,62 @@ gdbpy_lookup_static_symbol (PyObject *self, PyObject *args, PyObject *kw)
>    return sym_obj;
>  }
>  
> +/* Implementation of
> +   gdb.lookup_all_static_symbols (name [, domain) -> [symbol] or None.
> +
> +   Returns a list of all static symbols matching NAME in DOMAIN.  */
> +
> +PyObject *
> +gdbpy_lookup_all_static_symbols (PyObject *self, PyObject *args, PyObject *kw)
> +{
> +  const char *name;
> +  int domain = VAR_DOMAIN;
> +  static const char *keywords[] = { "name", "domain", NULL };
> +
> +  if (!gdb_PyArg_ParseTupleAndKeywords (args, kw, "s|i", keywords, &name,
> +					&domain))
> +    return NULL;
> +
> +  gdbpy_ref<> return_list (PyList_New (0));
> +  if (return_list == NULL)
> +    return NULL;
> +
> +  try
> +    {
> +      for (objfile *objfile : current_program_space->objfiles ())
> +	{
> +	  for (compunit_symtab *cust : objfile->compunits ())

This list only contains the already expanded compilation units.  So it won't find
the static symbols in CUs that are not yet expanded to full compunit symtabs.  Here's
an example:

test.c:

```
static int yo = 2;

int foo(int x);

int main() {
        return foo(4) + yo;
}
```

test2.c:

```
static int yo = 6;

int foo(int x) {
        return yo * x;
}
```

Compiled with

  gcc test.c test2.c -g3 -O0

$ ./gdb --data-directory=data-directory a.out -batch -ex "python print(gdb.lookup_all_static_symbols('yo'))"
[<gdb.Symbol object at 0x7f6223978e68>]
$ ./gdb --data-directory=data-directory a.out -batch -ex "python print(gdb.lookup_all_static_symbols('yo'))" -readnow
[<gdb.Symbol object at 0x7fef0b649e68>, <gdb.Symbol object at 0x7fef0b649f58>]

So I guess there should be a step where we expand CUs with matching symbols first.

Simon
  
Terekhov, Mikhail via Gdb-patches Oct. 21, 2019, 10:36 p.m. UTC | #3
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?

Either way, I'm happy with this patch; I've come around to y'all's view.

Christian
  

Patch

diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
index 0f12de94bba..2126effa12f 100644
--- a/gdb/doc/python.texi
+++ b/gdb/doc/python.texi
@@ -4880,6 +4880,41 @@ 
 information.
 @end defun
 
+@findex gdb.lookup_global_symbol
+@defun gdb.lookup_global_symbol (name @r{[}, domain@r{]})
+This function searches for a global symbol by name.
+The search scope can be restricted to by the domain argument.
+
+@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.
+@end defun
+
+@findex gdb.lookup_all_static_symbols
+@defun gdb.lookup_all_static_symbols (name @r{[}, domain@r{]})
+Similar to @code{gdb.lookup_static_symbol}, this function searches for
+global symbols with static linkage by name, and optionally restricted
+by the domain argument.  However, this function returns a list of all
+matching symbols found, not just the first one.
+
+@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 list of @code{gdb.Symbol} objects which could be empty
+if no matching symbols were 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
+
 A @code{gdb.Symbol} object has the following attributes:
 
 @defvar Symbol.type
diff --git a/gdb/python/py-symbol.c b/gdb/python/py-symbol.c
index ae9aca6c5d0..6e126f9f377 100644
--- a/gdb/python/py-symbol.c
+++ b/gdb/python/py-symbol.c
@@ -534,6 +534,62 @@  gdbpy_lookup_static_symbol (PyObject *self, PyObject *args, PyObject *kw)
   return sym_obj;
 }
 
+/* Implementation of
+   gdb.lookup_all_static_symbols (name [, domain) -> [symbol] or None.
+
+   Returns a list of all static symbols matching NAME in DOMAIN.  */
+
+PyObject *
+gdbpy_lookup_all_static_symbols (PyObject *self, PyObject *args, PyObject *kw)
+{
+  const char *name;
+  int domain = VAR_DOMAIN;
+  static const char *keywords[] = { "name", "domain", NULL };
+
+  if (!gdb_PyArg_ParseTupleAndKeywords (args, kw, "s|i", keywords, &name,
+					&domain))
+    return NULL;
+
+  gdbpy_ref<> return_list (PyList_New (0));
+  if (return_list == NULL)
+    return NULL;
+
+  try
+    {
+      for (objfile *objfile : current_program_space->objfiles ())
+	{
+	  for (compunit_symtab *cust : objfile->compunits ())
+	    {
+	      const struct blockvector *bv;
+	      const struct block *block;
+
+	      bv = COMPUNIT_BLOCKVECTOR (cust);
+	      block = BLOCKVECTOR_BLOCK (bv, STATIC_BLOCK);
+
+	      if (block != nullptr)
+		{
+		  symbol *symbol = lookup_symbol_in_static_block
+		    (name, block, (domain_enum) domain).symbol;
+
+		  if (symbol != nullptr)
+		    {
+		      PyObject *sym_obj
+			= symbol_to_symbol_object (symbol);
+		      if (PyList_Append (return_list.get (), sym_obj) == -1)
+			return NULL;
+		    }
+		}
+	    }
+	}
+    }
+  catch (const gdb_exception &except)
+    {
+      GDB_PY_HANDLE_EXCEPTION (except);
+    }
+
+  return return_list.release ();
+}
+
 /* This function is called when an objfile is about to be freed.
    Invalidate the symbol as further actions on the symbol would result
    in bad data.  All access to obj->symbol should be gated by
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index c5578430cff..e29e018a9d8 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -426,6 +426,8 @@  PyObject *gdbpy_lookup_global_symbol (PyObject *self, PyObject *args,
 				      PyObject *kw);
 PyObject *gdbpy_lookup_static_symbol (PyObject *self, PyObject *args,
 				      PyObject *kw);
+PyObject *gdbpy_lookup_all_static_symbols (PyObject *self, PyObject *args,
+					   PyObject *kw);
 PyObject *gdbpy_start_recording (PyObject *self, PyObject *args);
 PyObject *gdbpy_current_recording (PyObject *self, PyObject *args);
 PyObject *gdbpy_stop_recording (PyObject *self, PyObject *args);
diff --git a/gdb/python/python.c b/gdb/python/python.c
index ddf0e72d26f..80936e2ab7b 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -1994,6 +1994,10 @@  Return the symbol corresponding to the given name (or None)." },
     METH_VARARGS | METH_KEYWORDS,
     "lookup_static_symbol (name [, domain]) -> symbol\n\
 Return the static-linkage symbol corresponding to the given name (or None)." },
+  { "lookup_all_static_symbols", (PyCFunction) gdbpy_lookup_all_static_symbols,
+    METH_VARARGS | METH_KEYWORDS,
+    "lookup_all_static_symbols (name [, domain]) -> symbol\n\
+Return a list of all static-linkage symbols corresponding to the given name." },
 
   { "lookup_objfile", (PyCFunction) gdbpy_lookup_objfile,
     METH_VARARGS | METH_KEYWORDS,
diff --git a/gdb/testsuite/gdb.python/py-symbol.exp b/gdb/testsuite/gdb.python/py-symbol.exp
index 61960075565..116c32441f8 100644
--- a/gdb/testsuite/gdb.python/py-symbol.exp
+++ b/gdb/testsuite/gdb.python/py-symbol.exp
@@ -108,6 +108,10 @@  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"
+gdb_test "python print (gdb.lookup_all_static_symbols ('rr')\[0\].value ())" "99" \
+    "print value of gdb.lookup_all_static_symbols ('rr')\[0\], from the other file"
+gdb_test "python print (gdb.lookup_all_static_symbols ('rr')\[1\].value ())" "42" \
+    "print value of gdb.lookup_all_static_symbols ('rr')\[1\], from the other file"
 
 # Now continue back to the first source file.
 set linenum [gdb_get_line_number "Break at end."]
@@ -119,6 +123,10 @@  gdb_py_test_silent_cmd "python frame = gdb.selected_frame()" "Get Frame" 0
 # 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"
+gdb_test "python print (gdb.lookup_all_static_symbols ('rr')\[0\].value ())" "99" \
+    "print value of gdb.lookup_all_static_symbols ('rr')\[0\], from the main file"
+gdb_test "python print (gdb.lookup_all_static_symbols ('rr')\[1\].value ())" "42" \
+    "print value of gdb.lookup_all_static_symbols ('rr')\[1\], from the main file"
 
 # Test is_variable attribute.
 gdb_py_test_silent_cmd "python a = gdb.lookup_symbol(\'a\')" "Get variable a" 0