[patch] python(+solib error): save/restore error state

[patch] python(+solib error): save/restore error state

[Jan Kratochvil]

Hi,

downstream Bug:
	https://bugzilla.redhat.com/show_bug.cgi?id=639089

currently some unhandled errors are printed in an unrelated context later.

Patch goals:
 * ensure_python_env now saves/restores the Python error state of its caller.

 * ensure_python_env now internal-error warns on unhandled error in general.

   internal_warning is almost never used as it dumps GDB core the same way as
   internal_error.  Not targeted by this patch and used again:
     warning (_("internal error: [...]"));

 * source_python_script_for_objfile handles forgotten error explicitly.
   (as suggested by Tom)

 * solib_read_symbols prints GDB exceptions even for from_tty == 0.
   I understand "Loaded symbols for %s" should not be printed
   for from_tty == 0 but I do not understand why exceptions should be hidden,
   even for scripts.

   This is since - no specific message about such change there:
   e12b767fb287127671d514eef227486777f0a972
   Author: Pedro Alves
   Group errors for many missing shared libraries.
   http://sourceware.org/ml/gdb-patches/2010-04/msg00342.html
   http://sourceware.org/ml/gdb-cvs/2010-04/msg00126.html
   
   run_command_1 calls post_create_inferior with from_tty == 0 - this is why
   from_tty == 0 is in solib_read_symbols.
     /* Pass zero for FROM_TTY, because at this point the "run" command
        has done its thing; now we are setting up the running program.  */
     post_create_inferior (&current_target, 0);
   I do not agree with but this part is not required for this patch after the
   fix of solib_read_symbols.  There is more the problem of excessive "Reading
   symbols..." slowing GDB even 2x during startup just by the uninteresting
   messages - this may have been the goal for from_tty == 0 there.

 * Currently generally in use:
     if (value == NULL)
       {
	 gdbpy_print_stack ();
	 error (_("Error while executing Python code."));
       }
   should catch the gdbpy_print_stack output into a string variable and use it
   in the error call.  Not targeted by this patch.


Unfortunately this fix has no real testcase.  You can `yum install gdb-heap'
at least on Fedora 14 x86_64 and the provided testcase should show:
	(gdb) run 
	The program being debugged has been started already.
	Start it from the beginning? (y or n) y
	Starting program: .../gdb/testsuite/gdb.python/py-shared 
	LookupError: unknown encoding: IBM1047
	Error while reading shared library symbols:
	Error reading python script /usr/local/share/gdb/auto-load/lib64/ld-2.12.90.so-gdb.py for object file /lib64/ld-linux-x86-64.so.2

	Breakpoint 3, main (argc=1, argv=0x7fffffffdf58) at ./gdb.python/py-shared.c:25
	25        func1 ();
	(gdb) testcase ./gdb.python/py-shared.exp completed in 0 seconds

But nobody guarantees IBM1047 is an invalid charset for Python && supported
charset by glibc.  GDB does not accept a charset name unsupported by glibc.
And for example an invalid Python expression does not cause this kind of
delayed Python error state.

No regressions on {x86_64,x86_64-m32,i686}-fedora14snapshot-linux-gnu.
"Error while reading shared library symbols:" does not happen anywhere there.


Thanks,
Jan


2010-10-08  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* python/python.c
	(struct python_env) <error_type, error_value, error_traceback>: New
	fields.
	(restore_python_env): Handle PyErr_Occurred.  Call PyErr_Restore.
	(ensure_python_env): Call PyErr_Fetch.
	(source_python_script_for_objfile): Handle PyErr_Occurred.
	* solib.c (solib_read_symbols): Call exception_fprintf even without
	FROM_TTY.

--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -79,6 +79,7 @@ struct python_env
   PyGILState_STATE state;
   struct gdbarch *gdbarch;
   const struct language_defn *language;
+  PyObject *error_type, *error_value, *error_traceback;
 };
 
 static void
@@ -86,6 +87,16 @@ restore_python_env (void *p)
 {
   struct python_env *env = (struct python_env *)p;
 
+  /* Leftover Python error is forbidden by Python Exception Handling.  */
+  if (PyErr_Occurred ())
+    {
+      /* This order is similar to the one calling error afterwards. */
+      gdbpy_print_stack ();
+      warning (_("internal error: Unhandled Python exception"));
+    }
+
+  PyErr_Restore (env->error_type, env->error_value, env->error_traceback);
+
   PyGILState_Release (env->state);
   python_gdbarch = env->gdbarch;
   python_language = env->language;
@@ -108,6 +119,9 @@ ensure_python_env (struct gdbarch *gdbarch,
   python_gdbarch = gdbarch;
   python_language = language;
 
+  /* Save it and ensure ! PyErr_Occurred () afterwards.  */
+  PyErr_Fetch (&env->error_type, &env->error_value, &env->error_traceback);
+  
   return make_cleanup (restore_python_env, env);
 }
 
@@ -768,6 +782,13 @@ source_python_script_for_objfile (struct objfile *objfile,
      clear the error indicator.  */
   PyRun_SimpleFile (stream, file);
 
+  if (PyErr_Occurred ())
+    {
+      gdbpy_print_stack ();
+      error (_("Error reading python script %s for object file %s"), file,
+	     objfile->name);
+    }
+
   do_cleanups (cleanups);
   gdbpy_current_objfile = NULL;
 }
--- a/gdb/solib.c
+++ b/gdb/solib.c
@@ -652,12 +652,8 @@ solib_read_symbols (struct so_list *so, int flags)
 	}
 
       if (e.reason < 0)
-	{
-	  if (from_tty)
-	    exception_fprintf
-	      (gdb_stderr, e,
-	       _("Error while reading shared library symbols:\n"));
-	}
+	exception_fprintf (gdb_stderr, e,
+			  _("Error while reading shared library symbols:\n"));
       else
 	{
 	  if (from_tty || info_verbose)
--- a/gdb/testsuite/gdb.python/py-shared.exp
+++ b/gdb/testsuite/gdb.python/py-shared.exp
@@ -66,3 +66,11 @@ gdb_test "python print gdb.solib_name(long(func1))" "gdb/testsuite/gdb.python/py
 gdb_test "p &main" "" "main address"
 gdb_py_test_silent_cmd "python main = gdb.history(0)" "Aquire main address" 1
 gdb_test "python print gdb.solib_name(long(main))" "None" "test main solib location"
+
+# Test error while loading *-gdb.py
+
+gdb_breakpoint main
+
+gdb_test_no_output "set host-charset IBM1047"
+
+runto_main

Re: [patch] python(+solib error): save/restore error state

[Doug Evans]

On Fri, Oct 8, 2010 at 12:11 PM, Jan Kratochvil
<jan.kratochvil@redhat.com> wrote:
> @@ -768,6 +782,13 @@ source_python_script_for_objfile (struct objfile *objfile,
>      clear the error indicator.  */
>   PyRun_SimpleFile (stream, file);
>
> +  if (PyErr_Occurred ())
> +    {
> +      gdbpy_print_stack ();
> +      error (_("Error reading python script %s for object file %s"), file,
> +            objfile->name);
> +    }
> +
>   do_cleanups (cleanups);
>   gdbpy_current_objfile = NULL;
>  }

Hi.  Most of the patch seems great, but it raises a question that I
think needs to be answered.

The comment above this code says:

  /* Note: If an exception occurs python will print the traceback and
     clear the error indicator.  */

ISTM that either this comment is wrong or the above patch is wrong, or
some combination thereof.

Also, if there is a problem here, then presumably source_python_script
has the same problem, but I don't see it addressed in this patch.

Plus if PyRun_SimpleFile can leave an outstanding error, we should
research whether PyRun_SimpleString has the same problem.  I see
python.c checks the result of PyRun_SimpleString and calls
gdbpy_print_stack if there's an error.  Except that the python docs
say there is no way to get the error.

ref: http://docs.python.org/c-api/veryhigh.html?highlight=pyrun_simplestring#PyRun_SimpleString

The docs for PyRun_SimpleFile suggest the same, there is no way to get
the error.

So, am I misreading the python docs or do we need the above patch?
(and then if not, do we need the calls to gdbpy_print_stack after the
calls to PyRun_SimpleString? Some experiments are in order - off to do
some ...

Re: [patch] python(+solib error): save/restore error state

[Doug Evans]

On Fri, Oct 8, 2010 at 1:08 PM, Doug Evans <dje@google.com> wrote:
> On Fri, Oct 8, 2010 at 12:11 PM, Jan Kratochvil
> <jan.kratochvil@redhat.com> wrote:
>> @@ -768,6 +782,13 @@ source_python_script_for_objfile (struct objfile *objfile,
>>      clear the error indicator.  */
>>   PyRun_SimpleFile (stream, file);
>>
>> +  if (PyErr_Occurred ())
>> +    {
>> +      gdbpy_print_stack ();
>> +      error (_("Error reading python script %s for object file %s"), file,
>> +            objfile->name);
>> +    }
>> +
>>   do_cleanups (cleanups);
>>   gdbpy_current_objfile = NULL;
>>  }
>
> Hi.  Most of the patch seems great, but it raises a question that I
> think needs to be answered.
>
> The comment above this code says:
>
>  /* Note: If an exception occurs python will print the traceback and
>     clear the error indicator.  */
>
> ISTM that either this comment is wrong or the above patch is wrong, or
> some combination thereof.
>
> Also, if there is a problem here, then presumably source_python_script
> has the same problem, but I don't see it addressed in this patch.
>
> Plus if PyRun_SimpleFile can leave an outstanding error, we should
> research whether PyRun_SimpleString has the same problem.  I see
> python.c checks the result of PyRun_SimpleString and calls
> gdbpy_print_stack if there's an error.  Except that the python docs
> say there is no way to get the error.
>
> ref: http://docs.python.org/c-api/veryhigh.html?highlight=pyrun_simplestring#PyRun_SimpleString
>
> The docs for PyRun_SimpleFile suggest the same, there is no way to get
> the error.
>
> So, am I misreading the python docs or do we need the above patch?
> (and then if not, do we need the calls to gdbpy_print_stack after the
> calls to PyRun_SimpleString? Some experiments are in order - off to do
> some ...
>

Better yet, "use the source Luke ..." 1/2 :-).
Looking at the sources for PyRun_SimpleFile, I see PyErr_Print being
called before -1 is returned.
Similarly for PyRun_SimpleString.

Am I missing something?

Re: [patch] python(+solib error): save/restore error state

[Tom Tromey]

>>>>> "Doug" == Doug Evans <dje@google.com> writes:

Doug> Looking at the sources for PyRun_SimpleFile, I see PyErr_Print being
Doug> called before -1 is returned.
Doug> Similarly for PyRun_SimpleString.

Doug> Am I missing something?

In that case we should probably just call PyErr_Clear.

Tom

Re: [patch] python(+solib error): save/restore error state

[Doug Evans]

On Fri, Oct 8, 2010 at 2:27 PM, Tom Tromey <tromey@redhat.com> wrote:
>>>>>> "Doug" == Doug Evans <dje@google.com> writes:
>
> Doug> Looking at the sources for PyRun_SimpleFile, I see PyErr_Print being
> Doug> called before -1 is returned.
> Doug> Similarly for PyRun_SimpleString.
>
> Doug> Am I missing something?
>
> In that case we should probably just call PyErr_Clear.

Except that my understanding is that that is unnecessary.
PyErr_Print is defined to clear the error indicator.

For reference sake, according to the docs, the error is saved in
python vars sys.last_{type,value,traceback}.  We might want to
clear/save/restore those.

Re: [patch] python: save/restore/fix error state

[Jan Kratochvil]

On Fri, 08 Oct 2010 22:08:27 +0200, Doug Evans wrote:
> The comment above this code says:
> 
>   /* Note: If an exception occurs python will print the traceback and
>      clear the error indicator.  */
> 
> ISTM that either this comment is wrong or the above patch is wrong, or
> some combination thereof.

This comment is right, Python has no bug there, other gdb/python/ code has
bugs.  That is some gdb/python/ code returned success value while still
leaving the python exception set.  Such case is undefined and unchecked by
Python.  Python error state does not follow the errno POSIX semantics
	The setting of errno after a successful call to a function is
	unspecified [...].
as Python requires instead:
	The Python error state after a successful call to a function must be
	cleared.

I have checked callers of these functions, as a closure on the callers of the
top py-utils.c functions.  That does not mean this Python error state handling
verification is complete:
	python_string_to_unicode unicode_to_encoded_string
	unicode_to_encoded_python_string unicode_to_target_string
	unicode_to_target_python_string python_string_to_target_string
	python_string_to_target_python_string python_string_to_host_string
	target_string_to_unicode gdbpy_obj_to_string gdbpy_exception_to_string
	get_addr_from_python convert_value_from_python frapy_read_var
	gdbpy_get_display_hint valpy_getitem FIXME set_parameter_value
	compute_enum_values infpy_read_memory infpy_write_memory
	infpy_search_memory valpy_new valpy_call valpy_binop valpy_richcompare
	pretty_print_one_value set_attr parmpy_init

Therefore I have removed the check after PyRun_SimpleFile.

No regressions on {x86_64,x86_64-m32,i686}-fedora14snapshot-linux-gnu.


Thanks,
Jan


gdb/
2010-10-09  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* python/py-breakpoint.c (bppy_set_condition): New comment.
	* python/py-cmd.c (cmdpy_function): Call also gdbpy_print_stack for
	failed PyUnicode_Decode.
	(cmdpy_completer): Skip element for failed
	python_string_to_host_string.
	(cmdpy_init): Return -1 on failed python_string_to_host_string.
	* python/py-frame.c (frapy_read_var): Extend the function comment.
	* python/py-function.c (fnpy_init): Return -1 on failed
	python_string_to_host_string.
	* python/py-inferior.c (infpy_read_memory, infpy_write_memory): Extend
	the function comment.
	(infpy_search_memory): Extend the function comment.  Remove the
	PyErr_SetString call on already set error state.
	* python/py-param.c (set_parameter_value): Extend the function
	comment.  Return -1 on failed python_string_to_host_string, twice.
	(set_attr): Extend the function comment.
	(compute_enum_values): Extend the function comment.  New variable
	back_to.  Protect self->enumeration by BACK_TO cleanups.  Return 0 on
	failed python_string_to_host_string.
	(get_doc_string): Call gdbpy_print_stack on failed
	python_string_to_host_string.
	(parmpy_init): Extend the function comment.
	* python/py-prettyprint.c (pretty_print_one_value): Likewise.
	(gdbpy_get_display_hint, print_children): Call gdbpy_print_stack on
	failed python_string_to_host_string.
	* python/py-value.c (valpy_new, valpy_getitem, valpy_call)
	(valpy_binop, valpy_richcompare): Extend the function comment.
	* python/python.c
	(struct python_env) <error_type, error_value, error_traceback>: New
	fields.
	(restore_python_env): Handle PyErr_Occurred.  Call PyErr_Restore.
	(ensure_python_env): Call PyErr_Fetch.
	* varobj.c (update_dynamic_varobj_children): Call gdbpy_print_stack on
	failed convert_value_from_python.
	(value_get_print_value): Call gdbpy_print_stack on failed
	python_string_to_target_python_string.

gdb/testsuite/
2010-10-09  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gdb.python/py-error.exp: New file.
	* gdb.python/py-error.py: New file.

--- a/gdb/python/py-breakpoint.c
+++ b/gdb/python/py-breakpoint.c
@@ -420,6 +420,9 @@ bppy_get_condition (PyObject *self, void *closure)
   return PyString_Decode (str, strlen (str), host_charset (), NULL);
 }
 
+/* Returns 0 on success.  Returns -1 on error, with a python exception set.
+   */
+
 static int
 bppy_set_condition (PyObject *self, PyObject *newvalue, void *closure)
 {
--- a/gdb/python/py-cmd.c
+++ b/gdb/python/py-cmd.c
@@ -139,7 +139,10 @@ cmdpy_function (struct cmd_list_element *command, char *args, int from_tty)
     args = "";
   argobj = PyUnicode_Decode (args, strlen (args), host_charset (), NULL);
   if (! argobj)
-    error (_("Could not convert arguments to Python string."));
+    {
+      gdbpy_print_stack ();
+      error (_("Could not convert arguments to Python string."));
+    }
 
   ttyobj = from_tty ? Py_True : Py_False;
   Py_INCREF (ttyobj);
@@ -256,6 +259,12 @@ cmdpy_completer (struct cmd_list_element *command, char *text, char *word)
 	      continue;
 	    }
 	  result[out] = python_string_to_host_string (elt);
+	  if (result[out] == NULL)
+	    {
+	      /* Skip problem elements.  */
+	      PyErr_Clear ();
+	      continue;
+	    }
 	  ++out;
 	}
       result[out] = NULL;
@@ -466,7 +475,15 @@ cmdpy_init (PyObject *self, PyObject *args, PyObject *kw)
       PyObject *ds_obj = PyObject_GetAttr (self, gdbpy_doc_cst);
 
       if (ds_obj && gdbpy_is_string (ds_obj))
-	docstring = python_string_to_host_string (ds_obj);
+	{
+	  docstring = python_string_to_host_string (ds_obj);
+	  if (docstring == NULL)
+	    {
+	      xfree (cmd_name);
+	      xfree (pfx_name);
+	      return -1;
+	    }
+	}
     }
   if (! docstring)
     docstring = xstrdup (_("This command is not documented."));
--- a/gdb/python/py-frame.c
+++ b/gdb/python/py-frame.c
@@ -385,7 +385,8 @@ frapy_find_sal (PyObject *self, PyObject *args)
    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.  */
+   gdb.Symbol.  The block argument must be an instance of gdb.Block.  Returns
+   NULL on error, with a python exception set.  */
 static PyObject *
 frapy_read_var (PyObject *self, PyObject *args)
 {
--- a/gdb/python/py-function.c
+++ b/gdb/python/py-function.c
@@ -113,7 +113,14 @@ fnpy_init (PyObject *self, PyObject *args, PyObject *kwds)
     {
       PyObject *ds_obj = PyObject_GetAttrString (self, "__doc__");
       if (ds_obj && gdbpy_is_string (ds_obj))
-	docstring = python_string_to_host_string (ds_obj);
+	{
+	  docstring = python_string_to_host_string (ds_obj);
+	  if (docstring == NULL)
+	    {
+	      Py_DECREF (self);
+	      return -1;
+	    }
+	}
     }
   if (! docstring)
     docstring = xstrdup (_("This function is not documented."));
--- a/gdb/python/py-inferior.c
+++ b/gdb/python/py-inferior.c
@@ -293,7 +293,8 @@ gdbpy_inferiors (PyObject *unused, PyObject *unused2)
 
 /* Implementation of gdb.read_memory (address, length).
    Returns a Python buffer object with LENGTH bytes of the inferior's
-   memory at ADDRESS.  Both arguments are integers.  */
+   memory at ADDRESS.  Both arguments are integers.  Returns NULL on error,
+   with a python exception set.  */
 static PyObject *
 infpy_read_memory (PyObject *self, PyObject *args, PyObject *kw)
 {
@@ -361,7 +362,8 @@ infpy_read_memory (PyObject *self, PyObject *args, PyObject *kw)
    Writes the contents of BUFFER (a Python object supporting the read
    buffer protocol) at ADDRESS in the inferior's memory.  Write LENGTH
    bytes from BUFFER, or its entire contents if the argument is not
-   provided.  The function returns nothing.  */
+   provided.  The function returns nothing.  Returns NULL on error, with
+   a python exception set.  */
 static PyObject *
 infpy_write_memory (PyObject *self, PyObject *args, PyObject *kw)
 {
@@ -473,7 +475,8 @@ get_char_buffer (PyObject *self, Py_ssize_t segment, char **ptrptr)
    search from ADDRESS.  PATTERN is the pattern to search for (and
    must be a Python object supporting the buffer protocol).
    Returns a Python Long object holding the address where the pattern
-   was located, or if the pattern was not found, returns None.  */
+   was located, or if the pattern was not found, returns None.  Returns NULL
+   on error, with a python exception set.  */
 static PyObject *
 infpy_search_memory (PyObject *self, PyObject *args, PyObject *kw)
 {
@@ -511,12 +514,7 @@ infpy_search_memory (PyObject *self, PyObject *args, PyObject *kw)
 	}
     }
   else
-    {
-      PyErr_SetString (PyExc_RuntimeError,
-		       _("Cannot get search address/range from Python."));
-
-      return NULL;
-    }
+    return NULL;
 
   if (!PyObject_CheckReadBuffer (pattern))
     {
--- a/gdb/python/py-param.c
+++ b/gdb/python/py-param.c
@@ -110,8 +110,8 @@ get_attr (PyObject *obj, PyObject *attr_name)
   return PyObject_GenericGetAttr (obj, attr_name);
 }
 
-/* Set a parameter value from a Python value.  Return 0 on success, -1
-   on failure.  */
+/* Set a parameter value from a Python value.  Return 0 on success.  Returns
+   -1 on error, with a python exception set.  */
 static int
 set_parameter_value (parmpy_object *self, PyObject *value)
 {
@@ -142,7 +142,11 @@ set_parameter_value (parmpy_object *self, PyObject *value)
 	    self->value.stringval = NULL;
 	}
       else
-	self->value.stringval = python_string_to_host_string (value);
+	{
+	  self->value.stringval = python_string_to_host_string (value);
+	  if (self->value.stringval == NULL)
+	    return -1;
+	}
       break;
 
     case var_enum:
@@ -158,6 +162,8 @@ set_parameter_value (parmpy_object *self, PyObject *value)
 	  }
 
 	str = python_string_to_host_string (value);
+	if (str == NULL)
+	  return -1;
 	for (i = 0; self->enumeration[i]; ++i)
 	  if (! strcmp (self->enumeration[i], str))
 	    break;
@@ -258,7 +264,7 @@ set_parameter_value (parmpy_object *self, PyObject *value)
   return 0;
 }
 
-/* Set an attribute.  */
+/* Set an attribute.  Returns -1 on error, with a python exception set.  */
 static int
 set_attr (PyObject *obj, PyObject *attr_name, PyObject *val)
 {
@@ -358,12 +364,13 @@ add_setshow_generic (int parmclass, enum command_class cmdclass,
     }
 }
 
-/* A helper which computes enum values.  Returns 1 on success, 0 on
-   error.  */
+/* A helper which computes enum values.  Returns 1 on success.  Returns 0 on
+   error, with a python exception set.  */
 static int
 compute_enum_values (parmpy_object *self, PyObject *enum_values)
 {
   Py_ssize_t size, i;
+  struct cleanup *back_to;
 
   if (! enum_values)
     {
@@ -390,6 +397,7 @@ compute_enum_values (parmpy_object *self, PyObject *enum_values)
     }
 
   self->enumeration = xmalloc ((size + 1) * sizeof (char *));
+  back_to = make_cleanup (free_current_contents, &self->enumeration);
   memset (self->enumeration, 0, (size + 1) * sizeof (char *));
 
   for (i = 0; i < size; ++i)
@@ -397,16 +405,27 @@ compute_enum_values (parmpy_object *self, PyObject *enum_values)
       PyObject *item = PySequence_GetItem (enum_values, i);
 
       if (! item)
-	return 0;
+	{
+	  do_cleanups (back_to);
+	  return 0;
+	}
       if (! gdbpy_is_string (item))
 	{
+	  do_cleanups (back_to);
 	  PyErr_SetString (PyExc_RuntimeError, 
 			   _("The enumeration item not a string."));
 	  return 0;
 	}
       self->enumeration[i] = python_string_to_host_string (item);
+      if (self->enumeration[i] == NULL)
+	{
+	  do_cleanups (back_to);
+	  return 0;
+	}
+      make_cleanup (xfree, (char *) self->enumeration[i]);
     }
 
+  discard_cleanups (back_to);
   return 1;
 }
 
@@ -422,7 +441,11 @@ get_doc_string (PyObject *object, PyObject *attr)
       PyObject *ds_obj = PyObject_GetAttr (object, attr);
 
       if (ds_obj && gdbpy_is_string (ds_obj))
-	result = python_string_to_host_string (ds_obj);
+	{
+	  result = python_string_to_host_string (ds_obj);
+	  if (result == NULL)
+	    gdbpy_print_stack ();
+	}
     }
   if (! result)
     result = xstrdup (_("This command is not documented."));
@@ -449,8 +472,9 @@ get_doc_string (PyObject *object, PyObject *attr)
 
    The documentation for the parameter is taken from the doc string
    for the python class.
-   
-*/
+
+   Returns -1 on error, with a python exception set.  */
+
 static int
 parmpy_init (PyObject *self, PyObject *args, PyObject *kwds)
 {
--- a/gdb/python/py-prettyprint.c
+++ b/gdb/python/py-prettyprint.c
@@ -185,8 +185,8 @@ find_pretty_printer (PyObject *value)
    is returned.  If the function returns Py_NONE that means the pretty
    printer returned the Python None as a value.  Otherwise, if the
    function returns a value,  *OUT_VALUE is set to the value, and NULL
-   is returned.  On error, *OUT_VALUE is set to NULL, and NULL is
-   returned.  */
+   is returned.  On error, *OUT_VALUE is set to NULL, NULL is
+   returned, with a python exception set.  */
 
 static PyObject *
 pretty_print_one_value (PyObject *printer, struct value **out_value)
@@ -232,7 +232,11 @@ gdbpy_get_display_hint (PyObject *printer)
   if (hint)
     {
       if (gdbpy_is_string (hint))
-	result = python_string_to_host_string (hint);
+	{
+	  result = python_string_to_host_string (hint);
+	  if (result == NULL)
+	    gdbpy_print_stack ();
+	}
       Py_DECREF (hint);
     }
   else
@@ -555,7 +559,10 @@ print_children (PyObject *printer, const char *hint,
 	  else
 	    {
 	      output = python_string_to_host_string (py_v);
-	      fputs_filtered (output, stream);
+	      if (!output)
+		gdbpy_print_stack ();
+	      else
+		fputs_filtered (output, stream);
 	      xfree (output);
 	    }
 	}
--- a/gdb/python/py-value.c
+++ b/gdb/python/py-value.c
@@ -119,7 +119,8 @@ note_value (value_object *value_obj)
   values_in_python = value_obj;
 }
 
-/* Called when a new gdb.Value object needs to be allocated.  */
+/* Called when a new gdb.Value object needs to be allocated.  Returns NULL on
+   error, with a python exception set.  */
 static PyObject *
 valpy_new (PyTypeObject *subtype, PyObject *args, PyObject *keywords)
 {
@@ -438,7 +439,7 @@ valpy_length (PyObject *self)
 }
 
 /* Given string name of an element inside structure, return its value
-   object.  */
+   object.  Returns NULL on error, with a python exception set.  */
 static PyObject *
 valpy_getitem (PyObject *self, PyObject *key)
 {
@@ -499,7 +500,7 @@ valpy_setitem (PyObject *self, PyObject *key, PyObject *value)
 }
 
 /* Called by the Python interpreter to perform an inferior function
-   call on the value.  */
+   call on the value.  Returns NULL on error, with a python exception set.  */
 static PyObject *
 valpy_call (PyObject *self, PyObject *args, PyObject *keywords)
 {
@@ -619,7 +620,8 @@ enum valpy_opcode
   ((TYPE_CODE (TYPE) == TYPE_CODE_REF) ? (TYPE_TARGET_TYPE (TYPE)) : (TYPE))
 
 /* Returns a value object which is the result of applying the operation
-   specified by OPCODE to the given arguments.  */
+   specified by OPCODE to the given arguments.  Returns NULL on error, with
+   a python exception set.  */
 static PyObject *
 valpy_binop (enum valpy_opcode opcode, PyObject *self, PyObject *other)
 {
@@ -871,7 +873,8 @@ valpy_xor (PyObject *self, PyObject *other)
   return valpy_binop (VALPY_BITXOR, self, other);
 }
 
-/* Implements comparison operations for value objects.  */
+/* Implements comparison operations for value objects.  Returns NULL on error,
+   with a python exception set.  */
 static PyObject *
 valpy_richcompare (PyObject *self, PyObject *other, int op)
 {
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -79,6 +79,7 @@ struct python_env
   PyGILState_STATE state;
   struct gdbarch *gdbarch;
   const struct language_defn *language;
+  PyObject *error_type, *error_value, *error_traceback;
 };
 
 static void
@@ -86,6 +87,16 @@ restore_python_env (void *p)
 {
   struct python_env *env = (struct python_env *)p;
 
+  /* Leftover Python error is forbidden by Python Exception Handling.  */
+  if (PyErr_Occurred ())
+    {
+      /* This order is similar to the one calling error afterwards. */
+      gdbpy_print_stack ();
+      warning (_("internal error: Unhandled Python exception"));
+    }
+
+  PyErr_Restore (env->error_type, env->error_value, env->error_traceback);
+
   PyGILState_Release (env->state);
   python_gdbarch = env->gdbarch;
   python_language = env->language;
@@ -108,6 +119,9 @@ ensure_python_env (struct gdbarch *gdbarch,
   python_gdbarch = gdbarch;
   python_language = language;
 
+  /* Save it and ensure ! PyErr_Occurred () afterwards.  */
+  PyErr_Fetch (&env->error_type, &env->error_value, &env->error_traceback);
+  
   return make_cleanup (restore_python_env, env);
 }
 
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-error.exp
@@ -0,0 +1,56 @@
+# Copyright (C) 2010 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Test error while loading *-gdb.py.  IBM1047 is chosen as possibly supported
+# by glibc but unsupported by Python
+
+set testfile "py-error"
+
+load_lib gdb-python.exp
+
+# Start with a fresh gdb.
+gdb_exit
+gdb_start
+
+# Skip all tests if Python scripting is not enabled.
+if { [skip_python_tests] } { continue }
+
+set charset "IBM1047"
+
+set test2 "main reached"
+
+set test "set host-charset $charset"
+set test_regex [string_to_regexp $test]
+gdb_test_multiple $test $test {
+    -re "^$test_regex\r\n$gdb_prompt $" {
+	pass $test
+    }
+    -re "^$test_regex\r\nUndefined item: \"$charset\"\\.\r\n$gdb_prompt $" {
+	xfail $test
+	untested $test2
+	set test2 ""
+    }
+}
+
+if {$test2 == ""} {
+    return 0
+}
+
+set remote_python_file [remote_download host ${srcdir}/${subdir}/${testfile}.py]
+
+# argc=LookupError: unknown encoding: IBM1047
+gdb_test "source $remote_python_file" "Traceback.*ClassName.*\r\nLookupError: unknown encoding: $charset" $test2
+
+gdb_test "p 1" " = 1" "no delayed error"
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-error.py
@@ -0,0 +1,25 @@
+# Copyright (C) 2010 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+import gdb
+
+class ClassName(gdb.Command):
+    'a'
+    def __init__(self):
+        gdb.Command.__init__ (self, "ClassName", gdb.COMMAND_DATA, prefix=True)
+    def invoke(self, args, from_tty):
+        print
+
+ClassName()
--- a/gdb/varobj.c
+++ b/gdb/varobj.c
@@ -1052,6 +1052,8 @@ update_dynamic_varobj_children (struct varobj *var,
 	    error (_("Invalid item from the child list"));
 
 	  v = convert_value_from_python (py_v);
+	  if (v == NULL)
+	    gdbpy_print_stack ();
 	  install_dynamic_child (var, can_mention ? changed : NULL,
 				 can_mention ? new : NULL,
 				 can_mention ? unchanged : NULL,
@@ -2540,6 +2542,8 @@ value_get_print_value (struct value *value, enum varobj_display_formats format,
 			type = builtin_type (gdbarch)->builtin_char;
 			Py_DECREF (py_str);
 		      }
+		    else
+		      gdbpy_print_stack ();
 		  }
 		Py_DECREF (output);
 	      }

Re: [patch] python: save/restore/fix error state

[Tom Tromey]

Jan> -	docstring = python_string_to_host_string (ds_obj);
Jan> +	{
Jan> +	  docstring = python_string_to_host_string (ds_obj);
Jan> +	  if (docstring == NULL)
Jan> +	    {
Jan> +	      xfree (cmd_name);
Jan> +	      xfree (pfx_name);
Jan> +	      return -1;

It seems to me that inability to convert the docstring shouldn't be a
fatal problem for command registration.  We can just fall back to the
no-docstring code.

This occurs a couple of times.

Jan> -	self->value.stringval = python_string_to_host_string (value);
Jan> +	{
Jan> +	  self->value.stringval = python_string_to_host_string (value);
Jan> +	  if (self->value.stringval == NULL)
Jan> +	    return -1;
Jan> +	}

I think this should probably convert to a temporary variable before
assigning to self->value.


The rest looks good to me.  Thanks for doing this.

We were recently kicking around the idea of using Cython to write this
layer.  It has one nice advantage over doing it by hand: it handles
reference counting and error checking automatically.  It isn't clear it
can be made to work for gdb, though :(

Tom

Re: [patch] python: save/restore/fix error state

[Jan Kratochvil]

On Tue, 12 Oct 2010 21:55:22 +0200, Tom Tromey wrote:
> Jan> -	docstring = python_string_to_host_string (ds_obj);
> Jan> +	{
> Jan> +	  docstring = python_string_to_host_string (ds_obj);
> Jan> +	  if (docstring == NULL)
> Jan> +	    {
> Jan> +	      xfree (cmd_name);
> Jan> +	      xfree (pfx_name);
> Jan> +	      return -1;
> 
> It seems to me that inability to convert the docstring shouldn't be a
> fatal problem for command registration.  We can just fall back to the
> no-docstring code.

I had it that way first.  The problem is that way it prints:
	(gdb) source py-error.py
	LookupError: unknown encoding: IBM1047
	(gdb) _

which I do not find too useful.   With the error return it prints:
	(gdb) source py-error.py
	Traceback (most recent call last):
	  File "py-error.py", line 25, in <module>
	    ClassName()
	  File "py-error.py", line 21, in __init__
	    gdb.Command.__init__ (self, "ClassName", gdb.COMMAND_DATA, prefix=True)
	LookupError: unknown encoding: IBM1047
	(gdb) _

which I find more explanatory.  Why PyErr_Print() does not dump the whole
back(forward)trace when called from the inner block is unknown to me and it
seems to be a nature of Python to me.

Awaiting for the resolution of this case.



> Jan> -	self->value.stringval = python_string_to_host_string (value);
> Jan> +	{
> Jan> +	  self->value.stringval = python_string_to_host_string (value);
> Jan> +	  if (self->value.stringval == NULL)
> Jan> +	    return -1;
> Jan> +	}
> 
> I think this should probably convert to a temporary variable before
> assigning to self->value.

OK, yes, fixed.


Thanks,
Jan


gdb/
2010-10-12  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* python/py-breakpoint.c (bppy_set_condition): New comment.
	* python/py-cmd.c (cmdpy_function): Call also gdbpy_print_stack for
	failed PyUnicode_Decode.
	(cmdpy_completer): Skip element for failed
	python_string_to_host_string.
	(cmdpy_init): Return -1 on failed python_string_to_host_string.
	* python/py-frame.c (frapy_read_var): Extend the function comment.
	* python/py-function.c (fnpy_init): Return -1 on failed
	python_string_to_host_string.
	* python/py-inferior.c (infpy_read_memory, infpy_write_memory): Extend
	the function comment.
	(infpy_search_memory): Extend the function comment.  Remove the
	PyErr_SetString call on already set error state.
	* python/py-param.c (set_parameter_value): Extend the function
	comment.  Return -1 on failed python_string_to_host_string, twice.
	(set_attr): Extend the function comment.
	(compute_enum_values): Extend the function comment.  New variable
	back_to.  Protect self->enumeration by BACK_TO cleanups.  Return 0 on
	failed python_string_to_host_string.
	(get_doc_string): Call gdbpy_print_stack on failed
	python_string_to_host_string.
	(parmpy_init): Extend the function comment.
	* python/py-prettyprint.c (pretty_print_one_value): Likewise.
	(gdbpy_get_display_hint, print_children): Call gdbpy_print_stack on
	failed python_string_to_host_string.
	* python/py-value.c (valpy_new, valpy_getitem, valpy_call)
	(valpy_binop, valpy_richcompare): Extend the function comment.
	* python/python.c
	(struct python_env) <error_type, error_value, error_traceback>: New
	fields.
	(restore_python_env): Handle PyErr_Occurred.  Call PyErr_Restore.
	(ensure_python_env): Call PyErr_Fetch.
	* varobj.c (update_dynamic_varobj_children): Call gdbpy_print_stack on
	failed convert_value_from_python.
	(value_get_print_value): Call gdbpy_print_stack on failed
	python_string_to_target_python_string.

gdb/testsuite/
2010-10-12  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gdb.python/py-error.exp: New file.
	* gdb.python/py-error.py: New file.

--- a/gdb/python/py-breakpoint.c
+++ b/gdb/python/py-breakpoint.c
@@ -420,6 +420,9 @@ bppy_get_condition (PyObject *self, void *closure)
   return PyString_Decode (str, strlen (str), host_charset (), NULL);
 }
 
+/* Returns 0 on success.  Returns -1 on error, with a python exception set.
+   */
+
 static int
 bppy_set_condition (PyObject *self, PyObject *newvalue, void *closure)
 {
--- a/gdb/python/py-cmd.c
+++ b/gdb/python/py-cmd.c
@@ -139,7 +139,10 @@ cmdpy_function (struct cmd_list_element *command, char *args, int from_tty)
     args = "";
   argobj = PyUnicode_Decode (args, strlen (args), host_charset (), NULL);
   if (! argobj)
-    error (_("Could not convert arguments to Python string."));
+    {
+      gdbpy_print_stack ();
+      error (_("Could not convert arguments to Python string."));
+    }
 
   ttyobj = from_tty ? Py_True : Py_False;
   Py_INCREF (ttyobj);
@@ -256,6 +259,12 @@ cmdpy_completer (struct cmd_list_element *command, char *text, char *word)
 	      continue;
 	    }
 	  result[out] = python_string_to_host_string (elt);
+	  if (result[out] == NULL)
+	    {
+	      /* Skip problem elements.  */
+	      PyErr_Clear ();
+	      continue;
+	    }
 	  ++out;
 	}
       result[out] = NULL;
@@ -466,7 +475,15 @@ cmdpy_init (PyObject *self, PyObject *args, PyObject *kw)
       PyObject *ds_obj = PyObject_GetAttr (self, gdbpy_doc_cst);
 
       if (ds_obj && gdbpy_is_string (ds_obj))
-	docstring = python_string_to_host_string (ds_obj);
+	{
+	  docstring = python_string_to_host_string (ds_obj);
+	  if (docstring == NULL)
+	    {
+	      xfree (cmd_name);
+	      xfree (pfx_name);
+	      return -1;
+	    }
+	}
     }
   if (! docstring)
     docstring = xstrdup (_("This command is not documented."));
--- a/gdb/python/py-frame.c
+++ b/gdb/python/py-frame.c
@@ -385,7 +385,8 @@ frapy_find_sal (PyObject *self, PyObject *args)
    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.  */
+   gdb.Symbol.  The block argument must be an instance of gdb.Block.  Returns
+   NULL on error, with a python exception set.  */
 static PyObject *
 frapy_read_var (PyObject *self, PyObject *args)
 {
--- a/gdb/python/py-function.c
+++ b/gdb/python/py-function.c
@@ -113,7 +113,14 @@ fnpy_init (PyObject *self, PyObject *args, PyObject *kwds)
     {
       PyObject *ds_obj = PyObject_GetAttrString (self, "__doc__");
       if (ds_obj && gdbpy_is_string (ds_obj))
-	docstring = python_string_to_host_string (ds_obj);
+	{
+	  docstring = python_string_to_host_string (ds_obj);
+	  if (docstring == NULL)
+	    {
+	      Py_DECREF (self);
+	      return -1;
+	    }
+	}
     }
   if (! docstring)
     docstring = xstrdup (_("This function is not documented."));
--- a/gdb/python/py-inferior.c
+++ b/gdb/python/py-inferior.c
@@ -293,7 +293,8 @@ gdbpy_inferiors (PyObject *unused, PyObject *unused2)
 
 /* Implementation of gdb.read_memory (address, length).
    Returns a Python buffer object with LENGTH bytes of the inferior's
-   memory at ADDRESS.  Both arguments are integers.  */
+   memory at ADDRESS.  Both arguments are integers.  Returns NULL on error,
+   with a python exception set.  */
 static PyObject *
 infpy_read_memory (PyObject *self, PyObject *args, PyObject *kw)
 {
@@ -361,7 +362,8 @@ infpy_read_memory (PyObject *self, PyObject *args, PyObject *kw)
    Writes the contents of BUFFER (a Python object supporting the read
    buffer protocol) at ADDRESS in the inferior's memory.  Write LENGTH
    bytes from BUFFER, or its entire contents if the argument is not
-   provided.  The function returns nothing.  */
+   provided.  The function returns nothing.  Returns NULL on error, with
+   a python exception set.  */
 static PyObject *
 infpy_write_memory (PyObject *self, PyObject *args, PyObject *kw)
 {
@@ -473,7 +475,8 @@ get_char_buffer (PyObject *self, Py_ssize_t segment, char **ptrptr)
    search from ADDRESS.  PATTERN is the pattern to search for (and
    must be a Python object supporting the buffer protocol).
    Returns a Python Long object holding the address where the pattern
-   was located, or if the pattern was not found, returns None.  */
+   was located, or if the pattern was not found, returns None.  Returns NULL
+   on error, with a python exception set.  */
 static PyObject *
 infpy_search_memory (PyObject *self, PyObject *args, PyObject *kw)
 {
@@ -511,12 +514,7 @@ infpy_search_memory (PyObject *self, PyObject *args, PyObject *kw)
 	}
     }
   else
-    {
-      PyErr_SetString (PyExc_RuntimeError,
-		       _("Cannot get search address/range from Python."));
-
-      return NULL;
-    }
+    return NULL;
 
   if (!PyObject_CheckReadBuffer (pattern))
     {
--- a/gdb/python/py-param.c
+++ b/gdb/python/py-param.c
@@ -110,8 +110,8 @@ get_attr (PyObject *obj, PyObject *attr_name)
   return PyObject_GenericGetAttr (obj, attr_name);
 }
 
-/* Set a parameter value from a Python value.  Return 0 on success, -1
-   on failure.  */
+/* Set a parameter value from a Python value.  Return 0 on success.  Returns
+   -1 on error, with a python exception set.  */
 static int
 set_parameter_value (parmpy_object *self, PyObject *value)
 {
@@ -132,17 +132,25 @@ set_parameter_value (parmpy_object *self, PyObject *value)
 
 	  return -1;
 	}
-      if (self->value.stringval)
-	xfree (self->value.stringval);
       if (value == Py_None)
 	{
+	  xfree (self->value.stringval);
 	  if (self->type == var_optional_filename)
 	    self->value.stringval = xstrdup ("");
 	  else
 	    self->value.stringval = NULL;
 	}
       else
-	self->value.stringval = python_string_to_host_string (value);
+	{
+	  char *string;
+
+	  string = python_string_to_host_string (value);
+	  if (string == NULL)
+	    return -1;
+
+	  xfree (self->value.stringval);
+	  self->value.stringval = string;
+	}
       break;
 
     case var_enum:
@@ -158,6 +166,8 @@ set_parameter_value (parmpy_object *self, PyObject *value)
 	  }
 
 	str = python_string_to_host_string (value);
+	if (str == NULL)
+	  return -1;
 	for (i = 0; self->enumeration[i]; ++i)
 	  if (! strcmp (self->enumeration[i], str))
 	    break;
@@ -258,7 +268,7 @@ set_parameter_value (parmpy_object *self, PyObject *value)
   return 0;
 }
 
-/* Set an attribute.  */
+/* Set an attribute.  Returns -1 on error, with a python exception set.  */
 static int
 set_attr (PyObject *obj, PyObject *attr_name, PyObject *val)
 {
@@ -358,12 +368,13 @@ add_setshow_generic (int parmclass, enum command_class cmdclass,
     }
 }
 
-/* A helper which computes enum values.  Returns 1 on success, 0 on
-   error.  */
+/* A helper which computes enum values.  Returns 1 on success.  Returns 0 on
+   error, with a python exception set.  */
 static int
 compute_enum_values (parmpy_object *self, PyObject *enum_values)
 {
   Py_ssize_t size, i;
+  struct cleanup *back_to;
 
   if (! enum_values)
     {
@@ -390,6 +401,7 @@ compute_enum_values (parmpy_object *self, PyObject *enum_values)
     }
 
   self->enumeration = xmalloc ((size + 1) * sizeof (char *));
+  back_to = make_cleanup (free_current_contents, &self->enumeration);
   memset (self->enumeration, 0, (size + 1) * sizeof (char *));
 
   for (i = 0; i < size; ++i)
@@ -397,16 +409,27 @@ compute_enum_values (parmpy_object *self, PyObject *enum_values)
       PyObject *item = PySequence_GetItem (enum_values, i);
 
       if (! item)
-	return 0;
+	{
+	  do_cleanups (back_to);
+	  return 0;
+	}
       if (! gdbpy_is_string (item))
 	{
+	  do_cleanups (back_to);
 	  PyErr_SetString (PyExc_RuntimeError, 
 			   _("The enumeration item not a string."));
 	  return 0;
 	}
       self->enumeration[i] = python_string_to_host_string (item);
+      if (self->enumeration[i] == NULL)
+	{
+	  do_cleanups (back_to);
+	  return 0;
+	}
+      make_cleanup (xfree, (char *) self->enumeration[i]);
     }
 
+  discard_cleanups (back_to);
   return 1;
 }
 
@@ -422,7 +445,11 @@ get_doc_string (PyObject *object, PyObject *attr)
       PyObject *ds_obj = PyObject_GetAttr (object, attr);
 
       if (ds_obj && gdbpy_is_string (ds_obj))
-	result = python_string_to_host_string (ds_obj);
+	{
+	  result = python_string_to_host_string (ds_obj);
+	  if (result == NULL)
+	    gdbpy_print_stack ();
+	}
     }
   if (! result)
     result = xstrdup (_("This command is not documented."));
@@ -449,8 +476,9 @@ get_doc_string (PyObject *object, PyObject *attr)
 
    The documentation for the parameter is taken from the doc string
    for the python class.
-   
-*/
+
+   Returns -1 on error, with a python exception set.  */
+
 static int
 parmpy_init (PyObject *self, PyObject *args, PyObject *kwds)
 {
--- a/gdb/python/py-prettyprint.c
+++ b/gdb/python/py-prettyprint.c
@@ -185,8 +185,8 @@ find_pretty_printer (PyObject *value)
    is returned.  If the function returns Py_NONE that means the pretty
    printer returned the Python None as a value.  Otherwise, if the
    function returns a value,  *OUT_VALUE is set to the value, and NULL
-   is returned.  On error, *OUT_VALUE is set to NULL, and NULL is
-   returned.  */
+   is returned.  On error, *OUT_VALUE is set to NULL, NULL is
+   returned, with a python exception set.  */
 
 static PyObject *
 pretty_print_one_value (PyObject *printer, struct value **out_value)
@@ -232,7 +232,11 @@ gdbpy_get_display_hint (PyObject *printer)
   if (hint)
     {
       if (gdbpy_is_string (hint))
-	result = python_string_to_host_string (hint);
+	{
+	  result = python_string_to_host_string (hint);
+	  if (result == NULL)
+	    gdbpy_print_stack ();
+	}
       Py_DECREF (hint);
     }
   else
@@ -555,7 +559,10 @@ print_children (PyObject *printer, const char *hint,
 	  else
 	    {
 	      output = python_string_to_host_string (py_v);
-	      fputs_filtered (output, stream);
+	      if (!output)
+		gdbpy_print_stack ();
+	      else
+		fputs_filtered (output, stream);
 	      xfree (output);
 	    }
 	}
--- a/gdb/python/py-value.c
+++ b/gdb/python/py-value.c
@@ -119,7 +119,8 @@ note_value (value_object *value_obj)
   values_in_python = value_obj;
 }
 
-/* Called when a new gdb.Value object needs to be allocated.  */
+/* Called when a new gdb.Value object needs to be allocated.  Returns NULL on
+   error, with a python exception set.  */
 static PyObject *
 valpy_new (PyTypeObject *subtype, PyObject *args, PyObject *keywords)
 {
@@ -438,7 +439,7 @@ valpy_length (PyObject *self)
 }
 
 /* Given string name of an element inside structure, return its value
-   object.  */
+   object.  Returns NULL on error, with a python exception set.  */
 static PyObject *
 valpy_getitem (PyObject *self, PyObject *key)
 {
@@ -499,7 +500,7 @@ valpy_setitem (PyObject *self, PyObject *key, PyObject *value)
 }
 
 /* Called by the Python interpreter to perform an inferior function
-   call on the value.  */
+   call on the value.  Returns NULL on error, with a python exception set.  */
 static PyObject *
 valpy_call (PyObject *self, PyObject *args, PyObject *keywords)
 {
@@ -619,7 +620,8 @@ enum valpy_opcode
   ((TYPE_CODE (TYPE) == TYPE_CODE_REF) ? (TYPE_TARGET_TYPE (TYPE)) : (TYPE))
 
 /* Returns a value object which is the result of applying the operation
-   specified by OPCODE to the given arguments.  */
+   specified by OPCODE to the given arguments.  Returns NULL on error, with
+   a python exception set.  */
 static PyObject *
 valpy_binop (enum valpy_opcode opcode, PyObject *self, PyObject *other)
 {
@@ -871,7 +873,8 @@ valpy_xor (PyObject *self, PyObject *other)
   return valpy_binop (VALPY_BITXOR, self, other);
 }
 
-/* Implements comparison operations for value objects.  */
+/* Implements comparison operations for value objects.  Returns NULL on error,
+   with a python exception set.  */
 static PyObject *
 valpy_richcompare (PyObject *self, PyObject *other, int op)
 {
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -79,6 +79,7 @@ struct python_env
   PyGILState_STATE state;
   struct gdbarch *gdbarch;
   const struct language_defn *language;
+  PyObject *error_type, *error_value, *error_traceback;
 };
 
 static void
@@ -86,6 +87,16 @@ restore_python_env (void *p)
 {
   struct python_env *env = (struct python_env *)p;
 
+  /* Leftover Python error is forbidden by Python Exception Handling.  */
+  if (PyErr_Occurred ())
+    {
+      /* This order is similar to the one calling error afterwards. */
+      gdbpy_print_stack ();
+      warning (_("internal error: Unhandled Python exception"));
+    }
+
+  PyErr_Restore (env->error_type, env->error_value, env->error_traceback);
+
   PyGILState_Release (env->state);
   python_gdbarch = env->gdbarch;
   python_language = env->language;
@@ -108,6 +119,9 @@ ensure_python_env (struct gdbarch *gdbarch,
   python_gdbarch = gdbarch;
   python_language = language;
 
+  /* Save it and ensure ! PyErr_Occurred () afterwards.  */
+  PyErr_Fetch (&env->error_type, &env->error_value, &env->error_traceback);
+  
   return make_cleanup (restore_python_env, env);
 }
 
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-error.exp
@@ -0,0 +1,56 @@
+# Copyright (C) 2010 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Test error while loading *-gdb.py.  IBM1047 is chosen as possibly supported
+# by glibc but unsupported by Python
+
+set testfile "py-error"
+
+load_lib gdb-python.exp
+
+# Start with a fresh gdb.
+gdb_exit
+gdb_start
+
+# Skip all tests if Python scripting is not enabled.
+if { [skip_python_tests] } { continue }
+
+set charset "IBM1047"
+
+set test2 "main reached"
+
+set test "set host-charset $charset"
+set test_regex [string_to_regexp $test]
+gdb_test_multiple $test $test {
+    -re "^$test_regex\r\n$gdb_prompt $" {
+	pass $test
+    }
+    -re "^$test_regex\r\nUndefined item: \"$charset\"\\.\r\n$gdb_prompt $" {
+	xfail $test
+	untested $test2
+	set test2 ""
+    }
+}
+
+if {$test2 == ""} {
+    return 0
+}
+
+set remote_python_file [remote_download host ${srcdir}/${subdir}/${testfile}.py]
+
+# argc=LookupError: unknown encoding: IBM1047
+gdb_test "source $remote_python_file" "Traceback.*ClassName.*\r\nLookupError: unknown encoding: $charset" $test2
+
+gdb_test "p 1" " = 1" "no delayed error"
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-error.py
@@ -0,0 +1,25 @@
+# Copyright (C) 2010 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+import gdb
+
+class ClassName(gdb.Command):
+    'a'
+    def __init__(self):
+        gdb.Command.__init__ (self, "ClassName", gdb.COMMAND_DATA, prefix=True)
+    def invoke(self, args, from_tty):
+        print
+
+ClassName()
--- a/gdb/varobj.c
+++ b/gdb/varobj.c
@@ -1052,6 +1052,8 @@ update_dynamic_varobj_children (struct varobj *var,
 	    error (_("Invalid item from the child list"));
 
 	  v = convert_value_from_python (py_v);
+	  if (v == NULL)
+	    gdbpy_print_stack ();
 	  install_dynamic_child (var, can_mention ? changed : NULL,
 				 can_mention ? new : NULL,
 				 can_mention ? unchanged : NULL,
@@ -2540,6 +2542,8 @@ value_get_print_value (struct value *value, enum varobj_display_formats format,
 			type = builtin_type (gdbarch)->builtin_char;
 			Py_DECREF (py_str);
 		      }
+		    else
+		      gdbpy_print_stack ();
 		  }
 		Py_DECREF (output);
 	      }

Re: [patch] python: save/restore/fix error state

[Tom Tromey]

>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:

Tom> It seems to me that inability to convert the docstring shouldn't be a
Tom> fatal problem for command registration.  We can just fall back to the
Tom> no-docstring code.

Jan> I had it that way first.  The problem is that way it prints:
Jan> 	(gdb) source py-error.py
Jan> 	LookupError: unknown encoding: IBM1047
Jan> 	(gdb) _

Jan> which I do not find too useful.

[...]

I don't have a strong feeling either way.
This IBM1047 scenario is far-fetched.

Tom

Re: [patch] python: save/restore/fix error state

[Jan Kratochvil]

On Tue, 12 Oct 2010 23:39:02 +0200, Tom Tromey wrote:
> I don't have a strong feeling either way.

Checked-in the error existing version providing back(forward)traces as IMO
such case is already a problem and it should not be difficult to track it
down:
	http://sourceware.org/ml/gdb-cvs/2010-10/msg00082.html


> This IBM1047 scenario is far-fetched.

I have to note this problem occurs for 882 (75.51%) out of the 1168 charsets
supported by glibc.


Thanks,
Jan

Re: [patch] python: save/restore/fix error state

[Tom Tromey]

>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:

Tom> This IBM1047 scenario is far-fetched.

Jan> I have to note this problem occurs for 882 (75.51%) out of the 1168
Jan> charsets supported by glibc.

Ugh.  How many of those charsets are used by some locale?
If it is too many then we should probably look at some other approach.
We could do the conversion in gdb.  Or we could provide an iconv-based
python codec.

Tom

Re: [patch] python: save/restore/fix error state

[Jan Kratochvil]

On Wed, 13 Oct 2010 17:37:57 +0200, Tom Tromey wrote:
> >>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:
> Jan> I have to note this problem occurs for 882 (75.51%) out of the 1168
> Jan> charsets supported by glibc.
> 
> Ugh.  How many of those charsets are used by some locale?

After same offhand filtering at least for these:
	hy_AM.armscii8
	ka_GE.georgianps
	tg_TJ.koi8t
	vi_VN.tcvn
	zh_TW.euctw


> If it is too many then we should probably look at some other approach.
> We could do the conversion in gdb.  Or we could provide an iconv-based
> python codec.

Cannot it be reassigned to Python as a bug of its own?
BTW tested python-2.7-8.fc14.1.x86_64; there exists also python-3.x.


Thanks,
Jan

Re: [patch] python: save/restore/fix error state

[Tom Tromey]

>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:

Tom> Ugh.  How many of those charsets are used by some locale?

Jan> After same offhand filtering at least for these:
Jan> 	hy_AM.armscii8
Jan> 	ka_GE.georgianps
Jan> 	tg_TJ.koi8t
Jan> 	vi_VN.tcvn
Jan> 	zh_TW.euctw


Tom> If it is too many then we should probably look at some other approach.
Tom> We could do the conversion in gdb.  Or we could provide an iconv-based
Tom> python codec.

Jan> Cannot it be reassigned to Python as a bug of its own?

Yeah, I suppose so.

Tom