https://github.com/python/cpython/commit/470a0a68ebbbb4254f1a3e8e22cce0c3a0827055
commit: 470a0a68ebbbb4254f1a3e8e22cce0c3a0827055
branch: main
author: Mark Shannon <[email protected]>
committer: markshannon <[email protected]>
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",
_______________________________________________
Python-checkins mailing list -- [email protected]
To unsubscribe send an email to [email protected]
https://mail.python.org/mailman3/lists/python-checkins.python.org/
Member address: [email protected]
