diff options
author | Guido van Rossum <guido@python.org> | 2024-02-29 18:55:29 (GMT) |
---|---|---|
committer | GitHub <noreply@github.com> | 2024-02-29 18:55:29 (GMT) |
commit | 0656509033948780e6703391daca773c779041f7 (patch) | |
tree | f75c2c923b52ee8367c9f033cb911ac8dc99bfca | |
parent | 3b6f4cadf19e6a4edd2cbbbc96a0a4024b395648 (diff) | |
download | cpython-0656509033948780e6703391daca773c779041f7.zip cpython-0656509033948780e6703391daca773c779041f7.tar.gz cpython-0656509033948780e6703391daca773c779041f7.tar.bz2 |
gh-116088: Insert bottom checks after all sym_set_...() calls (#116089)
This changes the `sym_set_...()` functions to return a `bool` which is `false`
when the symbol is `bottom` after the operation.
All calls to such functions now check this result and go to `hit_bottom`,
a special error label that prints a different message and then reports
that it wasn't able to optimize the trace. No executor will be produced
in this case.
-rw-r--r-- | Include/internal/pycore_optimizer.h | 8 | ||||
-rw-r--r-- | Lib/test/test_capi/test_opt.py | 24 | ||||
-rw-r--r-- | Python/optimizer_analysis.c | 9 | ||||
-rw-r--r-- | Python/optimizer_bytecodes.c | 36 | ||||
-rw-r--r-- | Python/optimizer_cases.c.h | 36 | ||||
-rw-r--r-- | Python/optimizer_symbols.c | 36 | ||||
-rw-r--r-- | Tools/cases_generator/optimizer_generator.py | 4 |
7 files changed, 106 insertions, 47 deletions
diff --git a/Include/internal/pycore_optimizer.h b/Include/internal/pycore_optimizer.h index 4894f61..d32e6c0 100644 --- a/Include/internal/pycore_optimizer.h +++ b/Include/internal/pycore_optimizer.h @@ -91,10 +91,10 @@ extern _Py_UopsSymbol *_Py_uop_sym_new_type( extern _Py_UopsSymbol *_Py_uop_sym_new_const(_Py_UOpsContext *ctx, PyObject *const_val); extern _Py_UopsSymbol *_Py_uop_sym_new_null(_Py_UOpsContext *ctx); extern bool _Py_uop_sym_matches_type(_Py_UopsSymbol *sym, PyTypeObject *typ); -extern void _Py_uop_sym_set_null(_Py_UopsSymbol *sym); -extern void _Py_uop_sym_set_non_null(_Py_UopsSymbol *sym); -extern void _Py_uop_sym_set_type(_Py_UopsSymbol *sym, PyTypeObject *typ); -extern void _Py_uop_sym_set_const(_Py_UopsSymbol *sym, PyObject *const_val); +extern bool _Py_uop_sym_set_null(_Py_UopsSymbol *sym); +extern bool _Py_uop_sym_set_non_null(_Py_UopsSymbol *sym); +extern bool _Py_uop_sym_set_type(_Py_UopsSymbol *sym, PyTypeObject *typ); +extern bool _Py_uop_sym_set_const(_Py_UopsSymbol *sym, PyObject *const_val); extern bool _Py_uop_sym_is_bottom(_Py_UopsSymbol *sym); diff --git a/Lib/test/test_capi/test_opt.py b/Lib/test/test_capi/test_opt.py index a43726f..d217653 100644 --- a/Lib/test/test_capi/test_opt.py +++ b/Lib/test/test_capi/test_opt.py @@ -894,23 +894,33 @@ class TestUopsOptimization(unittest.TestCase): def test_type_inconsistency(self): ns = {} - exec(textwrap.dedent(""" + src = textwrap.dedent(""" def testfunc(n): for i in range(n): x = _test_global + _test_global - """), globals(), ns) + """) + exec(src, ns, ns) testfunc = ns['testfunc'] - # Must be a real global else it won't be optimized to _LOAD_CONST_INLINE - global _test_global - _test_global = 0 + ns['_test_global'] = 0 _, ex = self._run_with_optimizer(testfunc, 16) self.assertIsNone(ex) - _test_global = 1.2 + ns['_test_global'] = 1 _, ex = self._run_with_optimizer(testfunc, 16) self.assertIsNotNone(ex) uops = get_opnames(ex) - self.assertIn("_GUARD_BOTH_INT", uops) + self.assertNotIn("_GUARD_BOTH_INT", uops) self.assertIn("_BINARY_OP_ADD_INT", uops) + # Try again, but between the runs, set the global to a float. + # This should result in no executor the second time. + ns = {} + exec(src, ns, ns) + testfunc = ns['testfunc'] + ns['_test_global'] = 0 + _, ex = self._run_with_optimizer(testfunc, 16) + self.assertIsNone(ex) + ns['_test_global'] = 3.14 + _, ex = self._run_with_optimizer(testfunc, 16) + self.assertIsNone(ex) if __name__ == "__main__": diff --git a/Python/optimizer_analysis.c b/Python/optimizer_analysis.c index 2a7ef4e..a326e22 100644 --- a/Python/optimizer_analysis.c +++ b/Python/optimizer_analysis.c @@ -363,6 +363,15 @@ error: DPRINTF(1, "Encountered error in abstract interpreter\n"); _Py_uop_abstractcontext_fini(ctx); return 0; + +hit_bottom: + // Attempted to push a "bottom" (contradition) symbol onto the stack. + // This means that the abstract interpreter has hit unreachable code. + // We *could* generate an _EXIT_TRACE or _FATAL_ERROR here, but it's + // simpler to just admit failure and not create the executor. + DPRINTF(1, "Hit bottom in abstract interpreter\n"); + _Py_uop_abstractcontext_fini(ctx); + return 0; } diff --git a/Python/optimizer_bytecodes.c b/Python/optimizer_bytecodes.c index 928c22d..aa19a50 100644 --- a/Python/optimizer_bytecodes.c +++ b/Python/optimizer_bytecodes.c @@ -85,8 +85,12 @@ dummy_func(void) { sym_matches_type(right, &PyLong_Type)) { REPLACE_OP(this_instr, _NOP, 0, 0); } - sym_set_type(left, &PyLong_Type); - sym_set_type(right, &PyLong_Type); + if (!sym_set_type(left, &PyLong_Type)) { + goto hit_bottom; + } + if (!sym_set_type(right, &PyLong_Type)) { + goto hit_bottom; + } } op(_GUARD_BOTH_FLOAT, (left, right -- left, right)) { @@ -94,8 +98,12 @@ dummy_func(void) { sym_matches_type(right, &PyFloat_Type)) { REPLACE_OP(this_instr, _NOP, 0 ,0); } - sym_set_type(left, &PyFloat_Type); - sym_set_type(right, &PyFloat_Type); + if (!sym_set_type(left, &PyFloat_Type)) { + goto hit_bottom; + } + if (!sym_set_type(right, &PyFloat_Type)) { + goto hit_bottom; + } } op(_GUARD_BOTH_UNICODE, (left, right -- left, right)) { @@ -103,8 +111,12 @@ dummy_func(void) { sym_matches_type(right, &PyUnicode_Type)) { REPLACE_OP(this_instr, _NOP, 0 ,0); } - sym_set_type(left, &PyUnicode_Type); - sym_set_type(right, &PyUnicode_Type); + if (!sym_set_type(left, &PyUnicode_Type)) { + goto hit_bottom; + } + if (!sym_set_type(right, &PyUnicode_Type)) { + goto hit_bottom; + } } op(_BINARY_OP_ADD_INT, (left, right -- res)) { @@ -365,14 +377,20 @@ dummy_func(void) { op(_CHECK_FUNCTION_EXACT_ARGS, (func_version/2, callable, self_or_null, unused[oparg] -- callable, self_or_null, unused[oparg])) { - sym_set_type(callable, &PyFunction_Type); + if (!sym_set_type(callable, &PyFunction_Type)) { + goto hit_bottom; + } (void)self_or_null; (void)func_version; } op(_CHECK_CALL_BOUND_METHOD_EXACT_ARGS, (callable, null, unused[oparg] -- callable, null, unused[oparg])) { - sym_set_null(null); - sym_set_type(callable, &PyMethod_Type); + if (!sym_set_null(null)) { + goto hit_bottom; + } + if (!sym_set_type(callable, &PyMethod_Type)) { + goto hit_bottom; + } } op(_INIT_CALL_PY_EXACT_ARGS, (callable, self_or_null, args[oparg] -- new_frame: _Py_UOpsAbstractFrame *)) { diff --git a/Python/optimizer_cases.c.h b/Python/optimizer_cases.c.h index b38c03b..b34c573 100644 --- a/Python/optimizer_cases.c.h +++ b/Python/optimizer_cases.c.h @@ -172,8 +172,12 @@ sym_matches_type(right, &PyLong_Type)) { REPLACE_OP(this_instr, _NOP, 0, 0); } - sym_set_type(left, &PyLong_Type); - sym_set_type(right, &PyLong_Type); + if (!sym_set_type(left, &PyLong_Type)) { + goto hit_bottom; + } + if (!sym_set_type(right, &PyLong_Type)) { + goto hit_bottom; + } break; } @@ -276,8 +280,12 @@ sym_matches_type(right, &PyFloat_Type)) { REPLACE_OP(this_instr, _NOP, 0 ,0); } - sym_set_type(left, &PyFloat_Type); - sym_set_type(right, &PyFloat_Type); + if (!sym_set_type(left, &PyFloat_Type)) { + goto hit_bottom; + } + if (!sym_set_type(right, &PyFloat_Type)) { + goto hit_bottom; + } break; } @@ -383,8 +391,12 @@ sym_matches_type(right, &PyUnicode_Type)) { REPLACE_OP(this_instr, _NOP, 0 ,0); } - sym_set_type(left, &PyUnicode_Type); - sym_set_type(right, &PyUnicode_Type); + if (!sym_set_type(left, &PyUnicode_Type)) { + goto hit_bottom; + } + if (!sym_set_type(right, &PyUnicode_Type)) { + goto hit_bottom; + } break; } @@ -1397,8 +1409,12 @@ _Py_UopsSymbol *callable; null = stack_pointer[-1 - oparg]; callable = stack_pointer[-2 - oparg]; - sym_set_null(null); - sym_set_type(callable, &PyMethod_Type); + if (!sym_set_null(null)) { + goto hit_bottom; + } + if (!sym_set_type(callable, &PyMethod_Type)) { + goto hit_bottom; + } break; } @@ -1425,7 +1441,9 @@ self_or_null = stack_pointer[-1 - oparg]; callable = stack_pointer[-2 - oparg]; uint32_t func_version = (uint32_t)this_instr->operand; - sym_set_type(callable, &PyFunction_Type); + if (!sym_set_type(callable, &PyFunction_Type)) { + goto hit_bottom; + } (void)self_or_null; (void)func_version; break; diff --git a/Python/optimizer_symbols.c b/Python/optimizer_symbols.c index a529cc2..5c3ec2b 100644 --- a/Python/optimizer_symbols.c +++ b/Python/optimizer_symbols.c @@ -113,60 +113,68 @@ _Py_uop_sym_get_const(_Py_UopsSymbol *sym) return sym->const_val; } -void +bool _Py_uop_sym_set_type(_Py_UopsSymbol *sym, PyTypeObject *typ) { assert(typ != NULL && PyType_Check(typ)); if (sym->flags & IS_NULL) { sym_set_bottom(sym); - return; + return false; } if (sym->typ != NULL) { if (sym->typ != typ) { sym_set_bottom(sym); + return false; } - return; } - sym_set_flag(sym, NOT_NULL); - sym->typ = typ; + else { + sym_set_flag(sym, NOT_NULL); + sym->typ = typ; + } + return true; } -void +bool _Py_uop_sym_set_const(_Py_UopsSymbol *sym, PyObject *const_val) { assert(const_val != NULL); if (sym->flags & IS_NULL) { sym_set_bottom(sym); - return; + return false; } PyTypeObject *typ = Py_TYPE(const_val); if (sym->typ != NULL && sym->typ != typ) { sym_set_bottom(sym); - return; + return false; } if (sym->const_val != NULL) { if (sym->const_val != const_val) { // TODO: What if they're equal? sym_set_bottom(sym); + return false; } - return; } - sym_set_flag(sym, NOT_NULL); - sym->typ = typ; - sym->const_val = Py_NewRef(const_val); + else { + sym_set_flag(sym, NOT_NULL); + sym->typ = typ; + sym->const_val = Py_NewRef(const_val); + } + return true; } -void +bool _Py_uop_sym_set_null(_Py_UopsSymbol *sym) { sym_set_flag(sym, IS_NULL); + return !_Py_uop_sym_is_bottom(sym); } -void +bool _Py_uop_sym_set_non_null(_Py_UopsSymbol *sym) { sym_set_flag(sym, NOT_NULL); + return !_Py_uop_sym_is_bottom(sym); } diff --git a/Tools/cases_generator/optimizer_generator.py b/Tools/cases_generator/optimizer_generator.py index d3ce4c8..fca42b5 100644 --- a/Tools/cases_generator/optimizer_generator.py +++ b/Tools/cases_generator/optimizer_generator.py @@ -4,16 +4,12 @@ Writes the cases to optimizer_cases.c.h, which is #included in Python/optimizer_ """ import argparse -import os.path -import sys from analyzer import ( Analysis, Instruction, Uop, - Part, analyze_files, - Skip, StackItem, analysis_error, ) |