From 9e3c583954f75a6396d1935e000df85d89b78a3d Mon Sep 17 00:00:00 2001 From: "Miss Islington (bot)" <31488909+miss-islington@users.noreply.github.com> Date: Wed, 27 May 2020 16:00:01 -0700 Subject: [PATCH] bpo-40795: ctypes calls unraisablehook with an exception (GH-20452) If ctypes fails to convert the result of a callback or if a ctypes callback function raises an exception, sys.unraisablehook is now called with an exception set. Previously, the error was logged into stderr by PyErr_Print(). (cherry picked from commit 10228bad0452d94e66c964b625a0b61befa08e59) Co-authored-by: Victor Stinner --- Lib/ctypes/test/test_callbacks.py | 18 ++++++- Lib/ctypes/test/test_random_things.py | 51 ++++++++++--------- Lib/ctypes/test/test_unaligned_structures.py | 2 - .../2020-05-27-17-00-18.bpo-40795.eZSnHA.rst | 4 ++ Modules/_ctypes/callbacks.c | 43 +++++++++++----- Modules/_ctypes/callproc.c | 2 + 6 files changed, 79 insertions(+), 41 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2020-05-27-17-00-18.bpo-40795.eZSnHA.rst diff --git a/Lib/ctypes/test/test_callbacks.py b/Lib/ctypes/test/test_callbacks.py index 937a06d981b..d8e9c5a760e 100644 --- a/Lib/ctypes/test/test_callbacks.py +++ b/Lib/ctypes/test/test_callbacks.py @@ -1,5 +1,7 @@ import functools import unittest +from test import support + from ctypes import * from ctypes.test import need_symbol import _ctypes_test @@ -301,8 +303,22 @@ class SampleCallbacksTestCase(unittest.TestCase): with self.assertRaises(ArgumentError): cb(*args2) + def test_convert_result_error(self): + def func(): + return ("tuple",) + + proto = CFUNCTYPE(c_int) + ctypes_func = proto(func) + with support.catch_unraisable_exception() as cm: + # don't test the result since it is an uninitialized value + result = ctypes_func() + + self.assertIsInstance(cm.unraisable.exc_value, TypeError) + self.assertEqual(cm.unraisable.err_msg, + "Exception ignored on converting result " + "of ctypes callback function") + self.assertIs(cm.unraisable.object, func) -################################################################ if __name__ == '__main__': unittest.main() diff --git a/Lib/ctypes/test/test_random_things.py b/Lib/ctypes/test/test_random_things.py index ee5b2128ea0..2988e275cf4 100644 --- a/Lib/ctypes/test/test_random_things.py +++ b/Lib/ctypes/test/test_random_things.py @@ -1,5 +1,9 @@ from ctypes import * -import unittest, sys +import contextlib +from test import support +import unittest +import sys + def callback_func(arg): 42 / arg @@ -34,41 +38,40 @@ class CallbackTracbackTestCase(unittest.TestCase): # created, then a full traceback printed. When SystemExit is # raised in a callback function, the interpreter exits. - def capture_stderr(self, func, *args, **kw): - # helper - call function 'func', and return the captured stderr - import io - old_stderr = sys.stderr - logger = sys.stderr = io.StringIO() - try: - func(*args, **kw) - finally: - sys.stderr = old_stderr - return logger.getvalue() + @contextlib.contextmanager + def expect_unraisable(self, exc_type, exc_msg=None): + with support.catch_unraisable_exception() as cm: + yield + + self.assertIsInstance(cm.unraisable.exc_value, exc_type) + if exc_msg is not None: + self.assertEqual(str(cm.unraisable.exc_value), exc_msg) + self.assertEqual(cm.unraisable.err_msg, + "Exception ignored on calling ctypes " + "callback function") + self.assertIs(cm.unraisable.object, callback_func) def test_ValueError(self): cb = CFUNCTYPE(c_int, c_int)(callback_func) - out = self.capture_stderr(cb, 42) - self.assertEqual(out.splitlines()[-1], - "ValueError: 42") + with self.expect_unraisable(ValueError, '42'): + cb(42) def test_IntegerDivisionError(self): cb = CFUNCTYPE(c_int, c_int)(callback_func) - out = self.capture_stderr(cb, 0) - self.assertEqual(out.splitlines()[-1][:19], - "ZeroDivisionError: ") + with self.expect_unraisable(ZeroDivisionError): + cb(0) def test_FloatDivisionError(self): cb = CFUNCTYPE(c_int, c_double)(callback_func) - out = self.capture_stderr(cb, 0.0) - self.assertEqual(out.splitlines()[-1][:19], - "ZeroDivisionError: ") + with self.expect_unraisable(ZeroDivisionError): + cb(0.0) def test_TypeErrorDivisionError(self): cb = CFUNCTYPE(c_int, c_char_p)(callback_func) - out = self.capture_stderr(cb, b"spam") - self.assertEqual(out.splitlines()[-1], - "TypeError: " - "unsupported operand type(s) for /: 'int' and 'bytes'") + err_msg = "unsupported operand type(s) for /: 'int' and 'bytes'" + with self.expect_unraisable(TypeError, err_msg): + cb(b"spam") + if __name__ == '__main__': unittest.main() diff --git a/Lib/ctypes/test/test_unaligned_structures.py b/Lib/ctypes/test/test_unaligned_structures.py index bcacfc8184b..ee7fb45809b 100644 --- a/Lib/ctypes/test/test_unaligned_structures.py +++ b/Lib/ctypes/test/test_unaligned_structures.py @@ -27,7 +27,6 @@ for typ in [c_short, c_int, c_long, c_longlong, class TestStructures(unittest.TestCase): def test_native(self): for typ in structures: -## print typ.value self.assertEqual(typ.value.offset, 1) o = typ() o.value = 4 @@ -35,7 +34,6 @@ class TestStructures(unittest.TestCase): def test_swapped(self): for typ in byteswapped_structures: -## print >> sys.stderr, typ.value self.assertEqual(typ.value.offset, 1) o = typ() o.value = 4 diff --git a/Misc/NEWS.d/next/Library/2020-05-27-17-00-18.bpo-40795.eZSnHA.rst b/Misc/NEWS.d/next/Library/2020-05-27-17-00-18.bpo-40795.eZSnHA.rst new file mode 100644 index 00000000000..dd02fb05cab --- /dev/null +++ b/Misc/NEWS.d/next/Library/2020-05-27-17-00-18.bpo-40795.eZSnHA.rst @@ -0,0 +1,4 @@ +:mod:`ctypes` module: If ctypes fails to convert the result of a callback or +if a ctypes callback function raises an exception, sys.unraisablehook is now +called with an exception set. Previously, the error was logged into stderr +by :c:func:`PyErr_Print`. diff --git a/Modules/_ctypes/callbacks.c b/Modules/_ctypes/callbacks.c index 2b903c98e8e..29e8fac8c94 100644 --- a/Modules/_ctypes/callbacks.c +++ b/Modules/_ctypes/callbacks.c @@ -213,9 +213,6 @@ static void _CallPythonObject(void *mem, pArgs++; } -#define CHECK(what, x) \ -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); if (error_object == NULL) @@ -235,7 +232,10 @@ if (x == NULL) _PyTraceback_Add(what, "_ctypes/callbacks.c", __LINE__ - 1), PyEr } result = PyObject_CallObject(callable, arglist); - CHECK("'calling callback function'", result); + if (result == NULL) { + _PyErr_WriteUnraisableMsg("on calling ctypes callback function", + callable); + } #ifdef MS_WIN32 if (flags & FUNCFLAG_USE_LASTERROR) { @@ -251,16 +251,17 @@ if (x == NULL) _PyTraceback_Add(what, "_ctypes/callbacks.c", __LINE__ - 1), PyEr } Py_XDECREF(error_object); - if ((restype != &ffi_type_void) && result) { - PyObject *keep; + if (restype != &ffi_type_void && result) { assert(setfunc); + #ifdef WORDS_BIGENDIAN - /* See the corresponding code in callproc.c, around line 961 */ - if (restype->type != FFI_TYPE_FLOAT && restype->size < sizeof(ffi_arg)) + /* See the corresponding code in _ctypes_callproc(): + in callproc.c, around line 1219. */ + if (restype->type != FFI_TYPE_FLOAT && restype->size < sizeof(ffi_arg)) { mem = (char *)mem + sizeof(ffi_arg) - restype->size; + } #endif - keep = setfunc(mem, result, 0); - CHECK("'converting callback result'", keep); + /* keep is an object we have to keep alive so that the result stays valid. If there is no such object, the setfunc will have returned Py_None. @@ -270,18 +271,32 @@ if (x == NULL) _PyTraceback_Add(what, "_ctypes/callbacks.c", __LINE__ - 1), PyEr be the result. EXCEPT when restype is py_object - Python itself knows how to manage the refcount of these objects. */ - if (keep == NULL) /* Could not convert callback result. */ - PyErr_WriteUnraisable(callable); - else if (keep == Py_None) /* Nothing to keep */ + PyObject *keep = setfunc(mem, result, 0); + + if (keep == NULL) { + /* Could not convert callback result. */ + _PyErr_WriteUnraisableMsg("on converting result " + "of ctypes callback function", + callable); + } + else if (keep == Py_None) { + /* Nothing to keep */ Py_DECREF(keep); + } else if (setfunc != _ctypes_get_fielddesc("O")->setfunc) { if (-1 == PyErr_WarnEx(PyExc_RuntimeWarning, "memory leak in callback function.", 1)) - PyErr_WriteUnraisable(callable); + { + _PyErr_WriteUnraisableMsg("on converting result " + "of ctypes callback function", + callable); + } } } + Py_XDECREF(result); + Done: Py_XDECREF(arglist); PyGILState_Release(state); diff --git a/Modules/_ctypes/callproc.c b/Modules/_ctypes/callproc.c index 9bc28c26071..af6e1e8ce0b 100644 --- a/Modules/_ctypes/callproc.c +++ b/Modules/_ctypes/callproc.c @@ -1231,7 +1231,9 @@ PyObject *_ctypes_callproc(PPROC pProc, if (rtype->type != FFI_TYPE_FLOAT && rtype->type != FFI_TYPE_STRUCT && rtype->size < sizeof(ffi_arg)) + { resbuf = (char *)resbuf + sizeof(ffi_arg) - rtype->size; + } #endif #ifdef MS_WIN32