From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 124087 invoked by alias); 15 Oct 2019 19:28:54 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 123583 invoked by uid 89); 15 Oct 2019 19:28:54 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-18.0 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_STOCKGEN,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.1 spammy= X-HELO: simark.ca Received: from simark.ca (HELO simark.ca) (158.69.221.121) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 15 Oct 2019 19:28:52 +0000 Received: from [172.16.0.148] (192-222-181-218.qc.cable.ebox.net [192.222.181.218]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 2C6421E059; Tue, 15 Oct 2019 15:28:50 -0400 (EDT) Subject: Re: [PATCH] gdb/python: Introduce gdb.lookup_all_static_symbols To: Andrew Burgess , gdb-patches Cc: Christian Biesinger References: <20191015141515.GW4962@embecosm.com> <20191015164647.1837-1-andrew.burgess@embecosm.com> From: Simon Marchi Message-ID: <32eba92d-55a9-5694-cec5-80001d8ff1ae@simark.ca> Date: Tue, 15 Oct 2019 19:28:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.9.0 MIME-Version: 1.0 In-Reply-To: <20191015164647.1837-1-andrew.burgess@embecosm.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-SW-Source: 2019-10/txt/msg00448.txt.bz2 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 --data-directory=data-directory a.out -batch -ex "python print(gdb.lookup_all_static_symbols('yo'))" -readnow [, ] So I guess there should be a step where we expand CUs with matching symbols first. Simon