From c69a602329e4fed2fb01cdbfc68cec7682230892 Mon Sep 17 00:00:00 2001 From: Tomasz Pytel Date: Sat, 11 Jan 2025 15:16:46 -0500 Subject: [PATCH 1/8] gh-128632: fix segfault on nested __classdict__ type param --- Lib/test/test_syntax.py | 8 ++++++++ Python/assemble.c | 15 ++++++++------- Python/codegen.c | 3 +++ Python/compile.c | 3 +++ Python/symtable.c | 23 +++++++++++++++++------ 5 files changed, 39 insertions(+), 13 deletions(-) diff --git a/Lib/test/test_syntax.py b/Lib/test/test_syntax.py index 132e2b839627bc..f9aed2b7bb41f6 100644 --- a/Lib/test/test_syntax.py +++ b/Lib/test/test_syntax.py @@ -2607,6 +2607,14 @@ def test_continuation_bad_indentation(self): self.assertRaises(IndentationError, exec, code) + @support.cpython_only + def test_disallowed_type_param_names(self): + # See gh-128632 + self._check_error("class A[__class__]: pass", + "reserved name '__class__' can not be used for type parameter") + self._check_error("class A[__classdict__]: pass", + "reserved name '__classdict__' can not be used for type parameter") + @support.cpython_only def test_nested_named_except_blocks(self): code = "" diff --git a/Python/assemble.c b/Python/assemble.c index f7b88b519f5f71..9834c2b9f7ebf0 100644 --- a/Python/assemble.c +++ b/Python/assemble.c @@ -504,7 +504,7 @@ compute_localsplus_info(_PyCompile_CodeUnitMetadata *umd, int nlocalsplus, int nlocals = (int)PyDict_GET_SIZE(umd->u_varnames); // This counter mirrors the fix done in fix_cell_offsets(). - int numdropped = 0; + int numdropped = 0, cellvar_offset = -1; pos = 0; while (PyDict_Next(umd->u_cellvars, &pos, &k, &v)) { int has_name = PyDict_Contains(umd->u_varnames, k); @@ -515,14 +515,14 @@ compute_localsplus_info(_PyCompile_CodeUnitMetadata *umd, int nlocalsplus, continue; } - int offset = PyLong_AsInt(v); - if (offset == -1 && PyErr_Occurred()) { + cellvar_offset = PyLong_AsInt(v); + if (cellvar_offset == -1 && PyErr_Occurred()) { return ERROR; } - assert(offset >= 0); - offset += nlocals - numdropped; - assert(offset < nlocalsplus); - _Py_set_localsplus_info(offset, k, CO_FAST_CELL, names, kinds); + assert(cellvar_offset >= 0); + cellvar_offset += nlocals - numdropped; + assert(cellvar_offset < nlocalsplus); + _Py_set_localsplus_info(cellvar_offset, k, CO_FAST_CELL, names, kinds); } pos = 0; @@ -534,6 +534,7 @@ compute_localsplus_info(_PyCompile_CodeUnitMetadata *umd, int nlocalsplus, assert(offset >= 0); offset += nlocals - numdropped; assert(offset < nlocalsplus); + assert(offset > cellvar_offset); _Py_set_localsplus_info(offset, k, CO_FAST_FREE, names, kinds); } return SUCCESS; diff --git a/Python/codegen.c b/Python/codegen.c index 61707ba677097c..9ee4691cdd85ff 100644 --- a/Python/codegen.c +++ b/Python/codegen.c @@ -3065,6 +3065,9 @@ codegen_addop_yield(compiler *c, location loc) { return SUCCESS; } +/* XXX: Currently if this is used to insert a new name into u_freevars when + there are already entries in u_cellvars then the wrong index will be put + into u_freevars causing a hard error downstream. */ static int codegen_load_classdict_freevar(compiler *c, location loc) { diff --git a/Python/compile.c b/Python/compile.c index ef470830336dde..2c78914e95436c 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -970,6 +970,9 @@ _PyCompile_ResolveNameop(compiler *c, PyObject *mangled, int scope, break; } if (*optype != COMPILE_OP_FAST) { + /* XXX: Currently if this is used to insert a new name into u_freevars when + there are already entries in u_cellvars then the wrong index will be put + into u_freevars causing a hard error downstream. */ *arg = _PyCompile_DictAddObj(dict, mangled); RETURN_IF_ERROR(*arg); } diff --git a/Python/symtable.c b/Python/symtable.c index 49bd01ba68ac9e..a85626e64ee9e8 100644 --- a/Python/symtable.c +++ b/Python/symtable.c @@ -2536,11 +2536,22 @@ symtable_visit_expr(struct symtable *st, expr_ty e) static int symtable_visit_type_param_bound_or_default( struct symtable *st, expr_ty e, identifier name, - void *key, const char *ste_scope_info) + type_param_ty tp, const char *ste_scope_info) { + if (_PyUnicode_EqualToASCIIString(name, "__class__") || + _PyUnicode_EqualToASCIIString(name, "__classdict__")) { + + PyObject *error_msg = PyUnicode_FromFormat("reserved name '%U' can not be " + "used for type parameter", name); + PyErr_SetObject(PyExc_SyntaxError, error_msg); + Py_DECREF(error_msg); + SET_ERROR_LOCATION(st->st_filename, LOCATION(tp)); + return 0; + } + if (e) { int is_in_class = st->st_cur->ste_can_see_class_scope; - if (!symtable_enter_block(st, name, TypeVariableBlock, key, LOCATION(e))) { + if (!symtable_enter_block(st, name, TypeVariableBlock, (void *)tp, LOCATION(e))) { return 0; } @@ -2582,12 +2593,12 @@ symtable_visit_type_param(struct symtable *st, type_param_ty tp) // The only requirement for the key is that it is unique and it matches the logic in // compile.c where the scope is retrieved. if (!symtable_visit_type_param_bound_or_default(st, tp->v.TypeVar.bound, tp->v.TypeVar.name, - (void *)tp, ste_scope_info)) { + tp, ste_scope_info)) { return 0; } if (!symtable_visit_type_param_bound_or_default(st, tp->v.TypeVar.default_value, tp->v.TypeVar.name, - (void *)((uintptr_t)tp + 1), "a TypeVar default")) { + (type_param_ty)((uintptr_t)tp + 1), "a TypeVar default")) { return 0; } break; @@ -2597,7 +2608,7 @@ symtable_visit_type_param(struct symtable *st, type_param_ty tp) } if (!symtable_visit_type_param_bound_or_default(st, tp->v.TypeVarTuple.default_value, tp->v.TypeVarTuple.name, - (void *)tp, "a TypeVarTuple default")) { + tp, "a TypeVarTuple default")) { return 0; } break; @@ -2607,7 +2618,7 @@ symtable_visit_type_param(struct symtable *st, type_param_ty tp) } if (!symtable_visit_type_param_bound_or_default(st, tp->v.ParamSpec.default_value, tp->v.ParamSpec.name, - (void *)tp, "a ParamSpec default")) { + tp, "a ParamSpec default")) { return 0; } break; From c7ec3ed2e3366502630003a5ecc30a121cb0df13 Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" <43283697+blurb-it[bot]@users.noreply.github.com> Date: Sat, 11 Jan 2025 20:11:29 +0000 Subject: [PATCH 2/8] =?UTF-8?q?=F0=9F=93=9C=F0=9F=A4=96=20Added=20by=20blu?= =?UTF-8?q?rb=5Fit.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../2025-01-11-20-11-28.gh-issue-128632.ryhnKs.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Core_and_Builtins/2025-01-11-20-11-28.gh-issue-128632.ryhnKs.rst diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2025-01-11-20-11-28.gh-issue-128632.ryhnKs.rst b/Misc/NEWS.d/next/Core_and_Builtins/2025-01-11-20-11-28.gh-issue-128632.ryhnKs.rst new file mode 100644 index 00000000000000..4088b0197373cd --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2025-01-11-20-11-28.gh-issue-128632.ryhnKs.rst @@ -0,0 +1 @@ +fix segfault on nested __classdict__ type param From 8f813c13fdaa210514e421b901557ae58d7f9330 Mon Sep 17 00:00:00 2001 From: Tomasz Pytel Date: Sat, 11 Jan 2025 15:43:19 -0500 Subject: [PATCH 3/8] requested changes --- Lib/test/test_syntax.py | 8 ++++++++ Python/symtable.c | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_syntax.py b/Lib/test/test_syntax.py index f9aed2b7bb41f6..220df5105f9573 100644 --- a/Lib/test/test_syntax.py +++ b/Lib/test/test_syntax.py @@ -2614,6 +2614,14 @@ def test_disallowed_type_param_names(self): "reserved name '__class__' can not be used for type parameter") self._check_error("class A[__classdict__]: pass", "reserved name '__classdict__' can not be used for type parameter") + self._check_error("def f[__class__](): pass", + "reserved name '__class__' can not be used for type parameter") + self._check_error("def f[__classdict__](): pass", + "reserved name '__classdict__' can not be used for type parameter") + self._check_error("type T[__class__] = tuple[__class__]", + "reserved name '__class__' can not be used for type parameter") + self._check_error("type T[__classdict__] = tuple[__classdict__]", + "reserved name '__classdict__' can not be used for type parameter") @support.cpython_only def test_nested_named_except_blocks(self): diff --git a/Python/symtable.c b/Python/symtable.c index a85626e64ee9e8..6b2317399c2146 100644 --- a/Python/symtable.c +++ b/Python/symtable.c @@ -2541,7 +2541,7 @@ symtable_visit_type_param_bound_or_default( if (_PyUnicode_EqualToASCIIString(name, "__class__") || _PyUnicode_EqualToASCIIString(name, "__classdict__")) { - PyObject *error_msg = PyUnicode_FromFormat("reserved name '%U' can not be " + PyObject *error_msg = PyUnicode_FromFormat("reserved name '%U' cannot be " "used for type parameter", name); PyErr_SetObject(PyExc_SyntaxError, error_msg); Py_DECREF(error_msg); From 285be46d378126297595ee785d4e1e1ef2d447e1 Mon Sep 17 00:00:00 2001 From: Tomasz Pytel Date: Sat, 11 Jan 2025 16:22:25 -0500 Subject: [PATCH 4/8] disallow __classcell__ and __classdictcell__ --- Lib/test/test_syntax.py | 20 ++++++++------------ Python/symtable.c | 4 +++- 2 files changed, 11 insertions(+), 13 deletions(-) diff --git a/Lib/test/test_syntax.py b/Lib/test/test_syntax.py index 220df5105f9573..f8f0c94733ea51 100644 --- a/Lib/test/test_syntax.py +++ b/Lib/test/test_syntax.py @@ -2610,18 +2610,14 @@ def test_continuation_bad_indentation(self): @support.cpython_only def test_disallowed_type_param_names(self): # See gh-128632 - self._check_error("class A[__class__]: pass", - "reserved name '__class__' can not be used for type parameter") - self._check_error("class A[__classdict__]: pass", - "reserved name '__classdict__' can not be used for type parameter") - self._check_error("def f[__class__](): pass", - "reserved name '__class__' can not be used for type parameter") - self._check_error("def f[__classdict__](): pass", - "reserved name '__classdict__' can not be used for type parameter") - self._check_error("type T[__class__] = tuple[__class__]", - "reserved name '__class__' can not be used for type parameter") - self._check_error("type T[__classdict__] = tuple[__classdict__]", - "reserved name '__classdict__' can not be used for type parameter") + + for name in ('__class__', '__classdict__', '__classcell__', '__classdictcell__'): + self._check_error(f"class A[{name}]: pass", + f"reserved name '{name}' cannot be used for type parameter") + self._check_error(f"def f[{name}](): pass", + f"reserved name '{name}' cannot be used for type parameter") + self._check_error(f"type T[{name}] = tuple[{name}]", + f"reserved name '{name}' cannot be used for type parameter") @support.cpython_only def test_nested_named_except_blocks(self): diff --git a/Python/symtable.c b/Python/symtable.c index 6b2317399c2146..fccf79676ed0e7 100644 --- a/Python/symtable.c +++ b/Python/symtable.c @@ -2539,7 +2539,9 @@ symtable_visit_type_param_bound_or_default( type_param_ty tp, const char *ste_scope_info) { if (_PyUnicode_EqualToASCIIString(name, "__class__") || - _PyUnicode_EqualToASCIIString(name, "__classdict__")) { + _PyUnicode_EqualToASCIIString(name, "__classdict__") || + _PyUnicode_EqualToASCIIString(name, "__classcell__") || + _PyUnicode_EqualToASCIIString(name, "__classdictcell__")) { PyObject *error_msg = PyUnicode_FromFormat("reserved name '%U' cannot be " "used for type parameter", name); From 4d736e9d0d9dd7b4eecdd6982749ec2fa0c7b3a5 Mon Sep 17 00:00:00 2001 From: Tomasz Pytel Date: Sun, 12 Jan 2025 08:23:12 -0500 Subject: [PATCH 5/8] requested comment changes --- Python/assemble.c | 3 +++ Python/codegen.c | 3 --- Python/compile.c | 3 --- 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/Python/assemble.c b/Python/assemble.c index 9834c2b9f7ebf0..70788726f48534 100644 --- a/Python/assemble.c +++ b/Python/assemble.c @@ -534,6 +534,9 @@ compute_localsplus_info(_PyCompile_CodeUnitMetadata *umd, int nlocalsplus, assert(offset >= 0); offset += nlocals - numdropped; assert(offset < nlocalsplus); + /* XXX If the assertion below fails it is most likely because a freevar + was added to u_freevars with the wrong index due to not taking into + account cellvars already present, see gh-128632. */ assert(offset > cellvar_offset); _Py_set_localsplus_info(offset, k, CO_FAST_FREE, names, kinds); } diff --git a/Python/codegen.c b/Python/codegen.c index 9ee4691cdd85ff..61707ba677097c 100644 --- a/Python/codegen.c +++ b/Python/codegen.c @@ -3065,9 +3065,6 @@ codegen_addop_yield(compiler *c, location loc) { return SUCCESS; } -/* XXX: Currently if this is used to insert a new name into u_freevars when - there are already entries in u_cellvars then the wrong index will be put - into u_freevars causing a hard error downstream. */ static int codegen_load_classdict_freevar(compiler *c, location loc) { diff --git a/Python/compile.c b/Python/compile.c index 2c78914e95436c..ef470830336dde 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -970,9 +970,6 @@ _PyCompile_ResolveNameop(compiler *c, PyObject *mangled, int scope, break; } if (*optype != COMPILE_OP_FAST) { - /* XXX: Currently if this is used to insert a new name into u_freevars when - there are already entries in u_cellvars then the wrong index will be put - into u_freevars causing a hard error downstream. */ *arg = _PyCompile_DictAddObj(dict, mangled); RETURN_IF_ERROR(*arg); } From cee5fe48793954986d6ba8c6aeb8795978e9071c Mon Sep 17 00:00:00 2001 From: Tomasz Pytel Date: Fri, 24 Jan 2025 11:48:43 -0500 Subject: [PATCH 6/8] faster attr name compare maybe --- Python/symtable.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Python/symtable.c b/Python/symtable.c index fccf79676ed0e7..a09bb1bbd3b65d 100644 --- a/Python/symtable.c +++ b/Python/symtable.c @@ -2538,10 +2538,10 @@ symtable_visit_type_param_bound_or_default( struct symtable *st, expr_ty e, identifier name, type_param_ty tp, const char *ste_scope_info) { - if (_PyUnicode_EqualToASCIIString(name, "__class__") || - _PyUnicode_EqualToASCIIString(name, "__classdict__") || - _PyUnicode_EqualToASCIIString(name, "__classcell__") || - _PyUnicode_EqualToASCIIString(name, "__classdictcell__")) { + if (_PyUnicode_Equal(name, &_Py_ID(__class__)) || + _PyUnicode_Equal(name, &_Py_ID(__classdict__)) || + _PyUnicode_Equal(name, &_Py_ID(__classcell__)) || + _PyUnicode_Equal(name, &_Py_ID(__classdictcell__))) { PyObject *error_msg = PyUnicode_FromFormat("reserved name '%U' cannot be " "used for type parameter", name); From 151624adcf65153d716ad9a24821faac72a46334 Mon Sep 17 00:00:00 2001 From: Tomasz Pytel Date: Sat, 22 Feb 2025 08:22:20 -0500 Subject: [PATCH 7/8] requested changes --- Lib/test/test_syntax.py | 21 ++++++++++++++------- Python/symtable.c | 5 +---- 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/Lib/test/test_syntax.py b/Lib/test/test_syntax.py index 986229ced99183..12830c1454f604 100644 --- a/Lib/test/test_syntax.py +++ b/Lib/test/test_syntax.py @@ -2629,13 +2629,20 @@ def test_continuation_bad_indentation(self): def test_disallowed_type_param_names(self): # See gh-128632 - for name in ('__class__', '__classdict__', '__classcell__', '__classdictcell__'): - self._check_error(f"class A[{name}]: pass", - f"reserved name '{name}' cannot be used for type parameter") - self._check_error(f"def f[{name}](): pass", - f"reserved name '{name}' cannot be used for type parameter") - self._check_error(f"type T[{name}] = tuple[{name}]", - f"reserved name '{name}' cannot be used for type parameter") + self._check_error(f"class A[__classdict__]: pass", + f"reserved name '__classdict__' cannot be used for type parameter") + self._check_error(f"def f[__classdict__](): pass", + f"reserved name '__classdict__' cannot be used for type parameter") + self._check_error(f"type T[__classdict__] = tuple[__classdict__]", + f"reserved name '__classdict__' cannot be used for type parameter") + + # These compilations are here to make sure __class__, __classcell__ and __classdictcell__ + # don't break in the future like __classdict__ did in this case. + for name in ('__class__', '__classcell__', '__classdictcell__'): + compile(f""" +class A: + class B[{name}]: pass + """, "", mode="exec") @support.cpython_only def test_nested_named_except_blocks(self): diff --git a/Python/symtable.c b/Python/symtable.c index 1d10be8b4d6817..7e9987eb466a63 100644 --- a/Python/symtable.c +++ b/Python/symtable.c @@ -2517,10 +2517,7 @@ symtable_visit_type_param_bound_or_default( struct symtable *st, expr_ty e, identifier name, type_param_ty tp, const char *ste_scope_info) { - if (_PyUnicode_Equal(name, &_Py_ID(__class__)) || - _PyUnicode_Equal(name, &_Py_ID(__classdict__)) || - _PyUnicode_Equal(name, &_Py_ID(__classcell__)) || - _PyUnicode_Equal(name, &_Py_ID(__classdictcell__))) { + if (_PyUnicode_Equal(name, &_Py_ID(__classdict__))) { PyObject *error_msg = PyUnicode_FromFormat("reserved name '%U' cannot be " "used for type parameter", name); From e2c44d73ec44ff7721e82d4a9b6a4776ed63c64a Mon Sep 17 00:00:00 2001 From: Jelle Zijlstra Date: Fri, 28 Feb 2025 16:26:32 -0800 Subject: [PATCH 8/8] Update Misc/NEWS.d/next/Core_and_Builtins/2025-01-11-20-11-28.gh-issue-128632.ryhnKs.rst --- .../2025-01-11-20-11-28.gh-issue-128632.ryhnKs.rst | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2025-01-11-20-11-28.gh-issue-128632.ryhnKs.rst b/Misc/NEWS.d/next/Core_and_Builtins/2025-01-11-20-11-28.gh-issue-128632.ryhnKs.rst index 4088b0197373cd..8cb23fc2d9e78e 100644 --- a/Misc/NEWS.d/next/Core_and_Builtins/2025-01-11-20-11-28.gh-issue-128632.ryhnKs.rst +++ b/Misc/NEWS.d/next/Core_and_Builtins/2025-01-11-20-11-28.gh-issue-128632.ryhnKs.rst @@ -1 +1,2 @@ -fix segfault on nested __classdict__ type param +Disallow ``__classdict__`` as the name of a type parameter. Using this +name would previously crash the interpreter in some circumstances.