From 336f3e4019b93fa555c04e10cfcf87b694b2d830 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Wed, 9 Nov 2022 11:45:12 -0800 Subject: [PATCH 1/2] Don't perform jumps before error handling --- Python/bytecodes.c | 34 +++++++++++++++++++--------------- Python/generated_cases.c.h | 34 +++++++++++++++++++--------------- 2 files changed, 38 insertions(+), 30 deletions(-) diff --git a/Python/bytecodes.c b/Python/bytecodes.c index d2df56ef7ba330..bd860b59bbcfc9 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -1152,6 +1152,8 @@ dummy_func( PyObject *name = GETITEM(names, oparg); next_instr--; if (_Py_Specialize_StoreAttr(owner, next_instr, name)) { + // "undo" the rewind so end up in the correct handler: + next_instr++; goto error; } DISPATCH_SAME_OPARG(); @@ -1735,6 +1737,8 @@ dummy_func( PyObject *name = GETITEM(names, oparg>>1); next_instr--; if (_Py_Specialize_LoadAttr(owner, next_instr, name)) { + // "undo" the rewind so end up in the correct handler: + next_instr++; goto error; } DISPATCH_SAME_OPARG(); @@ -3144,7 +3148,6 @@ dummy_func( PyObject *callable = PEEK(2); DEOPT_IF(callable != (PyObject *)&PyUnicode_Type, CALL); STAT_INC(CALL, hit); - JUMPBY(INLINE_CACHE_ENTRIES_CALL); PyObject *arg = TOP(); PyObject *res = PyObject_Str(arg); Py_DECREF(arg); @@ -3154,6 +3157,7 @@ dummy_func( if (res == NULL) { goto error; } + JUMPBY(INLINE_CACHE_ENTRIES_CALL); CHECK_EVAL_BREAKER(); } @@ -3165,7 +3169,6 @@ dummy_func( PyObject *callable = PEEK(2); DEOPT_IF(callable != (PyObject *)&PyTuple_Type, CALL); STAT_INC(CALL, hit); - JUMPBY(INLINE_CACHE_ENTRIES_CALL); PyObject *arg = TOP(); PyObject *res = PySequence_Tuple(arg); Py_DECREF(arg); @@ -3175,6 +3178,7 @@ dummy_func( if (res == NULL) { goto error; } + JUMPBY(INLINE_CACHE_ENTRIES_CALL); CHECK_EVAL_BREAKER(); } @@ -3188,7 +3192,6 @@ dummy_func( PyTypeObject *tp = (PyTypeObject *)callable; DEOPT_IF(tp->tp_vectorcall == NULL, CALL); STAT_INC(CALL, hit); - JUMPBY(INLINE_CACHE_ENTRIES_CALL); STACK_SHRINK(total_args); PyObject *res = tp->tp_vectorcall((PyObject *)tp, stack_pointer, total_args-kwnames_len, call_shape.kwnames); @@ -3203,6 +3206,7 @@ dummy_func( if (res == NULL) { goto error; } + JUMPBY(INLINE_CACHE_ENTRIES_CALL); CHECK_EVAL_BREAKER(); } @@ -3218,7 +3222,6 @@ dummy_func( DEOPT_IF(!PyCFunction_CheckExact(callable), CALL); DEOPT_IF(PyCFunction_GET_FLAGS(callable) != METH_O, CALL); STAT_INC(CALL, hit); - JUMPBY(INLINE_CACHE_ENTRIES_CALL); PyCFunction cfunc = PyCFunction_GET_FUNCTION(callable); // This is slower but CPython promises to check all non-vectorcall // function calls. @@ -3237,6 +3240,7 @@ dummy_func( if (res == NULL) { goto error; } + JUMPBY(INLINE_CACHE_ENTRIES_CALL); CHECK_EVAL_BREAKER(); } @@ -3252,7 +3256,6 @@ dummy_func( DEOPT_IF(PyCFunction_GET_FLAGS(callable) != METH_FASTCALL, CALL); STAT_INC(CALL, hit); - JUMPBY(INLINE_CACHE_ENTRIES_CALL); PyCFunction cfunc = PyCFunction_GET_FUNCTION(callable); STACK_SHRINK(total_args); /* res = func(self, args, nargs) */ @@ -3277,6 +3280,7 @@ dummy_func( */ goto error; } + JUMPBY(INLINE_CACHE_ENTRIES_CALL); CHECK_EVAL_BREAKER(); } @@ -3291,7 +3295,6 @@ dummy_func( DEOPT_IF(PyCFunction_GET_FLAGS(callable) != (METH_FASTCALL | METH_KEYWORDS), CALL); STAT_INC(CALL, hit); - JUMPBY(INLINE_CACHE_ENTRIES_CALL); STACK_SHRINK(total_args); /* res = func(self, args, nargs, kwnames) */ _PyCFunctionFastWithKeywords cfunc = @@ -3316,6 +3319,7 @@ dummy_func( if (res == NULL) { goto error; } + JUMPBY(INLINE_CACHE_ENTRIES_CALL); CHECK_EVAL_BREAKER(); } @@ -3331,7 +3335,6 @@ dummy_func( PyInterpreterState *interp = _PyInterpreterState_GET(); DEOPT_IF(callable != interp->callable_cache.len, CALL); STAT_INC(CALL, hit); - JUMPBY(INLINE_CACHE_ENTRIES_CALL); PyObject *arg = TOP(); Py_ssize_t len_i = PyObject_Length(arg); if (len_i < 0) { @@ -3347,6 +3350,7 @@ dummy_func( if (res == NULL) { goto error; } + JUMPBY(INLINE_CACHE_ENTRIES_CALL); } // stack effect: (__0, __array[oparg] -- ) @@ -3361,7 +3365,6 @@ dummy_func( PyInterpreterState *interp = _PyInterpreterState_GET(); DEOPT_IF(callable != interp->callable_cache.isinstance, CALL); STAT_INC(CALL, hit); - JUMPBY(INLINE_CACHE_ENTRIES_CALL); PyObject *cls = POP(); PyObject *inst = TOP(); int retval = PyObject_IsInstance(inst, cls); @@ -3380,6 +3383,7 @@ dummy_func( if (res == NULL) { goto error; } + JUMPBY(INLINE_CACHE_ENTRIES_CALL); } // stack effect: (__0, __array[oparg] -- ) @@ -3393,9 +3397,6 @@ dummy_func( PyObject *list = SECOND(); DEOPT_IF(!PyList_Check(list), CALL); STAT_INC(CALL, hit); - // CALL + POP_TOP - JUMPBY(INLINE_CACHE_ENTRIES_CALL + 1); - assert(_Py_OPCODE(next_instr[-1]) == POP_TOP); PyObject *arg = POP(); if (_PyList_AppendTakeRef((PyListObject *)list, arg) < 0) { goto error; @@ -3403,6 +3404,9 @@ dummy_func( STACK_SHRINK(2); Py_DECREF(list); Py_DECREF(callable); + // CALL + POP_TOP + JUMPBY(INLINE_CACHE_ENTRIES_CALL + 1); + assert(_Py_OPCODE(next_instr[-1]) == POP_TOP); } // stack effect: (__0, __array[oparg] -- ) @@ -3420,7 +3424,6 @@ dummy_func( PyObject *self = SECOND(); DEOPT_IF(!Py_IS_TYPE(self, callable->d_common.d_type), CALL); STAT_INC(CALL, hit); - JUMPBY(INLINE_CACHE_ENTRIES_CALL); PyCFunction cfunc = meth->ml_meth; // This is slower but CPython promises to check all non-vectorcall // function calls. @@ -3438,6 +3441,7 @@ dummy_func( if (res == NULL) { goto error; } + JUMPBY(INLINE_CACHE_ENTRIES_CALL); CHECK_EVAL_BREAKER(); } @@ -3454,7 +3458,6 @@ dummy_func( PyObject *self = PEEK(total_args); DEOPT_IF(!Py_IS_TYPE(self, d_type), CALL); STAT_INC(CALL, hit); - JUMPBY(INLINE_CACHE_ENTRIES_CALL); int nargs = total_args-1; STACK_SHRINK(nargs); _PyCFunctionFastWithKeywords cfunc = @@ -3475,6 +3478,7 @@ dummy_func( if (res == NULL) { goto error; } + JUMPBY(INLINE_CACHE_ENTRIES_CALL); CHECK_EVAL_BREAKER(); } @@ -3492,7 +3496,6 @@ dummy_func( DEOPT_IF(!Py_IS_TYPE(self, callable->d_common.d_type), CALL); DEOPT_IF(meth->ml_flags != METH_NOARGS, CALL); STAT_INC(CALL, hit); - JUMPBY(INLINE_CACHE_ENTRIES_CALL); PyCFunction cfunc = meth->ml_meth; // This is slower but CPython promises to check all non-vectorcall // function calls. @@ -3509,6 +3512,7 @@ dummy_func( if (res == NULL) { goto error; } + JUMPBY(INLINE_CACHE_ENTRIES_CALL); CHECK_EVAL_BREAKER(); } @@ -3526,7 +3530,6 @@ dummy_func( PyObject *self = PEEK(total_args); DEOPT_IF(!Py_IS_TYPE(self, callable->d_common.d_type), CALL); STAT_INC(CALL, hit); - JUMPBY(INLINE_CACHE_ENTRIES_CALL); _PyCFunctionFast cfunc = (_PyCFunctionFast)(void(*)(void))meth->ml_meth; int nargs = total_args-1; @@ -3544,6 +3547,7 @@ dummy_func( if (res == NULL) { goto error; } + JUMPBY(INLINE_CACHE_ENTRIES_CALL); CHECK_EVAL_BREAKER(); } diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index da6b34038cdedf..0030a1d9a4cb83 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -1143,6 +1143,8 @@ PyObject *name = GETITEM(names, oparg); next_instr--; if (_Py_Specialize_StoreAttr(owner, next_instr, name)) { + // "undo" the rewind so end up in the correct handler: + next_instr++; goto error; } DISPATCH_SAME_OPARG(); @@ -1726,6 +1728,8 @@ PyObject *name = GETITEM(names, oparg>>1); next_instr--; if (_Py_Specialize_LoadAttr(owner, next_instr, name)) { + // "undo" the rewind so end up in the correct handler: + next_instr++; goto error; } DISPATCH_SAME_OPARG(); @@ -3134,7 +3138,6 @@ PyObject *callable = PEEK(2); DEOPT_IF(callable != (PyObject *)&PyUnicode_Type, CALL); STAT_INC(CALL, hit); - JUMPBY(INLINE_CACHE_ENTRIES_CALL); PyObject *arg = TOP(); PyObject *res = PyObject_Str(arg); Py_DECREF(arg); @@ -3144,6 +3147,7 @@ if (res == NULL) { goto error; } + JUMPBY(INLINE_CACHE_ENTRIES_CALL); CHECK_EVAL_BREAKER(); DISPATCH(); } @@ -3155,7 +3159,6 @@ PyObject *callable = PEEK(2); DEOPT_IF(callable != (PyObject *)&PyTuple_Type, CALL); STAT_INC(CALL, hit); - JUMPBY(INLINE_CACHE_ENTRIES_CALL); PyObject *arg = TOP(); PyObject *res = PySequence_Tuple(arg); Py_DECREF(arg); @@ -3165,6 +3168,7 @@ if (res == NULL) { goto error; } + JUMPBY(INLINE_CACHE_ENTRIES_CALL); CHECK_EVAL_BREAKER(); DISPATCH(); } @@ -3178,7 +3182,6 @@ PyTypeObject *tp = (PyTypeObject *)callable; DEOPT_IF(tp->tp_vectorcall == NULL, CALL); STAT_INC(CALL, hit); - JUMPBY(INLINE_CACHE_ENTRIES_CALL); STACK_SHRINK(total_args); PyObject *res = tp->tp_vectorcall((PyObject *)tp, stack_pointer, total_args-kwnames_len, call_shape.kwnames); @@ -3193,6 +3196,7 @@ if (res == NULL) { goto error; } + JUMPBY(INLINE_CACHE_ENTRIES_CALL); CHECK_EVAL_BREAKER(); DISPATCH(); } @@ -3208,7 +3212,6 @@ DEOPT_IF(!PyCFunction_CheckExact(callable), CALL); DEOPT_IF(PyCFunction_GET_FLAGS(callable) != METH_O, CALL); STAT_INC(CALL, hit); - JUMPBY(INLINE_CACHE_ENTRIES_CALL); PyCFunction cfunc = PyCFunction_GET_FUNCTION(callable); // This is slower but CPython promises to check all non-vectorcall // function calls. @@ -3227,6 +3230,7 @@ if (res == NULL) { goto error; } + JUMPBY(INLINE_CACHE_ENTRIES_CALL); CHECK_EVAL_BREAKER(); DISPATCH(); } @@ -3242,7 +3246,6 @@ DEOPT_IF(PyCFunction_GET_FLAGS(callable) != METH_FASTCALL, CALL); STAT_INC(CALL, hit); - JUMPBY(INLINE_CACHE_ENTRIES_CALL); PyCFunction cfunc = PyCFunction_GET_FUNCTION(callable); STACK_SHRINK(total_args); /* res = func(self, args, nargs) */ @@ -3267,6 +3270,7 @@ */ goto error; } + JUMPBY(INLINE_CACHE_ENTRIES_CALL); CHECK_EVAL_BREAKER(); DISPATCH(); } @@ -3281,7 +3285,6 @@ DEOPT_IF(PyCFunction_GET_FLAGS(callable) != (METH_FASTCALL | METH_KEYWORDS), CALL); STAT_INC(CALL, hit); - JUMPBY(INLINE_CACHE_ENTRIES_CALL); STACK_SHRINK(total_args); /* res = func(self, args, nargs, kwnames) */ _PyCFunctionFastWithKeywords cfunc = @@ -3306,6 +3309,7 @@ if (res == NULL) { goto error; } + JUMPBY(INLINE_CACHE_ENTRIES_CALL); CHECK_EVAL_BREAKER(); DISPATCH(); } @@ -3321,7 +3325,6 @@ PyInterpreterState *interp = _PyInterpreterState_GET(); DEOPT_IF(callable != interp->callable_cache.len, CALL); STAT_INC(CALL, hit); - JUMPBY(INLINE_CACHE_ENTRIES_CALL); PyObject *arg = TOP(); Py_ssize_t len_i = PyObject_Length(arg); if (len_i < 0) { @@ -3337,6 +3340,7 @@ if (res == NULL) { goto error; } + JUMPBY(INLINE_CACHE_ENTRIES_CALL); DISPATCH(); } @@ -3351,7 +3355,6 @@ PyInterpreterState *interp = _PyInterpreterState_GET(); DEOPT_IF(callable != interp->callable_cache.isinstance, CALL); STAT_INC(CALL, hit); - JUMPBY(INLINE_CACHE_ENTRIES_CALL); PyObject *cls = POP(); PyObject *inst = TOP(); int retval = PyObject_IsInstance(inst, cls); @@ -3370,6 +3373,7 @@ if (res == NULL) { goto error; } + JUMPBY(INLINE_CACHE_ENTRIES_CALL); DISPATCH(); } @@ -3383,9 +3387,6 @@ PyObject *list = SECOND(); DEOPT_IF(!PyList_Check(list), CALL); STAT_INC(CALL, hit); - // CALL + POP_TOP - JUMPBY(INLINE_CACHE_ENTRIES_CALL + 1); - assert(_Py_OPCODE(next_instr[-1]) == POP_TOP); PyObject *arg = POP(); if (_PyList_AppendTakeRef((PyListObject *)list, arg) < 0) { goto error; @@ -3393,6 +3394,9 @@ STACK_SHRINK(2); Py_DECREF(list); Py_DECREF(callable); + // CALL + POP_TOP + JUMPBY(INLINE_CACHE_ENTRIES_CALL + 1); + assert(_Py_OPCODE(next_instr[-1]) == POP_TOP); DISPATCH(); } @@ -3410,7 +3414,6 @@ PyObject *self = SECOND(); DEOPT_IF(!Py_IS_TYPE(self, callable->d_common.d_type), CALL); STAT_INC(CALL, hit); - JUMPBY(INLINE_CACHE_ENTRIES_CALL); PyCFunction cfunc = meth->ml_meth; // This is slower but CPython promises to check all non-vectorcall // function calls. @@ -3428,6 +3431,7 @@ if (res == NULL) { goto error; } + JUMPBY(INLINE_CACHE_ENTRIES_CALL); CHECK_EVAL_BREAKER(); DISPATCH(); } @@ -3444,7 +3448,6 @@ PyObject *self = PEEK(total_args); DEOPT_IF(!Py_IS_TYPE(self, d_type), CALL); STAT_INC(CALL, hit); - JUMPBY(INLINE_CACHE_ENTRIES_CALL); int nargs = total_args-1; STACK_SHRINK(nargs); _PyCFunctionFastWithKeywords cfunc = @@ -3465,6 +3468,7 @@ if (res == NULL) { goto error; } + JUMPBY(INLINE_CACHE_ENTRIES_CALL); CHECK_EVAL_BREAKER(); DISPATCH(); } @@ -3482,7 +3486,6 @@ DEOPT_IF(!Py_IS_TYPE(self, callable->d_common.d_type), CALL); DEOPT_IF(meth->ml_flags != METH_NOARGS, CALL); STAT_INC(CALL, hit); - JUMPBY(INLINE_CACHE_ENTRIES_CALL); PyCFunction cfunc = meth->ml_meth; // This is slower but CPython promises to check all non-vectorcall // function calls. @@ -3499,6 +3502,7 @@ if (res == NULL) { goto error; } + JUMPBY(INLINE_CACHE_ENTRIES_CALL); CHECK_EVAL_BREAKER(); DISPATCH(); } @@ -3516,7 +3520,6 @@ PyObject *self = PEEK(total_args); DEOPT_IF(!Py_IS_TYPE(self, callable->d_common.d_type), CALL); STAT_INC(CALL, hit); - JUMPBY(INLINE_CACHE_ENTRIES_CALL); _PyCFunctionFast cfunc = (_PyCFunctionFast)(void(*)(void))meth->ml_meth; int nargs = total_args-1; @@ -3534,6 +3537,7 @@ if (res == NULL) { goto error; } + JUMPBY(INLINE_CACHE_ENTRIES_CALL); CHECK_EVAL_BREAKER(); DISPATCH(); } From 02085ee9849e6e6d2a5ce74e04240e43f29fd43e Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Wed, 9 Nov 2022 12:07:29 -0800 Subject: [PATCH 2/2] blurb add --- .../2022-11-09-12-07-24.gh-issue-99298.NeArAJ.rst | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2022-11-09-12-07-24.gh-issue-99298.NeArAJ.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2022-11-09-12-07-24.gh-issue-99298.NeArAJ.rst b/Misc/NEWS.d/next/Core and Builtins/2022-11-09-12-07-24.gh-issue-99298.NeArAJ.rst new file mode 100644 index 00000000000000..8908bfaa8e25f6 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2022-11-09-12-07-24.gh-issue-99298.NeArAJ.rst @@ -0,0 +1,2 @@ +Fix an issue that could potentially cause incorrect error handling for some +bytecode instructions.