From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 77702 invoked by alias); 4 Nov 2019 17:12:09 -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 77691 invoked by uid 89); 4 Nov 2019 17:12:09 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-6.3 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.1 spammy=demo, Notice X-HELO: mail-wr1-f46.google.com Received: from mail-wr1-f46.google.com (HELO mail-wr1-f46.google.com) (209.85.221.46) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 04 Nov 2019 17:12:07 +0000 Received: by mail-wr1-f46.google.com with SMTP id l10so18012601wrb.2 for ; Mon, 04 Nov 2019 09:12:07 -0800 (PST) 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=ixkTdqaLvlzGAMDNlhK6Gqf/y+f8RdHbo9DPJQ4BAQc=; b=cZ74YbElhcdE6Xkl873dcJFOhz839wvWtNegjFkhIgXxFTjNpIjdXgHf0snu1NnJqc XoXQwmNPQMdbD9qHEA3zF3bpAO/Bu9ZYJ97o7WZuFWpdbKM5FI5gnutTqzQ1rJlfXV8q C9hpeqwq7LaGIrdhrPsZXzujLCl6X3teBj6CEQdMRsxlvD35hTKsvNGq4guvHs9PLOc+ mmh3q4hQRV9dX1C6vFKzUjNg5HDHMzhANZZRcthz9gmOFbPGYI3TrHJ/ZewIzqpX++Bk 5eLXiBqsC/MLDgR53aalbv0narY92J6fBHVo4IiRBq0Sd4SLv+3Vnj4Vv1ItR169Avgx NH6A== Return-Path: Received: from localhost (cust64-dsl91-135-5.idnet.net. [91.135.5.64]) by smtp.gmail.com with ESMTPSA id w9sm18838800wrt.85.2019.11.04.09.12.04 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Mon, 04 Nov 2019 09:12:04 -0800 (PST) Date: Mon, 04 Nov 2019 17:12:00 -0000 From: Andrew Burgess To: Simon Marchi Cc: Christian Biesinger , gdb-patches Subject: Re: [PATCH] gdb/python: Introduce gdb.lookup_all_static_symbols Message-ID: <20191104171204.GB11037@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> <20191101115304.GR4962@embecosm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Fortune: The man on tops walks a lonely street User-Agent: Mutt/1.9.2 (2017-12-15) X-IsSubscribed: yes X-SW-Source: 2019-11/txt/msg00094.txt.bz2 * Simon Marchi [2019-11-01 09:55:07 -0400]: > On 2019-11-01 7:53 a.m., Andrew Burgess wrote: > >> 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) ? ] > > I would at least make sure the naming is consistent with gdb.lookup_global_symbol, > so have the _symbol(s) suffix. So, lookup_static_global_symbol(s). > > I don't think that lookup_static_symbols is particularly confusing. As a user, I > don't think I would expect it to return static symbols from local scopes. > > I think what makes lookup_static_symbol ambiguous is that we're considering making > it take a block as parameter. Then, I agree, a user could think that it will find > static symbols from local scopes. > > But AFAIK, we're considering passing a block to lookup_static_symbol only because > of a current shortcoming in the API, that we don't have an object type > representing a compilation unit. If lookup_static_symbol accepted a compilation > unit object instead, then I don't think there would be a similar confusion > (especially that it is singular, and returns a single symbol). > > Would it be an option to not add lookup_static_symbol for now, and instead work > on having a type representing compilation units, and then think about introducing > lookup_static_symbol again? Christian, can you achieve what you wanted using > lookup_static_symbols plus some filtering instead? How about I merged patches #1 and #2, but drop #3 for now? This would give: lookup_static_symbol - returns the global static symbol for the current compilation unit, or whatever else GDB can find if there's nothing in the current CU. This is basically inline with what we get from 'print symbol_name' at the CLI (if we limit symbol_name to just global static things). AND, lookup_static_symbols - returns the list of all global static symbols. We no longer have a block parameter passed to 'lookup_static_symbol', so hopefully and confusion can be avoided. If/when we later add a Python wrapper for CU we can _extend_ the API for lookup_static_symbol to also take a CU and so provide the "look over there" type behaviour in a clearer way. Would everyone be happy with this? Thanks, Andrew > > > 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. > > Ok, that's the behavior I expected (to not find static local symbols), thanks > for clearing that up. > > Simon