From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 35014 invoked by alias); 28 Oct 2019 05:08:01 -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 34980 invoked by uid 89); 28 Oct 2019 05:08:00 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-29.4 required=5.0 tests=AWL,BAYES_00,ENV_AND_HDR_SPF_MATCH,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_STOCKGEN,RCVD_IN_DNSWL_NONE,SPF_PASS,USER_IN_DEF_SPF_WL autolearn=ham version=3.3.1 spammy=andrewburgessembecosmcom, U*andrew.burgess, andrew.burgess@embecosm.com, pc X-HELO: mail-wr1-f48.google.com Received: from mail-wr1-f48.google.com (HELO mail-wr1-f48.google.com) (209.85.221.48) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 28 Oct 2019 05:07:57 +0000 Received: by mail-wr1-f48.google.com with SMTP id w18so8423820wrt.3 for ; Sun, 27 Oct 2019 22:07:57 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=B+IBmZErSr9eVTc0GDQ3FW8GubA4C7qdoMbGF3Cx6Yg=; b=b2Jtel2tYJNlGRwTDJL9aET4KCRcT6Sw7/adHrdSz8Wfgw64Z8aU6l+VGiM/kNcrsM XaU4fEmrYmVaURNxj0q5tcFOSO38u3PxLAtmEIplZXotw5ACGKBtnYj1p/G+bsjXLQIt 4MKs+I32jyg4+COb4MLQH3aOu9AOTsoKVquF0dalgTICIbV6XNCWmdHt4BmTik4kXudT LjHXTsL5nYS2CCDn9fuVeOh6Stt27ngmSKLGO3yIx+/5W85RDKn5rIdiVMsTFdL5J5/u Ed3ZvLdi9DZO6t+nrfHbK7SbQ0W7VqByLN6CvTug8OZtQum0W0ZiSj4ahCHoQdlgOuru peDw== MIME-Version: 1.0 References: <20191015141515.GW4962@embecosm.com> <20191015164647.1837-1-andrew.burgess@embecosm.com> <32eba92d-55a9-5694-cec5-80001d8ff1ae@simark.ca> <20191023191354.GH4962@embecosm.com> In-Reply-To: <20191023191354.GH4962@embecosm.com> From: "Christian Biesinger via gdb-patches" Reply-To: Christian Biesinger Date: Mon, 28 Oct 2019 05:08:00 -0000 Message-ID: Subject: Re: [PATCH] gdb/python: Introduce gdb.lookup_all_static_symbols To: Andrew Burgess Cc: Simon Marchi , gdb-patches Content-Type: text/plain; charset="UTF-8" X-IsSubscribed: yes X-SW-Source: 2019-10/txt/msg00980.txt.bz2 On Wed, Oct 23, 2019 at 2:13 PM Andrew Burgess wrote: > > * Christian Biesinger [2019-10-21 17:36:37 -0500]: > > > On Tue, Oct 15, 2019 at 2:28 PM Simon Marchi 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 > 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" \