Re: [PATCH] gdb/python: Introduce gdb.lookup_all_static_symbols

[Andrew Burgess <andrew.burgess@embecosm.com>]

* Simon Marchi <simark@simark.ca> [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 "<string>", line 1, in <module>
> >   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