Re: Python: fetch value when building gdb.Value object

[Jan Kratochvil <jan dot kratochvil at redhat dot com>]

On Wed, 28 Sep 2011 22:40:50 +0200, Paul Koning wrote:
> If I use RETURN_MASK_ERROR and control/C is hit, does that mean the
> currently running code aborts and the outer handler that does have
> RETURN_MASK_ALL is entered?

Yes.

> If so, most of the Python code seems to be a candidate for
> RETURN_MASK_ERROR.

In fact I do not know, it is a Python thing.

RETURN_MASK_ALL is right if returned PyExc_KeyboardInterrupt will really abort
any execution of Python code.  It is probably so, as suggested by:
	http://docs.python.org/library/exceptions.html#exceptions.KeyboardInterrupt

RETURN_MASK_ERROR is right otherwise, but only if it is safe to longjmp out
from a code called by Python.  This may not be true.  Python may be C++
exceptions throwing safe but it cannot be safe for the GDB longjmp exceptions.
But this case would mean Python is buggy for CTRL-C on its own so
RETURN_MASK_ERROR probably is not right.


> One exception is valpy_getitem (in py-value.c) since it has an xfree(value)
> after the TRY_CATCH but before the GDB_PY_HANDLE_EXCEPTION (outside the
> TRY_CATCH block, though -- is that right?)

It is right as long as RETURN_MASK_ALL is right.  It is more usual to call
make_cleanup though - as shown in the attached change.  The change could be
just a code cleanup without functionality change if that RETURN_MASK_ALL is
kept there.


> Probably related: I just tried an infinite Python loop and found that
> control-C had no effect.  I wonder if the Python interpreter is setting up
> its own control-C trap (quite possibly -- that's a Python exception after
> all) and we're losing it somewhere along the lines.

Yes, this is the kind of bug from it, thanks for checking it.


Thanks,
Jan


only FYI, not intended for commit:

gdb/
2011-10-01  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* python/py-value.c (valpy_getitem): New variable back_to.  Register
	xfree of field to it.  Call do_cleanups for it.  Use RETURN_MASK_ERROR
	instead of RETURN_MASK_ALL.

--- a/gdb/python/py-value.c
+++ b/gdb/python/py-value.c
@@ -448,6 +448,7 @@ valpy_getitem (PyObject *self, PyObject *key)
   char *field = NULL;
   struct value *res_val = NULL;
   volatile struct gdb_exception except;
+  struct cleanup *back_to;
 
   if (gdbpy_is_string (key))
     {  
@@ -456,7 +457,9 @@ valpy_getitem (PyObject *self, PyObject *key)
 	return NULL;
     }
 
-  TRY_CATCH (except, RETURN_MASK_ALL)
+  back_to = make_cleanup (xfree, field);
+
+  TRY_CATCH (except, RETURN_MASK_ERROR)
     {
       struct value *tmp = self_value->value;
 
@@ -485,10 +488,10 @@ valpy_getitem (PyObject *self, PyObject *key)
 	    }
 	}
     }
-
-  xfree (field);
   GDB_PY_HANDLE_EXCEPTION (except);
 
+  do_cleanups (back_to);
+
   return res_val ? value_to_value_object (res_val) : NULL;
 }