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.

7 files changed, 154 insertions, 48 deletions

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;
             }