Issue python#22462: Fix pyexpat's creation of a dummy frame to make it appear in exception tracebacks.

pitrou · pitrou · commit 94262ebc9c12 · 2014-10-08T20:02:40.000+02:00
Initial patch by Mark Shannon.
diff --git 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
@@ -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
@@ -166,6 +166,9 @@ Core and Builtins
 Library
 -------
 
+- Issue #22462: Fix pyexpat's creation of a dummy frame to make it
+  appear in exception tracebacks.
+
 - Issue #21965: Add support for in-memory SSL to the ssl module.  Patch
   by Geert Jansen.
 
diff --git 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
@@ -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
@@ -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
@@ -15,8 +15,6 @@ module pyexpat
 
 #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};
 
@@ -217,121 +215,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;
 }
 
@@ -383,7 +277,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;
@@ -515,7 +409,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);
@@ -544,7 +438,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); \
@@ -676,7 +570,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
@@ -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)
 {

Original file line number	Diff line number	Diff line change
`@@ -919,7 +919,7 @@ static PyObject GetResult(PyObject restype, void result, PyObject checker)`
`919`	`919`
`920`	`920`	`v = PyObject_CallFunctionObjArgs(checker, retval, NULL);`
`921`	`921`	`if (v == NULL)`
`922`		`- _ctypes_add_traceback("GetResult", "_ctypes/callproc.c", __LINE__-2);`
	`922`	`+ _PyTraceback_Add("GetResult", "_ctypes/callproc.c", __LINE__-2);`
`923`	`923`	`Py_DECREF(retval);`
`924`	`924`	`return v;`
`925`	`925`	`}`