Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Simon Marchi <simark@simark.ca>
To: Andrew Burgess <andrew.burgess@embecosm.com>,
	gdb-patches <gdb-patches@sourceware.org>
Cc: Christian Biesinger <cbiesinger@google.com>
Subject: Re: [PATCH] gdb/python: Introduce gdb.lookup_all_static_symbols
Date: Tue, 15 Oct 2019 19:28:00 -0000	[thread overview]
Message-ID: <32eba92d-55a9-5694-cec5-80001d8ff1ae@simark.ca> (raw)
In-Reply-To: <20191015164647.1837-1-andrew.burgess@embecosm.com>

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


  parent reply	other threads:[~2019-10-15 19:28 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-23 16:46 [PATCH] gdb/python: smarter symbol lookup for gdb.lookup_static_symbol Andrew Burgess
2019-09-23 17:18 ` Eli Zaretskii
2019-10-08 11:07   ` [PATCHv2] " Andrew Burgess
2019-10-08 12:01     ` Eli Zaretskii
2019-10-08 16:01     ` Christian Biesinger via gdb-patches
2019-10-15 14:15       ` Andrew Burgess
2019-10-15 16:46         ` [PATCH] gdb/python: Introduce gdb.lookup_all_static_symbols Andrew Burgess
2019-10-15 17:07           ` Eli Zaretskii
2019-10-23 19:15             ` Andrew Burgess
2019-10-24  2:31               ` Eli Zaretskii
2019-10-15 19:28           ` Simon Marchi [this message]
2019-10-15 23:32             ` Andrew Burgess
2019-10-21 22:37             ` Christian Biesinger via gdb-patches
2019-10-23 19:14               ` Andrew Burgess
2019-10-24  3:11                 ` Simon Marchi
2019-10-24 16:53                   ` Andrew Burgess
2019-10-28  5:08                 ` Christian Biesinger via gdb-patches
2019-11-01 11:53                   ` Andrew Burgess
2019-11-01 13:55                     ` Simon Marchi
2019-11-04 17:12                       ` Andrew Burgess
2019-11-04 18:28                         ` Simon Marchi
2019-11-09  6:23                           ` Christian Biesinger via gdb-patches
2019-11-10 22:27                             ` Andrew Burgess
2019-11-10 22:37                               ` Christian Biesinger via gdb-patches
2019-11-11 16:27                                 ` Andrew Burgess
2019-11-11 16:31                                   ` Christian Biesinger via gdb-patches
2019-10-16 21:53         ` [PATCHv2] gdb/python: smarter symbol lookup for gdb.lookup_static_symbol Christian Biesinger via gdb-patches

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=32eba92d-55a9-5694-cec5-80001d8ff1ae@simark.ca \
    --to=simark@simark.ca \
    --cc=andrew.burgess@embecosm.com \
    --cc=cbiesinger@google.com \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox