From c43d20bac16da25db5eeb12772fad15be9fe5326 Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Fri, 3 Feb 2023 10:51:27 -0800 Subject: [PATCH 1/3] Special-case CHECK_EVAL_BREAKER() --- Tools/cases_generator/generate_cases.py | 15 ++++++++++++--- Tools/cases_generator/test_generator.py | 9 +++++++-- 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/Tools/cases_generator/generate_cases.py b/Tools/cases_generator/generate_cases.py index 3925583b40e728..1f16aee373041e 100644 --- a/Tools/cases_generator/generate_cases.py +++ b/Tools/cases_generator/generate_cases.py @@ -230,7 +230,8 @@ def __init__(self, inst: parser.InstDef): self.kind = inst.kind self.name = inst.name self.block = inst.block - self.block_text, self.predictions = extract_block_text(self.block) + self.block_text, self.check_eval_breaker, self.predictions = \ + extract_block_text(self.block) self.always_exits = always_exits(self.block_text) self.cache_effects = [ effect for effect in inst.inputs if isinstance(effect, parser.CacheEffect) @@ -1016,6 +1017,8 @@ def write_instr(self, instr: Instruction) -> None: if not instr.always_exits: for prediction in instr.predictions: self.out.emit(f"PREDICT({prediction});") + if instr.check_eval_breaker: + self.out.emit("CHECK_EVAL_BREAKER();") self.out.emit(f"DISPATCH();") def write_super(self, sup: SuperInstruction) -> None: @@ -1091,7 +1094,7 @@ def wrap_super_or_macro(self, up: SuperOrMacroInstruction): self.out.emit(f"DISPATCH();") -def extract_block_text(block: parser.Block) -> tuple[list[str], list[str]]: +def extract_block_text(block: parser.Block) -> tuple[list[str], bool, list[str]]: # Get lines of text with proper dedent blocklines = block.text.splitlines(True) @@ -1111,6 +1114,12 @@ def extract_block_text(block: parser.Block) -> tuple[list[str], list[str]]: while blocklines and not blocklines[-1].strip(): blocklines.pop() + # Separate CHECK_EVAL_BREAKER() macro from end + check_eval_breaker = \ + blocklines != [] and blocklines[-1].strip() == "CHECK_EVAL_BREAKER();" + if check_eval_breaker: + del blocklines[-1] + # Separate PREDICT(...) macros from end predictions: list[str] = [] while blocklines and ( @@ -1119,7 +1128,7 @@ def extract_block_text(block: parser.Block) -> tuple[list[str], list[str]]: predictions.insert(0, m.group(1)) blocklines.pop() - return blocklines, predictions + return blocklines, check_eval_breaker, predictions def always_exits(lines: list[str]) -> bool: diff --git a/Tools/cases_generator/test_generator.py b/Tools/cases_generator/test_generator.py index 9df97d24ab6f43..2c952308b8968d 100644 --- a/Tools/cases_generator/test_generator.py +++ b/Tools/cases_generator/test_generator.py @@ -177,15 +177,16 @@ def test_overlap(): """ run_cases_test(input, output) -def test_predictions(): +def test_predictions_and_eval_breaker(): input = """ inst(OP1, (--)) { } inst(OP2, (--)) { } - inst(OP3, (--)) { + inst(OP3, (arg -- res)) { DEOPT_IF(xxx, OP1); PREDICT(OP2); + CHECK_EVAL_BREAKER(); } """ output = """ @@ -200,8 +201,12 @@ def test_predictions(): } TARGET(OP3) { + PyObject *arg = PEEK(1); + PyObject *res; DEOPT_IF(xxx, OP1); + POKE(1, res); PREDICT(OP2); + CHECK_EVAL_BREAKER(); DISPATCH(); } """ From f2bad0debbbdaf74e79410a1d6d12aea1337c867 Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Mon, 6 Feb 2023 17:14:40 -0800 Subject: [PATCH 2/3] Modernize CALL_FUNCTION_EX --- Python/bytecodes.c | 18 ++++-------------- Python/generated_cases.c.h | 23 +++++++++++------------ Python/opcode_metadata.h | 4 ++-- 3 files changed, 17 insertions(+), 28 deletions(-) diff --git a/Python/bytecodes.c b/Python/bytecodes.c index ec0439af98e632..72fa4fa4890752 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -2949,20 +2949,14 @@ dummy_func( CHECK_EVAL_BREAKER(); } - // error: CALL_FUNCTION_EX has irregular stack effect - inst(CALL_FUNCTION_EX) { - PyObject *func, *callargs, *kwargs = NULL, *result; - if (oparg & 0x01) { - kwargs = POP(); + inst(CALL_FUNCTION_EX, (null, func, callargs, kwargs if (oparg & 1) -- result)) { + if (oparg & 1) { // DICT_MERGE is called before this opcode if there are kwargs. // It converts all dict subtypes in kwargs into regular dicts. assert(PyDict_CheckExact(kwargs)); } - callargs = POP(); - func = TOP(); if (!PyTuple_CheckExact(callargs)) { if (check_args_iterable(tstate, func, callargs) < 0) { - Py_DECREF(callargs); goto error; } Py_SETREF(callargs, PySequence_Tuple(callargs)); @@ -2977,12 +2971,8 @@ dummy_func( Py_DECREF(callargs); Py_XDECREF(kwargs); - STACK_SHRINK(1); - assert(TOP() == NULL); - SET_TOP(result); - if (result == NULL) { - goto error; - } + assert(null == NULL); + ERROR_IF(result == NULL, error); CHECK_EVAL_BREAKER(); } diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index 4e511f43d95028..5605491b11b446 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -3565,18 +3565,18 @@ TARGET(CALL_FUNCTION_EX) { PREDICTED(CALL_FUNCTION_EX); - PyObject *func, *callargs, *kwargs = NULL, *result; - if (oparg & 0x01) { - kwargs = POP(); + PyObject *kwargs = (oparg & 1) ? PEEK(((oparg & 1) ? 1 : 0)) : NULL; + PyObject *callargs = PEEK(1 + ((oparg & 1) ? 1 : 0)); + PyObject *func = PEEK(2 + ((oparg & 1) ? 1 : 0)); + PyObject *null = PEEK(3 + ((oparg & 1) ? 1 : 0)); + PyObject *result; + if (oparg & 1) { // DICT_MERGE is called before this opcode if there are kwargs. // It converts all dict subtypes in kwargs into regular dicts. assert(PyDict_CheckExact(kwargs)); } - callargs = POP(); - func = TOP(); if (!PyTuple_CheckExact(callargs)) { if (check_args_iterable(tstate, func, callargs) < 0) { - Py_DECREF(callargs); goto error; } Py_SETREF(callargs, PySequence_Tuple(callargs)); @@ -3591,12 +3591,11 @@ Py_DECREF(callargs); Py_XDECREF(kwargs); - STACK_SHRINK(1); - assert(TOP() == NULL); - SET_TOP(result); - if (result == NULL) { - goto error; - } + assert(null == NULL); + if (result == NULL) { STACK_SHRINK(((oparg & 1) ? 1 : 0)); goto pop_3_error; } + STACK_SHRINK(((oparg & 1) ? 1 : 0)); + STACK_SHRINK(2); + POKE(1, result); CHECK_EVAL_BREAKER(); DISPATCH(); } diff --git a/Python/opcode_metadata.h b/Python/opcode_metadata.h index ed26ff00c7b182..707edcc93b788b 100644 --- a/Python/opcode_metadata.h +++ b/Python/opcode_metadata.h @@ -323,7 +323,7 @@ _PyOpcode_num_popped(int opcode, int oparg, bool jump) { case CALL_NO_KW_METHOD_DESCRIPTOR_FAST: return -1; case CALL_FUNCTION_EX: - return -1; + return ((oparg & 1) ? 1 : 0) + 3; case MAKE_FUNCTION: return ((oparg & 0x01) ? 1 : 0) + ((oparg & 0x02) ? 1 : 0) + ((oparg & 0x04) ? 1 : 0) + ((oparg & 0x08) ? 1 : 0) + 1; case RETURN_GENERATOR: @@ -669,7 +669,7 @@ _PyOpcode_num_pushed(int opcode, int oparg, bool jump) { case CALL_NO_KW_METHOD_DESCRIPTOR_FAST: return -1; case CALL_FUNCTION_EX: - return -1; + return 1; case MAKE_FUNCTION: return 1; case RETURN_GENERATOR: From 19307bad9df775d7db31c0df1e806486293c8cc2 Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Tue, 7 Feb 2023 12:25:22 -0800 Subject: [PATCH 3/3] Fix crash due to refcount shenanigans Also avoid compiler warning for unused variable 'null' (used in assert only). --- Python/bytecodes.c | 9 +++++---- Python/generated_cases.c.h | 8 ++++---- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/Python/bytecodes.c b/Python/bytecodes.c index 72fa4fa4890752..bb853be421910c 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -2949,7 +2949,7 @@ dummy_func( CHECK_EVAL_BREAKER(); } - inst(CALL_FUNCTION_EX, (null, func, callargs, kwargs if (oparg & 1) -- result)) { + inst(CALL_FUNCTION_EX, (unused, func, callargs, kwargs if (oparg & 1) -- result)) { if (oparg & 1) { // DICT_MERGE is called before this opcode if there are kwargs. // It converts all dict subtypes in kwargs into regular dicts. @@ -2959,10 +2959,11 @@ dummy_func( if (check_args_iterable(tstate, func, callargs) < 0) { goto error; } - Py_SETREF(callargs, PySequence_Tuple(callargs)); - if (callargs == NULL) { + PyObject *tuple = PySequence_Tuple(callargs); + if (tuple == NULL) { goto error; } + Py_SETREF(callargs, tuple); } assert(PyTuple_CheckExact(callargs)); @@ -2971,7 +2972,7 @@ dummy_func( Py_DECREF(callargs); Py_XDECREF(kwargs); - assert(null == NULL); + assert(PEEK(3 + (oparg & 1)) == NULL); ERROR_IF(result == NULL, error); CHECK_EVAL_BREAKER(); } diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index 5605491b11b446..7669ea94b4d37f 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -3568,7 +3568,6 @@ PyObject *kwargs = (oparg & 1) ? PEEK(((oparg & 1) ? 1 : 0)) : NULL; PyObject *callargs = PEEK(1 + ((oparg & 1) ? 1 : 0)); PyObject *func = PEEK(2 + ((oparg & 1) ? 1 : 0)); - PyObject *null = PEEK(3 + ((oparg & 1) ? 1 : 0)); PyObject *result; if (oparg & 1) { // DICT_MERGE is called before this opcode if there are kwargs. @@ -3579,10 +3578,11 @@ if (check_args_iterable(tstate, func, callargs) < 0) { goto error; } - Py_SETREF(callargs, PySequence_Tuple(callargs)); - if (callargs == NULL) { + PyObject *tuple = PySequence_Tuple(callargs); + if (tuple == NULL) { goto error; } + Py_SETREF(callargs, tuple); } assert(PyTuple_CheckExact(callargs)); @@ -3591,7 +3591,7 @@ Py_DECREF(callargs); Py_XDECREF(kwargs); - assert(null == NULL); + assert(PEEK(3 + (oparg & 1)) == NULL); if (result == NULL) { STACK_SHRINK(((oparg & 1) ? 1 : 0)); goto pop_3_error; } STACK_SHRINK(((oparg & 1) ? 1 : 0)); STACK_SHRINK(2);