From 309d7cc5df4e2bf3086c49eb2b1b56b929554500 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Fri, 13 Mar 2020 16:39:12 +0100 Subject: [PATCH] bpo-35370: Add _PyEval_SetTrace() function (GH-18975) * sys.settrace(), sys.setprofile() and _lsprof.Profiler.enable() now properly report PySys_Audit() error if "sys.setprofile" or "sys.settrace" audit event is denied. * Add _PyEval_SetProfile() and _PyEval_SetTrace() function: similar to PyEval_SetProfile() and PyEval_SetTrace() but take a tstate parameter and return -1 on error. * Add _PyObject_FastCallTstate() function. --- Doc/c-api/init.rst | 5 + Include/cpython/abstract.h | 8 +- Include/cpython/ceval.h | 2 + .../2020-03-13-14-41-28.bpo-35370.df50Q7.rst | 3 + Modules/_lsprof.c | 35 +++++-- Python/ceval.c | 96 +++++++++++++------ Python/sysmodule.c | 72 +++++++++----- 7 files changed, 156 insertions(+), 65 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2020-03-13-14-41-28.bpo-35370.df50Q7.rst diff --git a/Doc/c-api/init.rst b/Doc/c-api/init.rst index badea5a0136..747278c60b0 100644 --- a/Doc/c-api/init.rst +++ b/Doc/c-api/init.rst @@ -1498,6 +1498,8 @@ Python-level trace functions in previous versions. profile function is called for all monitored events except :const:`PyTrace_LINE` :const:`PyTrace_OPCODE` and :const:`PyTrace_EXCEPTION`. + The caller must hold the :term:`GIL`. + .. c:function:: void PyEval_SetTrace(Py_tracefunc func, PyObject *obj) @@ -1508,6 +1510,9 @@ Python-level trace functions in previous versions. will not receive :const:`PyTrace_C_CALL`, :const:`PyTrace_C_EXCEPTION` or :const:`PyTrace_C_RETURN` as a value for the *what* parameter. + The caller must hold the :term:`GIL`. + + .. _advanced-debugging: Advanced Debugger Support diff --git a/Include/cpython/abstract.h b/Include/cpython/abstract.h index c0b0182fd5d..48cf25c3afb 100644 --- a/Include/cpython/abstract.h +++ b/Include/cpython/abstract.h @@ -142,12 +142,18 @@ PyAPI_FUNC(PyObject *) PyObject_VectorcallDict( "tuple" and keyword arguments "dict". "dict" may also be NULL */ PyAPI_FUNC(PyObject *) PyVectorcall_Call(PyObject *callable, PyObject *tuple, PyObject *dict); +static inline PyObject * +_PyObject_FastCallTstate(PyThreadState *tstate, PyObject *func, PyObject *const *args, Py_ssize_t nargs) +{ + return _PyObject_VectorcallTstate(tstate, func, args, (size_t)nargs, NULL); +} + /* Same as PyObject_Vectorcall except without keyword arguments */ static inline PyObject * _PyObject_FastCall(PyObject *func, PyObject *const *args, Py_ssize_t nargs) { PyThreadState *tstate = PyThreadState_GET(); - return _PyObject_VectorcallTstate(tstate, func, args, (size_t)nargs, NULL); + return _PyObject_FastCallTstate(tstate, func, args, nargs); } /* Call a callable without any arguments diff --git a/Include/cpython/ceval.h b/Include/cpython/ceval.h index 00c749949c4..74599370c10 100644 --- a/Include/cpython/ceval.h +++ b/Include/cpython/ceval.h @@ -7,7 +7,9 @@ extern "C" { #endif PyAPI_FUNC(void) PyEval_SetProfile(Py_tracefunc, PyObject *); +PyAPI_DATA(int) _PyEval_SetProfile(PyThreadState *tstate, Py_tracefunc func, PyObject *arg); PyAPI_FUNC(void) PyEval_SetTrace(Py_tracefunc, PyObject *); +PyAPI_FUNC(int) _PyEval_SetTrace(PyThreadState *tstate, Py_tracefunc func, PyObject *arg); PyAPI_FUNC(int) _PyEval_GetCoroutineOriginTrackingDepth(void); PyAPI_FUNC(void) _PyEval_SetAsyncGenFirstiter(PyObject *); PyAPI_FUNC(PyObject *) _PyEval_GetAsyncGenFirstiter(void); diff --git a/Misc/NEWS.d/next/Library/2020-03-13-14-41-28.bpo-35370.df50Q7.rst b/Misc/NEWS.d/next/Library/2020-03-13-14-41-28.bpo-35370.df50Q7.rst new file mode 100644 index 00000000000..a65fe6396d6 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2020-03-13-14-41-28.bpo-35370.df50Q7.rst @@ -0,0 +1,3 @@ +sys.settrace(), sys.setprofile() and _lsprof.Profiler.enable() now properly +report :c:func:`PySys_Audit` error if "sys.setprofile" or "sys.settrace" +audit event is denied. diff --git a/Modules/_lsprof.c b/Modules/_lsprof.c index 2718c61944e..7115fee1f2e 100644 --- a/Modules/_lsprof.c +++ b/Modules/_lsprof.c @@ -578,8 +578,9 @@ static PyObject* profiler_getstats(ProfilerObject *pObj, PyObject* noarg) { statscollector_t collect; - if (pending_exception(pObj)) + if (pending_exception(pObj)) { return NULL; + } if (!pObj->externalTimer || pObj->externalTimerUnit == 0.0) { _PyTime_t onesec = _PyTime_FromSeconds(1); collect.factor = (double)1 / onesec; @@ -639,9 +640,15 @@ profiler_enable(ProfilerObject *self, PyObject *args, PyObject *kwds) if (!PyArg_ParseTupleAndKeywords(args, kwds, "|ii:enable", kwlist, &subcalls, &builtins)) return NULL; - if (setSubcalls(self, subcalls) < 0 || setBuiltins(self, builtins) < 0) + if (setSubcalls(self, subcalls) < 0 || setBuiltins(self, builtins) < 0) { return NULL; - PyEval_SetProfile(profiler_callback, (PyObject*)self); + } + + PyThreadState *tstate = PyThreadState_GET(); + if (_PyEval_SetProfile(tstate, profiler_callback, (PyObject*)self) < 0) { + return NULL; + } + self->flags |= POF_ENABLED; Py_RETURN_NONE; } @@ -671,11 +678,16 @@ Stop collecting profiling information.\n\ static PyObject* profiler_disable(ProfilerObject *self, PyObject* noarg) { - self->flags &= ~POF_ENABLED; - PyEval_SetProfile(NULL, NULL); - flush_unmatched(self); - if (pending_exception(self)) + PyThreadState *tstate = PyThreadState_GET(); + if (_PyEval_SetProfile(tstate, NULL, NULL) < 0) { return NULL; + } + self->flags &= ~POF_ENABLED; + + flush_unmatched(self); + if (pending_exception(self)) { + return NULL; + } Py_RETURN_NONE; } @@ -695,8 +707,13 @@ profiler_clear(ProfilerObject *pObj, PyObject* noarg) static void profiler_dealloc(ProfilerObject *op) { - if (op->flags & POF_ENABLED) - PyEval_SetProfile(NULL, NULL); + if (op->flags & POF_ENABLED) { + PyThreadState *tstate = PyThreadState_GET(); + if (_PyEval_SetProfile(tstate, NULL, NULL) < 0) { + PyErr_WriteUnraisable((PyObject *)op); + } + } + flush_unmatched(op); clearEntries(op); Py_XDECREF(op->externalTimer); diff --git a/Python/ceval.c b/Python/ceval.c index ccd1c06a39c..fc4f718de28 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -4586,51 +4586,85 @@ maybe_call_line_trace(Py_tracefunc func, PyObject *obj, return result; } +int +_PyEval_SetProfile(PyThreadState *tstate, Py_tracefunc func, PyObject *arg) +{ + assert(tstate != NULL); + /* The caller must hold the GIL */ + assert(PyGILState_Check()); + + /* Call PySys_Audit() in the context of the current thread state, + even if tstate is not the current thread state. */ + if (PySys_Audit("sys.setprofile", NULL) < 0) { + return -1; + } + + PyObject *profileobj = tstate->c_profileobj; + + tstate->c_profilefunc = NULL; + tstate->c_profileobj = NULL; + /* Must make sure that tracing is not ignored if 'profileobj' is freed */ + tstate->use_tracing = tstate->c_tracefunc != NULL; + Py_XDECREF(profileobj); + + Py_XINCREF(arg); + tstate->c_profileobj = arg; + tstate->c_profilefunc = func; + + /* Flag that tracing or profiling is turned on */ + tstate->use_tracing = (func != NULL) || (tstate->c_tracefunc != NULL); + return 0; +} + void PyEval_SetProfile(Py_tracefunc func, PyObject *arg) { - if (PySys_Audit("sys.setprofile", NULL) < 0) { - return; + PyThreadState *tstate = _PyThreadState_GET(); + (void)_PyEval_SetProfile(tstate, func, arg); +} + +int +_PyEval_SetTrace(PyThreadState *tstate, Py_tracefunc func, PyObject *arg) +{ + assert(tstate != NULL); + /* The caller must hold the GIL */ + assert(PyGILState_Check()); + + /* Call PySys_Audit() in the context of the current thread state, + even if tstate is not the current thread state. */ + if (PySys_Audit("sys.settrace", NULL) < 0) { + return -1; } - PyThreadState *tstate = _PyThreadState_GET(); - PyObject *temp = tstate->c_profileobj; + struct _ceval_runtime_state *ceval = &tstate->interp->runtime->ceval; + PyObject *traceobj = tstate->c_traceobj; + ceval->tracing_possible += (func != NULL) - (tstate->c_tracefunc != NULL); + + tstate->c_tracefunc = NULL; + tstate->c_traceobj = NULL; + /* Must make sure that profiling is not ignored if 'traceobj' is freed */ + tstate->use_tracing = (tstate->c_profilefunc != NULL); + Py_XDECREF(traceobj); + Py_XINCREF(arg); - tstate->c_profilefunc = NULL; - tstate->c_profileobj = NULL; - /* Must make sure that tracing is not ignored if 'temp' is freed */ - tstate->use_tracing = tstate->c_tracefunc != NULL; - Py_XDECREF(temp); - tstate->c_profilefunc = func; - tstate->c_profileobj = arg; + tstate->c_traceobj = arg; + tstate->c_tracefunc = func; + /* Flag that tracing or profiling is turned on */ - tstate->use_tracing = (func != NULL) || (tstate->c_tracefunc != NULL); + tstate->use_tracing = ((func != NULL) + || (tstate->c_profilefunc != NULL)); + + return 0; } void PyEval_SetTrace(Py_tracefunc func, PyObject *arg) { - if (PySys_Audit("sys.settrace", NULL) < 0) { - return; - } - - _PyRuntimeState *runtime = &_PyRuntime; - PyThreadState *tstate = _PyRuntimeState_GetThreadState(runtime); - PyObject *temp = tstate->c_traceobj; - runtime->ceval.tracing_possible += (func != NULL) - (tstate->c_tracefunc != NULL); - Py_XINCREF(arg); - tstate->c_tracefunc = NULL; - tstate->c_traceobj = NULL; - /* Must make sure that profiling is not ignored if 'temp' is freed */ - tstate->use_tracing = tstate->c_profilefunc != NULL; - Py_XDECREF(temp); - tstate->c_tracefunc = func; - tstate->c_traceobj = arg; - /* Flag that tracing or profiling is turned on */ - tstate->use_tracing = ((func != NULL) - || (tstate->c_profilefunc != NULL)); + PyThreadState *tstate = _PyThreadState_GET(); + (void)_PyEval_SetTrace(tstate, func, arg); } + void _PyEval_SetCoroutineOriginTrackingDepth(PyThreadState *tstate, int new_depth) { diff --git a/Python/sysmodule.c b/Python/sysmodule.c index 2a8d12c03c0..c78a6273805 100644 --- a/Python/sysmodule.c +++ b/Python/sysmodule.c @@ -876,7 +876,7 @@ trace_init(void) static PyObject * -call_trampoline(PyObject* callback, +call_trampoline(PyThreadState *tstate, PyObject* callback, PyFrameObject *frame, int what, PyObject *arg) { if (PyFrame_FastToLocalsWithError(frame) < 0) { @@ -889,7 +889,7 @@ call_trampoline(PyObject* callback, stack[2] = (arg != NULL) ? arg : Py_None; /* call the Python-level function */ - PyObject *result = _PyObject_FastCall(callback, stack, 3); + PyObject *result = _PyObject_FastCallTstate(tstate, callback, stack, 3); PyFrame_LocalsToFast(frame, 1); if (result == NULL) { @@ -903,15 +903,17 @@ static int profile_trampoline(PyObject *self, PyFrameObject *frame, int what, PyObject *arg) { - PyObject *result; - - if (arg == NULL) + if (arg == NULL) { arg = Py_None; - result = call_trampoline(self, frame, what, arg); + } + + PyThreadState *tstate = _PyThreadState_GET(); + PyObject *result = call_trampoline(tstate, self, frame, what, arg); if (result == NULL) { - PyEval_SetProfile(NULL, NULL); + _PyEval_SetProfile(tstate, NULL, NULL); return -1; } + Py_DECREF(result); return 0; } @@ -921,20 +923,24 @@ trace_trampoline(PyObject *self, PyFrameObject *frame, int what, PyObject *arg) { PyObject *callback; - PyObject *result; - - if (what == PyTrace_CALL) + if (what == PyTrace_CALL) { callback = self; - else + } + else { callback = frame->f_trace; - if (callback == NULL) + } + if (callback == NULL) { return 0; - result = call_trampoline(callback, frame, what, arg); + } + + PyThreadState *tstate = _PyThreadState_GET(); + PyObject *result = call_trampoline(tstate, callback, frame, what, arg); if (result == NULL) { - PyEval_SetTrace(NULL, NULL); + _PyEval_SetTrace(tstate, NULL, NULL); Py_CLEAR(frame->f_trace); return -1; } + if (result != Py_None) { Py_XSETREF(frame->f_trace, result); } @@ -947,12 +953,21 @@ trace_trampoline(PyObject *self, PyFrameObject *frame, static PyObject * sys_settrace(PyObject *self, PyObject *args) { - if (trace_init() == -1) + if (trace_init() == -1) { return NULL; - if (args == Py_None) - PyEval_SetTrace(NULL, NULL); - else - PyEval_SetTrace(trace_trampoline, args); + } + + PyThreadState *tstate = _PyThreadState_GET(); + if (args == Py_None) { + if (_PyEval_SetTrace(tstate, NULL, NULL) < 0) { + return NULL; + } + } + else { + if (_PyEval_SetTrace(tstate, trace_trampoline, args) < 0) { + return NULL; + } + } Py_RETURN_NONE; } @@ -987,12 +1002,21 @@ sys_gettrace_impl(PyObject *module) static PyObject * sys_setprofile(PyObject *self, PyObject *args) { - if (trace_init() == -1) + if (trace_init() == -1) { return NULL; - if (args == Py_None) - PyEval_SetProfile(NULL, NULL); - else - PyEval_SetProfile(profile_trampoline, args); + } + + PyThreadState *tstate = _PyThreadState_GET(); + if (args == Py_None) { + if (_PyEval_SetProfile(tstate, NULL, NULL) < 0) { + return NULL; + } + } + else { + if (_PyEval_SetProfile(tstate, profile_trampoline, args) < 0) { + return NULL; + } + } Py_RETURN_NONE; }