https://github.com/python/cpython/commit/470a0a68ebbbb4254f1a3e8e22cce0c3a08... commit: 470a0a68ebbbb4254f1a3e8e22cce0c3a0827055 branch: main author: Mark Shannon <mark@hotpy.org> committer: markshannon <mark@hotpy.org> date: 2025-01-22T10:51:37Z summary: GH-128682: Change a couple of functions to only steal references on success. (GH-129132) Change PyTuple_FromStackRefSteal and PyList_FromStackRefSteal to only steal on success to avoid escaping files: M Include/internal/pycore_list.h M Include/internal/pycore_opcode_metadata.h M Include/internal/pycore_tuple.h M Include/internal/pycore_uop_metadata.h M Objects/listobject.c M Objects/tupleobject.c M Python/bytecodes.c M Python/ceval.c M Python/executor_cases.c.h M Python/generated_cases.c.h M Tools/cases_generator/analyzer.py diff --git a/Include/internal/pycore_list.h b/Include/internal/pycore_list.h index 836ff30abfcedb..5d817891408481 100644 --- a/Include/internal/pycore_list.h +++ b/Include/internal/pycore_list.h @@ -61,7 +61,7 @@ typedef struct { union _PyStackRef; -PyAPI_FUNC(PyObject *)_PyList_FromStackRefSteal(const union _PyStackRef *src, Py_ssize_t n); +PyAPI_FUNC(PyObject *)_PyList_FromStackRefStealOnSuccess(const union _PyStackRef *src, Py_ssize_t n); PyAPI_FUNC(PyObject *)_PyList_AsTupleAndClear(PyListObject *v); #ifdef __cplusplus diff --git a/Include/internal/pycore_opcode_metadata.h b/Include/internal/pycore_opcode_metadata.h index 396b3a58ad08ec..22ed269c5f1686 100644 --- a/Include/internal/pycore_opcode_metadata.h +++ b/Include/internal/pycore_opcode_metadata.h @@ -2031,12 +2031,12 @@ const struct opcode_metadata _PyOpcode_opcode_metadata[266] = { [BINARY_SUBSCR_LIST_INT] = { true, INSTR_FMT_IXC, HAS_DEOPT_FLAG | HAS_ESCAPES_FLAG }, [BINARY_SUBSCR_STR_INT] = { true, INSTR_FMT_IXC, HAS_DEOPT_FLAG | HAS_ESCAPES_FLAG }, [BINARY_SUBSCR_TUPLE_INT] = { true, INSTR_FMT_IXC, HAS_DEOPT_FLAG | HAS_ESCAPES_FLAG }, - [BUILD_LIST] = { true, INSTR_FMT_IB, HAS_ARG_FLAG | HAS_ERROR_FLAG }, + [BUILD_LIST] = { true, INSTR_FMT_IB, HAS_ARG_FLAG | HAS_ERROR_FLAG | HAS_ERROR_NO_POP_FLAG }, [BUILD_MAP] = { true, INSTR_FMT_IB, HAS_ARG_FLAG | HAS_ERROR_FLAG | HAS_ESCAPES_FLAG }, [BUILD_SET] = { true, INSTR_FMT_IB, HAS_ARG_FLAG | HAS_ERROR_FLAG | HAS_ESCAPES_FLAG }, [BUILD_SLICE] = { true, INSTR_FMT_IB, HAS_ARG_FLAG | HAS_ERROR_FLAG }, [BUILD_STRING] = { true, INSTR_FMT_IB, HAS_ARG_FLAG | HAS_ERROR_FLAG }, - [BUILD_TUPLE] = { true, INSTR_FMT_IB, HAS_ARG_FLAG | HAS_ERROR_FLAG }, + [BUILD_TUPLE] = { true, INSTR_FMT_IB, HAS_ARG_FLAG | HAS_ERROR_FLAG | HAS_ERROR_NO_POP_FLAG }, [CACHE] = { true, INSTR_FMT_IX, 0 }, [CALL] = { true, INSTR_FMT_IBC00, HAS_ARG_FLAG | HAS_EVAL_BREAK_FLAG | HAS_ERROR_FLAG | HAS_ERROR_NO_POP_FLAG | HAS_ESCAPES_FLAG }, [CALL_ALLOC_AND_ENTER_INIT] = { true, INSTR_FMT_IBC00, HAS_ARG_FLAG | HAS_DEOPT_FLAG | HAS_ERROR_FLAG | HAS_ERROR_NO_POP_FLAG | HAS_ESCAPES_FLAG }, diff --git a/Include/internal/pycore_tuple.h b/Include/internal/pycore_tuple.h index 82b875241f4116..dc68d6648b9ec8 100644 --- a/Include/internal/pycore_tuple.h +++ b/Include/internal/pycore_tuple.h @@ -21,7 +21,7 @@ extern PyStatus _PyTuple_InitGlobalObjects(PyInterpreterState *); #define _PyTuple_ITEMS(op) _Py_RVALUE(_PyTuple_CAST(op)->ob_item) PyAPI_FUNC(PyObject *)_PyTuple_FromArray(PyObject *const *, Py_ssize_t); -PyAPI_FUNC(PyObject *)_PyTuple_FromStackRefSteal(const union _PyStackRef *, Py_ssize_t); +PyAPI_FUNC(PyObject *)_PyTuple_FromStackRefStealOnSuccess(const union _PyStackRef *, Py_ssize_t); PyAPI_FUNC(PyObject *)_PyTuple_FromArraySteal(PyObject *const *, Py_ssize_t); typedef struct { diff --git a/Include/internal/pycore_uop_metadata.h b/Include/internal/pycore_uop_metadata.h index 6cc825ed62ed95..4d97a26ec6478a 100644 --- a/Include/internal/pycore_uop_metadata.h +++ b/Include/internal/pycore_uop_metadata.h @@ -137,8 +137,8 @@ const uint16_t _PyUop_Flags[MAX_UOP_ID+1] = { [_STORE_DEREF] = HAS_ARG_FLAG | HAS_FREE_FLAG | HAS_ESCAPES_FLAG, [_COPY_FREE_VARS] = HAS_ARG_FLAG, [_BUILD_STRING] = HAS_ARG_FLAG | HAS_ERROR_FLAG, - [_BUILD_TUPLE] = HAS_ARG_FLAG | HAS_ERROR_FLAG, - [_BUILD_LIST] = HAS_ARG_FLAG | HAS_ERROR_FLAG, + [_BUILD_TUPLE] = HAS_ARG_FLAG | HAS_ERROR_FLAG | HAS_ERROR_NO_POP_FLAG, + [_BUILD_LIST] = HAS_ARG_FLAG | HAS_ERROR_FLAG | HAS_ERROR_NO_POP_FLAG, [_LIST_EXTEND] = HAS_ARG_FLAG | HAS_ERROR_FLAG | HAS_ESCAPES_FLAG, [_SET_UPDATE] = HAS_ARG_FLAG | HAS_ERROR_FLAG | HAS_ESCAPES_FLAG, [_BUILD_SET] = HAS_ARG_FLAG | HAS_ERROR_FLAG | HAS_ESCAPES_FLAG, diff --git a/Objects/listobject.c b/Objects/listobject.c index bbd53e7de94a31..099e65c0c25fed 100644 --- a/Objects/listobject.c +++ b/Objects/listobject.c @@ -3190,7 +3190,7 @@ _PyList_AsTupleAndClear(PyListObject *self) } PyObject * -_PyList_FromStackRefSteal(const _PyStackRef *src, Py_ssize_t n) +_PyList_FromStackRefStealOnSuccess(const _PyStackRef *src, Py_ssize_t n) { if (n == 0) { return PyList_New(0); @@ -3198,9 +3198,6 @@ _PyList_FromStackRefSteal(const _PyStackRef *src, Py_ssize_t n) PyListObject *list = (PyListObject *)PyList_New(n); if (list == NULL) { - for (Py_ssize_t i = 0; i < n; i++) { - PyStackRef_CLOSE(src[i]); - } return NULL; } diff --git a/Objects/tupleobject.c b/Objects/tupleobject.c index 002002eb455556..7fe8553030a02e 100644 --- a/Objects/tupleobject.c +++ b/Objects/tupleobject.c @@ -391,16 +391,13 @@ _PyTuple_FromArray(PyObject *const *src, Py_ssize_t n) } PyObject * -_PyTuple_FromStackRefSteal(const _PyStackRef *src, Py_ssize_t n) +_PyTuple_FromStackRefStealOnSuccess(const _PyStackRef *src, Py_ssize_t n) { if (n == 0) { return tuple_get_empty(); } PyTupleObject *tuple = tuple_alloc(n); if (tuple == NULL) { - for (Py_ssize_t i = 0; i < n; i++) { - PyStackRef_CLOSE(src[i]); - } return NULL; } PyObject **dst = tuple->ob_item; diff --git a/Python/bytecodes.c b/Python/bytecodes.c index 8bed29ca2c8fc7..c78d496adcd086 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -1852,16 +1852,20 @@ dummy_func( } inst(BUILD_TUPLE, (values[oparg] -- tup)) { - PyObject *tup_o = _PyTuple_FromStackRefSteal(values, oparg); + PyObject *tup_o = _PyTuple_FromStackRefStealOnSuccess(values, oparg); + if (tup_o == NULL) { + ERROR_NO_POP(); + } INPUTS_DEAD(); - ERROR_IF(tup_o == NULL, error); tup = PyStackRef_FromPyObjectSteal(tup_o); } inst(BUILD_LIST, (values[oparg] -- list)) { - PyObject *list_o = _PyList_FromStackRefSteal(values, oparg); + PyObject *list_o = _PyList_FromStackRefStealOnSuccess(values, oparg); + if (list_o == NULL) { + ERROR_NO_POP(); + } INPUTS_DEAD(); - ERROR_IF(list_o == NULL, error); list = PyStackRef_FromPyObjectSteal(list_o); } diff --git a/Python/ceval.c b/Python/ceval.c index 58a54467f06bb0..171b383d547e35 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -1527,7 +1527,12 @@ initialize_locals(PyThreadState *tstate, PyFunctionObject *func, u = (PyObject *)&_Py_SINGLETON(tuple_empty); } else { - u = _PyTuple_FromStackRefSteal(args + n, argcount - n); + u = _PyTuple_FromStackRefStealOnSuccess(args + n, argcount - n); + if (u == NULL) { + for (Py_ssize_t i = n; i < argcount; i++) { + PyStackRef_CLOSE(args[i]); + } + } } if (u == NULL) { goto fail_post_positional; diff --git a/Python/executor_cases.c.h b/Python/executor_cases.c.h index 93072304e6e111..d2da9a8acf9d97 100644 --- a/Python/executor_cases.c.h +++ b/Python/executor_cases.c.h @@ -2240,8 +2240,10 @@ _PyStackRef tup; oparg = CURRENT_OPARG(); values = &stack_pointer[-oparg]; - PyObject *tup_o = _PyTuple_FromStackRefSteal(values, oparg); - if (tup_o == NULL) JUMP_TO_ERROR(); + PyObject *tup_o = _PyTuple_FromStackRefStealOnSuccess(values, oparg); + if (tup_o == NULL) { + JUMP_TO_ERROR(); + } tup = PyStackRef_FromPyObjectSteal(tup_o); stack_pointer[-oparg] = tup; stack_pointer += 1 - oparg; @@ -2254,8 +2256,10 @@ _PyStackRef list; oparg = CURRENT_OPARG(); values = &stack_pointer[-oparg]; - PyObject *list_o = _PyList_FromStackRefSteal(values, oparg); - if (list_o == NULL) JUMP_TO_ERROR(); + PyObject *list_o = _PyList_FromStackRefStealOnSuccess(values, oparg); + if (list_o == NULL) { + JUMP_TO_ERROR(); + } list = PyStackRef_FromPyObjectSteal(list_o); stack_pointer[-oparg] = list; stack_pointer += 1 - oparg; diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index d0ab667a8bc8ab..7b49f336383a7a 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -732,10 +732,8 @@ _PyStackRef *values; _PyStackRef list; values = &stack_pointer[-oparg]; - PyObject *list_o = _PyList_FromStackRefSteal(values, oparg); + PyObject *list_o = _PyList_FromStackRefStealOnSuccess(values, oparg); if (list_o == NULL) { - stack_pointer += -oparg; - assert(WITHIN_STACK_BOUNDS()); goto error; } list = PyStackRef_FromPyObjectSteal(list_o); @@ -905,10 +903,8 @@ _PyStackRef *values; _PyStackRef tup; values = &stack_pointer[-oparg]; - PyObject *tup_o = _PyTuple_FromStackRefSteal(values, oparg); + PyObject *tup_o = _PyTuple_FromStackRefStealOnSuccess(values, oparg); if (tup_o == NULL) { - stack_pointer += -oparg; - assert(WITHIN_STACK_BOUNDS()); goto error; } tup = PyStackRef_FromPyObjectSteal(tup_o); diff --git a/Tools/cases_generator/analyzer.py b/Tools/cases_generator/analyzer.py index 1f15c3bb9c88af..e8ee314879893d 100644 --- a/Tools/cases_generator/analyzer.py +++ b/Tools/cases_generator/analyzer.py @@ -581,7 +581,7 @@ def has_error_without_pop(op: parser.InstDef) -> bool: "_PyGen_GetGeneratorFromFrame", "_PyInterpreterState_GET", "_PyList_AppendTakeRef", - "_PyList_FromStackRefSteal", + "_PyList_FromStackRefStealOnSuccess", "_PyList_ITEMS", "_PyLong_Add", "_PyLong_CompactValue", @@ -600,8 +600,7 @@ def has_error_without_pop(op: parser.InstDef) -> bool: "_PyObject_InlineValues", "_PyObject_ManagedDictPointer", "_PyThreadState_HasStackSpace", - "_PyTuple_FromArraySteal", - "_PyTuple_FromStackRefSteal", + "_PyTuple_FromStackRefStealOnSuccess", "_PyTuple_ITEMS", "_PyType_HasFeature", "_PyType_NewManagedObject",