From b74a6f14b94d36fb72b1344663e81776bf450847 Mon Sep 17 00:00:00 2001 From: Steve Dower Date: Thu, 28 Nov 2019 08:46:23 -0800 Subject: bpo-38920: Add audit hooks for when sys.excepthook and sys.unraisablehook are invoked (GH-17392) Also fixes some potential segfaults in unraisable hook handling. --- Doc/library/sys.rst | 23 ++++++- Lib/test/audit-tests.py | 37 +++++++++++ Lib/test/test_audit.py | 56 ++++++++++++----- .../2019-11-26-09-16-47.bpo-38920.Vx__sT.rst | 2 + Python/errors.c | 72 ++++++++++++---------- Python/pythonrun.c | 8 +++ Python/sysmodule.c | 4 +- 7 files changed, 154 insertions(+), 48 deletions(-) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2019-11-26-09-16-47.bpo-38920.Vx__sT.rst diff --git a/Doc/library/sys.rst b/Doc/library/sys.rst index 8d0de479e9..1cf19b8ad6 100644 --- a/Doc/library/sys.rst +++ b/Doc/library/sys.rst @@ -36,13 +36,18 @@ always available. .. audit-event:: sys.addaudithook "" sys.addaudithook Raise an auditing event ``sys.addaudithook`` with no arguments. If any - existing hooks raise an exception derived from :class:`Exception`, the + existing hooks raise an exception derived from :class:`RuntimeError`, the new hook will not be added and the exception suppressed. As a result, callers cannot assume that their hook has been added unless they control all existing hooks. .. versionadded:: 3.8 + .. versionchanged:: 3.8.1 + + Exceptions derived from :class:`Exception` but not :class:`RuntimeError` + are no longer suppressed. + .. impl-detail:: When tracing is enabled (see :func:`settrace`), Python hooks are only @@ -308,6 +313,15 @@ always available. before the program exits. The handling of such top-level exceptions can be customized by assigning another three-argument function to ``sys.excepthook``. + .. audit-event:: sys.excepthook hook,type,value,traceback sys.excepthook + + Raise an auditing event ``sys.excepthook`` with arguments ``hook``, + ``type``, ``value``, ``traceback`` when an uncaught exception occurs. + If no hook has been set, ``hook`` may be ``None``. If any hook raises + an exception derived from :class:`RuntimeError` the call to the hook will + be suppressed. Otherwise, the audit hook exception will be reported as + unraisable and ``sys.excepthook`` will be called. + .. seealso:: The :func:`sys.unraisablehook` function handles unraisable exceptions @@ -1563,6 +1577,13 @@ always available. See also :func:`excepthook` which handles uncaught exceptions. + .. audit-event:: sys.unraisablehook hook,unraisable sys.unraisablehook + + Raise an auditing event ``sys.unraisablehook`` with arguments + ``hook``, ``unraisable`` when an exception that cannot be handled occurs. + The ``unraisable`` object is the same as what will be passed to the hook. + If no hook has been set, ``hook`` may be ``None``. + .. versionadded:: 3.8 .. data:: version diff --git a/Lib/test/audit-tests.py b/Lib/test/audit-tests.py index ddeff22030..ed08612c04 100644 --- a/Lib/test/audit-tests.py +++ b/Lib/test/audit-tests.py @@ -263,13 +263,50 @@ def test_cantrace(): def test_mmap(): import mmap + with TestHook() as hook: mmap.mmap(-1, 8) assertEqual(hook.seen[0][1][:2], (-1, 8)) +def test_excepthook(): + def excepthook(exc_type, exc_value, exc_tb): + if exc_type is not RuntimeError: + sys.__excepthook__(exc_type, exc_value, exc_tb) + + def hook(event, args): + if event == "sys.excepthook": + if not isinstance(args[2], args[1]): + raise TypeError(f"Expected isinstance({args[2]!r}, " f"{args[1]!r})") + if args[0] != excepthook: + raise ValueError(f"Expected {args[0]} == {excepthook}") + print(event, repr(args[2])) + + sys.addaudithook(hook) + sys.excepthook = excepthook + raise RuntimeError("fatal-error") + + +def test_unraisablehook(): + from _testcapi import write_unraisable_exc + + def unraisablehook(hookargs): + pass + + def hook(event, args): + if event == "sys.unraisablehook": + if args[0] != unraisablehook: + raise ValueError(f"Expected {args[0]} == {unraisablehook}") + print(event, repr(args[1].exc_value), args[1].err_msg) + + sys.addaudithook(hook) + sys.unraisablehook = unraisablehook + write_unraisable_exc(RuntimeError("nonfatal-error"), "for audit hook test", None) + + if __name__ == "__main__": from test.libregrtest.setup import suppress_msvcrt_asserts + suppress_msvcrt_asserts(False) test = sys.argv[1] diff --git a/Lib/test/test_audit.py b/Lib/test/test_audit.py index 41f9fae102..31a0855927 100644 --- a/Lib/test/test_audit.py +++ b/Lib/test/test_audit.py @@ -24,7 +24,23 @@ class AuditTest(unittest.TestCase): sys.stdout.writelines(p.stdout) sys.stderr.writelines(p.stderr) if p.returncode: - self.fail(''.join(p.stderr)) + self.fail("".join(p.stderr)) + + def run_python(self, *args): + events = [] + with subprocess.Popen( + [sys.executable, "-X utf8", AUDIT_TESTS_PY, *args], + encoding="utf-8", + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + ) as p: + p.wait() + sys.stderr.writelines(p.stderr) + return ( + p.returncode, + [line.strip().partition(" ") for line in p.stdout], + "".join(p.stderr), + ) def test_basic(self): self.do_test("test_basic") @@ -36,19 +52,11 @@ class AuditTest(unittest.TestCase): self.do_test("test_block_add_hook_baseexception") def test_finalize_hooks(self): - events = [] - with subprocess.Popen( - [sys.executable, "-X utf8", AUDIT_TESTS_PY, "test_finalize_hooks"], - encoding="utf-8", - stdout=subprocess.PIPE, - stderr=subprocess.PIPE, - ) as p: - p.wait() - for line in p.stdout: - events.append(line.strip().partition(" ")) - sys.stderr.writelines(p.stderr) - if p.returncode: - self.fail(''.join(p.stderr)) + returncode, events, stderr = self.run_python("test_finalize_hooks") + if stderr: + print(stderr, file=sys.stderr) + if returncode: + self.fail(stderr) firstId = events[0][2] self.assertSequenceEqual( @@ -76,6 +84,26 @@ class AuditTest(unittest.TestCase): def test_mmap(self): self.do_test("test_mmap") + def test_excepthook(self): + returncode, events, stderr = self.run_python("test_excepthook") + if not returncode: + self.fail(f"Expected fatal exception\n{stderr}") + + self.assertSequenceEqual( + [("sys.excepthook", " ", "RuntimeError('fatal-error')")], events + ) + + def test_unraisablehook(self): + returncode, events, stderr = self.run_python("test_unraisablehook") + if returncode: + self.fail(stderr) + + self.assertEqual(events[0][0], "sys.unraisablehook") + self.assertEqual( + events[0][2], + "RuntimeError('nonfatal-error') Exception ignored for audit hook test", + ) + if __name__ == "__main__": unittest.main() diff --git a/Misc/NEWS.d/next/Core and Builtins/2019-11-26-09-16-47.bpo-38920.Vx__sT.rst b/Misc/NEWS.d/next/Core and Builtins/2019-11-26-09-16-47.bpo-38920.Vx__sT.rst new file mode 100644 index 0000000000..2e9e443dd9 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2019-11-26-09-16-47.bpo-38920.Vx__sT.rst @@ -0,0 +1,2 @@ +Add audit hooks for when :func:`sys.excepthook` and +:func:`sys.unraisablehook` are invoked diff --git a/Python/errors.c b/Python/errors.c index 8a94afdd8c..197d9779b3 100644 --- a/Python/errors.c +++ b/Python/errors.c @@ -1367,44 +1367,54 @@ _PyErr_WriteUnraisableMsg(const char *err_msg_str, PyObject *obj) } } + PyObject *hook_args = make_unraisable_hook_args( + tstate, exc_type, exc_value, exc_tb, err_msg, obj); + if (hook_args == NULL) { + err_msg_str = ("Exception ignored on building " + "sys.unraisablehook arguments"); + goto error; + } + _Py_IDENTIFIER(unraisablehook); PyObject *hook = _PySys_GetObjectId(&PyId_unraisablehook); - if (hook != NULL && hook != Py_None) { - PyObject *hook_args; - - hook_args = make_unraisable_hook_args(tstate, exc_type, exc_value, - exc_tb, err_msg, obj); - if (hook_args != NULL) { - PyObject *args[1] = {hook_args}; - PyObject *res = _PyObject_FastCall(hook, args, 1); - Py_DECREF(hook_args); - if (res != NULL) { - Py_DECREF(res); - goto done; - } - - err_msg_str = "Exception ignored in sys.unraisablehook"; - } - else { - err_msg_str = ("Exception ignored on building " - "sys.unraisablehook arguments"); - } + if (hook == NULL) { + Py_DECREF(hook_args); + goto default_hook; + } - Py_XDECREF(err_msg); - err_msg = PyUnicode_FromString(err_msg_str); - if (err_msg == NULL) { - PyErr_Clear(); - } + if (PySys_Audit("sys.unraisablehook", "OO", hook, hook_args) < 0) { + Py_DECREF(hook_args); + err_msg_str = "Exception ignored in audit hook"; + obj = NULL; + goto error; + } - /* sys.unraisablehook failed: log its error using default hook */ - Py_XDECREF(exc_type); - Py_XDECREF(exc_value); - Py_XDECREF(exc_tb); - _PyErr_Fetch(tstate, &exc_type, &exc_value, &exc_tb); + if (hook == Py_None) { + Py_DECREF(hook_args); + goto default_hook; + } - obj = hook; + PyObject *args[1] = {hook_args}; + PyObject *res = _PyObject_FastCall(hook, args, 1); + Py_DECREF(hook_args); + if (res != NULL) { + Py_DECREF(res); + goto done; } + /* sys.unraisablehook failed: log its error using default hook */ + obj = hook; + err_msg_str = NULL; + +error: + /* err_msg_str and obj have been updated and we have a new exception */ + Py_XSETREF(err_msg, PyUnicode_FromString(err_msg_str ? + err_msg_str : "Exception ignored in sys.unraisablehook")); + Py_XDECREF(exc_type); + Py_XDECREF(exc_value); + Py_XDECREF(exc_tb); + _PyErr_Fetch(tstate, &exc_type, &exc_value, &exc_tb); + default_hook: /* Call the default unraisable hook (ignore failure) */ (void)write_unraisable_exc(tstate, exc_type, exc_value, exc_tb, diff --git a/Python/pythonrun.c b/Python/pythonrun.c index c9afa8f8ba..a7da143077 100644 --- a/Python/pythonrun.c +++ b/Python/pythonrun.c @@ -695,6 +695,14 @@ _PyErr_PrintEx(PyThreadState *tstate, int set_sys_last_vars) } } hook = _PySys_GetObjectId(&PyId_excepthook); + if (PySys_Audit("sys.excepthook", "OOOO", hook ? hook : Py_None, + exception, v, tb) < 0) { + if (PyErr_ExceptionMatches(PyExc_RuntimeError)) { + PyErr_Clear(); + goto done; + } + _PyErr_WriteUnraisableMsg("in audit hook", NULL); + } if (hook) { PyObject* stack[3]; PyObject *result; diff --git a/Python/sysmodule.c b/Python/sysmodule.c index 5b0fb813b4..1255665d02 100644 --- a/Python/sysmodule.c +++ b/Python/sysmodule.c @@ -293,8 +293,8 @@ PySys_AddAuditHook(Py_AuditHookFunction hook, void *userData) /* Cannot invoke hooks until we are initialized */ if (Py_IsInitialized()) { if (PySys_Audit("sys.addaudithook", NULL) < 0) { - if (PyErr_ExceptionMatches(PyExc_Exception)) { - /* We do not report errors derived from Exception */ + if (PyErr_ExceptionMatches(PyExc_RuntimeError)) { + /* We do not report errors derived from RuntimeError */ PyErr_Clear(); return 0; } -- cgit v1.2.1