From 00ee6d506ec399d30687c6cb420c8b3e22664577 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Thu, 10 Nov 2022 08:46:56 -0800 Subject: [PATCH] GH-99298: Don't perform jumps before error handling (GH-99299) --- ...2-11-09-12-07-24.gh-issue-99298.NeArAJ.rst | 2 ++ Python/bytecodes.c | 34 +++++++++++-------- Python/generated_cases.c.h | 34 +++++++++++-------- 3 files changed, 40 insertions(+), 30 deletions(-) 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. diff --git a/Python/bytecodes.c b/Python/bytecodes.c index ed12b94e3142c1..80f9b1ed8d6ba9 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -1146,6 +1146,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(); @@ -1725,6 +1727,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(); @@ -3113,7 +3117,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); @@ -3123,6 +3126,7 @@ dummy_func( if (res == NULL) { goto error; } + JUMPBY(INLINE_CACHE_ENTRIES_CALL); CHECK_EVAL_BREAKER(); } @@ -3134,7 +3138,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); @@ -3144,6 +3147,7 @@ dummy_func( if (res == NULL) { goto error; } + JUMPBY(INLINE_CACHE_ENTRIES_CALL); CHECK_EVAL_BREAKER(); } @@ -3157,7 +3161,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, kwnames); @@ -3172,6 +3175,7 @@ dummy_func( if (res == NULL) { goto error; } + JUMPBY(INLINE_CACHE_ENTRIES_CALL); CHECK_EVAL_BREAKER(); } @@ -3187,7 +3191,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. @@ -3206,6 +3209,7 @@ dummy_func( if (res == NULL) { goto error; } + JUMPBY(INLINE_CACHE_ENTRIES_CALL); CHECK_EVAL_BREAKER(); } @@ -3221,7 +3225,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) */ @@ -3246,6 +3249,7 @@ dummy_func( */ goto error; } + JUMPBY(INLINE_CACHE_ENTRIES_CALL); CHECK_EVAL_BREAKER(); } @@ -3260,7 +3264,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 = @@ -3285,6 +3288,7 @@ dummy_func( if (res == NULL) { goto error; } + JUMPBY(INLINE_CACHE_ENTRIES_CALL); CHECK_EVAL_BREAKER(); } @@ -3300,7 +3304,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) { @@ -3316,6 +3319,7 @@ dummy_func( if (res == NULL) { goto error; } + JUMPBY(INLINE_CACHE_ENTRIES_CALL); } // stack effect: (__0, __array[oparg] -- ) @@ -3330,7 +3334,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); @@ -3349,6 +3352,7 @@ dummy_func( if (res == NULL) { goto error; } + JUMPBY(INLINE_CACHE_ENTRIES_CALL); } // stack effect: (__0, __array[oparg] -- ) @@ -3362,9 +3366,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; @@ -3372,6 +3373,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] -- ) @@ -3389,7 +3393,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. @@ -3407,6 +3410,7 @@ dummy_func( if (res == NULL) { goto error; } + JUMPBY(INLINE_CACHE_ENTRIES_CALL); CHECK_EVAL_BREAKER(); } @@ -3423,7 +3427,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 = @@ -3444,6 +3447,7 @@ dummy_func( if (res == NULL) { goto error; } + JUMPBY(INLINE_CACHE_ENTRIES_CALL); CHECK_EVAL_BREAKER(); } @@ -3461,7 +3465,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. @@ -3478,6 +3481,7 @@ dummy_func( if (res == NULL) { goto error; } + JUMPBY(INLINE_CACHE_ENTRIES_CALL); CHECK_EVAL_BREAKER(); } @@ -3495,7 +3499,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; @@ -3513,6 +3516,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 0487851db6106e..c8045cc189483d 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -1137,6 +1137,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(); @@ -1716,6 +1718,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(); @@ -3104,7 +3108,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); @@ -3114,6 +3117,7 @@ if (res == NULL) { goto error; } + JUMPBY(INLINE_CACHE_ENTRIES_CALL); CHECK_EVAL_BREAKER(); DISPATCH(); } @@ -3125,7 +3129,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); @@ -3135,6 +3138,7 @@ if (res == NULL) { goto error; } + JUMPBY(INLINE_CACHE_ENTRIES_CALL); CHECK_EVAL_BREAKER(); DISPATCH(); } @@ -3148,7 +3152,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, kwnames); @@ -3163,6 +3166,7 @@ if (res == NULL) { goto error; } + JUMPBY(INLINE_CACHE_ENTRIES_CALL); CHECK_EVAL_BREAKER(); DISPATCH(); } @@ -3178,7 +3182,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. @@ -3197,6 +3200,7 @@ if (res == NULL) { goto error; } + JUMPBY(INLINE_CACHE_ENTRIES_CALL); CHECK_EVAL_BREAKER(); DISPATCH(); } @@ -3212,7 +3216,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) */ @@ -3237,6 +3240,7 @@ */ goto error; } + JUMPBY(INLINE_CACHE_ENTRIES_CALL); CHECK_EVAL_BREAKER(); DISPATCH(); } @@ -3251,7 +3255,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 = @@ -3276,6 +3279,7 @@ if (res == NULL) { goto error; } + JUMPBY(INLINE_CACHE_ENTRIES_CALL); CHECK_EVAL_BREAKER(); DISPATCH(); } @@ -3291,7 +3295,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) { @@ -3307,6 +3310,7 @@ if (res == NULL) { goto error; } + JUMPBY(INLINE_CACHE_ENTRIES_CALL); DISPATCH(); } @@ -3321,7 +3325,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); @@ -3340,6 +3343,7 @@ if (res == NULL) { goto error; } + JUMPBY(INLINE_CACHE_ENTRIES_CALL); DISPATCH(); } @@ -3353,9 +3357,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; @@ -3363,6 +3364,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(); } @@ -3380,7 +3384,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. @@ -3398,6 +3401,7 @@ if (res == NULL) { goto error; } + JUMPBY(INLINE_CACHE_ENTRIES_CALL); CHECK_EVAL_BREAKER(); DISPATCH(); } @@ -3414,7 +3418,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 = @@ -3435,6 +3438,7 @@ if (res == NULL) { goto error; } + JUMPBY(INLINE_CACHE_ENTRIES_CALL); CHECK_EVAL_BREAKER(); DISPATCH(); } @@ -3452,7 +3456,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. @@ -3469,6 +3472,7 @@ if (res == NULL) { goto error; } + JUMPBY(INLINE_CACHE_ENTRIES_CALL); CHECK_EVAL_BREAKER(); DISPATCH(); } @@ -3486,7 +3490,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; @@ -3504,6 +3507,7 @@ if (res == NULL) { goto error; } + JUMPBY(INLINE_CACHE_ENTRIES_CALL); CHECK_EVAL_BREAKER(); DISPATCH(); }