From 0ddbf4795f40d2d3386dc56ffa264b50a015f6c9 Mon Sep 17 00:00:00 2001 From: Antoine Pitrou Date: Wed, 8 Oct 2014 20:00:09 +0200 Subject: [PATCH] Issue #22462: Fix pyexpat's creation of a dummy frame to make it appear in exception tracebacks. Initial patch by Mark Shannon. --- Include/traceback.h | 1 + Lib/test/test_pyexpat.py | 18 +++++- Misc/NEWS | 3 + Modules/_ctypes/callbacks.c | 45 +------------- Modules/_ctypes/callproc.c | 2 +- Modules/_ctypes/ctypes.h | 2 - Modules/pyexpat.c | 120 +++--------------------------------- Python/traceback.c | 33 ++++++++++ 8 files changed, 63 insertions(+), 161 deletions(-) diff --git a/Include/traceback.h b/Include/traceback.h index 77347077e91..12d467a08df 100644 --- a/Include/traceback.h +++ b/Include/traceback.h @@ -24,6 +24,7 @@ PyAPI_FUNC(int) PyTraceBack_Here(struct _frame *); PyAPI_FUNC(int) PyTraceBack_Print(PyObject *, PyObject *); #ifndef Py_LIMITED_API PyAPI_FUNC(int) _Py_DisplaySourceLine(PyObject *, PyObject *, int, int); +PyAPI_FUNC(void) _PyTraceback_Add(char *, char *, int); #endif /* Reveal traceback type so we can typecheck traceback objects */ diff --git a/Lib/test/test_pyexpat.py b/Lib/test/test_pyexpat.py index 95a614bb9b7..26f1961bd91 100644 --- a/Lib/test/test_pyexpat.py +++ b/Lib/test/test_pyexpat.py @@ -2,7 +2,9 @@ # handler, are obscure and unhelpful. from io import BytesIO +import os import unittest +import traceback from xml.parsers import expat from xml.parsers.expat import errors @@ -419,7 +421,11 @@ class HandlerExceptionTest(unittest.TestCase): def StartElementHandler(self, name, attrs): raise RuntimeError(name) - def test(self): + def check_traceback_entry(self, entry, filename, funcname): + self.assertEqual(os.path.basename(entry[0]), filename) + self.assertEqual(entry[2], funcname) + + def test_exception(self): parser = expat.ParserCreate() parser.StartElementHandler = self.StartElementHandler try: @@ -429,6 +435,16 @@ def test(self): self.assertEqual(e.args[0], 'a', "Expected RuntimeError for element 'a', but" + \ " found %r" % e.args[0]) + # Check that the traceback contains the relevant line in pyexpat.c + entries = traceback.extract_tb(e.__traceback__) + self.assertEqual(len(entries), 3) + self.check_traceback_entry(entries[0], + "test_pyexpat.py", "test_exception") + self.check_traceback_entry(entries[1], + "pyexpat.c", "StartElement") + self.check_traceback_entry(entries[2], + "test_pyexpat.py", "StartElementHandler") + self.assertIn('call_with_frame("StartElement"', entries[1][3]) # Test Current* members: diff --git a/Misc/NEWS b/Misc/NEWS index 93eedd5e2c8..4fe01097cd2 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -24,6 +24,9 @@ Core and Builtins Library ------- +- Issue #22462: Fix pyexpat's creation of a dummy frame to make it + appear in exception tracebacks. + - Issue #21173: Fix len() on a WeakKeyDictionary when .clear() was called with an iterator alive. diff --git a/Modules/_ctypes/callbacks.c b/Modules/_ctypes/callbacks.c index ec8fd12a896..7cd61640f81 100644 --- a/Modules/_ctypes/callbacks.c +++ b/Modules/_ctypes/callbacks.c @@ -92,49 +92,6 @@ PrintError(char *msg, ...) } -/* after code that pyrex generates */ -void _ctypes_add_traceback(char *funcname, char *filename, int lineno) -{ - PyObject *py_globals = 0; - PyCodeObject *py_code = 0; - PyFrameObject *py_frame = 0; - PyObject *exception, *value, *tb; - - /* (Save and) Clear the current exception. Python functions must not be - called with an exception set. Calling Python functions happens when - the codec of the filesystem encoding is implemented in pure Python. */ - PyErr_Fetch(&exception, &value, &tb); - - py_globals = PyDict_New(); - if (!py_globals) - goto bad; - py_code = PyCode_NewEmpty(filename, funcname, lineno); - if (!py_code) - goto bad; - py_frame = PyFrame_New( - PyThreadState_Get(), /*PyThreadState *tstate,*/ - py_code, /*PyCodeObject *code,*/ - py_globals, /*PyObject *globals,*/ - 0 /*PyObject *locals*/ - ); - if (!py_frame) - goto bad; - py_frame->f_lineno = lineno; - - PyErr_Restore(exception, value, tb); - PyTraceBack_Here(py_frame); - - Py_DECREF(py_globals); - Py_DECREF(py_code); - Py_DECREF(py_frame); - return; - - bad: - Py_XDECREF(py_globals); - Py_XDECREF(py_code); - Py_XDECREF(py_frame); -} - #ifdef MS_WIN32 /* * We must call AddRef() on non-NULL COM pointers we receive as arguments @@ -254,7 +211,7 @@ static void _CallPythonObject(void *mem, } #define CHECK(what, x) \ -if (x == NULL) _ctypes_add_traceback(what, "_ctypes/callbacks.c", __LINE__ - 1), PyErr_Print() +if (x == NULL) _PyTraceback_Add(what, "_ctypes/callbacks.c", __LINE__ - 1), PyErr_Print() if (flags & (FUNCFLAG_USE_ERRNO | FUNCFLAG_USE_LASTERROR)) { error_object = _ctypes_get_errobj(&space); diff --git a/Modules/_ctypes/callproc.c b/Modules/_ctypes/callproc.c index 74119a3b4d0..dcabbdf361d 100644 --- a/Modules/_ctypes/callproc.c +++ b/Modules/_ctypes/callproc.c @@ -919,7 +919,7 @@ static PyObject *GetResult(PyObject *restype, void *result, PyObject *checker) v = PyObject_CallFunctionObjArgs(checker, retval, NULL); if (v == NULL) - _ctypes_add_traceback("GetResult", "_ctypes/callproc.c", __LINE__-2); + _PyTraceback_Add("GetResult", "_ctypes/callproc.c", __LINE__-2); Py_DECREF(retval); return v; } diff --git a/Modules/_ctypes/ctypes.h b/Modules/_ctypes/ctypes.h index ac8341e5c3a..de6c7649117 100644 --- a/Modules/_ctypes/ctypes.h +++ b/Modules/_ctypes/ctypes.h @@ -353,8 +353,6 @@ extern char *_ctypes_conversion_errors; extern void _ctypes_free_closure(void *); extern void *_ctypes_alloc_closure(void); -extern void _ctypes_add_traceback(char *, char *, int); - extern PyObject *PyCData_FromBaseObj(PyObject *type, PyObject *base, Py_ssize_t index, char *adr); extern char *_ctypes_alloc_format_string(const char *prefix, const char *suffix); extern char *_ctypes_alloc_format_string_with_shape(int ndim, diff --git a/Modules/pyexpat.c b/Modules/pyexpat.c index 97f2b567717..4ced53b611a 100644 --- a/Modules/pyexpat.c +++ b/Modules/pyexpat.c @@ -8,8 +8,6 @@ #define XML_COMBINED_VERSION (10000*XML_MAJOR_VERSION+100*XML_MINOR_VERSION+XML_MICRO_VERSION) -#define FIX_TRACE - static XML_Memory_Handling_Suite ExpatMemoryHandler = { PyObject_Malloc, PyObject_Realloc, PyObject_Free}; @@ -210,121 +208,17 @@ flag_error(xmlparseobject *self) error_external_entity_ref_handler); } -static PyCodeObject* -getcode(enum HandlerTypes slot, char* func_name, int lineno) -{ - if (handler_info[slot].tb_code == NULL) { - handler_info[slot].tb_code = - PyCode_NewEmpty(__FILE__, func_name, lineno); - } - return handler_info[slot].tb_code; -} - -#ifdef FIX_TRACE -static int -trace_frame(PyThreadState *tstate, PyFrameObject *f, int code, PyObject *val) -{ - int result = 0; - if (!tstate->use_tracing || tstate->tracing) - return 0; - if (tstate->c_profilefunc != NULL) { - tstate->tracing++; - result = tstate->c_profilefunc(tstate->c_profileobj, - f, code , val); - tstate->use_tracing = ((tstate->c_tracefunc != NULL) - || (tstate->c_profilefunc != NULL)); - tstate->tracing--; - if (result) - return result; - } - if (tstate->c_tracefunc != NULL) { - tstate->tracing++; - result = tstate->c_tracefunc(tstate->c_traceobj, - f, code , val); - tstate->use_tracing = ((tstate->c_tracefunc != NULL) - || (tstate->c_profilefunc != NULL)); - tstate->tracing--; - } - return result; -} - -static int -trace_frame_exc(PyThreadState *tstate, PyFrameObject *f) -{ - PyObject *type, *value, *traceback, *arg; - int err; - - if (tstate->c_tracefunc == NULL) - return 0; - - PyErr_Fetch(&type, &value, &traceback); - if (value == NULL) { - value = Py_None; - Py_INCREF(value); - } - arg = PyTuple_Pack(3, type, value, traceback); - if (arg == NULL) { - PyErr_Restore(type, value, traceback); - return 0; - } - err = trace_frame(tstate, f, PyTrace_EXCEPTION, arg); - Py_DECREF(arg); - if (err == 0) - PyErr_Restore(type, value, traceback); - else { - Py_XDECREF(type); - Py_XDECREF(value); - Py_XDECREF(traceback); - } - return err; -} -#endif - static PyObject* -call_with_frame(PyCodeObject *c, PyObject* func, PyObject* args, +call_with_frame(char *funcname, int lineno, PyObject* func, PyObject* args, xmlparseobject *self) { - PyThreadState *tstate = PyThreadState_GET(); - PyFrameObject *f; - PyObject *res, *globals; + PyObject *res; - if (c == NULL) - return NULL; - - globals = PyEval_GetGlobals(); - if (globals == NULL) { - return NULL; - } - - f = PyFrame_New(tstate, c, globals, NULL); - if (f == NULL) - return NULL; - tstate->frame = f; -#ifdef FIX_TRACE - if (trace_frame(tstate, f, PyTrace_CALL, Py_None) < 0) { - return NULL; - } -#endif res = PyEval_CallObject(func, args); if (res == NULL) { - if (tstate->curexc_traceback == NULL) - PyTraceBack_Here(f); + _PyTraceback_Add(funcname, __FILE__, lineno); XML_StopParser(self->itself, XML_FALSE); -#ifdef FIX_TRACE - if (trace_frame_exc(tstate, f) < 0) { - return NULL; - } } - else { - if (trace_frame(tstate, f, PyTrace_RETURN, res) < 0) { - Py_CLEAR(res); - } - } -#else - } -#endif - tstate->frame = f->f_back; - Py_DECREF(f); return res; } @@ -376,7 +270,7 @@ call_character_handler(xmlparseobject *self, const XML_Char *buffer, int len) PyTuple_SET_ITEM(args, 0, temp); /* temp is now a borrowed reference; consider it unused. */ self->in_callback = 1; - temp = call_with_frame(getcode(CharacterData, "CharacterData", __LINE__), + temp = call_with_frame("CharacterData", __LINE__, self->handlers[CharacterData], args, self); /* temp is an owned reference again, or NULL */ self->in_callback = 0; @@ -508,7 +402,7 @@ my_StartElementHandler(void *userData, } /* Container is now a borrowed reference; ignore it. */ self->in_callback = 1; - rv = call_with_frame(getcode(StartElement, "StartElement", __LINE__), + rv = call_with_frame("StartElement", __LINE__, self->handlers[StartElement], args, self); self->in_callback = 0; Py_DECREF(args); @@ -537,7 +431,7 @@ my_##NAME##Handler PARAMS {\ args = Py_BuildValue PARAM_FORMAT ;\ if (!args) { flag_error(self); return RETURN;} \ self->in_callback = 1; \ - rv = call_with_frame(getcode(NAME,#NAME,__LINE__), \ + rv = call_with_frame(#NAME,__LINE__, \ self->handlers[NAME], args, self); \ self->in_callback = 0; \ Py_DECREF(args); \ @@ -669,7 +563,7 @@ my_ElementDeclHandler(void *userData, goto finally; } self->in_callback = 1; - rv = call_with_frame(getcode(ElementDecl, "ElementDecl", __LINE__), + rv = call_with_frame("ElementDecl", __LINE__, self->handlers[ElementDecl], args, self); self->in_callback = 0; if (rv == NULL) { diff --git a/Python/traceback.c b/Python/traceback.c index 2ece192db90..bbe6f2eebbd 100644 --- a/Python/traceback.c +++ b/Python/traceback.c @@ -142,6 +142,39 @@ PyTraceBack_Here(PyFrameObject *frame) return 0; } +/* Insert a frame into the traceback for (funcname, filename, lineno). */ +void _PyTraceback_Add(char *funcname, char *filename, int lineno) +{ + PyObject *globals = NULL; + PyCodeObject *code = NULL; + PyFrameObject *frame = NULL; + PyObject *exception, *value, *tb; + + /* Save and clear the current exception. Python functions must not be + called with an exception set. Calling Python functions happens when + the codec of the filesystem encoding is implemented in pure Python. */ + PyErr_Fetch(&exception, &value, &tb); + + globals = PyDict_New(); + if (!globals) + goto done; + code = PyCode_NewEmpty(filename, funcname, lineno); + if (!code) + goto done; + frame = PyFrame_New(PyThreadState_Get(), code, globals, NULL); + if (!frame) + goto done; + frame->f_lineno = lineno; + + PyErr_Restore(exception, value, tb); + PyTraceBack_Here(frame); + +done: + Py_XDECREF(globals); + Py_XDECREF(code); + Py_XDECREF(frame); +} + static PyObject * _Py_FindSourceFile(PyObject *filename, char* namebuf, size_t namelen, PyObject *io) {