From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 115051 invoked by alias); 1 Nov 2019 11:53:12 -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 115043 invoked by uid 89); 1 Nov 2019 11:53:12 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-23.1 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_STOCKGEN,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.1 spammy=Frame, 2019-10-21, Accept, discussing X-HELO: mail-wr1-f42.google.com Received: from mail-wr1-f42.google.com (HELO mail-wr1-f42.google.com) (209.85.221.42) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 01 Nov 2019 11:53:09 +0000 Received: by mail-wr1-f42.google.com with SMTP id e6so7584166wrw.1 for ; Fri, 01 Nov 2019 04:53:09 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=embecosm.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=vLVQj0w/kbxoZUYyT5LavAg6honHW0XalmbWYA+5ZZg=; b=R8kNDgIitErLll8cYqi+vSDD/6yUp3h9Sa8TXp7yrVryN2Js6LIXT2QmgB0qajkSkc 9yDOxoI5lXJV0v41/tVrDeJyWJpoJpphPwIH/Mz8/zxwME6zG4wZ3oWVLQF9Pj9SDbgw MO2NJ9nKsYRFWP1pxVJMLl/W7SFVTO6YOAE7o8JJDyDyFTxk33rgrw0ZxyQHPbT1svxs c3znO0o5UPdu5CRLq0n57dkZ4KX9Cah0GLSwuXOnhgbtfX8YceqErG65tlFjFw8ghQJX Rs6J7irNop+xP7hEzAwZRQT81ozX2wjpfRzogeTvfwGqLWq/HeA5a85hgNkyd2f3f5XO ZILQ== Return-Path: Received: from localhost (host86-128-12-122.range86-128.btcentralplus.com. [86.128.12.122]) by smtp.gmail.com with ESMTPSA id z189sm10590043wmc.25.2019.11.01.04.53.05 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Fri, 01 Nov 2019 04:53:05 -0700 (PDT) Date: Fri, 01 Nov 2019 11:53:00 -0000 From: Andrew Burgess To: Christian Biesinger Cc: Simon Marchi , gdb-patches Subject: Re: [PATCH] gdb/python: Introduce gdb.lookup_all_static_symbols Message-ID: <20191101115304.GR4962@embecosm.com> References: <20191015141515.GW4962@embecosm.com> <20191015164647.1837-1-andrew.burgess@embecosm.com> <32eba92d-55a9-5694-cec5-80001d8ff1ae@simark.ca> <20191023191354.GH4962@embecosm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Fortune: Your society will be sought by people of taste and refinement. X-Editor: GNU Emacs [ http://www.gnu.org/software/emacs ] User-Agent: Mutt/1.9.2 (2017-12-15) X-IsSubscribed: yes X-SW-Source: 2019-11/txt/msg00014.txt.bz2 * Christian Biesinger [2019-10-28 00:07:17 -0500]: > 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? This is not correct, both lookup_static_symbol AND lookup_static_symbols BOTH only lookup file level static globals. [ SIDE-ISSUE : The name for both of these is obviously a possible sauce of confusion, maybe before this function makes it into a release we should consider renaming it/them? How about lookup_static_global(s) ? ] Here's a demo session using the above 3 patches, the test file is: static int global_v1 = 1; static int global_v2 = 2; int use_them () { return global_v1 + global_v2; } int main () { static int global_v2 = 3; static int global_v3 = 4; return use_them () + global_v2 + global_v3; } And my GDB session: (gdb) start Temporary breakpoint 1 at 0x40049a: file test.c, line 16. Starting program: /projects/gdb/tmp/static-syms/test Temporary breakpoint 1, main () at test.c:16 16 return use_them () + global_v2 + global_v3; (gdb) python print (gdb.lookup_static_symbol ('global_v1').value ()) 1 (gdb) python print (gdb.lookup_static_symbol ('global_v2').value ()) 2 (gdb) python print (gdb.lookup_static_symbol ('global_v3').value ()) Traceback (most recent call last): File "", line 1, in AttributeError: 'NoneType' object has no attribute 'value' Error while executing Python code. (gdb) Notice that despite being inside main, we see the file level 'global_v2' and completely miss the 'global_v3'. This is inline with the original behaviour of this function before I started messing with it. One source of confusion may have been that I added a call to lookup_symbol_in_static_block, this doesn't find a static symbol in a block, or the first static symbol between block and the global scope, but instead finds the static scope for a block and looks for a matching symbol in that block. The other source of confusion was my talking about 'print symbol_name'. You are correct that in the above test if I did 'print global_v2' I would see the static within main, so in that sense what I've implemented is _not_ like 'print symbol_name'. The only comparison I meant to draw with 'print symbol_name' was that if I do try to access 'global_v1' while in main I know I'll get the global_v1 from _this_ file, not a global_v1 from some other file. With the new 3rd patch a user can now from Python say, if I was in some other block, which static global would I see, which I think for a scripting interface is helpful. Then with the second patch the user can also find all static globals. However, anything you can find with lookup_static_symbol will _always_ be in the list returned by lookup_static_symbols. Hope this clears things up, Thanks, Andrew > > 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" \