From 345ad57b3816cd9255c71a02788104a9145aff2e Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Tue, 1 Apr 2025 22:13:00 -0400 Subject: [PATCH 01/14] Fix crash when the stackref is NULL. --- Python/bytecodes.c | 2 ++ Python/executor_cases.c.h | 8 ++++++++ Python/generated_cases.c.h | 10 ++++++++++ 3 files changed, 20 insertions(+) diff --git a/Python/bytecodes.c b/Python/bytecodes.c index 8d7e137c82943a..91cc94d698a5c7 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -4304,6 +4304,7 @@ dummy_func( EXIT_IF(meth->ml_flags != (METH_FASTCALL|METH_KEYWORDS)); PyTypeObject *d_type = method->d_common.d_type; PyObject *self = PyStackRef_AsPyObjectBorrow(arguments[0]); + EXIT_IF(self == NULL); EXIT_IF(!Py_IS_TYPE(self, d_type)); STAT_INC(CALL, hit); int nargs = total_args - 1; @@ -4382,6 +4383,7 @@ dummy_func( PyMethodDef *meth = method->d_method; EXIT_IF(meth->ml_flags != METH_FASTCALL); PyObject *self = PyStackRef_AsPyObjectBorrow(arguments[0]); + EXIT_IF(self == NULL); EXIT_IF(!Py_IS_TYPE(self, method->d_common.d_type)); STAT_INC(CALL, hit); int nargs = total_args - 1; diff --git a/Python/executor_cases.c.h b/Python/executor_cases.c.h index ca64d7557e33d5..542deecf42ba15 100644 --- a/Python/executor_cases.c.h +++ b/Python/executor_cases.c.h @@ -5973,6 +5973,10 @@ } PyTypeObject *d_type = method->d_common.d_type; PyObject *self = PyStackRef_AsPyObjectBorrow(arguments[0]); + if (self == NULL) { + UOP_STAT_INC(uopcode, miss); + JUMP_TO_JUMP_TARGET(); + } if (!Py_IS_TYPE(self, d_type)) { UOP_STAT_INC(uopcode, miss); JUMP_TO_JUMP_TARGET(); @@ -6126,6 +6130,10 @@ JUMP_TO_JUMP_TARGET(); } PyObject *self = PyStackRef_AsPyObjectBorrow(arguments[0]); + if (self == NULL) { + UOP_STAT_INC(uopcode, miss); + JUMP_TO_JUMP_TARGET(); + } if (!Py_IS_TYPE(self, method->d_common.d_type)) { UOP_STAT_INC(uopcode, miss); JUMP_TO_JUMP_TARGET(); diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index 41ea054d3f5eea..9654704904482e 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -3421,6 +3421,11 @@ JUMP_TO_PREDICTED(CALL); } PyObject *self = PyStackRef_AsPyObjectBorrow(arguments[0]); + if (self == NULL) { + UPDATE_MISS_STATS(CALL); + assert(_PyOpcode_Deopt[opcode] == (CALL)); + JUMP_TO_PREDICTED(CALL); + } if (!Py_IS_TYPE(self, method->d_common.d_type)) { UPDATE_MISS_STATS(CALL); assert(_PyOpcode_Deopt[opcode] == (CALL)); @@ -3543,6 +3548,11 @@ } PyTypeObject *d_type = method->d_common.d_type; PyObject *self = PyStackRef_AsPyObjectBorrow(arguments[0]); + if (self == NULL) { + UPDATE_MISS_STATS(CALL); + assert(_PyOpcode_Deopt[opcode] == (CALL)); + JUMP_TO_PREDICTED(CALL); + } if (!Py_IS_TYPE(self, d_type)) { UPDATE_MISS_STATS(CALL); assert(_PyOpcode_Deopt[opcode] == (CALL)); From 1914aca1332fa93a1f621c1e994b5044b2135a8c Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Tue, 1 Apr 2025 22:16:41 -0400 Subject: [PATCH 02/14] Add a test. --- Lib/test/test_types.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/Lib/test/test_types.py b/Lib/test/test_types.py index f014f7e9ee08c9..ee0a9aeecea430 100644 --- a/Lib/test/test_types.py +++ b/Lib/test/test_types.py @@ -646,6 +646,19 @@ def test_traceback_and_frame_types(self): def test_capsule_type(self): self.assertIsInstance(_datetime.datetime_CAPI, types.CapsuleType) + def test_gh131998(self): + # GH-131998: The specialized instruction would get tricked into dereferencing + # a bound "self" that didn't exist if subsequently called unbound. + def call(part): + part.pop() + + + try: + call(["a"]) + call(list) + except: + pass + class UnionTests(unittest.TestCase): From 5101034b37db2c45bed9192d975bcc2043cb1dcb Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Tue, 1 Apr 2025 22:22:46 -0400 Subject: [PATCH 03/14] Add a test case. --- Lib/test/test_types.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_types.py b/Lib/test/test_types.py index ee0a9aeecea430..04c63266dd63ab 100644 --- a/Lib/test/test_types.py +++ b/Lib/test/test_types.py @@ -4,6 +4,7 @@ run_with_locale, cpython_only, no_rerun, MISSING_C_DOCSTRINGS, ) +from test.support.script_helper import assert_python_ok import collections.abc from collections import namedtuple, UserDict import copy @@ -649,15 +650,18 @@ def test_capsule_type(self): def test_gh131998(self): # GH-131998: The specialized instruction would get tricked into dereferencing # a bound "self" that didn't exist if subsequently called unbound. + code = """if True: + import glob def call(part): part.pop() - try: - call(["a"]) + call(['a']) call(list) except: pass + """ + assert_python_ok("-c", code) class UnionTests(unittest.TestCase): From 2233afe1227de5059254bded38ad835152b09bcf Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Tue, 1 Apr 2025 22:24:24 -0400 Subject: [PATCH 04/14] Add blurb --- .../2025-04-01-22-24-19.gh-issue-131998.DvmZcT.rst | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 Misc/NEWS.d/next/Core_and_Builtins/2025-04-01-22-24-19.gh-issue-131998.DvmZcT.rst diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2025-04-01-22-24-19.gh-issue-131998.DvmZcT.rst b/Misc/NEWS.d/next/Core_and_Builtins/2025-04-01-22-24-19.gh-issue-131998.DvmZcT.rst new file mode 100644 index 00000000000000..e83004d925ad1e --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2025-04-01-22-24-19.gh-issue-131998.DvmZcT.rst @@ -0,0 +1,2 @@ +Fix a crash when using an unbound method :term:`descriptor` object in a +function where a bound method descriptor was used. From 538c83b85c533de43fe5d3e6c78c529edc3cd6ba Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Wed, 2 Apr 2025 05:50:30 -0400 Subject: [PATCH 05/14] Update Lib/test/test_types.py Co-authored-by: sobolevn --- Lib/test/test_types.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_types.py b/Lib/test/test_types.py index 04c63266dd63ab..f4ece14a570ae7 100644 --- a/Lib/test/test_types.py +++ b/Lib/test/test_types.py @@ -655,10 +655,10 @@ def test_gh131998(self): def call(part): part.pop() + call(['a']) try: - call(['a']) call(list) - except: + except TypeError: pass """ assert_python_ok("-c", code) From 04b61df6a4ba6321fca02bf7f4a8ccb53cc5e0c0 Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Wed, 2 Apr 2025 06:03:55 -0400 Subject: [PATCH 06/14] Try removing the 'import glob' now. --- Lib/test/test_types.py | 1 - 1 file changed, 1 deletion(-) diff --git a/Lib/test/test_types.py b/Lib/test/test_types.py index f4ece14a570ae7..6a82afc5665f70 100644 --- a/Lib/test/test_types.py +++ b/Lib/test/test_types.py @@ -651,7 +651,6 @@ def test_gh131998(self): # GH-131998: The specialized instruction would get tricked into dereferencing # a bound "self" that didn't exist if subsequently called unbound. code = """if True: - import glob def call(part): part.pop() From cd12c28dcaa9e48e0bb8d7ef3f50c567f09e1351 Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Wed, 2 Apr 2025 06:06:22 -0400 Subject: [PATCH 07/14] Add back the 'import glob', it doesn't reproduce. --- Lib/test/test_types.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Lib/test/test_types.py b/Lib/test/test_types.py index 6a82afc5665f70..a4e9a396c5423b 100644 --- a/Lib/test/test_types.py +++ b/Lib/test/test_types.py @@ -651,6 +651,10 @@ def test_gh131998(self): # GH-131998: The specialized instruction would get tricked into dereferencing # a bound "self" that didn't exist if subsequently called unbound. code = """if True: + # The optimizer is finicky, and the easiest way (that I know of) + # to get it to reproduce in CI is by importing a Python module at runtime. + import glob + def call(part): part.pop() From f73dd5935697b23964d5a25b0c240dfe87438af5 Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Wed, 2 Apr 2025 06:11:32 -0400 Subject: [PATCH 08/14] Update Lib/test/test_types.py Co-authored-by: Victor Stinner --- Lib/test/test_types.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_types.py b/Lib/test/test_types.py index a4e9a396c5423b..7b560d679c7465 100644 --- a/Lib/test/test_types.py +++ b/Lib/test/test_types.py @@ -647,7 +647,7 @@ def test_traceback_and_frame_types(self): def test_capsule_type(self): self.assertIsInstance(_datetime.datetime_CAPI, types.CapsuleType) - def test_gh131998(self): + def test_call_unbound_crash(self): # GH-131998: The specialized instruction would get tricked into dereferencing # a bound "self" that didn't exist if subsequently called unbound. code = """if True: From a2a07ea432947c2e5ba21a013eff2d3365a7abfb Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Wed, 2 Apr 2025 16:59:24 -0400 Subject: [PATCH 09/14] Check for 'total_args' instead of 'self' --- Python/bytecodes.c | 6 ++++-- Python/executor_cases.c.h | 18 ++++++++++-------- Python/generated_cases.c.h | 22 ++++++++++++---------- 3 files changed, 26 insertions(+), 20 deletions(-) diff --git a/Python/bytecodes.c b/Python/bytecodes.c index 91cc94d698a5c7..76fa962762ee22 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -4298,13 +4298,14 @@ dummy_func( arguments--; total_args++; } + EXIT_IF(total_args == 0); PyMethodDescrObject *method = (PyMethodDescrObject *)callable_o; EXIT_IF(!Py_IS_TYPE(method, &PyMethodDescr_Type)); PyMethodDef *meth = method->d_method; EXIT_IF(meth->ml_flags != (METH_FASTCALL|METH_KEYWORDS)); PyTypeObject *d_type = method->d_common.d_type; PyObject *self = PyStackRef_AsPyObjectBorrow(arguments[0]); - EXIT_IF(self == NULL); + assert(self != NULL); EXIT_IF(!Py_IS_TYPE(self, d_type)); STAT_INC(CALL, hit); int nargs = total_args - 1; @@ -4377,13 +4378,14 @@ dummy_func( arguments--; total_args++; } + EXIT_IF(total_args == 0); PyMethodDescrObject *method = (PyMethodDescrObject *)callable_o; /* Builtin METH_FASTCALL methods, without keywords */ EXIT_IF(!Py_IS_TYPE(method, &PyMethodDescr_Type)); PyMethodDef *meth = method->d_method; EXIT_IF(meth->ml_flags != METH_FASTCALL); PyObject *self = PyStackRef_AsPyObjectBorrow(arguments[0]); - EXIT_IF(self == NULL); + assert(self != NULL); EXIT_IF(!Py_IS_TYPE(self, method->d_common.d_type)); STAT_INC(CALL, hit); int nargs = total_args - 1; diff --git a/Python/executor_cases.c.h b/Python/executor_cases.c.h index 542deecf42ba15..81507ccbed7ccd 100644 --- a/Python/executor_cases.c.h +++ b/Python/executor_cases.c.h @@ -5961,6 +5961,10 @@ arguments--; total_args++; } + if (total_args == 0) { + UOP_STAT_INC(uopcode, miss); + JUMP_TO_JUMP_TARGET(); + } PyMethodDescrObject *method = (PyMethodDescrObject *)callable_o; if (!Py_IS_TYPE(method, &PyMethodDescr_Type)) { UOP_STAT_INC(uopcode, miss); @@ -5973,10 +5977,7 @@ } PyTypeObject *d_type = method->d_common.d_type; PyObject *self = PyStackRef_AsPyObjectBorrow(arguments[0]); - if (self == NULL) { - UOP_STAT_INC(uopcode, miss); - JUMP_TO_JUMP_TARGET(); - } + assert(self != NULL); if (!Py_IS_TYPE(self, d_type)) { UOP_STAT_INC(uopcode, miss); JUMP_TO_JUMP_TARGET(); @@ -6118,6 +6119,10 @@ arguments--; total_args++; } + if (total_args == 0) { + UOP_STAT_INC(uopcode, miss); + JUMP_TO_JUMP_TARGET(); + } PyMethodDescrObject *method = (PyMethodDescrObject *)callable_o; /* Builtin METH_FASTCALL methods, without keywords */ if (!Py_IS_TYPE(method, &PyMethodDescr_Type)) { @@ -6130,10 +6135,7 @@ JUMP_TO_JUMP_TARGET(); } PyObject *self = PyStackRef_AsPyObjectBorrow(arguments[0]); - if (self == NULL) { - UOP_STAT_INC(uopcode, miss); - JUMP_TO_JUMP_TARGET(); - } + assert(self != NULL); if (!Py_IS_TYPE(self, method->d_common.d_type)) { UOP_STAT_INC(uopcode, miss); JUMP_TO_JUMP_TARGET(); diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index 9654704904482e..872c64a39b8e2a 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -3407,6 +3407,11 @@ arguments--; total_args++; } + if (total_args == 0) { + UPDATE_MISS_STATS(CALL); + assert(_PyOpcode_Deopt[opcode] == (CALL)); + JUMP_TO_PREDICTED(CALL); + } PyMethodDescrObject *method = (PyMethodDescrObject *)callable_o; /* Builtin METH_FASTCALL methods, without keywords */ if (!Py_IS_TYPE(method, &PyMethodDescr_Type)) { @@ -3421,11 +3426,7 @@ JUMP_TO_PREDICTED(CALL); } PyObject *self = PyStackRef_AsPyObjectBorrow(arguments[0]); - if (self == NULL) { - UPDATE_MISS_STATS(CALL); - assert(_PyOpcode_Deopt[opcode] == (CALL)); - JUMP_TO_PREDICTED(CALL); - } + assert(self != NULL); if (!Py_IS_TYPE(self, method->d_common.d_type)) { UPDATE_MISS_STATS(CALL); assert(_PyOpcode_Deopt[opcode] == (CALL)); @@ -3534,6 +3535,11 @@ arguments--; total_args++; } + if (total_args == 0) { + UPDATE_MISS_STATS(CALL); + assert(_PyOpcode_Deopt[opcode] == (CALL)); + JUMP_TO_PREDICTED(CALL); + } PyMethodDescrObject *method = (PyMethodDescrObject *)callable_o; if (!Py_IS_TYPE(method, &PyMethodDescr_Type)) { UPDATE_MISS_STATS(CALL); @@ -3548,11 +3554,7 @@ } PyTypeObject *d_type = method->d_common.d_type; PyObject *self = PyStackRef_AsPyObjectBorrow(arguments[0]); - if (self == NULL) { - UPDATE_MISS_STATS(CALL); - assert(_PyOpcode_Deopt[opcode] == (CALL)); - JUMP_TO_PREDICTED(CALL); - } + assert(self != NULL); if (!Py_IS_TYPE(self, d_type)) { UPDATE_MISS_STATS(CALL); assert(_PyOpcode_Deopt[opcode] == (CALL)); From 0eb6933d428ea9707b6d8c46b127685652db4cb0 Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Mon, 7 Apr 2025 07:27:21 -0400 Subject: [PATCH 10/14] Fix the damn import. --- Lib/test/test_types.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_types.py b/Lib/test/test_types.py index 124064264c0b7a..d231664a453659 100644 --- a/Lib/test/test_types.py +++ b/Lib/test/test_types.py @@ -4,7 +4,8 @@ run_with_locale, cpython_only, no_rerun, MISSING_C_DOCSTRINGS, EqualToForwardRef, ) -from test.support.script_helper import assert_python_ok, import_fresh_module +from test.support.script_helper import assert_python_ok +from test.support.import_helper import import_fresh_module import collections.abc from collections import namedtuple, UserDict import copy From d1d5c0a933727235fe184fd64ee54bf2a4d8ade8 Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Tue, 8 Apr 2025 05:40:04 -0400 Subject: [PATCH 11/14] Apply suggestions from code review Co-authored-by: Mark Shannon --- Lib/test/test_types.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_types.py b/Lib/test/test_types.py index d231664a453659..30cf6c1e2cf898 100644 --- a/Lib/test/test_types.py +++ b/Lib/test/test_types.py @@ -681,9 +681,11 @@ def test_call_unbound_crash(self): import glob def call(part): + [] + ([] + []) part.pop() - call(['a']) + for _ in range(3): + call(['a']) try: call(list) except TypeError: From 02617146e87560280232dc2c3a6031afa6cc3157 Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Tue, 8 Apr 2025 05:40:43 -0400 Subject: [PATCH 12/14] Remove 'import glob' --- Lib/test/test_types.py | 1 - 1 file changed, 1 deletion(-) diff --git a/Lib/test/test_types.py b/Lib/test/test_types.py index 30cf6c1e2cf898..f2bf7243d2a350 100644 --- a/Lib/test/test_types.py +++ b/Lib/test/test_types.py @@ -678,7 +678,6 @@ def test_call_unbound_crash(self): code = """if True: # The optimizer is finicky, and the easiest way (that I know of) # to get it to reproduce in CI is by importing a Python module at runtime. - import glob def call(part): [] + ([] + []) From 99037ae701584b5f80b8d869bc74e38119c4c398 Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Tue, 8 Apr 2025 05:41:53 -0400 Subject: [PATCH 13/14] Fix stray newline change. --- Lib/test/test_types.py | 1 + 1 file changed, 1 insertion(+) diff --git a/Lib/test/test_types.py b/Lib/test/test_types.py index f2bf7243d2a350..88772211a30df8 100644 --- a/Lib/test/test_types.py +++ b/Lib/test/test_types.py @@ -6,6 +6,7 @@ ) from test.support.script_helper import assert_python_ok from test.support.import_helper import import_fresh_module + import collections.abc from collections import namedtuple, UserDict import copy From d87de4106c6b2086ebd7d24fec7c3e5d39a02f93 Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Tue, 8 Apr 2025 05:43:05 -0400 Subject: [PATCH 14/14] Remove comment. --- Lib/test/test_types.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/Lib/test/test_types.py b/Lib/test/test_types.py index 88772211a30df8..d80d317aab1ddc 100644 --- a/Lib/test/test_types.py +++ b/Lib/test/test_types.py @@ -677,8 +677,6 @@ def test_call_unbound_crash(self): # GH-131998: The specialized instruction would get tricked into dereferencing # a bound "self" that didn't exist if subsequently called unbound. code = """if True: - # The optimizer is finicky, and the easiest way (that I know of) - # to get it to reproduce in CI is by importing a Python module at runtime. def call(part): [] + ([] + [])