Message ID | 1454606973-31017-5-git-send-email-jeffm@suse.com |
---|---|
State | New, archived |
Headers |
Received: (qmail 16971 invoked by alias); 4 Feb 2016 17:29:42 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <gdb-patches.sourceware.org> List-Unsubscribe: <mailto:gdb-patches-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:gdb-patches-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/gdb-patches/> List-Post: <mailto:gdb-patches@sourceware.org> List-Help: <mailto:gdb-patches-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 16919 invoked by uid 89); 4 Feb 2016 17:29:41 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.2 spammy=sk:symbol_ X-HELO: mx2.suse.de Received: from mx2.suse.de (HELO mx2.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (CAMELLIA256-SHA encrypted) ESMTPS; Thu, 04 Feb 2016 17:29:40 +0000 Received: from relay1.suse.de (charybdis-ext.suse.de [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id 3152CAC01 for <gdb-patches@sourceware.org>; Thu, 4 Feb 2016 17:29:37 +0000 (UTC) Received: by starscream.home.jeffm.io (Postfix, from userid 1000) id 7D8E08998A; Thu, 4 Feb 2016 12:29:34 -0500 (EST) From: jeffm@suse.com To: gdb-patches@sourceware.org Cc: Jeff Mahoney <jeffm@suse.com> Subject: [PATCH 4/7] py-symbol: Require a frame for lookup_symbol only when necessary Date: Thu, 4 Feb 2016 12:29:30 -0500 Message-Id: <1454606973-31017-5-git-send-email-jeffm@suse.com> In-Reply-To: <1454606973-31017-1-git-send-email-jeffm@suse.com> References: <1454606973-31017-1-git-send-email-jeffm@suse.com> X-IsSubscribed: yes |
Commit Message
Jeff Mahoney
Feb. 4, 2016, 5:29 p.m. UTC
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.
---
gdb/python/py-symbol.c | 31 +++++++++++++++++--------------
1 file changed, 17 insertions(+), 14 deletions(-)
Comments
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? Cheers Phil
-----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-----
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. Thanks. I've rewritten this patch email reply from the garbled so hopefully you can make sense of it! ;) All of these patches need documentation in the user manual, and also new tests for 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)) { Space after the function name and before the (. This and others. > + 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 > } > > - 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 > - > ret_tuple = PyTuple_New (2); > if (!ret_tuple) > return NULL; 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? Cheers Phil
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)) { + 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 } - 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 - ret_tuple = PyTuple_New (2); if (!ret_tuple) return NULL;