From 26fc7a3627f896ef0cb8aef062fdfe8a165e0ab7 Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Tue, 31 Jan 2023 00:38:38 +0000 Subject: [PATCH 1/5] gh-98831: rewrite GET_LEN, GET_ITER, BEFORE_WITH and a few simple opcodes in the instruction definition DSL --- Python/bytecodes.c | 62 ++++++++++++++------------------------ Python/generated_cases.c.h | 48 +++++++++++++++-------------- Python/opcode_metadata.h | 28 ++++++++--------- 3 files changed, 61 insertions(+), 77 deletions(-) diff --git a/Python/bytecodes.c b/Python/bytecodes.c index d1e59f7908b580..97fe07462fe181 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -2053,8 +2053,7 @@ dummy_func( } } - // stack effect: ( -- ) - inst(JUMP_BACKWARD_NO_INTERRUPT) { + inst(JUMP_BACKWARD_NO_INTERRUPT, (--)) { /* This bytecode is used in the `yield from` or `await` loop. * If there is an interrupt, we want it handled in the innermost * generator or coroutine, so we deliberately do not check it here. @@ -2063,18 +2062,12 @@ dummy_func( JUMPBY(-oparg); } - // stack effect: ( -- __0) - inst(GET_LEN) { + inst(GET_LEN, (obj -- obj, len_o)) { // PUSH(len(TOS)) - Py_ssize_t len_i = PyObject_Length(TOP()); - if (len_i < 0) { - goto error; - } - PyObject *len_o = PyLong_FromSsize_t(len_i); - if (len_o == NULL) { - goto error; - } - PUSH(len_o); + Py_ssize_t len_i = PyObject_Length(obj); + ERROR_IF(len_i < 0, error); + len_o = PyLong_FromSsize_t(len_i); + ERROR_IF(len_o == NULL, error); } inst(MATCH_CLASS, (subject, type, names -- attrs)) { @@ -2110,15 +2103,11 @@ dummy_func( ERROR_IF(values_or_none == NULL, error); } - // stack effect: ( -- ) - inst(GET_ITER) { + inst(GET_ITER, (iterable -- iter)) { /* before: [obj]; after [getiter(obj)] */ - PyObject *iterable = TOP(); - PyObject *iter = PyObject_GetIter(iterable); - Py_DECREF(iterable); - SET_TOP(iter); - if (iter == NULL) - goto error; + iter = PyObject_GetIter(iterable); + DECREF_INPUTS(); + ERROR_IF(iter == NULL, error); } // stack effect: ( -- ) @@ -2313,10 +2302,10 @@ dummy_func( PREDICT(GET_AWAITABLE); } - // stack effect: ( -- __0) - inst(BEFORE_WITH) { - PyObject *mgr = TOP(); - PyObject *res; + inst(BEFORE_WITH, (mgr -- exit, res)) { + /* pop the context manager, push its __exit__ and the + * value returned from calling its __enter__ + */ PyObject *enter = _PyObject_LookupSpecial(mgr, &_Py_ID(__enter__)); if (enter == NULL) { if (!_PyErr_Occurred(tstate)) { @@ -2325,9 +2314,9 @@ dummy_func( "context manager protocol", Py_TYPE(mgr)->tp_name); } - goto error; + ERROR_IF(true, error); } - PyObject *exit = _PyObject_LookupSpecial(mgr, &_Py_ID(__exit__)); + exit = _PyObject_LookupSpecial(mgr, &_Py_ID(__exit__)); if (exit == NULL) { if (!_PyErr_Occurred(tstate)) { _PyErr_Format(tstate, PyExc_TypeError, @@ -2337,16 +2326,12 @@ dummy_func( Py_TYPE(mgr)->tp_name); } Py_DECREF(enter); - goto error; + ERROR_IF(true, error); } - SET_TOP(exit); - Py_DECREF(mgr); + DECREF_INPUTS(); res = _PyObject_CallNoArgs(enter); Py_DECREF(enter); - if (res == NULL) { - goto error; - } - PUSH(res); + ERROR_IF(res == NULL, error); } inst(WITH_EXCEPT_START, (exit_func, lasti, unused, val -- exit_func, lasti, unused, val, res)) { @@ -2469,8 +2454,7 @@ dummy_func( GO_TO_INSTRUCTION(CALL_PY_EXACT_ARGS); } - // stack effect: ( -- ) - inst(KW_NAMES) { + inst(KW_NAMES, (--)) { assert(kwnames == NULL); assert(oparg < PyTuple_GET_SIZE(consts)); kwnames = GETITEM(consts, oparg); @@ -3252,8 +3236,7 @@ dummy_func( PEEK(oparg) = top; } - // stack effect: ( -- ) - inst(EXTENDED_ARG) { + inst(EXTENDED_ARG, (--)) { assert(oparg); assert(cframe.use_tracing == 0); opcode = _Py_OPCODE(*next_instr); @@ -3262,8 +3245,7 @@ dummy_func( DISPATCH_GOTO(); } - // stack effect: ( -- ) - inst(CACHE) { + inst(CACHE, (--)) { Py_UNREACHABLE(); } diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index 3ee30ae8df9e3c..1123b862602538 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -2460,16 +2460,15 @@ } TARGET(GET_LEN) { + PyObject *obj = PEEK(1); + PyObject *len_o; // PUSH(len(TOS)) - Py_ssize_t len_i = PyObject_Length(TOP()); - if (len_i < 0) { - goto error; - } - PyObject *len_o = PyLong_FromSsize_t(len_i); - if (len_o == NULL) { - goto error; - } - PUSH(len_o); + Py_ssize_t len_i = PyObject_Length(obj); + if (len_i < 0) goto error; + len_o = PyLong_FromSsize_t(len_i); + if (len_o == NULL) goto error; + STACK_GROW(1); + POKE(1, len_o); DISPATCH(); } @@ -2532,13 +2531,13 @@ } TARGET(GET_ITER) { + PyObject *iterable = PEEK(1); + PyObject *iter; /* before: [obj]; after [getiter(obj)] */ - PyObject *iterable = TOP(); - PyObject *iter = PyObject_GetIter(iterable); + iter = PyObject_GetIter(iterable); Py_DECREF(iterable); - SET_TOP(iter); - if (iter == NULL) - goto error; + if (iter == NULL) goto pop_1_error; + POKE(1, iter); DISPATCH(); } @@ -2736,8 +2735,12 @@ } TARGET(BEFORE_WITH) { - PyObject *mgr = TOP(); + PyObject *mgr = PEEK(1); + PyObject *exit; PyObject *res; + /* pop the context manager, push its __exit__ and the + * value returned from calling its __enter__ + */ PyObject *enter = _PyObject_LookupSpecial(mgr, &_Py_ID(__enter__)); if (enter == NULL) { if (!_PyErr_Occurred(tstate)) { @@ -2746,9 +2749,9 @@ "context manager protocol", Py_TYPE(mgr)->tp_name); } - goto error; + if (true) goto pop_1_error; } - PyObject *exit = _PyObject_LookupSpecial(mgr, &_Py_ID(__exit__)); + exit = _PyObject_LookupSpecial(mgr, &_Py_ID(__exit__)); if (exit == NULL) { if (!_PyErr_Occurred(tstate)) { _PyErr_Format(tstate, PyExc_TypeError, @@ -2758,16 +2761,15 @@ Py_TYPE(mgr)->tp_name); } Py_DECREF(enter); - goto error; + if (true) goto pop_1_error; } - SET_TOP(exit); Py_DECREF(mgr); res = _PyObject_CallNoArgs(enter); Py_DECREF(enter); - if (res == NULL) { - goto error; - } - PUSH(res); + if (res == NULL) goto pop_1_error; + STACK_GROW(1); + POKE(1, res); + POKE(2, exit); DISPATCH(); } diff --git a/Python/opcode_metadata.h b/Python/opcode_metadata.h index e76ddda2f0292d..c40e40ff324d11 100644 --- a/Python/opcode_metadata.h +++ b/Python/opcode_metadata.h @@ -245,9 +245,9 @@ _PyOpcode_num_popped(int opcode, int oparg) { case JUMP_IF_TRUE_OR_POP: return -1; case JUMP_BACKWARD_NO_INTERRUPT: - return -1; + return 0; case GET_LEN: - return -1; + return 1; case MATCH_CLASS: return 3; case MATCH_MAPPING: @@ -257,7 +257,7 @@ _PyOpcode_num_popped(int opcode, int oparg) { case MATCH_KEYS: return 2; case GET_ITER: - return -1; + return 1; case GET_YIELD_FROM_ITER: return -1; case FOR_ITER: @@ -273,7 +273,7 @@ _PyOpcode_num_popped(int opcode, int oparg) { case BEFORE_ASYNC_WITH: return -1; case BEFORE_WITH: - return -1; + return 1; case WITH_EXCEPT_START: return 4; case PUSH_EXC_INFO: @@ -287,7 +287,7 @@ _PyOpcode_num_popped(int opcode, int oparg) { case CALL_BOUND_METHOD_EXACT_ARGS: return -1; case KW_NAMES: - return -1; + return 0; case CALL: return -1; case CALL_PY_EXACT_ARGS: @@ -339,9 +339,9 @@ _PyOpcode_num_popped(int opcode, int oparg) { case SWAP: return -1; case EXTENDED_ARG: - return -1; + return 0; case CACHE: - return -1; + return 0; default: Py_UNREACHABLE(); } @@ -591,9 +591,9 @@ _PyOpcode_num_pushed(int opcode, int oparg) { case JUMP_IF_TRUE_OR_POP: return -1; case JUMP_BACKWARD_NO_INTERRUPT: - return -1; + return 0; case GET_LEN: - return -1; + return 2; case MATCH_CLASS: return 1; case MATCH_MAPPING: @@ -603,7 +603,7 @@ _PyOpcode_num_pushed(int opcode, int oparg) { case MATCH_KEYS: return 3; case GET_ITER: - return -1; + return 1; case GET_YIELD_FROM_ITER: return -1; case FOR_ITER: @@ -619,7 +619,7 @@ _PyOpcode_num_pushed(int opcode, int oparg) { case BEFORE_ASYNC_WITH: return -1; case BEFORE_WITH: - return -1; + return 2; case WITH_EXCEPT_START: return 5; case PUSH_EXC_INFO: @@ -633,7 +633,7 @@ _PyOpcode_num_pushed(int opcode, int oparg) { case CALL_BOUND_METHOD_EXACT_ARGS: return -1; case KW_NAMES: - return -1; + return 0; case CALL: return -1; case CALL_PY_EXACT_ARGS: @@ -685,9 +685,9 @@ _PyOpcode_num_pushed(int opcode, int oparg) { case SWAP: return -1; case EXTENDED_ARG: - return -1; + return 0; case CACHE: - return -1; + return 0; default: Py_UNREACHABLE(); } From 6bfec269fec0b2f61590356a3b1e04e249d4e909 Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Tue, 31 Jan 2023 01:10:57 +0000 Subject: [PATCH 2/5] goto instead of ERROR_IF to avoid leaks --- Python/bytecodes.c | 4 ++-- Python/generated_cases.c.h | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Python/bytecodes.c b/Python/bytecodes.c index 97fe07462fe181..3d387bb39ad05d 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -2314,7 +2314,7 @@ dummy_func( "context manager protocol", Py_TYPE(mgr)->tp_name); } - ERROR_IF(true, error); + goto error; } exit = _PyObject_LookupSpecial(mgr, &_Py_ID(__exit__)); if (exit == NULL) { @@ -2326,7 +2326,7 @@ dummy_func( Py_TYPE(mgr)->tp_name); } Py_DECREF(enter); - ERROR_IF(true, error); + goto error; } DECREF_INPUTS(); res = _PyObject_CallNoArgs(enter); diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index 1123b862602538..4c28845a4e64a6 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -2749,7 +2749,7 @@ "context manager protocol", Py_TYPE(mgr)->tp_name); } - if (true) goto pop_1_error; + goto error; } exit = _PyObject_LookupSpecial(mgr, &_Py_ID(__exit__)); if (exit == NULL) { @@ -2761,7 +2761,7 @@ Py_TYPE(mgr)->tp_name); } Py_DECREF(enter); - if (true) goto pop_1_error; + goto error; } Py_DECREF(mgr); res = _PyObject_CallNoArgs(enter); From 2ee6241848fc2b33229801b4663d6626012a2843 Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Tue, 31 Jan 2023 01:40:55 +0000 Subject: [PATCH 3/5] revert BEFORE_WITH --- Python/bytecodes.c | 18 +++++++++++------- Python/generated_cases.c.h | 17 +++++++---------- 2 files changed, 18 insertions(+), 17 deletions(-) diff --git a/Python/bytecodes.c b/Python/bytecodes.c index 3d387bb39ad05d..b333ba3894cf6f 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -2302,10 +2302,10 @@ dummy_func( PREDICT(GET_AWAITABLE); } - inst(BEFORE_WITH, (mgr -- exit, res)) { - /* pop the context manager, push its __exit__ and the - * value returned from calling its __enter__ - */ + // stack effect: ( -- __0) + inst(BEFORE_WITH) { + PyObject *mgr = TOP(); + PyObject *res; PyObject *enter = _PyObject_LookupSpecial(mgr, &_Py_ID(__enter__)); if (enter == NULL) { if (!_PyErr_Occurred(tstate)) { @@ -2316,7 +2316,7 @@ dummy_func( } goto error; } - exit = _PyObject_LookupSpecial(mgr, &_Py_ID(__exit__)); + PyObject *exit = _PyObject_LookupSpecial(mgr, &_Py_ID(__exit__)); if (exit == NULL) { if (!_PyErr_Occurred(tstate)) { _PyErr_Format(tstate, PyExc_TypeError, @@ -2328,10 +2328,14 @@ dummy_func( Py_DECREF(enter); goto error; } - DECREF_INPUTS(); + SET_TOP(exit); + Py_DECREF(mgr); res = _PyObject_CallNoArgs(enter); Py_DECREF(enter); - ERROR_IF(res == NULL, error); + if (res == NULL) { + goto error; + } + PUSH(res); } inst(WITH_EXCEPT_START, (exit_func, lasti, unused, val -- exit_func, lasti, unused, val, res)) { diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index 4c28845a4e64a6..6e03dd56666bcb 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -2735,12 +2735,8 @@ } TARGET(BEFORE_WITH) { - PyObject *mgr = PEEK(1); - PyObject *exit; + PyObject *mgr = TOP(); PyObject *res; - /* pop the context manager, push its __exit__ and the - * value returned from calling its __enter__ - */ PyObject *enter = _PyObject_LookupSpecial(mgr, &_Py_ID(__enter__)); if (enter == NULL) { if (!_PyErr_Occurred(tstate)) { @@ -2751,7 +2747,7 @@ } goto error; } - exit = _PyObject_LookupSpecial(mgr, &_Py_ID(__exit__)); + PyObject *exit = _PyObject_LookupSpecial(mgr, &_Py_ID(__exit__)); if (exit == NULL) { if (!_PyErr_Occurred(tstate)) { _PyErr_Format(tstate, PyExc_TypeError, @@ -2763,13 +2759,14 @@ Py_DECREF(enter); goto error; } + SET_TOP(exit); Py_DECREF(mgr); res = _PyObject_CallNoArgs(enter); Py_DECREF(enter); - if (res == NULL) goto pop_1_error; - STACK_GROW(1); - POKE(1, res); - POKE(2, exit); + if (res == NULL) { + goto error; + } + PUSH(res); DISPATCH(); } From 0a5f78dda13878be08d1eef08a83343c7896142d Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Tue, 31 Jan 2023 01:47:16 +0000 Subject: [PATCH 4/5] Revert "revert BEFORE_WITH" This reverts commit 2ee6241848fc2b33229801b4663d6626012a2843. --- Python/bytecodes.c | 18 +++++++----------- Python/generated_cases.c.h | 17 ++++++++++------- 2 files changed, 17 insertions(+), 18 deletions(-) diff --git a/Python/bytecodes.c b/Python/bytecodes.c index b333ba3894cf6f..3d387bb39ad05d 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -2302,10 +2302,10 @@ dummy_func( PREDICT(GET_AWAITABLE); } - // stack effect: ( -- __0) - inst(BEFORE_WITH) { - PyObject *mgr = TOP(); - PyObject *res; + inst(BEFORE_WITH, (mgr -- exit, res)) { + /* pop the context manager, push its __exit__ and the + * value returned from calling its __enter__ + */ PyObject *enter = _PyObject_LookupSpecial(mgr, &_Py_ID(__enter__)); if (enter == NULL) { if (!_PyErr_Occurred(tstate)) { @@ -2316,7 +2316,7 @@ dummy_func( } goto error; } - PyObject *exit = _PyObject_LookupSpecial(mgr, &_Py_ID(__exit__)); + exit = _PyObject_LookupSpecial(mgr, &_Py_ID(__exit__)); if (exit == NULL) { if (!_PyErr_Occurred(tstate)) { _PyErr_Format(tstate, PyExc_TypeError, @@ -2328,14 +2328,10 @@ dummy_func( Py_DECREF(enter); goto error; } - SET_TOP(exit); - Py_DECREF(mgr); + DECREF_INPUTS(); res = _PyObject_CallNoArgs(enter); Py_DECREF(enter); - if (res == NULL) { - goto error; - } - PUSH(res); + ERROR_IF(res == NULL, error); } inst(WITH_EXCEPT_START, (exit_func, lasti, unused, val -- exit_func, lasti, unused, val, res)) { diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index 6e03dd56666bcb..4c28845a4e64a6 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -2735,8 +2735,12 @@ } TARGET(BEFORE_WITH) { - PyObject *mgr = TOP(); + PyObject *mgr = PEEK(1); + PyObject *exit; PyObject *res; + /* pop the context manager, push its __exit__ and the + * value returned from calling its __enter__ + */ PyObject *enter = _PyObject_LookupSpecial(mgr, &_Py_ID(__enter__)); if (enter == NULL) { if (!_PyErr_Occurred(tstate)) { @@ -2747,7 +2751,7 @@ } goto error; } - PyObject *exit = _PyObject_LookupSpecial(mgr, &_Py_ID(__exit__)); + exit = _PyObject_LookupSpecial(mgr, &_Py_ID(__exit__)); if (exit == NULL) { if (!_PyErr_Occurred(tstate)) { _PyErr_Format(tstate, PyExc_TypeError, @@ -2759,14 +2763,13 @@ Py_DECREF(enter); goto error; } - SET_TOP(exit); Py_DECREF(mgr); res = _PyObject_CallNoArgs(enter); Py_DECREF(enter); - if (res == NULL) { - goto error; - } - PUSH(res); + if (res == NULL) goto pop_1_error; + STACK_GROW(1); + POKE(1, res); + POKE(2, exit); DISPATCH(); } From a92549887af2f81f3ffd3b136a986ed8aad3bd06 Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Tue, 31 Jan 2023 01:50:18 +0000 Subject: [PATCH 5/5] fix refleak of exit when __enter__ raises --- Python/bytecodes.c | 5 ++++- Python/generated_cases.c.h | 5 ++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/Python/bytecodes.c b/Python/bytecodes.c index 3d387bb39ad05d..5b3bf4ec7ba796 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -2331,7 +2331,10 @@ dummy_func( DECREF_INPUTS(); res = _PyObject_CallNoArgs(enter); Py_DECREF(enter); - ERROR_IF(res == NULL, error); + if (res == NULL) { + Py_DECREF(exit); + ERROR_IF(true, error); + } } inst(WITH_EXCEPT_START, (exit_func, lasti, unused, val -- exit_func, lasti, unused, val, res)) { diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index 4c28845a4e64a6..661fe27f327b33 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -2766,7 +2766,10 @@ Py_DECREF(mgr); res = _PyObject_CallNoArgs(enter); Py_DECREF(enter); - if (res == NULL) goto pop_1_error; + if (res == NULL) { + Py_DECREF(exit); + if (true) goto pop_1_error; + } STACK_GROW(1); POKE(1, res); POKE(2, exit);