From patchwork Thu Dec 27 19:26:33 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tom Tromey X-Patchwork-Id: 30866 Received: (qmail 16009 invoked by alias); 27 Dec 2018 19:26:50 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 15883 invoked by uid 89); 27 Dec 2018 19:26:49 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=clearing, 708, sk:python-, sk:python X-HELO: gateway32.websitewelcome.com Received: from gateway32.websitewelcome.com (HELO gateway32.websitewelcome.com) (192.185.145.101) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 27 Dec 2018 19:26:44 +0000 Received: from cm13.websitewelcome.com (cm13.websitewelcome.com [100.42.49.6]) by gateway32.websitewelcome.com (Postfix) with ESMTP id 1FDCA79E329 for ; Thu, 27 Dec 2018 13:26:43 -0600 (CST) Received: from box5379.bluehost.com ([162.241.216.53]) by cmsmtp with SMTP id cbIpgeuTPYTGMcbIpgShBT; Thu, 27 Dec 2018 13:26:43 -0600 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=tromey.com; s=default; h=References:In-Reply-To:Message-Id:Date:Subject:Cc:To:From: Sender:Reply-To:MIME-Version:Content-Type:Content-Transfer-Encoding: Content-ID:Content-Description:Resent-Date:Resent-From:Resent-Sender: Resent-To:Resent-Cc:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=jaK39PzbQxSWSBJ2yqORK/diGERBeXlDhglytUx8miM=; b=BVgSU60Bl7E8irNoxWE+vijHLy sRJ0JMhZCFQAmEpfCin4yTF18fLCdv+fATliEny3gZEQTP7CTVI6jlCn5fvS5VbdifAtyZ3hxebp7 PIpIdwdH6BXwl5YVKVKKSS03S; Received: from 75-166-72-210.hlrn.qwest.net ([75.166.72.210]:47364 helo=bapiya.Home) by box5379.bluehost.com with esmtpsa (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.91) (envelope-from ) id 1gcbIo-0004pC-ID; Thu, 27 Dec 2018 13:26:42 -0600 From: Tom Tromey To: gdb-patches@sourceware.org Cc: Tom Tromey Subject: [PATCH 1/5] Use a wrapper for PyErr_Fetch Date: Thu, 27 Dec 2018 12:26:33 -0700 Message-Id: <20181227192637.17862-2-tom@tromey.com> In-Reply-To: <20181227192637.17862-1-tom@tromey.com> References: <20181227192637.17862-1-tom@tromey.com> 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 2018-12-27 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 | 54 ++++++++++++++----------------- 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, 119 insertions(+), 71 deletions(-) diff --git a/gdb/python/py-prettyprint.c b/gdb/python/py-prettyprint.c index fa4107c30d..a66c88ad3e 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 e0aedb5b58..ffe49b2f4e 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,10 +419,10 @@ 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, gdbpy_gdberror_exc) + 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 ()); @@ -433,10 +430,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 d21c2faf64..66cfa59026 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 8946ef85af..4cb76c3abb 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 1ac54f9b57..cbb3615020 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 @@ -664,8 +721,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 f790d48cc2..e651773318 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); } }