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

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

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