summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorGuido van Rossum <guido@python.org>2024-02-29 18:55:29 (GMT)
committerGitHub <noreply@github.com>2024-02-29 18:55:29 (GMT)
commit0656509033948780e6703391daca773c779041f7 (patch)
treef75c2c923b52ee8367c9f033cb911ac8dc99bfca
parent3b6f4cadf19e6a4edd2cbbbc96a0a4024b395648 (diff)
downloadcpython-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.h8
-rw-r--r--Lib/test/test_capi/test_opt.py24
-rw-r--r--Python/optimizer_analysis.c9
-rw-r--r--Python/optimizer_bytecodes.c36
-rw-r--r--Python/optimizer_cases.c.h36
-rw-r--r--Python/optimizer_symbols.c36
-rw-r--r--Tools/cases_generator/optimizer_generator.py4
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,
)