From 05fadc96f20df1f72013bf368892499b88a23ec4 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Fri, 23 May 2025 13:35:45 -0600 Subject: [PATCH] Revert "gh-132775: Use _PyCode GetScriptXIData() (gh-134511)" This reverts commit 09e72cf091d03479eddcb3c4526f5c6af56d31a0. --- Lib/test/support/interpreters/channels.py | 2 +- Lib/test/test__interpreters.py | 25 +- Lib/test/test_interpreters/test_api.py | 23 +- Modules/_interpretersmodule.c | 294 ++++++++++++++++------ 4 files changed, 240 insertions(+), 104 deletions(-) diff --git a/Lib/test/support/interpreters/channels.py b/Lib/test/support/interpreters/channels.py index 3b6e0f0effd969..7a2bd7d63f808f 100644 --- a/Lib/test/support/interpreters/channels.py +++ b/Lib/test/support/interpreters/channels.py @@ -69,7 +69,7 @@ def list_all(): if not hasattr(send, '_unboundop'): send._set_unbound(unboundop) else: - assert send._unbound[0] == unboundop + assert send._unbound[0] == op channels.append(chan) return channels diff --git a/Lib/test/test__interpreters.py b/Lib/test/test__interpreters.py index ad3ebbfdff64a7..63fdaad8de7ef5 100644 --- a/Lib/test/test__interpreters.py +++ b/Lib/test/test__interpreters.py @@ -474,15 +474,13 @@ def setUp(self): def test_signatures(self): # See https://github.com/python/cpython/issues/126654 - msg = r'_interpreters.exec\(\) argument 3 must be dict, not int' + msg = "expected 'shared' to be a dict" with self.assertRaisesRegex(TypeError, msg): _interpreters.exec(self.id, 'a', 1) with self.assertRaisesRegex(TypeError, msg): _interpreters.exec(self.id, 'a', shared=1) - msg = r'_interpreters.run_string\(\) argument 3 must be dict, not int' with self.assertRaisesRegex(TypeError, msg): _interpreters.run_string(self.id, 'a', shared=1) - msg = r'_interpreters.run_func\(\) argument 3 must be dict, not int' with self.assertRaisesRegex(TypeError, msg): _interpreters.run_func(self.id, lambda: None, shared=1) @@ -954,8 +952,7 @@ def test_invalid_syntax(self): """) with self.subTest('script'): - with self.assertRaises(SyntaxError): - _interpreters.run_string(self.id, script) + self.assert_run_failed(SyntaxError, script) with self.subTest('module'): modname = 'spam_spam_spam' @@ -1022,19 +1019,12 @@ def script(): with open(w, 'w', encoding="utf-8") as spipe: with contextlib.redirect_stdout(spipe): print('it worked!', end='') - failed = None def f(): - nonlocal failed - try: - _interpreters.set___main___attrs(self.id, dict(w=w)) - _interpreters.run_func(self.id, script) - except Exception as exc: - failed = exc + _interpreters.set___main___attrs(self.id, dict(w=w)) + _interpreters.run_func(self.id, script) t = threading.Thread(target=f) t.start() t.join() - if failed: - raise Exception from failed with open(r, encoding="utf-8") as outfile: out = outfile.read() @@ -1063,16 +1053,19 @@ def test_closure(self): spam = True def script(): assert spam - with self.assertRaises(ValueError): + + with self.assertRaises(TypeError): _interpreters.run_func(self.id, script) + # XXX This hasn't been fixed yet. + @unittest.expectedFailure def test_return_value(self): def script(): return 'spam' with self.assertRaises(ValueError): _interpreters.run_func(self.id, script) -# @unittest.skip("we're not quite there yet") + @unittest.skip("we're not quite there yet") def test_args(self): with self.subTest('args'): def script(a, b=0): diff --git a/Lib/test/test_interpreters/test_api.py b/Lib/test/test_interpreters/test_api.py index 165949167ceba8..1e2d572b1cbb81 100644 --- a/Lib/test/test_interpreters/test_api.py +++ b/Lib/test/test_interpreters/test_api.py @@ -839,16 +839,9 @@ def test_bad_script(self): interp.exec(10) def test_bytes_for_script(self): - r, w = self.pipe() - RAN = b'R' - DONE = b'D' interp = interpreters.create() - interp.exec(f"""if True: - import os - os.write({w}, {RAN!r}) - """) - os.write(w, DONE) - self.assertEqual(os.read(r, 1), RAN) + with self.assertRaises(TypeError): + interp.exec(b'print("spam")') def test_with_background_threads_still_running(self): r_interp, w_interp = self.pipe() @@ -1017,6 +1010,8 @@ def test_call(self): for i, (callable, args, kwargs) in enumerate([ (call_func_noop, (), {}), + (call_func_return_shareable, (), {}), + (call_func_return_not_shareable, (), {}), (Spam.noop, (), {}), ]): with self.subTest(f'success case #{i+1}'): @@ -1041,8 +1036,6 @@ def test_call(self): (call_func_complex, ('custom', 'spam!'), {}), (call_func_complex, ('custom-inner', 'eggs!'), {}), (call_func_complex, ('???',), {'exc': ValueError('spam')}), - (call_func_return_shareable, (), {}), - (call_func_return_not_shareable, (), {}), ]): with self.subTest(f'invalid case #{i+1}'): with self.assertRaises(Exception): @@ -1058,6 +1051,8 @@ def test_call_in_thread(self): for i, (callable, args, kwargs) in enumerate([ (call_func_noop, (), {}), + (call_func_return_shareable, (), {}), + (call_func_return_not_shareable, (), {}), (Spam.noop, (), {}), ]): with self.subTest(f'success case #{i+1}'): @@ -1084,8 +1079,6 @@ def test_call_in_thread(self): (call_func_complex, ('custom', 'spam!'), {}), (call_func_complex, ('custom-inner', 'eggs!'), {}), (call_func_complex, ('???',), {'exc': ValueError('spam')}), - (call_func_return_shareable, (), {}), - (call_func_return_not_shareable, (), {}), ]): with self.subTest(f'invalid case #{i+1}'): if args or kwargs: @@ -1625,8 +1618,8 @@ def test_exec(self): def test_call(self): with self.subTest('no args'): interpid = _interpreters.create() - with self.assertRaises(ValueError): - _interpreters.call(interpid, call_func_return_shareable) + exc = _interpreters.call(interpid, call_func_return_shareable) + self.assertIs(exc, None) with self.subTest('uncaught exception'): interpid = _interpreters.create() diff --git a/Modules/_interpretersmodule.c b/Modules/_interpretersmodule.c index 376517ab92360f..f4807fd214b19e 100644 --- a/Modules/_interpretersmodule.c +++ b/Modules/_interpretersmodule.c @@ -9,6 +9,7 @@ #include "pycore_code.h" // _PyCode_HAS_EXECUTORS() #include "pycore_crossinterp.h" // _PyXIData_t #include "pycore_pyerrors.h" // _PyErr_GetRaisedException() +#include "pycore_function.h" // _PyFunction_VerifyStateless() #include "pycore_interp.h" // _PyInterpreterState_IDIncref() #include "pycore_modsupport.h" // _PyArg_BadArgument() #include "pycore_namespace.h" // _PyNamespace_New() @@ -360,6 +361,81 @@ _get_current_xibufferview_type(void) } +/* Python code **************************************************************/ + +static const char * +check_code_str(PyUnicodeObject *text) +{ + assert(text != NULL); + if (PyUnicode_GET_LENGTH(text) == 0) { + return "too short"; + } + + // XXX Verify that it parses? + + return NULL; +} + +#ifndef NDEBUG +static int +code_has_args(PyCodeObject *code) +{ + assert(code != NULL); + return (code->co_argcount > 0 + || code->co_posonlyargcount > 0 + || code->co_kwonlyargcount > 0 + || code->co_flags & (CO_VARARGS | CO_VARKEYWORDS)); +} +#endif + +#define RUN_TEXT 1 +#define RUN_CODE 2 + +static const char * +get_code_str(PyObject *arg, Py_ssize_t *len_p, PyObject **bytes_p, int *flags_p) +{ + const char *codestr = NULL; + Py_ssize_t len = -1; + PyObject *bytes_obj = NULL; + int flags = 0; + + if (PyUnicode_Check(arg)) { + assert(PyUnicode_Check(arg) + && (check_code_str((PyUnicodeObject *)arg) == NULL)); + codestr = PyUnicode_AsUTF8AndSize(arg, &len); + if (codestr == NULL) { + return NULL; + } + if (strlen(codestr) != (size_t)len) { + PyErr_SetString(PyExc_ValueError, + "source code string cannot contain null bytes"); + return NULL; + } + flags = RUN_TEXT; + } + else { + assert(PyCode_Check(arg)); + assert(_PyCode_VerifyStateless( + PyThreadState_Get(), (PyCodeObject *)arg, NULL, NULL, NULL) == 0); + assert(!code_has_args((PyCodeObject *)arg)); + flags = RUN_CODE; + + // Serialize the code object. + bytes_obj = PyMarshal_WriteObjectToString(arg, Py_MARSHAL_VERSION); + if (bytes_obj == NULL) { + return NULL; + } + codestr = PyBytes_AS_STRING(bytes_obj); + len = PyBytes_GET_SIZE(bytes_obj); + } + + *flags_p = flags; + *bytes_p = bytes_obj; + *len_p = len; + return codestr; +} + + /* interpreter-specific code ************************************************/ static int @@ -423,14 +499,22 @@ config_from_object(PyObject *configobj, PyInterpreterConfig *config) static int -_run_script(_PyXIData_t *script, PyObject *ns) +_run_script(PyObject *ns, const char *codestr, Py_ssize_t codestrlen, int flags) { - PyObject *code = _PyXIData_NewObject(script); - if (code == NULL) { - return -1; + PyObject *result = NULL; + if (flags & RUN_TEXT) { + result = PyRun_StringFlags(codestr, Py_file_input, ns, ns, NULL); + } + else if (flags & RUN_CODE) { + PyObject *code = PyMarshal_ReadObjectFromString(codestr, codestrlen); + if (code != NULL) { + result = PyEval_EvalCode(code, ns, ns); + Py_DECREF(code); + } + } + else { + Py_UNREACHABLE(); } - PyObject *result = PyEval_EvalCode(code, ns, ns); - Py_DECREF(code); if (result == NULL) { return -1; } @@ -439,11 +523,12 @@ _run_script(_PyXIData_t *script, PyObject *ns) } static int -_exec_in_interpreter(PyThreadState *tstate, PyInterpreterState *interp, - _PyXIData_t *script, PyObject *shareables, +_run_in_interpreter(PyInterpreterState *interp, + const char *codestr, Py_ssize_t codestrlen, + PyObject *shareables, int flags, PyObject **p_excinfo) { - assert(!_PyErr_Occurred(tstate)); + assert(!PyErr_Occurred()); _PyXI_session *session = _PyXI_NewSession(); if (session == NULL) { return -1; @@ -451,7 +536,7 @@ _exec_in_interpreter(PyThreadState *tstate, PyInterpreterState *interp, // Prep and switch interpreters. if (_PyXI_Enter(session, interp, shareables) < 0) { - if (_PyErr_Occurred(tstate)) { + if (PyErr_Occurred()) { // If an error occured at this step, it means that interp // was not prepared and switched. _PyXI_FreeSession(session); @@ -473,7 +558,7 @@ _exec_in_interpreter(PyThreadState *tstate, PyInterpreterState *interp, if (mainns == NULL) { goto finally; } - res = _run_script(script, mainns); + res = _run_script(mainns, codestr, codestrlen, flags); finally: // Clean up and switch back. @@ -867,23 +952,104 @@ PyDoc_STRVAR(set___main___attrs_doc, Bind the given attributes in the interpreter's __main__ module."); -static void -unwrap_not_shareable(PyThreadState *tstate) +static PyUnicodeObject * +convert_script_arg(PyThreadState *tstate, + PyObject *arg, const char *fname, const char *displayname, + const char *expected) { - PyObject *exctype = _PyXIData_GetNotShareableErrorType(tstate); - if (!_PyErr_ExceptionMatches(tstate, exctype)) { - return; + PyUnicodeObject *str = NULL; + if (PyUnicode_CheckExact(arg)) { + str = (PyUnicodeObject *)Py_NewRef(arg); } - PyObject *exc = _PyErr_GetRaisedException(tstate); - PyObject *cause = PyException_GetCause(exc); - if (cause != NULL) { - Py_DECREF(exc); - exc = cause; + else if (PyUnicode_Check(arg)) { + // XXX str = PyUnicode_FromObject(arg); + str = (PyUnicodeObject *)Py_NewRef(arg); } else { - assert(PyException_GetContext(exc) == NULL); + _PyArg_BadArgument(fname, displayname, expected, arg); + return NULL; } + + const char *err = check_code_str(str); + if (err != NULL) { + Py_DECREF(str); + _PyErr_Format(tstate, PyExc_ValueError, + "%.200s(): bad script text (%s)", fname, err); + return NULL; + } + + return str; +} + +static PyCodeObject * +convert_code_arg(PyThreadState *tstate, + PyObject *arg, const char *fname, const char *displayname, + const char *expected) +{ + PyObject *cause; + PyCodeObject *code = NULL; + if (PyFunction_Check(arg)) { + // For now we allow globals, so we can't use + // _PyFunction_VerifyStateless(). + PyObject *codeobj = PyFunction_GetCode(arg); + if (_PyCode_VerifyStateless( + tstate, (PyCodeObject *)codeobj, NULL, NULL, NULL) < 0) { + goto chained; + } + code = (PyCodeObject *)Py_NewRef(codeobj); + } + else if (PyCode_Check(arg)) { + if (_PyCode_VerifyStateless( + tstate, (PyCodeObject *)arg, NULL, NULL, NULL) < 0) { + goto chained; + } + code = (PyCodeObject *)Py_NewRef(arg); + } + else { + _PyArg_BadArgument(fname, displayname, expected, arg); + return NULL; + } + + return code; + +chained: + cause = _PyErr_GetRaisedException(tstate); + assert(cause != NULL); + _PyArg_BadArgument(fname, displayname, expected, arg); + PyObject *exc = _PyErr_GetRaisedException(tstate); + PyException_SetCause(exc, cause); _PyErr_SetRaisedException(tstate, exc); + return NULL; +} + +static int +_interp_exec(PyObject *self, PyInterpreterState *interp, + PyObject *code_arg, PyObject *shared_arg, PyObject **p_excinfo) +{ + if (shared_arg != NULL && !PyDict_CheckExact(shared_arg)) { + PyErr_SetString(PyExc_TypeError, "expected 'shared' to be a dict"); + return -1; + } + + // Extract code. + Py_ssize_t codestrlen = -1; + PyObject *bytes_obj = NULL; + int flags = 0; + const char *codestr = get_code_str(code_arg, + &codestrlen, &bytes_obj, &flags); + if (codestr == NULL) { + return -1; + } + + // Run the code in the interpreter. + int res = _run_in_interpreter(interp, codestr, codestrlen, + shared_arg, flags, p_excinfo); + Py_XDECREF(bytes_obj); + if (res < 0) { + return -1; + } + + return 0; } static PyObject * @@ -896,9 +1062,8 @@ interp_exec(PyObject *self, PyObject *args, PyObject *kwds) PyObject *shared = NULL; int restricted = 0; if (!PyArg_ParseTupleAndKeywords(args, kwds, - "OO|O!$p:" FUNCNAME, kwlist, - &id, &code, &PyDict_Type, &shared, - &restricted)) + "OO|O$p:" FUNCNAME, kwlist, + &id, &code, &shared, &restricted)) { return NULL; } @@ -910,17 +1075,22 @@ interp_exec(PyObject *self, PyObject *args, PyObject *kwds) return NULL; } - // We don't need the script to be "pure", which means it can use - // global variables. They will be resolved against __main__. - _PyXIData_t xidata = {0}; - if (_PyCode_GetScriptXIData(tstate, code, &xidata) < 0) { - unwrap_not_shareable(tstate); + const char *expected = "a string, a function, or a code object"; + if (PyUnicode_Check(code)) { + code = (PyObject *)convert_script_arg(tstate, code, FUNCNAME, + "argument 2", expected); + } + else { + code = (PyObject *)convert_code_arg(tstate, code, FUNCNAME, + "argument 2", expected); + } + if (code == NULL) { return NULL; } PyObject *excinfo = NULL; - int res = _exec_in_interpreter(tstate, interp, &xidata, shared, &excinfo); - _PyXIData_Release(&xidata); + int res = _interp_exec(self, interp, code, shared, &excinfo); + Py_DECREF(code); if (res < 0) { assert((excinfo == NULL) != (PyErr_Occurred() == NULL)); return excinfo; @@ -956,9 +1126,8 @@ interp_run_string(PyObject *self, PyObject *args, PyObject *kwds) PyObject *shared = NULL; int restricted = 0; if (!PyArg_ParseTupleAndKeywords(args, kwds, - "OU|O!$p:" FUNCNAME, kwlist, - &id, &script, &PyDict_Type, &shared, - &restricted)) + "OU|O$p:" FUNCNAME, kwlist, + &id, &script, &shared, &restricted)) { return NULL; } @@ -970,20 +1139,15 @@ interp_run_string(PyObject *self, PyObject *args, PyObject *kwds) return NULL; } - if (PyFunction_Check(script) || PyCode_Check(script)) { - _PyArg_BadArgument(FUNCNAME, "argument 2", "a string", script); - return NULL; - } - - _PyXIData_t xidata = {0}; - if (_PyCode_GetScriptXIData(tstate, script, &xidata) < 0) { - unwrap_not_shareable(tstate); + script = (PyObject *)convert_script_arg(tstate, script, FUNCNAME, + "argument 2", "a string"); + if (script == NULL) { return NULL; } PyObject *excinfo = NULL; - int res = _exec_in_interpreter(tstate, interp, &xidata, shared, &excinfo); - _PyXIData_Release(&xidata); + int res = _interp_exec(self, interp, script, shared, &excinfo); + Py_DECREF(script); if (res < 0) { assert((excinfo == NULL) != (PyErr_Occurred() == NULL)); return excinfo; @@ -1009,9 +1173,8 @@ interp_run_func(PyObject *self, PyObject *args, PyObject *kwds) PyObject *shared = NULL; int restricted = 0; if (!PyArg_ParseTupleAndKeywords(args, kwds, - "OO|O!$p:" FUNCNAME, kwlist, - &id, &func, &PyDict_Type, &shared, - &restricted)) + "OO|O$p:" FUNCNAME, kwlist, + &id, &func, &shared, &restricted)) { return NULL; } @@ -1023,29 +1186,16 @@ interp_run_func(PyObject *self, PyObject *args, PyObject *kwds) return NULL; } - // We don't worry about checking globals. They will be resolved - // against __main__. - PyObject *code; - if (PyFunction_Check(func)) { - code = PyFunction_GET_CODE(func); - } - else if (PyCode_Check(func)) { - code = func; - } - else { - _PyArg_BadArgument(FUNCNAME, "argument 2", "a function", func); - return NULL; - } - - _PyXIData_t xidata = {0}; - if (_PyCode_GetScriptXIData(tstate, code, &xidata) < 0) { - unwrap_not_shareable(tstate); + PyCodeObject *code = convert_code_arg(tstate, func, FUNCNAME, + "argument 2", + "a function or a code object"); + if (code == NULL) { return NULL; } PyObject *excinfo = NULL; - int res = _exec_in_interpreter(tstate, interp, &xidata, shared, &excinfo); - _PyXIData_Release(&xidata); + int res = _interp_exec(self, interp, (PyObject *)code, shared, &excinfo); + Py_DECREF(code); if (res < 0) { assert((excinfo == NULL) != (PyErr_Occurred() == NULL)); return excinfo; @@ -1098,15 +1248,15 @@ interp_call(PyObject *self, PyObject *args, PyObject *kwds) return NULL; } - _PyXIData_t xidata = {0}; - if (_PyCode_GetPureScriptXIData(tstate, callable, &xidata) < 0) { - unwrap_not_shareable(tstate); + PyObject *code = (PyObject *)convert_code_arg(tstate, callable, FUNCNAME, + "argument 2", "a function"); + if (code == NULL) { return NULL; } PyObject *excinfo = NULL; - int res = _exec_in_interpreter(tstate, interp, &xidata, NULL, &excinfo); - _PyXIData_Release(&xidata); + int res = _interp_exec(self, interp, code, NULL, &excinfo); + Py_DECREF(code); if (res < 0) { assert((excinfo == NULL) != (PyErr_Occurred() == NULL)); return excinfo;