From 0ca0240c0b1ac854fefdf51c557920edc2bcdfb4 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Tue, 5 Sep 2023 23:48:08 +0200 Subject: [PATCH] gh-85283: Add PySys_AuditTuple() function sys.audit() now has assertions to check that the event argument is not NULL and that the format argument does not use the "N" format. Add tests on PySys_AuditTuple(). --- Doc/c-api/sys.rst | 21 +++++-- Doc/whatsnew/3.13.rst | 4 ++ Include/cpython/sysmodule.h | 6 +- Lib/test/test_embed.py | 3 + ...3-09-06-00-14-49.gh-issue-85283.GKY0Cc.rst | 3 + Programs/_testembed.c | 59 ++++++++++++++++++- Python/sysmodule.c | 22 ++++++- 7 files changed, 109 insertions(+), 9 deletions(-) create mode 100644 Misc/NEWS.d/next/C API/2023-09-06-00-14-49.gh-issue-85283.GKY0Cc.rst diff --git a/Doc/c-api/sys.rst b/Doc/c-api/sys.rst index aed625c5f6cdae..e3c54b075114ff 100644 --- a/Doc/c-api/sys.rst +++ b/Doc/c-api/sys.rst @@ -291,19 +291,24 @@ 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 option must not be used. It 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. 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 +317,14 @@ 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 pass arguments as a Python object. + *args* must be a :class:`tuple`. To pass no arguments, *args* can be *NULL*. + + .. 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 index 7a62963203e164..d5987ae31ce68d 100644 --- a/Doc/whatsnew/3.13.rst +++ b/Doc/whatsnew/3.13.rst @@ -1013,6 +1013,10 @@ New Features ``_PyThreadState_UncheckedGet()``. (Contributed by Victor Stinner in :gh:`108867`.) +* 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 index e028fb7ecd5230..36c4f89432067b 100644 --- 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 index dc476ef83c2519..46c9b03c4178e9 100644 --- a/Lib/test/test_embed.py +++ b/Lib/test/test_embed.py @@ -1716,6 +1716,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 new file mode 100644 index 00000000000000..811551cba73b3a --- /dev/null +++ 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 index bc991020d0fa77..e66c51818227c4 100644 --- a/Programs/_testembed.c +++ b/Programs/_testembed.c @@ -1278,11 +1278,16 @@ static int _test_audit(Py_ssize_t setValue) printf("Set event failed"); return 4; } + if (PyErr_Occurred()) { + printf("Exception raised"); + return 5; + } if (sawSet != 42) { printf("Failed to see *userData change\n"); - return 5; + return 6; } + return 0; } @@ -1296,6 +1301,57 @@ static int test_audit(void) return result; } +static int test_audit_tuple(void) +{ +#define ASSERT(TEST, EXITCODE) \ + if (!(TEST)) { \ + printf("ERROR test failed at %s:%i\n", __FILE__, __LINE__); \ + return (EXITCODE); \ + } + + Py_ssize_t sawSet = 0; + + // we need at least one hook, otherwise code checking for + // PySys_AuditTuple() is skipped. + PySys_AddAuditHook(_audit_hook, &sawSet); + _testembed_Py_InitializeFromConfig(); + + ASSERT(!PyErr_Occurred(), 0); + + // pass Python tuple object + PyObject *tuple = Py_BuildValue("(i)", 444); + if (tuple == NULL) { + goto error; + } + ASSERT(PySys_AuditTuple("_testembed.set", tuple) == 0, 10); + ASSERT(!PyErr_Occurred(), 11); + ASSERT(sawSet == 444, 12); + Py_DECREF(tuple); + + // pass Python int object + PyObject *int_arg = PyLong_FromLong(555); + if (int_arg == NULL) { + goto error; + } + ASSERT(PySys_AuditTuple("_testembed.set", int_arg) == -1, 20); + ASSERT(PyErr_ExceptionMatches(PyExc_TypeError), 21); + PyErr_Clear(); + Py_DECREF(int_arg); + + // NULL is accepted and means "no arguments" + ASSERT(PySys_AuditTuple("_testembed.test_audit_tuple", NULL) == 0, 30); + ASSERT(!PyErr_Occurred(), 31); + + Py_Finalize(); + return 0; + +error: + PyErr_Print(); + return 1; + +#undef ASSERT +} + static volatile int _audit_subinterpreter_interpreter_count = 0; static int _audit_subinterpreter_hook(const char *event, PyObject *args, void *userdata) @@ -2140,6 +2196,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 index b00301765e1890..a7ce07d28ae7df 100644 --- a/Python/sysmodule.c +++ b/Python/sysmodule.c @@ -191,9 +191,7 @@ static int sys_audit_tstate(PyThreadState *ts, const char *event, const char *argFormat, va_list vargs) { - /* 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(event != NULL); assert(!argFormat || !strchr(argFormat, 'N')); if (!ts) { @@ -338,6 +336,21 @@ PySys_Audit(const char *event, const char *argFormat, ...) return res; } +int +PySys_AuditTuple(const char *event, PyObject *args) +{ + if (args == NULL) { + return PySys_Audit(event, NULL); + } + + if (!PyTuple_Check(args)) { + PyErr_Format(PyExc_TypeError, "args must be tuple, got %s", + Py_TYPE(args)->tp_name); + return -1; + } + return PySys_Audit(event, "O", args); +} + /* 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. @@ -509,6 +522,9 @@ sys_audit(PyObject *self, PyObject *const *args, Py_ssize_t argc) return NULL; } + assert(args[0] != NULL); + assert(PyUnicode_Check(args[0])); + if (!should_audit(tstate->interp)) { Py_RETURN_NONE; }