From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 28766 invoked by alias); 1 Nov 2019 13:55:13 -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 28758 invoked by uid 89); 1 Nov 2019 13:55:12 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-5.9 required=5.0 tests=AWL,BAYES_00,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.1 spammy=H*F:D*ca 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; Fri, 01 Nov 2019 13:55:11 +0000 Received: from [172.16.0.155] (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 B72171E56E; Fri, 1 Nov 2019 09:55:08 -0400 (EDT) Subject: Re: [PATCH] gdb/python: Introduce gdb.lookup_all_static_symbols To: Andrew Burgess , Christian Biesinger Cc: gdb-patches 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> From: Simon Marchi Message-ID: Date: Fri, 01 Nov 2019 13:55: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: <20191101115304.GR4962@embecosm.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-SW-Source: 2019-11/txt/msg00022.txt.bz2 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? > 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