gh-85283: Add PySys_AuditTuple() function

vstinner · vstinner · commit 23867319fe84 · 2023-09-06T00:21:38.000+02:00
PySys_Audit() now raises an exception if the event argument is NULL
or if the "N" format is used in the *format* argument.

Add tests on PySys_AuditTuple() and on new PySys_Audit() error code
paths.
diff --git a/Doc/c-api/sys.rst b/Doc/c-api/sys.rst
@@ -291,19 +291,21 @@ accessible to C code.  They all work with the current interpreter thread's
    Raise an auditing event with any active hooks. Return zero for success
    and non-zero with an exception set on failure.
 
+   The *event* string argument must not be *NULL*.
+
    If any hooks have been added, *format* and other arguments will be used
    to construct a tuple to pass. Apart from ``N``, the same format characters
    as used in :c:func:`Py_BuildValue` are available. If the built value is not
-   a tuple, it will be added into a single-element tuple. (The ``N`` format
-   option consumes a reference, but since there is no way to know whether
-   arguments to this function will be consumed, using it may cause reference
-   leaks.)
+   a tuple, it will be added into a single-element tuple. The ``N`` format
+   must not be used in *format*.
 
    Note that ``#`` format characters should always be treated as
    :c:type:`Py_ssize_t`, regardless of whether ``PY_SSIZE_T_CLEAN`` was defined.
 
    :func:`sys.audit` performs the same function from Python code.
 
+   See also :c:func:`PySys_AuditTuple`.
+
    .. versionadded:: 3.8
 
    .. versionchanged:: 3.8.2
@@ -312,6 +314,15 @@ accessible to C code.  They all work with the current interpreter thread's
       unavoidable deprecation warning was raised.
 
 
+.. c:function:: int PySys_AuditTuple(const char *event, PyObject *args)
+
+   Similar to :c:func:`PySys_Audit`, but event pass arguments as a Python
+   :class:`tuple` object. *args* can be *NULL* to pass no arguments: it is
+   treated the same as passing an empty tuple.
+
+   .. versionadded:: 3.13
+
+
 .. c:function:: int PySys_AddAuditHook(Py_AuditHookFunction hook, void *userData)
 
    Append the callable *hook* to the list of active auditing hooks.
diff --git a/Doc/whatsnew/3.13.rst b/Doc/whatsnew/3.13.rst
@@ -924,6 +924,10 @@ New Features
   references) now supports the :ref:`Limited API <limited-c-api>`.
   (Contributed by Victor Stinner in :gh:`108634`.)
 
+* Add :c:func:`PySys_AuditTuple` function: similar to :c:func:`PySys_Audit`,
+  but pass event arguments as a Python :class:`tuple` object.
+  (Contributed by Victor Stinner in :gh:`85283`.)
+
 Porting to Python 3.13
 ----------------------
 
diff --git a/Include/cpython/sysmodule.h b/Include/cpython/sysmodule.h
@@ -6,6 +6,10 @@ typedef int(*Py_AuditHookFunction)(const char *, PyObject *, void *);
 
 PyAPI_FUNC(int) PySys_Audit(
     const char *event,
-    const char *argFormat,
+    const char *format,
     ...);
 PyAPI_FUNC(int) PySys_AddAuditHook(Py_AuditHookFunction, void*);
+
+PyAPI_FUNC(int) PySys_AuditTuple(
+    const char *event,
+    PyObject *args);
diff --git a/Lib/test/test_embed.py b/Lib/test/test_embed.py
@@ -1703,6 +1703,9 @@ def test_open_code_hook(self):
     def test_audit(self):
         self.run_embedded_interpreter("test_audit")
 
+    def test_audit_tuple(self):
+        self.run_embedded_interpreter("test_audit_tuple")
+
     def test_audit_subinterpreter(self):
         self.run_embedded_interpreter("test_audit_subinterpreter")
 
diff --git a/Misc/NEWS.d/next/C API/2023-09-06-00-14-49.gh-issue-85283.GKY0Cc.rst b/Misc/NEWS.d/next/C API/2023-09-06-00-14-49.gh-issue-85283.GKY0Cc.rst
@@ -0,0 +1,3 @@
+Add :c:func:`PySys_AuditTuple` function: similar to :c:func:`PySys_Audit`,
+but pass event arguments as a Python :class:`tuple` object.  Patch by Victor
+Stinner.
diff --git a/Programs/_testembed.c b/Programs/_testembed.c
@@ -1276,7 +1276,30 @@ static int _test_audit(Py_ssize_t setValue)
         printf("Failed to see *userData change\n");
         return 5;
     }
+
+    // event argument must not be NULL (format can be NULL)
+    assert(!PyErr_Occurred());
+    assert(PySys_Audit(NULL, NULL) == -1);
+    assert(PyErr_ExceptionMatches(PyExc_ValueError));
+    PyErr_Clear();
+
+    // 'N' must not be used in the format
+    PyObject *arg = PyUnicode_FromString("arg");
+    if (arg == NULL) {
+        goto error;
+    }
+    Py_ssize_t refcnt = Py_REFCNT(arg);
+    assert(PySys_Audit("_testembed.raise", "N", arg) == -1);
+    assert(PyErr_ExceptionMatches(PyExc_ValueError));
+    PyErr_Clear();
+    assert(Py_REFCNT(arg) == refcnt);
+    Py_DECREF(arg);
+
     return 0;
+
+error:
+    PyErr_Print();
+    return 1;
 }
 
 static int test_audit(void)
@@ -1289,6 +1312,43 @@ static int test_audit(void)
     return result;
 }
 
+static int test_audit_tuple(void)
+{
+    _testembed_Py_InitializeFromConfig();
+
+    assert(!PyErr_Occurred());
+
+    // pass tuple
+    PyObject *tuple = Py_BuildValue("ii", 3, 5);
+    if (tuple == NULL) {
+        goto error;
+    }
+    assert(PySys_AuditTuple("_testembed.test_audit_tuple", tuple) == 0);
+    assert(!PyErr_Occurred());
+    Py_DECREF(tuple);
+
+    // NULL is accepted and means "no arguments"
+    assert(PySys_AuditTuple("_testembed.test_audit_tuple", NULL) == 0);
+    assert(!PyErr_Occurred());
+
+    // wrong argument type
+    PyObject *not_tuple = PyLong_FromLong(123);
+    if (not_tuple == NULL) {
+        goto error;
+    }
+    assert(PySys_AuditTuple("_testembed.test_audit_tuple", not_tuple) == -1);
+    assert(PyErr_ExceptionMatches(PyExc_TypeError));
+    PyErr_Clear();
+    Py_DECREF(not_tuple);
+
+    Py_Finalize();
+    return 0;
+
+error:
+    PyErr_Print();
+    return 1;
+}
+
 static volatile int _audit_subinterpreter_interpreter_count = 0;
 
 static int _audit_subinterpreter_hook(const char *event, PyObject *args, void *userdata)
@@ -2133,6 +2193,7 @@ static struct TestCase TestCases[] = {
     // Audit
     {"test_open_code_hook", test_open_code_hook},
     {"test_audit", test_audit},
+    {"test_audit_tuple", test_audit_tuple},
     {"test_audit_subinterpreter", test_audit_subinterpreter},
     {"test_audit_run_command", test_audit_run_command},
     {"test_audit_run_file", test_audit_run_file},
diff --git a/Python/sysmodule.c b/Python/sysmodule.c
@@ -183,13 +183,9 @@ should_audit(PyInterpreterState *interp)
 
 
 static int
-sys_audit_tstate(PyThreadState *ts, const char *event,
-                 const char *argFormat, va_list vargs)
+sys_audit_tuple(PyThreadState *ts, const char *event, PyObject *eventArgs)
 {
-    /* N format is inappropriate, because you do not know
-       whether the reference is consumed by the call.
-       Assert rather than exception for perf reasons */
-    assert(!argFormat || !strchr(argFormat, 'N'));
+    assert(PyTuple_Check(eventArgs));
 
     if (!ts) {
         /* Audit hooks cannot be called with a NULL thread state */
@@ -200,14 +196,19 @@ sys_audit_tstate(PyThreadState *ts, const char *event,
        the current Python thread state. */
     assert(ts == _PyThreadState_GET());
 
+    if (event == NULL) {
+        _PyErr_SetString(ts, PyExc_ValueError,
+                         "event argument must not be NULL");
+        return -1;
+    }
+
     /* Early exit when no hooks are registered */
     PyInterpreterState *is = ts->interp;
     if (!should_audit(is)) {
         return 0;
     }
 
     PyObject *eventName = NULL;
-    PyObject *eventArgs = NULL;
     PyObject *hooks = NULL;
     PyObject *hook = NULL;
     int res = -1;
@@ -217,21 +218,6 @@ sys_audit_tstate(PyThreadState *ts, const char *event,
 
     PyObject *exc = _PyErr_GetRaisedException(ts);
 
-    /* Initialize event args now */
-    if (argFormat && argFormat[0]) {
-        eventArgs = Py_VaBuildValue(argFormat, vargs);
-        if (eventArgs && !PyTuple_Check(eventArgs)) {
-            PyObject *argTuple = PyTuple_Pack(1, eventArgs);
-            Py_SETREF(eventArgs, argTuple);
-        }
-    }
-    else {
-        eventArgs = PyTuple_New(0);
-    }
-    if (!eventArgs) {
-        goto exit;
-    }
-
     /* Call global hooks
      *
      * We don't worry about any races on hooks getting added,
@@ -298,7 +284,6 @@ sys_audit_tstate(PyThreadState *ts, const char *event,
     Py_XDECREF(hook);
     Py_XDECREF(hooks);
     Py_XDECREF(eventName);
-    Py_XDECREF(eventArgs);
 
     if (!res) {
         _PyErr_SetRaisedException(ts, exc);
@@ -311,28 +296,85 @@ sys_audit_tstate(PyThreadState *ts, const char *event,
     return res;
 }
 
+static int
+sys_audit_vargs(PyThreadState *tstate, const char *event,
+                const char *format, va_list vargs)
+{
+    if (format && strchr(format, 'N')) {
+        _PyErr_SetString(tstate, PyExc_ValueError,
+                         "format argument must not use the 'N' format");
+        return -1;
+    }
+
+    PyObject *args;
+    if (format && format[0]) {
+        args = Py_VaBuildValue(format, vargs);
+        if (args && !PyTuple_Check(args)) {
+            PyObject *argTuple = PyTuple_Pack(1, args);
+            Py_SETREF(args, argTuple);
+        }
+    }
+    else {
+        args = PyTuple_New(0);
+    }
+    if (!args) {
+        return -1;
+    }
+
+    int res = sys_audit_tuple(tstate, event, args);
+    Py_DECREF(args);
+    return res;
+}
+
 int
 _PySys_Audit(PyThreadState *tstate, const char *event,
-             const char *argFormat, ...)
+             const char *format, ...)
 {
     va_list vargs;
-    va_start(vargs, argFormat);
-    int res = sys_audit_tstate(tstate, event, argFormat, vargs);
+    va_start(vargs, format);
+    int res = sys_audit_vargs(tstate, event, format, vargs);
     va_end(vargs);
     return res;
 }
 
 int
-PySys_Audit(const char *event, const char *argFormat, ...)
+PySys_Audit(const char *event, const char *format, ...)
 {
     PyThreadState *tstate = _PyThreadState_GET();
     va_list vargs;
-    va_start(vargs, argFormat);
-    int res = sys_audit_tstate(tstate, event, argFormat, vargs);
+    va_start(vargs, format);
+    int res = sys_audit_vargs(tstate, event, format, vargs);
     va_end(vargs);
     return res;
 }
 
+int
+PySys_AuditTuple(const char *event, PyObject *args)
+{
+    PyThreadState *tstate = _PyThreadState_GET();
+    int delete_args = 0;
+
+    if (args == NULL) {
+        delete_args = 1;
+        args = PyTuple_New(0);
+        if (args == NULL) {
+            return -1;
+        }
+    }
+    else if (!PyTuple_Check(args)) {
+        _PyErr_Format(tstate, PyExc_TypeError,
+                      "expected tuple, got %s", Py_TYPE(args)->tp_name);
+        return -1;
+    }
+
+    int res = sys_audit_tuple(tstate, event, args);
+
+    if (delete_args) {
+        Py_DECREF(args);
+    }
+    return res;
+}
+
 /* We expose this function primarily for our own cleanup during
  * finalization. In general, it should not need to be called,
  * and as such the function is not exported.

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	+Add :c:func:`PySys_AuditTuple` function: similar to :c:func:`PySys_Audit`,
	`2`	+but pass event arguments as a Python :class:`tuple` object. Patch by Victor
	`3`	`+Stinner.`