Skip to content

Commit 0656509

Browse files
gvanrossumgvanrossum
authored
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.
1 parent 3b6f4ca commit 0656509

File tree

7 files changed

+106
-47
lines changed

7 files changed

+106
-47
lines changed

‎Include/internal/pycore_optimizer.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -91,10 +91,10 @@ extern _Py_UopsSymbol *_Py_uop_sym_new_type(
9191
extern _Py_UopsSymbol *_Py_uop_sym_new_const(_Py_UOpsContext *ctx, PyObject *const_val);
9292
extern _Py_UopsSymbol *_Py_uop_sym_new_null(_Py_UOpsContext *ctx);
9393
extern bool _Py_uop_sym_matches_type(_Py_UopsSymbol *sym, PyTypeObject *typ);
94-
extern void _Py_uop_sym_set_null(_Py_UopsSymbol *sym);
95-
extern void _Py_uop_sym_set_non_null(_Py_UopsSymbol *sym);
96-
extern void _Py_uop_sym_set_type(_Py_UopsSymbol *sym, PyTypeObject *typ);
97-
extern void _Py_uop_sym_set_const(_Py_UopsSymbol *sym, PyObject *const_val);
94+
extern bool _Py_uop_sym_set_null(_Py_UopsSymbol *sym);
95+
extern bool _Py_uop_sym_set_non_null(_Py_UopsSymbol *sym);
96+
extern bool _Py_uop_sym_set_type(_Py_UopsSymbol *sym, PyTypeObject *typ);
97+
extern bool _Py_uop_sym_set_const(_Py_UopsSymbol *sym, PyObject *const_val);
9898
extern bool _Py_uop_sym_is_bottom(_Py_UopsSymbol *sym);
9999

100100

‎Lib/test/test_capi/test_opt.py

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -894,23 +894,33 @@ def testfunc(n):
894894

895895
def test_type_inconsistency(self):
896896
ns = {}
897-
exec(textwrap.dedent("""
897+
src = textwrap.dedent("""
898898
def testfunc(n):
899899
for i in range(n):
900900
x = _test_global + _test_global
901-
"""), globals(), ns)
901+
""")
902+
exec(src, ns, ns)
902903
testfunc = ns['testfunc']
903-
# Must be a real global else it won't be optimized to _LOAD_CONST_INLINE
904-
global _test_global
905-
_test_global = 0
904+
ns['_test_global'] = 0
906905
_, ex = self._run_with_optimizer(testfunc, 16)
907906
self.assertIsNone(ex)
908-
_test_global = 1.2
907+
ns['_test_global'] = 1
909908
_, ex = self._run_with_optimizer(testfunc, 16)
910909
self.assertIsNotNone(ex)
911910
uops = get_opnames(ex)
912-
self.assertIn("_GUARD_BOTH_INT", uops)
911+
self.assertNotIn("_GUARD_BOTH_INT", uops)
913912
self.assertIn("_BINARY_OP_ADD_INT", uops)
913+
# Try again, but between the runs, set the global to a float.
914+
# This should result in no executor the second time.
915+
ns = {}
916+
exec(src, ns, ns)
917+
testfunc = ns['testfunc']
918+
ns['_test_global'] = 0
919+
_, ex = self._run_with_optimizer(testfunc, 16)
920+
self.assertIsNone(ex)
921+
ns['_test_global'] = 3.14
922+
_, ex = self._run_with_optimizer(testfunc, 16)
923+
self.assertIsNone(ex)
914924

915925

916926
if __name__ == "__main__":

‎Python/optimizer_analysis.c

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -363,6 +363,15 @@ optimize_uops(
363363
DPRINTF(1, "Encountered error in abstract interpreter\n");
364364
_Py_uop_abstractcontext_fini(ctx);
365365
return 0;
366+
367+
hit_bottom:
368+
// Attempted to push a "bottom" (contradition) symbol onto the stack.
369+
// This means that the abstract interpreter has hit unreachable code.
370+
// We *could* generate an _EXIT_TRACE or _FATAL_ERROR here, but it's
371+
// simpler to just admit failure and not create the executor.
372+
DPRINTF(1, "Hit bottom in abstract interpreter\n");
373+
_Py_uop_abstractcontext_fini(ctx);
374+
return 0;
366375
}
367376

368377

‎Python/optimizer_bytecodes.c

Lines changed: 27 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -85,26 +85,38 @@ dummy_func(void) {
8585
sym_matches_type(right, &PyLong_Type)) {
8686
REPLACE_OP(this_instr, _NOP, 0, 0);
8787
}
88-
sym_set_type(left, &PyLong_Type);
89-
sym_set_type(right, &PyLong_Type);
88+
if (!sym_set_type(left, &PyLong_Type)) {
89+
goto hit_bottom;
90+
}
91+
if (!sym_set_type(right, &PyLong_Type)) {
92+
goto hit_bottom;
93+
}
9094
}
9195

9296
op(_GUARD_BOTH_FLOAT, (left, right -- left, right)) {
9397
if (sym_matches_type(left, &PyFloat_Type) &&
9498
sym_matches_type(right, &PyFloat_Type)) {
9599
REPLACE_OP(this_instr, _NOP, 0 ,0);
96100
}
97-
sym_set_type(left, &PyFloat_Type);
98-
sym_set_type(right, &PyFloat_Type);
101+
if (!sym_set_type(left, &PyFloat_Type)) {
102+
goto hit_bottom;
103+
}
104+
if (!sym_set_type(right, &PyFloat_Type)) {
105+
goto hit_bottom;
106+
}
99107
}
100108

101109
op(_GUARD_BOTH_UNICODE, (left, right -- left, right)) {
102110
if (sym_matches_type(left, &PyUnicode_Type) &&
103111
sym_matches_type(right, &PyUnicode_Type)) {
104112
REPLACE_OP(this_instr, _NOP, 0 ,0);
105113
}
106-
sym_set_type(left, &PyUnicode_Type);
107-
sym_set_type(right, &PyUnicode_Type);
114+
if (!sym_set_type(left, &PyUnicode_Type)) {
115+
goto hit_bottom;
116+
}
117+
if (!sym_set_type(right, &PyUnicode_Type)) {
118+
goto hit_bottom;
119+
}
108120
}
109121

110122
op(_BINARY_OP_ADD_INT, (left, right -- res)) {
@@ -365,14 +377,20 @@ dummy_func(void) {
365377

366378

367379
op(_CHECK_FUNCTION_EXACT_ARGS, (func_version/2, callable, self_or_null, unused[oparg] -- callable, self_or_null, unused[oparg])) {
368-
sym_set_type(callable, &PyFunction_Type);
380+
if (!sym_set_type(callable, &PyFunction_Type)) {
381+
goto hit_bottom;
382+
}
369383
(void)self_or_null;
370384
(void)func_version;
371385
}
372386

373387
op(_CHECK_CALL_BOUND_METHOD_EXACT_ARGS, (callable, null, unused[oparg] -- callable, null, unused[oparg])) {
374-
sym_set_null(null);
375-
sym_set_type(callable, &PyMethod_Type);
388+
if (!sym_set_null(null)) {
389+
goto hit_bottom;
390+
}
391+
if (!sym_set_type(callable, &PyMethod_Type)) {
392+
goto hit_bottom;
393+
}
376394
}
377395

378396
op(_INIT_CALL_PY_EXACT_ARGS, (callable, self_or_null, args[oparg] -- new_frame: _Py_UOpsAbstractFrame *)) {

‎Python/optimizer_cases.c.h

Lines changed: 27 additions & 9 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎Python/optimizer_symbols.c

Lines changed: 22 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -113,60 +113,68 @@ _Py_uop_sym_get_const(_Py_UopsSymbol *sym)
113113
return sym->const_val;
114114
}
115115

116-
void
116+
bool
117117
_Py_uop_sym_set_type(_Py_UopsSymbol *sym, PyTypeObject *typ)
118118
{
119119
assert(typ != NULL && PyType_Check(typ));
120120
if (sym->flags & IS_NULL) {
121121
sym_set_bottom(sym);
122-
return;
122+
return false;
123123
}
124124
if (sym->typ != NULL) {
125125
if (sym->typ != typ) {
126126
sym_set_bottom(sym);
127+
return false;
127128
}
128-
return;
129129
}
130-
sym_set_flag(sym, NOT_NULL);
131-
sym->typ = typ;
130+
else {
131+
sym_set_flag(sym, NOT_NULL);
132+
sym->typ = typ;
133+
}
134+
return true;
132135
}
133136

134-
void
137+
bool
135138
_Py_uop_sym_set_const(_Py_UopsSymbol *sym, PyObject *const_val)
136139
{
137140
assert(const_val != NULL);
138141
if (sym->flags & IS_NULL) {
139142
sym_set_bottom(sym);
140-
return;
143+
return false;
141144
}
142145
PyTypeObject *typ = Py_TYPE(const_val);
143146
if (sym->typ != NULL && sym->typ != typ) {
144147
sym_set_bottom(sym);
145-
return;
148+
return false;
146149
}
147150
if (sym->const_val != NULL) {
148151
if (sym->const_val != const_val) {
149152
// TODO: What if they're equal?
150153
sym_set_bottom(sym);
154+
return false;
151155
}
152-
return;
153156
}
154-
sym_set_flag(sym, NOT_NULL);
155-
sym->typ = typ;
156-
sym->const_val = Py_NewRef(const_val);
157+
else {
158+
sym_set_flag(sym, NOT_NULL);
159+
sym->typ = typ;
160+
sym->const_val = Py_NewRef(const_val);
161+
}
162+
return true;
157163
}
158164

159165

160-
void
166+
bool
161167
_Py_uop_sym_set_null(_Py_UopsSymbol *sym)
162168
{
163169
sym_set_flag(sym, IS_NULL);
170+
return !_Py_uop_sym_is_bottom(sym);
164171
}
165172

166-
void
173+
bool
167174
_Py_uop_sym_set_non_null(_Py_UopsSymbol *sym)
168175
{
169176
sym_set_flag(sym, NOT_NULL);
177+
return !_Py_uop_sym_is_bottom(sym);
170178
}
171179

172180

‎Tools/cases_generator/optimizer_generator.py

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,16 +4,12 @@
44
"""
55

66
import argparse
7-
import os.path
8-
import sys
97

108
from analyzer import (
119
Analysis,
1210
Instruction,
1311
Uop,
14-
Part,
1512
analyze_files,
16-
Skip,
1713
StackItem,
1814
analysis_error,
1915
)

0 commit comments

Comments
 (0)