* [python][patch] Fix 11036 @ 2010-02-25 14:29 Phil Muldoon 2010-02-25 18:34 ` Tom Tromey 0 siblings, 1 reply; 6+ messages in thread From: Phil Muldoon @ 2010-02-25 14:29 UTC (permalink / raw) To: gdb-patches This patch fixes python/11036. It allows access to variables that have the same name in different scopes to be differentiated in read_var with an optional block argument. This allows the API user to refine the read_var search. What do you think? Cheers, Phil -- ChangeLog 2010-02-25 Phil Muldoon <pmuldoon@redhat.com> PR python/11036 * python/py-frame.c (frapy_read_var): Add block argument and logic to cope with user provided blocks. doc/ChangeLog 2010-02-25 Phil Muldoon <pmuldoon@redhat.com> * gdb.texinfo (Frames In Python): Add block parameter and description to read_var text. testsuite/ChangeLog 2010-02-25 Phil Muldoon <pmuldoon@redhat.com> * gdb.python/py-frame.exp: Add read_var block tests. * gdb.python/py-frame.c (block): New function. -- diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 0a7191e..d8b7572 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,9 @@ +2010-02-25 Phil Muldoon <pmuldoon@redhat.com> + + PR python/11036 + * python/py-frame.c (frapy_read_var): Add block argument and logic + to cope with user provided blocks. + 2010-02-24 Pedro Alves <pedro@codesourcery.com> * mi/mi-main.c (mi_cmd_execute): Fix typo. diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo index 82742d4..ded9aa2 100644 --- a/gdb/doc/gdb.texinfo +++ b/gdb/doc/gdb.texinfo @@ -20795,9 +20795,12 @@ Return the frame's symtab and line object. @xref{Symbol Tables In Python}. @end defmethod -@defmethod Frame read_var variable -Return the value of the given variable in this frame. @var{variable} must -be a string. +@defmethod Frame read_var variable @r{[}block@r{]} +Return the value of the given variable in this frame. If the optional +block argument is provided, search for the variable from that current +block; otherwise start at the frame's current block (which is +determined by the frame's current program counter). @var{variable} +must be a string. @var{block} must be a @code{gdb.Block} object. @end defmethod @defmethod Frame select diff --git a/gdb/python/py-frame.c b/gdb/python/py-frame.c index 94211db..46c0bb0 100644 --- a/gdb/python/py-frame.c +++ b/gdb/python/py-frame.c @@ -380,20 +380,23 @@ frapy_find_sal (PyObject *self, PyObject *args) return sal_obj; } -/* Implementation of gdb.Frame.read_var_value (self, variable) -> gdb.Value. - Returns the value of the given variable in this frame. The argument must be - a string. Returns None if GDB can't find the specified variable. */ - +/* Implementation of gdb.Frame.read_var_value (self, variable, + [block]) -> gdb.Value. If the optional block argument is provided + start the search from that block, otherwise search from the frame's + current block (determined by examining the resume address of the + frame). The variable argument must be a string. The block + argument must be an instance of gdb.Block. Returns None if GDB + can't find the specified variable. */ static PyObject * frapy_read_var (PyObject *self, PyObject *args) { struct frame_info *frame; - PyObject *sym_obj; + PyObject *sym_obj, *block_obj = NULL; struct symbol *var = NULL; /* gcc-4.3.2 false warning. */ struct value *val = NULL; volatile struct gdb_exception except; - if (!PyArg_ParseTuple (args, "O", &sym_obj)) + if (!PyArg_ParseTuple (args, "O|O", &sym_obj, &block_obj)) return NULL; if (PyObject_TypeCheck (sym_obj, &symbol_object_type)) @@ -410,11 +413,19 @@ frapy_read_var (PyObject *self, PyObject *args) return NULL; cleanup = make_cleanup (xfree, var_name); + if (block_obj) + { + block = block_object_to_block (block_obj); + if (block == NULL) + return NULL; + } + TRY_CATCH (except, RETURN_MASK_ALL) { FRAPY_REQUIRE_VALID ((frame_object *) self, frame); - block = block_for_pc (get_frame_address_in_block (frame)); + if (!block) + block = block_for_pc (get_frame_address_in_block (frame)); var = lookup_symbol (var_name, block, VAR_DOMAIN, NULL); } GDB_PY_HANDLE_EXCEPTION (except); diff --git a/gdb/testsuite/gdb.python/py-frame.c b/gdb/testsuite/gdb.python/py-frame.c index 22eb9f2..82db341 100644 --- a/gdb/testsuite/gdb.python/py-frame.c +++ b/gdb/testsuite/gdb.python/py-frame.c @@ -8,7 +8,23 @@ int f1 (int a, int b) return f2(a) + b; } +int block (void) +{ + int i = 99; + { + double i = 1.1; + double f = 2.2; + { + const char *i = "stuff"; + const char *f = "foo"; + const char *b = "bar"; + return 0; /* Block break here. */ + } + } +} + int main (int argc, char *argv[]) { + block (); return f1 (1, 2); } diff --git a/gdb/testsuite/gdb.python/py-frame.exp b/gdb/testsuite/gdb.python/py-frame.exp index 6989207..150e737 100644 --- a/gdb/testsuite/gdb.python/py-frame.exp +++ b/gdb/testsuite/gdb.python/py-frame.exp @@ -56,6 +56,30 @@ if ![runto_main] then { return 0 } +gdb_breakpoint [gdb_get_line_number "Block break here."] +gdb_continue_to_breakpoint "Block break here." +gdb_py_test_silent_cmd "python bf1 = gdb.selected_frame ()" "get frame" 0 + +# First test that read_var is unaffected by PR 11036 changes. +gdb_test "python print bf1.read_var(\"i\")" "\"stuff\"" "test i" +gdb_test "python print bf1.read_var(\"f\")" "\"foo\"" "test f" +gdb_test "python print bf1.read_var(\"b\")" "\"bar\"" "test b" + +# Test the read_var function in another block other than the current +# block (in this case, the super block). Test thar read_var is reading +# the correct variables of i and f but they are the correct value and type. +gdb_py_test_silent_cmd "python sb = bf1.block().superblock" "get superblock" 0 +gdb_test "python print bf1.read_var(\"i\", sb)" "1.1.*" "test i = 1.1" +gdb_test "python print bf1.read_var(\"i\", sb).type" "double" "test double i" +gdb_test "python print bf1.read_var(\"f\", sb)" "2.2.*" "test f = 2.2" +gdb_test "python print bf1.read_var(\"f\", sb).type" "double" "test double f" + +# And again test another outerblock, this time testing "i" is the +# correct value and type. +gdb_py_test_silent_cmd "python sb = sb.superblock" "get superblock" 0 +gdb_test "python print bf1.read_var(\"i\", sb)" "99" "test i = 99" +gdb_test "python print bf1.read_var(\"i\", sb).type" "int" "test int i" + gdb_breakpoint "f2" gdb_continue_to_breakpoint "breakpoint at f2" gdb_test "up" "" "" ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [python][patch] Fix 11036 2010-02-25 14:29 [python][patch] Fix 11036 Phil Muldoon @ 2010-02-25 18:34 ` Tom Tromey 2010-02-26 12:38 ` Phil Muldoon 0 siblings, 1 reply; 6+ messages in thread From: Tom Tromey @ 2010-02-25 18:34 UTC (permalink / raw) To: Phil Muldoon; +Cc: gdb-patches >>>>> "Phil" == Phil Muldoon <pmuldoon@redhat.com> writes: Phil> 2010-02-25 Phil Muldoon <pmuldoon@redhat.com> Phil> PR python/11036 Phil> * python/py-frame.c (frapy_read_var): Add block argument and logic Phil> to cope with user provided blocks. Phil> + argument must be an instance of gdb.Block. Returns None if GDB Phil> + can't find the specified variable. */ This last sentence in this comment seems to be wrong. Sometimes the function returns None, sometimes it throws an exception. I think the function should behave more predictably somehow. Phil> + block = block_object_to_block (block_obj); Phil> + if (block == NULL) Phil> + return NULL; When block_object_to_block returns NULL, it does not set the Python exception. That makes this code erroneous. In other respects the code bits seem reasonable to me. Tom ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [python][patch] Fix 11036 2010-02-25 18:34 ` Tom Tromey @ 2010-02-26 12:38 ` Phil Muldoon 2010-02-26 14:56 ` Eli Zaretskii 2010-02-26 23:16 ` Tom Tromey 0 siblings, 2 replies; 6+ messages in thread From: Phil Muldoon @ 2010-02-26 12:38 UTC (permalink / raw) To: Tom Tromey; +Cc: gdb-patches On Thu, Feb 25, 2010 at 11:33:49AM -0700, Tom Tromey wrote: > >>>>> "Phil" == Phil Muldoon <pmuldoon@redhat.com> writes: > Phil> + argument must be an instance of gdb.Block. Returns None if GDB > Phil> + can't find the specified variable. */ > > This last sentence in this comment seems to be wrong. > Sometimes the function returns None, sometimes it throws an exception. > I think the function should behave more predictably somehow. I took a look at the code that lies beneath the instance where this function could return None. It seems that read_var_value can (according to the comments heading that function) return NULL. I'm stumped when this can happen, as it deals with each LOC_* scenario, throwing the relevant errors when needed. We talked a little about this on irc last night, and even in the case of symbols where the value have been optimized away, read_var still returns non-null: (gdb) py f = gdb.selected_frame() (gdb) py print f.read_var("optimize_me") <value optimized out> If you examine the runtime, the return here is a valid value, not a NULL. In any case, I think that if read_var returns NULL, then this is an error case. If anyone knows this to be different let me know and I can go back to returning None in this case. To reflect this, if the result of read_var is NULL, I raise an exception. What do you think? > Phil> + block = block_object_to_block (block_obj); > Phil> + if (block == NULL) > Phil> + return NULL; Ok, fixed. I have also updated the comments and the documentation to reflect the new decisions. Cheers, Phil -- diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo index f6105b7..23aca55 100644 --- a/gdb/doc/gdb.texinfo +++ b/gdb/doc/gdb.texinfo @@ -20799,9 +20799,13 @@ Return the frame's symtab and line object. @xref{Symbol Tables In Python}. @end defmethod -@defmethod Frame read_var variable -Return the value of the given variable in this frame. @var{variable} must -be a string. +@defmethod Frame read_var variable @r{[}block@r{]} +Return the value of the given variable in this frame. If the optional +block argument is provided, search for the variable from that current +block; otherwise start at the frame's current block (which is +determined by the frame's current program counter). @var{variable} +must be a string or a @code{gdb.Symbol} object. @var{block} must be +a @code{gdb.Block} object. @end defmethod @defmethod Frame select diff --git a/gdb/python/py-frame.c b/gdb/python/py-frame.c index 94211db..3b5320c 100644 --- a/gdb/python/py-frame.c +++ b/gdb/python/py-frame.c @@ -380,20 +380,22 @@ frapy_find_sal (PyObject *self, PyObject *args) return sal_obj; } -/* Implementation of gdb.Frame.read_var_value (self, variable) -> gdb.Value. - Returns the value of the given variable in this frame. The argument must be - a string. Returns None if GDB can't find the specified variable. */ - +/* Implementation of gdb.Frame.read_var_value (self, variable, + [block]) -> gdb.Value. If the optional block argument is provided + start the search from that block, otherwise search from the frame's + current block (determined by examining the resume address of the + frame). The variable argument must be a string or an instance of a + gdb.Symbol. The block argument must be an instance of gdb.Block. */ static PyObject * frapy_read_var (PyObject *self, PyObject *args) { struct frame_info *frame; - PyObject *sym_obj; + PyObject *sym_obj, *block_obj = NULL; struct symbol *var = NULL; /* gcc-4.3.2 false warning. */ struct value *val = NULL; volatile struct gdb_exception except; - if (!PyArg_ParseTuple (args, "O", &sym_obj)) + if (!PyArg_ParseTuple (args, "O|O", &sym_obj, &block_obj)) return NULL; if (PyObject_TypeCheck (sym_obj, &symbol_object_type)) @@ -410,11 +412,23 @@ frapy_read_var (PyObject *self, PyObject *args) return NULL; cleanup = make_cleanup (xfree, var_name); + if (block_obj) + { + block = block_object_to_block (block_obj); + if (!block) + { + PyErr_SetString (PyExc_RuntimeError, + _("Second argument must be block.")); + return NULL; + } + } + TRY_CATCH (except, RETURN_MASK_ALL) { FRAPY_REQUIRE_VALID ((frame_object *) self, frame); - block = block_for_pc (get_frame_address_in_block (frame)); + if (!block) + block = block_for_pc (get_frame_address_in_block (frame)); var = lookup_symbol (var_name, block, VAR_DOMAIN, NULL); } GDB_PY_HANDLE_EXCEPTION (except); @@ -445,10 +459,15 @@ frapy_read_var (PyObject *self, PyObject *args) } GDB_PY_HANDLE_EXCEPTION (except); - if (val) - return value_to_value_object (val); + if (!val) + { + PyErr_Format (PyExc_ValueError, + _("Variable cannot be found for symbol '%s'."), + SYMBOL_NATURAL_NAME (var)); + return NULL; + } - Py_RETURN_NONE; + return value_to_value_object (val); } /* Select this frame. */ diff --git a/gdb/testsuite/gdb.python/py-frame.c b/gdb/testsuite/gdb.python/py-frame.c index 22eb9f2..82db341 100644 --- a/gdb/testsuite/gdb.python/py-frame.c +++ b/gdb/testsuite/gdb.python/py-frame.c @@ -8,7 +8,23 @@ int f1 (int a, int b) return f2(a) + b; } +int block (void) +{ + int i = 99; + { + double i = 1.1; + double f = 2.2; + { + const char *i = "stuff"; + const char *f = "foo"; + const char *b = "bar"; + return 0; /* Block break here. */ + } + } +} + int main (int argc, char *argv[]) { + block (); return f1 (1, 2); } diff --git a/gdb/testsuite/gdb.python/py-frame.exp b/gdb/testsuite/gdb.python/py-frame.exp index 6989207..150e737 100644 --- a/gdb/testsuite/gdb.python/py-frame.exp +++ b/gdb/testsuite/gdb.python/py-frame.exp @@ -56,6 +56,30 @@ if ![runto_main] then { return 0 } +gdb_breakpoint [gdb_get_line_number "Block break here."] +gdb_continue_to_breakpoint "Block break here." +gdb_py_test_silent_cmd "python bf1 = gdb.selected_frame ()" "get frame" 0 + +# First test that read_var is unaffected by PR 11036 changes. +gdb_test "python print bf1.read_var(\"i\")" "\"stuff\"" "test i" +gdb_test "python print bf1.read_var(\"f\")" "\"foo\"" "test f" +gdb_test "python print bf1.read_var(\"b\")" "\"bar\"" "test b" + +# Test the read_var function in another block other than the current +# block (in this case, the super block). Test thar read_var is reading +# the correct variables of i and f but they are the correct value and type. +gdb_py_test_silent_cmd "python sb = bf1.block().superblock" "get superblock" 0 +gdb_test "python print bf1.read_var(\"i\", sb)" "1.1.*" "test i = 1.1" +gdb_test "python print bf1.read_var(\"i\", sb).type" "double" "test double i" +gdb_test "python print bf1.read_var(\"f\", sb)" "2.2.*" "test f = 2.2" +gdb_test "python print bf1.read_var(\"f\", sb).type" "double" "test double f" + +# And again test another outerblock, this time testing "i" is the +# correct value and type. +gdb_py_test_silent_cmd "python sb = sb.superblock" "get superblock" 0 +gdb_test "python print bf1.read_var(\"i\", sb)" "99" "test i = 99" +gdb_test "python print bf1.read_var(\"i\", sb).type" "int" "test int i" + gdb_breakpoint "f2" gdb_continue_to_breakpoint "breakpoint at f2" gdb_test "up" "" "" ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [python][patch] Fix 11036 2010-02-26 12:38 ` Phil Muldoon @ 2010-02-26 14:56 ` Eli Zaretskii 2010-02-26 23:16 ` Tom Tromey 1 sibling, 0 replies; 6+ messages in thread From: Eli Zaretskii @ 2010-02-26 14:56 UTC (permalink / raw) To: Phil Muldoon; +Cc: tromey, gdb-patches > Date: Fri, 26 Feb 2010 12:37:50 +0000 > From: Phil Muldoon <pmuldoon@redhat.com> > Cc: gdb-patches@sourceware.org > > I have also updated the comments and the documentation to reflect the > new decisions. Thanks. I have a couple of comments: > +@defmethod Frame read_var variable @r{[}block@r{]} > +Return the value of the given variable in this frame. If the optional > +block argument is provided, search for the variable from that current > +block; otherwise start at the frame's current block (which is Please use @var for the arguments: Return the value of @var{variable} in this frame. If the optional argument @var{block} is provided, search for the variable from that block; otherwise start at the frame's current block (which is (note that I also slightly reworded your text). Okay with these changes. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [python][patch] Fix 11036 2010-02-26 12:38 ` Phil Muldoon 2010-02-26 14:56 ` Eli Zaretskii @ 2010-02-26 23:16 ` Tom Tromey 2010-02-28 22:06 ` Phil Muldoon 1 sibling, 1 reply; 6+ messages in thread From: Tom Tromey @ 2010-02-26 23:16 UTC (permalink / raw) To: Phil Muldoon; +Cc: gdb-patches >>>>> "Phil" == Phil Muldoon <pmuldoon@redhat.com> writes: Phil> If you examine the runtime, the return here is a valid value, not Phil> a NULL. In any case, I think that if read_var returns NULL, then this Phil> is an error case. If anyone knows this to be different let me know and Phil> I can go back to returning None in this case. To reflect this, if the Phil> result of read_var is NULL, I raise an exception. What do you think? I think this is fine. I dug through the code a bit and I also couldn't see when read_var_value would return NULL. Perhaps it is from before the LOC_COMPUTED case was changed to set the "optimized out" flag? (Just a random hypothesis based on the documentation for symbol_computed_ops::read_variable.) This patch is ok. Tom ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [python][patch] Fix 11036 2010-02-26 23:16 ` Tom Tromey @ 2010-02-28 22:06 ` Phil Muldoon 0 siblings, 0 replies; 6+ messages in thread From: Phil Muldoon @ 2010-02-28 22:06 UTC (permalink / raw) To: Tom Tromey; +Cc: gdb-patches, eliz On Fri, Feb 26, 2010 at 04:16:09PM -0700, Tom Tromey wrote: > I think this is fine. On Fri, Feb 26, 2010 at 02:55 PM, Eli Zaretskii wrote: > (note that I also slightly reworded your text). > > Okay with these changes. So committed: http://sourceware.org/ml/gdb-cvs/2010-02/msg00223.html Cheers, and thanks for the help. Phil Muldoon ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-02-28 22:06 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2010-02-25 14:29 [python][patch] Fix 11036 Phil Muldoon 2010-02-25 18:34 ` Tom Tromey 2010-02-26 12:38 ` Phil Muldoon 2010-02-26 14:56 ` Eli Zaretskii 2010-02-26 23:16 ` Tom Tromey 2010-02-28 22:06 ` Phil Muldoon
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox