From 5c329e6ab4c7bba9b83155571b150756210001df Mon Sep 17 00:00:00 2001 From: Tom Tromey Date: Thu, 27 Dec 2018 11:32:01 -0700 Subject: [PATCH] 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 * 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) : Remove. : 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 | 18 +++++++++++++ gdb/python/py-prettyprint.c | 12 ++------- gdb/python/py-utils.c | 58 ++++++++++++++++++----------------------- gdb/python/py-value.c | 10 ++++---- gdb/python/py-varobj.c | 10 ++------ gdb/python/python-internal.h | 61 +++++++++++++++++++++++++++++++++++++++++--- gdb/python/python.c | 25 ++++++++---------- 7 files changed, 121 insertions(+), 73 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index da92e3f..af27de2 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,21 @@ +2019-01-03 Tom Tromey + + * 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) : Remove. + : 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 * linux-nat.c (delete_lwp_cleanup): Delete. diff --git a/gdb/python/py-prettyprint.c b/gdb/python/py-prettyprint.c index d69a86b..ac0506f 100644 --- a/gdb/python/py-prettyprint.c +++ b/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; - - PyErr_Fetch (&type, &value, &trace); - - gdbpy_ref<> type_ref (type); - gdbpy_ref<> value_ref (value); - gdbpy_ref<> trace_ref (trace); - - gdb::unique_xmalloc_ptr - msg (gdbpy_exception_to_string (type, value)); + gdbpy_err_fetch fetched_error; + gdb::unique_xmalloc_ptr msg = fetched_error.to_string (); if (msg == NULL || *msg == '\0') fprintf_filtered (stream, _("")); diff --git a/gdb/python/py-utils.c b/gdb/python/py-utils.c index 66b2a67..0f838c2 100644 --- a/gdb/python/py-utils.c +++ b/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 - 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. */ +/* See python-internal.h. */ gdb::unique_xmalloc_ptr -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. - pvalue is not a string when raise "foo" is used, instead it is None - and ptype is "foo". - So the algorithm we use is to print `str (pvalue)' if it's not - None, otherwise we print `str (ptype)'. + value is a string when PyErr_SetString is used. + value is not a string when raise "foo" is used, instead it is None + and type is "foo". + So the algorithm we use is to print `str (value)' if it's not + 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) - return gdbpy_obj_to_string (pvalue); + if (m_error_value && m_error_value != Py_None) + 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 +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; - - 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 - msg (gdbpy_exception_to_string (ptype, pvalue)); + gdbpy_err_fetch fetched_error; + gdb::unique_xmalloc_ptr msg = fetched_error.to_string (); 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) - || msg == NULL || *msg == '\0') + else if (! fetched_error.type_matches (gdbpy_gdberror_exc) + || 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 - { - Py_XDECREF (ptype); - Py_XDECREF (pvalue); - Py_XDECREF (ptraceback); - error ("%s", msg.get ()); - } + error ("%s", msg.get ()); } diff --git a/gdb/python/py-value.c b/gdb/python/py-value.c index 74835e9..9cc56e6 100644 --- a/gdb/python/py-value.c +++ b/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; - - PyErr_Fetch (&etype, &evalue, &etraceback); + gdbpy_err_fetch fetched_error; 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 diff --git a/gdb/python/py-varobj.c b/gdb/python/py-varobj.c index 38da443..274b2de 100644 --- a/gdb/python/py-varobj.c +++ b/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; - - PyErr_Fetch (&type, &value, &trace); - gdb::unique_xmalloc_ptr - value_str (gdbpy_exception_to_string (type, value)); - Py_XDECREF (type); - Py_XDECREF (value); - Py_XDECREF (trace); + gdbpy_err_fetch fetched_error; + gdb::unique_xmalloc_ptr value_str = fetched_error.to_string (); if (value_str == NULL) { gdbpy_print_stack (); diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h index 51919b7..b5d9840 100644 --- a/gdb/python/python-internal.h +++ b/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 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 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 m_error; }; /* Like gdbpy_enter, but takes a varobj. This is a subclass just to @@ -665,8 +722,6 @@ gdb::unique_xmalloc_ptr 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 gdbpy_obj_to_string (PyObject *obj); -gdb::unique_xmalloc_ptr 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, diff --git a/gdb/python/python.c b/gdb/python/python.c index be3ee87..d8505e9 100644 --- a/gdb/python/python.c +++ b/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); - - /* Fetch the error message contained within ptype, pvalue. */ - gdb::unique_xmalloc_ptr - msg (gdbpy_exception_to_string (ptype, pvalue)); - gdb::unique_xmalloc_ptr type (gdbpy_obj_to_string (ptype)); + gdb::unique_xmalloc_ptr msg = fetched_error.to_string (); + gdb::unique_xmalloc_ptr type; + /* Don't compute TYPE if MSG already indicates that there is an + error. */ + if (msg != NULL) + 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); } } -- 2.7.4