Use a wrapper for PyErr_Fetch

This introduces a new class that wraps PyErr_Fetch and PyErr_Restore,
and then changes all the callers in gdb to use it.  This reduces the
amount of explicit reference counting that is done in the Python code.
I also found and fixed a latent bug in gdbpy_print_stack -- it was not
correctly checking some error conditions, nor clearing the exception
when needed.

gdb/ChangeLog
2019-01-03  Tom Tromey  <tom@tromey.com>

	* python/python.c (gdbpy_enter, ~gdbpy_enter): Update.
	(gdbpy_print_stack): Use gdbpy_err_fetch.
	* python/python-internal.h (class gdbpy_err_fetch): New class.
	(class gdbpy_enter) <m_error_type, m_error_value,
	m_error_traceback>: Remove.
	<m_error>: New member.
	(gdbpy_exception_to_string): Don't declare.
	* python/py-varobj.c (py_varobj_iter_next): Use gdbpy_err_fetch.
	* python/py-value.c (convert_value_from_python): Use
	gdbpy_err_fetch.
	* python/py-utils.c (gdbpy_err_fetch::to_string): Rename from
	gdbpy_exception_to_string.
	(gdbpy_handle_exception): Use gdbpy_err_fetch.
	* python/py-prettyprint.c (print_stack_unless_memory_error): Use
	gdbpy_err_fetch.

gdb/ChangeLog

@@ -1,3 +1,21 @@
+2019-01-03  Tom Tromey  <tom@tromey.com>
+
+	* python/python.c (gdbpy_enter, ~gdbpy_enter): Update.
+	(gdbpy_print_stack): Use gdbpy_err_fetch.
+	* python/python-internal.h (class gdbpy_err_fetch): New class.
+	(class gdbpy_enter) <m_error_type, m_error_value,
+	m_error_traceback>: Remove.
+	<m_error>: New member.
+	(gdbpy_exception_to_string): Don't declare.
+	* python/py-varobj.c (py_varobj_iter_next): Use gdbpy_err_fetch.
+	* python/py-value.c (convert_value_from_python): Use
+	gdbpy_err_fetch.
+	* python/py-utils.c (gdbpy_err_fetch::to_string): Rename from
+	gdbpy_exception_to_string.
+	(gdbpy_handle_exception): Use gdbpy_err_fetch.
+	* python/py-prettyprint.c (print_stack_unless_memory_error): Use
+	gdbpy_err_fetch.
+
 2019-01-03  Andrew Burgess  <andrew.burgess@embecosm.com>
 
 	* linux-nat.c (delete_lwp_cleanup): Delete.

gdb/python/py-prettyprint.c

@@ -259,16 +259,8 @@ print_stack_unless_memory_error (struct ui_file *stream)
 {
   if (PyErr_ExceptionMatches (gdbpy_gdb_memory_error))
     {
-      PyObject *type, *value, *trace;
+      gdbpy_err_fetch fetched_error;
-
+      gdb::unique_xmalloc_ptr<char> msg = fetched_error.to_string ();
-      PyErr_Fetch (&type, &value, &trace);
-
-      gdbpy_ref<> type_ref (type);
-      gdbpy_ref<> value_ref (value);
-      gdbpy_ref<> trace_ref (trace);
-
-      gdb::unique_xmalloc_ptr<char>
-	msg (gdbpy_exception_to_string (type, value));
 
       if (msg == NULL || *msg == '\0')
 	fprintf_filtered (stream, _("<error reading variable>"));

gdb/python/py-utils.c

@@ -203,28 +203,33 @@ gdbpy_obj_to_string (PyObject *obj)
   return NULL;
 }
 
-/* Return the string representation of the exception represented by
+/* See python-internal.h.  */
-   TYPE, VALUE which is assumed to have been obtained with PyErr_Fetch,
-   i.e., the error indicator is currently clear.
-   If the result is NULL a python error occurred, the caller must clear it.  */
 
 gdb::unique_xmalloc_ptr<char>
-gdbpy_exception_to_string (PyObject *ptype, PyObject *pvalue)
+gdbpy_err_fetch::to_string () const
 {
   /* There are a few cases to consider.
      For example:
-     pvalue is a string when PyErr_SetString is used.
+     value is a string when PyErr_SetString is used.
-     pvalue is not a string when raise "foo" is used, instead it is None
+     value is not a string when raise "foo" is used, instead it is None
-     and ptype is "foo".
+     and type is "foo".
-     So the algorithm we use is to print `str (pvalue)' if it's not
+     So the algorithm we use is to print `str (value)' if it's not
-     None, otherwise we print `str (ptype)'.
+     None, otherwise we print `str (type)'.
      Using str (aka PyObject_Str) will fetch the error message from
      gdb.GdbError ("message").  */
 
-  if (pvalue && pvalue != Py_None)
+  if (m_error_value && m_error_value != Py_None)
-    return gdbpy_obj_to_string (pvalue);
+    return gdbpy_obj_to_string (m_error_value);
   else
-    return gdbpy_obj_to_string (ptype);
+    return gdbpy_obj_to_string (m_error_type);
+}
+
+/* See python-internal.h.  */
+
+gdb::unique_xmalloc_ptr<char>
+gdbpy_err_fetch::type_to_string () const
+{
+  return gdbpy_obj_to_string (m_error_type);
 }
 
 /* Convert a GDB exception to the appropriate Python exception.
@@ -394,16 +399,8 @@ gdb_pymodule_addobject (PyObject *module, const char *name, PyObject *object)
 void
 gdbpy_handle_exception ()
 {
-  PyObject *ptype, *pvalue, *ptraceback;
+  gdbpy_err_fetch fetched_error;
-
+  gdb::unique_xmalloc_ptr<char> msg = fetched_error.to_string ();
-  PyErr_Fetch (&ptype, &pvalue, &ptraceback);
-
-  /* Try to fetch an error message contained within ptype, pvalue.
-     When fetching the error message we need to make our own copy,
-     we no longer own ptype, pvalue after the call to PyErr_Restore.  */
-
-  gdb::unique_xmalloc_ptr<char>
-    msg (gdbpy_exception_to_string (ptype, pvalue));
 
   if (msg == NULL)
     {
@@ -422,12 +419,12 @@ gdbpy_handle_exception ()
      for user errors.  However, a missing message for gdb.GdbError
      exceptions is arguably a bug, so we flag it as such.  */
 
-  if (PyErr_GivenExceptionMatches (ptype, PyExc_KeyboardInterrupt))
+  if (fetched_error.type_matches (PyExc_KeyboardInterrupt))
     throw_quit ("Quit");
-  else if (! PyErr_GivenExceptionMatches (ptype, gdbpy_gdberror_exc)
+  else if (! fetched_error.type_matches (gdbpy_gdberror_exc)
-      || msg == NULL || *msg == '\0')
+	   || msg == NULL || *msg == '\0')
     {
-      PyErr_Restore (ptype, pvalue, ptraceback);
+      fetched_error.restore ();
       gdbpy_print_stack ();
       if (msg != NULL && *msg != '\0')
 	error (_("Error occurred in Python: %s"), msg.get ());
@@ -435,10 +432,5 @@ gdbpy_handle_exception ()
 	error (_("Error occurred in Python."));
     }
   else
-    {
+    error ("%s", msg.get ());
-      Py_XDECREF (ptype);
-      Py_XDECREF (pvalue);
-      Py_XDECREF (ptraceback);
-      error ("%s", msg.get ());
-    }
 }

gdb/python/py-value.c

@@ -1661,9 +1661,7 @@ convert_value_from_python (PyObject *obj)
 	         ULONGEST instead.  */
 	      if (PyErr_ExceptionMatches (PyExc_OverflowError))
 		{
-		  PyObject *etype, *evalue, *etraceback;
+		  gdbpy_err_fetch fetched_error;
-
-		  PyErr_Fetch (&etype, &evalue, &etraceback);
 		  gdbpy_ref<> zero (PyInt_FromLong (0));
 
 		  /* Check whether obj is positive.  */
@@ -1676,8 +1674,10 @@ convert_value_from_python (PyObject *obj)
 			value = value_from_ulongest (builtin_type_upylong, ul);
 		    }
 		  else
-		    /* There's nothing we can do.  */
+		    {
-		    PyErr_Restore (etype, evalue, etraceback);
+		      /* There's nothing we can do.  */
+		      fetched_error.restore ();
+		    }
 		}
 	    }
 	  else

gdb/python/py-varobj.c

@@ -70,14 +70,8 @@ py_varobj_iter_next (struct varobj_iter *self)
       /* If we got a memory error, just use the text as the item.  */
       if (PyErr_ExceptionMatches (gdbpy_gdb_memory_error))
 	{
-	  PyObject *type, *value, *trace;
+	  gdbpy_err_fetch fetched_error;
-
+	  gdb::unique_xmalloc_ptr<char> value_str = fetched_error.to_string ();
-	  PyErr_Fetch (&type, &value, &trace);
-	  gdb::unique_xmalloc_ptr<char>
-	    value_str (gdbpy_exception_to_string (type, value));
-	  Py_XDECREF (type);
-	  Py_XDECREF (value);
-	  Py_XDECREF (trace);
 	  if (value_str == NULL)
 	    {
 	      gdbpy_print_stack ();

gdb/python/python-internal.h

@@ -591,6 +591,60 @@ int gdbpy_initialize_xmethods (void)
 int gdbpy_initialize_unwind (void)
   CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION;
 
+/* A wrapper for PyErr_Fetch that handles reference counting for the
+   caller.  */
+class gdbpy_err_fetch
+{
+public:
+
+  gdbpy_err_fetch ()
+  {
+    PyErr_Fetch (&m_error_type, &m_error_value, &m_error_traceback);
+  }
+
+  ~gdbpy_err_fetch ()
+  {
+    Py_XDECREF (m_error_type);
+    Py_XDECREF (m_error_value);
+    Py_XDECREF (m_error_traceback);
+  }
+
+  /* Call PyErr_Restore using the values stashed in this object.
+     After this call, this object is invalid and neither the to_string
+     nor restore methods may be used again.  */
+
+  void restore ()
+  {
+    PyErr_Restore (m_error_type, m_error_value, m_error_traceback);
+    m_error_type = nullptr;
+    m_error_value = nullptr;
+    m_error_traceback = nullptr;
+  }
+
+  /* Return the string representation of the exception represented by
+     this object.  If the result is NULL a python error occurred, the
+     caller must clear it.  */
+
+  gdb::unique_xmalloc_ptr<char> to_string () const;
+
+  /* Return the string representation of the type of the exception
+     represented by this object.  If the result is NULL a python error
+     occurred, the caller must clear it.  */
+
+  gdb::unique_xmalloc_ptr<char> type_to_string () const;
+
+  /* Return true if the stored type matches TYPE, false otherwise.  */
+
+  bool type_matches (PyObject *type) const
+  {
+    return PyErr_GivenExceptionMatches (m_error_type, type);
+  }
+
+private:
+
+  PyObject *m_error_type, *m_error_value, *m_error_traceback;
+};
+
 /* Called before entering the Python interpreter to install the
    current language and architecture to be used for Python values.
    Also set the active extension language for GDB so that SIGINT's
@@ -612,7 +666,10 @@ class gdbpy_enter
   PyGILState_STATE m_state;
   struct gdbarch *m_gdbarch;
   const struct language_defn *m_language;
-  PyObject *m_error_type, *m_error_value, *m_error_traceback;
+
+  /* An optional is used here because we don't want to call
+     PyErr_Fetch too early.  */
+  gdb::optional<gdbpy_err_fetch> m_error;
 };
 
 /* Like gdbpy_enter, but takes a varobj.  This is a subclass just to
@@ -665,8 +722,6 @@ gdb::unique_xmalloc_ptr<char> python_string_to_host_string (PyObject *obj);
 gdbpy_ref<> host_string_to_python_string (const char *str);
 int gdbpy_is_string (PyObject *obj);
 gdb::unique_xmalloc_ptr<char> gdbpy_obj_to_string (PyObject *obj);
-gdb::unique_xmalloc_ptr<char> gdbpy_exception_to_string (PyObject *ptype,
-							 PyObject *pvalue);
 
 int gdbpy_is_lazy_string (PyObject *result);
 void gdbpy_extract_lazy_string (PyObject *string, CORE_ADDR *addr,

gdb/python/python.c

@@ -214,7 +214,7 @@ gdbpy_enter::gdbpy_enter  (struct gdbarch *gdbarch,
   python_language = language;
 
   /* Save it and ensure ! PyErr_Occurred () afterwards.  */
-  PyErr_Fetch (&m_error_type, &m_error_value, &m_error_traceback);
+  m_error.emplace ();
 }
 
 gdbpy_enter::~gdbpy_enter ()
@@ -227,7 +227,7 @@ gdbpy_enter::~gdbpy_enter ()
       warning (_("internal error: Unhandled Python exception"));
     }
 
-  PyErr_Restore (m_error_type, m_error_value, m_error_traceback);
+  m_error->restore ();
 
   PyGILState_Release (m_state);
   python_gdbarch = m_gdbarch;
@@ -1234,24 +1234,25 @@ gdbpy_print_stack (void)
   /* Print "message", just error print message.  */
   else
     {
-      PyObject *ptype, *pvalue, *ptraceback;
+      gdbpy_err_fetch fetched_error;
 
-      PyErr_Fetch (&ptype, &pvalue, &ptraceback);
+      gdb::unique_xmalloc_ptr<char> msg = fetched_error.to_string ();
-
+      gdb::unique_xmalloc_ptr<char> type;
-      /* Fetch the error message contained within ptype, pvalue.  */
+      /* Don't compute TYPE if MSG already indicates that there is an
-      gdb::unique_xmalloc_ptr<char>
+	 error.  */
-	msg (gdbpy_exception_to_string (ptype, pvalue));
+      if (msg != NULL)
-      gdb::unique_xmalloc_ptr<char> type (gdbpy_obj_to_string (ptype));
+	type = fetched_error.type_to_string ();
 
       TRY
 	{
-	  if (msg == NULL)
+	  if (msg == NULL || type == NULL)
 	    {
 	      /* An error occurred computing the string representation of the
 		 error message.  */
 	      fprintf_filtered (gdb_stderr,
 				_("Error occurred computing Python error" \
 				  "message.\n"));
+	      PyErr_Clear ();
 	    }
 	  else
 	    fprintf_filtered (gdb_stderr, "Python Exception %s %s: \n",
@@ -1261,10 +1262,6 @@ gdbpy_print_stack (void)
 	{
 	}
       END_CATCH
-
-      Py_XDECREF (ptype);
-      Py_XDECREF (pvalue);
-      Py_XDECREF (ptraceback);
     }
 }