From bea33f5e1db6e4a554919a82894f44568576e979 Mon Sep 17 00:00:00 2001 From: Steve Dower Date: Thu, 28 Nov 2019 08:46:11 -0800 Subject: [PATCH] bpo-38920: Add audit hooks for when sys.excepthook and sys.unraisable hooks 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 | 76 +++++++++++-------- Python/pythonrun.c | 8 ++ Python/sysmodule.c | 4 +- 7 files changed, 156 insertions(+), 50 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 2f33445a6f9..a824fb95e8e 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 @@ -1540,6 +1554,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 ddeff22030a..ed08612c041 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 41f9fae1022..31a08559273 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 00000000000..2e9e443dd99 --- /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 1783084c336..d65707e7f97 100644 --- a/Python/errors.c +++ b/Python/errors.c @@ -1391,43 +1391,53 @@ _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 *res = _PyObject_CallOneArg(hook, hook_args); - 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"); - } - - Py_XDECREF(err_msg); - err_msg = PyUnicode_FromString(err_msg_str); - if (err_msg == NULL) { - PyErr_Clear(); - } - - /* 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); - - obj = hook; + if (hook == NULL) { + Py_DECREF(hook_args); + goto default_hook; } + 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; + } + + if (hook == Py_None) { + Py_DECREF(hook_args); + goto default_hook; + } + + PyObject *res = _PyObject_CallOneArg(hook, hook_args); + 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 77a95ceb94b..b68a0b5572a 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 30c7e987461..78b9d22821f 100644 --- a/Python/sysmodule.c +++ b/Python/sysmodule.c @@ -323,8 +323,8 @@ PySys_AddAuditHook(Py_AuditHookFunction hook, void *userData) /* Cannot invoke hooks until we are initialized */ if (runtime->initialized) { if (PySys_Audit("sys.addaudithook", NULL) < 0) { - if (_PyErr_ExceptionMatches(tstate, PyExc_Exception)) { - /* We do not report errors derived from Exception */ + if (_PyErr_ExceptionMatches(tstate, PyExc_RuntimeError)) { + /* We do not report errors derived from RuntimeError */ _PyErr_Clear(tstate); return 0; }