Re: [PATCH 4/7] py-symbol: Require a frame for lookup_symbol only when necessary

[Jeff Mahoney <jeffm at suse dot com>]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 2/5/16 7:17 AM, Phil Muldoon wrote:
> On 04/02/16 17:29, jeffm@suse.com wrote:
>> From: Jeff Mahoney <jeffm@suse.com> > > gdbpy_lookup_symbol
>> requires a frame prior to doing the symbol lookup but > a frame
>> isn't necessary for many symbol types.  Calling >
>> symbol_read_needs_frame will tell us if it's necessary but we
>> need to > have already looked up the symbol to use it.  This
>> patch puts the > lookup first and then only resolves the frame if
>> one is required. > > This allows us to lookup static symbols
>> directly from python rather > than using gdb.eval_and_parse.
> 
> Please add documentation and further tests for new functionality.
> 
>> --- >  gdb/python/py-symbol.c | 31
>> +++++++++++++++++-------------- >  1 file changed, 17
>> insertions(+), 14 deletions(-) > > diff --git
>> a/gdb/python/py-symbol.c b/gdb/python/py-symbol.c > index
>> cbbc9e2..c7f0ff8 100644 > --- a/gdb/python/py-symbol.c > +++
>> b/gdb/python/py-symbol.c > @@ -382,14 +382,28 @@
>> gdbpy_lookup_symbol (PyObject *self, PyObject *args, PyObject
>> *kw) >  >    if (block_obj) >      block = block_object_to_block
>> (block_obj); > -  else > + > +  TRY > +    { > +      symbol =
>> lookup_symbol (name, block, (domain_enum) domain, > +
>> &is_a_field_of_this).symbol; > +    } > +  CATCH (except,
>> RETURN_MASK_ALL) > +    { > +      GDB_PY_HANDLE_EXCEPTION
>> (except); > +    } > +  END_CATCH > + > +  if (symbol && !block)
>> >      { >        struct frame_info *selected_frame; >  >
>> TRY >      { > -      selected_frame = get_selected_frame (_("No
>> frame selected.")); > -      block = get_frame_block
>> (selected_frame, NULL); > +      if
> (symbol_read_needs_frame(symbol)) {
> 
> Formatting nit, space after function name and before (.
> 
>> +        selected_frame = get_selected_frame (_("No frame
>> selected.")); > +        block = get_frame_block (selected_frame,
>> NULL); > +      } >      } >        CATCH (except,
>> RETURN_MASK_ALL) >      { > @@ -398,17 +412,6 @@
>> gdbpy_lookup_symbol (PyObject *self, PyObject *args, PyObject
>> *kw) >        END_CATCH >      }
> 
> My only concern here is (and the context is not meaningful enough
> to see), is that you've removed an "else" above which is an
> unconditional branch, and replaced it with an IF instead. What
> happens when both IFs can fail?

This patch is broken.  It works for my need to look up static symbols
but it fails the py-symbol testsuite.  We look up the block and never
use it, so it's no wonder it's failing.

- -Jeff


- -- 
Jeff Mahoney
SUSE Labs
-----BEGIN PGP SIGNATURE-----
Version: GnuPG/MacGPG2 v2.0.19 (Darwin)
Comment: GPGTools - http://gpgtools.org

iQIcBAEBAgAGBQJWtMjbAAoJEB57S2MheeWyJhcQALzkE3DaXUyTHxaSovnUbe6+
bFleikXDjbL4UKYwT/0iJfVoaH8weoWMiFfrYVXJgCqXIUEX9mtUPfMXq5NHyJ71
0FgQa9pRVCK2sS/4jpPjMwL40AU4DI4XYOHbSowZfm9kn28yY+uP6UkbiWsv5dDn
egNkSzjyrX4MzMHSzZIDqNBGqPdp03nLBKyrjl8iLtgUK6HZwBponk2Y50l8d+gy
M6FP4xXMUqKgW0JBW+P1V/S6YowW+RCaHgBrmwISe9QU79M3s3Jpi+VD+WxEO8HY
rUrlOXzJhMm/IcF9r4ROmYGXFdQdOjlMyTQtOkyCe8+VYlwOsc107GzipNnQ2mRf
Rd2+2UZNs+430pcpivq+h+wDlcM5Cu6QWDFa9/TIHw97n776YeQ6bZHcSCXFOvd2
FrWsOqQoM2zNTKTjuxCJyr2lfgHxFTiSggVrX1Glmw2sVdwks5DvoGXNa8lnq1fp
PGII0edxMPg1neOhZZ24j/k2dB1dZmAOMuzUg6OggSiVSj31R+ZmarE/xRHfXqPk
kTfuJHX45dpBTZQ+X6VgWkYzG6OHhRV2ilWHky7PETM3LTpTVFNGqOxEEYZach0/
C5SV6Zykp+wjkZTVLL/q3YgPRHJj+SUKRRBEj7qfG4PcyeI0ybsDVfQ8ZEh6JANI
WWRyy3/zyUbONSV77I/R
=eJFa
-----END PGP SIGNATURE-----