[Python-checkins] gh-104615: don't make unsafe swaps in apply_static_swaps (#104620)
carljm
webhook-mailer at python.org
Thu May 18 17:22:22 EDT 2023
https://github.com/python/cpython/commit/0589c6a4d3d822cace42050198cb9a5e99c879ad
commit: 0589c6a4d3d822cace42050198cb9a5e99c879ad
branch: main
author: Carl Meyer <carl at oddbird.net>
committer: carljm <carl at oddbird.net>
date: 2023-05-18T21:22:03Z
summary:
gh-104615: don't make unsafe swaps in apply_static_swaps (#104620)
files:
A Misc/NEWS.d/next/Core and Builtins/2023-05-18-13-00-21.gh-issue-104615.h_rtw2.rst
M Include/internal/pycore_compile.h
M Include/internal/pycore_global_objects_fini_generated.h
M Include/internal/pycore_global_strings.h
M Include/internal/pycore_runtime_init_generated.h
M Include/internal/pycore_unicodeobject_generated.h
M Lib/test/support/bytecode_helper.py
M Lib/test/test_compile.py
M Lib/test/test_listcomps.py
M Lib/test/test_peepholer.py
M Modules/_testinternalcapi.c
M Modules/clinic/_testinternalcapi.c.h
M Python/compile.c
M Python/flowgraph.c
diff --git a/Include/internal/pycore_compile.h b/Include/internal/pycore_compile.h
index 499f55f3e276..80a637e5bf9a 100644
--- a/Include/internal/pycore_compile.h
+++ b/Include/internal/pycore_compile.h
@@ -105,7 +105,8 @@ PyAPI_FUNC(PyObject*) _PyCompile_CodeGen(
PyAPI_FUNC(PyObject*) _PyCompile_OptimizeCfg(
PyObject *instructions,
- PyObject *consts);
+ PyObject *consts,
+ int nlocals);
PyAPI_FUNC(PyCodeObject*)
_PyCompile_Assemble(_PyCompile_CodeUnitMetadata *umd, PyObject *filename,
diff --git a/Include/internal/pycore_global_objects_fini_generated.h b/Include/internal/pycore_global_objects_fini_generated.h
index 24a268ac8c43..eeaa2ad96c90 100644
--- a/Include/internal/pycore_global_objects_fini_generated.h
+++ b/Include/internal/pycore_global_objects_fini_generated.h
@@ -1071,6 +1071,7 @@ _PyStaticObjects_CheckRefcnt(PyInterpreterState *interp) {
_PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(newline));
_PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(newlines));
_PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(next));
+ _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(nlocals));
_PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(node_depth));
_PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(node_offset));
_PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(ns));
diff --git a/Include/internal/pycore_global_strings.h b/Include/internal/pycore_global_strings.h
index c1005d051552..5cc790d126be 100644
--- a/Include/internal/pycore_global_strings.h
+++ b/Include/internal/pycore_global_strings.h
@@ -559,6 +559,7 @@ struct _Py_global_strings {
STRUCT_FOR_ID(newline)
STRUCT_FOR_ID(newlines)
STRUCT_FOR_ID(next)
+ STRUCT_FOR_ID(nlocals)
STRUCT_FOR_ID(node_depth)
STRUCT_FOR_ID(node_offset)
STRUCT_FOR_ID(ns)
diff --git a/Include/internal/pycore_runtime_init_generated.h b/Include/internal/pycore_runtime_init_generated.h
index ff1dee6eacfe..0cb24a92dffa 100644
--- a/Include/internal/pycore_runtime_init_generated.h
+++ b/Include/internal/pycore_runtime_init_generated.h
@@ -1065,6 +1065,7 @@ extern "C" {
INIT_ID(newline), \
INIT_ID(newlines), \
INIT_ID(next), \
+ INIT_ID(nlocals), \
INIT_ID(node_depth), \
INIT_ID(node_offset), \
INIT_ID(ns), \
diff --git a/Include/internal/pycore_unicodeobject_generated.h b/Include/internal/pycore_unicodeobject_generated.h
index ba6b37f1bf55..fe2479beb8a3 100644
--- a/Include/internal/pycore_unicodeobject_generated.h
+++ b/Include/internal/pycore_unicodeobject_generated.h
@@ -1518,6 +1518,9 @@ _PyUnicode_InitStaticStrings(PyInterpreterState *interp) {
string = &_Py_ID(next);
assert(_PyUnicode_CheckConsistency(string, 1));
_PyUnicode_InternInPlace(interp, &string);
+ string = &_Py_ID(nlocals);
+ assert(_PyUnicode_CheckConsistency(string, 1));
+ _PyUnicode_InternInPlace(interp, &string);
string = &_Py_ID(node_depth);
assert(_PyUnicode_CheckConsistency(string, 1));
_PyUnicode_InternInPlace(interp, &string);
diff --git a/Lib/test/support/bytecode_helper.py b/Lib/test/support/bytecode_helper.py
index 7b577f54b8ad..388d1266773c 100644
--- a/Lib/test/support/bytecode_helper.py
+++ b/Lib/test/support/bytecode_helper.py
@@ -130,10 +130,10 @@ def generate_code(self, ast):
class CfgOptimizationTestCase(CompilationStepTestCase):
- def get_optimized(self, insts, consts):
+ def get_optimized(self, insts, consts, nlocals=0):
insts = self.normalize_insts(insts)
insts = self.complete_insts_info(insts)
- insts = optimize_cfg(insts, consts)
+ insts = optimize_cfg(insts, consts, nlocals)
return insts, consts
class AssemblerTestCase(CompilationStepTestCase):
diff --git a/Lib/test/test_compile.py b/Lib/test/test_compile.py
index c68b9ce38846..784c0550cc09 100644
--- a/Lib/test/test_compile.py
+++ b/Lib/test/test_compile.py
@@ -1168,6 +1168,24 @@ def foo(param, lambda_exp):
""")
compile(code, "<test>", "exec")
+ def test_apply_static_swaps(self):
+ def f(x, y):
+ a, a = x, y
+ return a
+ self.assertEqual(f("x", "y"), "y")
+
+ def test_apply_static_swaps_2(self):
+ def f(x, y, z):
+ a, b, a = x, y, z
+ return a
+ self.assertEqual(f("x", "y", "z"), "z")
+
+ def test_apply_static_swaps_3(self):
+ def f(x, y, z):
+ a, a, b = x, y, z
+ return a
+ self.assertEqual(f("x", "y", "z"), "y")
+
@requires_debug_ranges()
class TestSourcePositions(unittest.TestCase):
diff --git a/Lib/test/test_listcomps.py b/Lib/test/test_listcomps.py
index 985274dfd6cb..ae63f4a35394 100644
--- a/Lib/test/test_listcomps.py
+++ b/Lib/test/test_listcomps.py
@@ -484,6 +484,12 @@ def test_nested_listcomp_in_lambda(self):
"""
self._check_in_scopes(code, {"z": 1, "out": [(3, 2, 1)]})
+ def test_assign_to_comp_iter_var_in_outer_function(self):
+ code = """
+ a = [1 for a in [0]]
+ """
+ self._check_in_scopes(code, {"a": [1]}, scopes=["function"])
+
__test__ = {'doctests' : doctests}
diff --git a/Lib/test/test_peepholer.py b/Lib/test/test_peepholer.py
index bf7fc421a9df..255e92804214 100644
--- a/Lib/test/test_peepholer.py
+++ b/Lib/test/test_peepholer.py
@@ -971,13 +971,14 @@ def trace(frame, event, arg):
self.assertNotInBytecode(f, "LOAD_FAST_CHECK")
-class DirectiCfgOptimizerTests(CfgOptimizationTestCase):
+class DirectCfgOptimizerTests(CfgOptimizationTestCase):
def cfg_optimization_test(self, insts, expected_insts,
- consts=None, expected_consts=None):
+ consts=None, expected_consts=None,
+ nlocals=0):
if expected_consts is None:
expected_consts = consts
- opt_insts, opt_consts = self.get_optimized(insts, consts)
+ opt_insts, opt_consts = self.get_optimized(insts, consts, nlocals)
expected_insts = self.normalize_insts(expected_insts)
self.assertInstructionsMatch(opt_insts, expected_insts)
self.assertEqual(opt_consts, expected_consts)
@@ -1058,6 +1059,19 @@ def test_conditional_jump_backward_const_condition(self):
]
self.cfg_optimization_test(insts, expected_insts, consts=list(range(5)))
+ def test_no_unsafe_static_swap(self):
+ # We can't change order of two stores to the same location
+ insts = [
+ ('LOAD_CONST', 0, 1),
+ ('LOAD_CONST', 1, 2),
+ ('LOAD_CONST', 2, 3),
+ ('SWAP', 3, 4),
+ ('STORE_FAST', 1, 4),
+ ('STORE_FAST', 1, 4),
+ ('POP_TOP', 0, 4),
+ ('RETURN_VALUE', 5)
+ ]
+ self.cfg_optimization_test(insts, insts, consts=list(range(3)), nlocals=1)
if __name__ == "__main__":
unittest.main()
diff --git a/Misc/NEWS.d/next/Core and Builtins/2023-05-18-13-00-21.gh-issue-104615.h_rtw2.rst b/Misc/NEWS.d/next/Core and Builtins/2023-05-18-13-00-21.gh-issue-104615.h_rtw2.rst
new file mode 100644
index 000000000000..447291a619dd
--- /dev/null
+++ b/Misc/NEWS.d/next/Core and Builtins/2023-05-18-13-00-21.gh-issue-104615.h_rtw2.rst
@@ -0,0 +1,2 @@
+Fix wrong ordering of assignments in code like ``a, a = x, y``. Contributed by
+Carl Meyer.
diff --git a/Modules/_testinternalcapi.c b/Modules/_testinternalcapi.c
index 5802f1d4ff5f..3ee332393083 100644
--- a/Modules/_testinternalcapi.c
+++ b/Modules/_testinternalcapi.c
@@ -616,16 +616,17 @@ _testinternalcapi.optimize_cfg -> object
instructions: object
consts: object
+ nlocals: int
Apply compiler optimizations to an instruction list.
[clinic start generated code]*/
static PyObject *
_testinternalcapi_optimize_cfg_impl(PyObject *module, PyObject *instructions,
- PyObject *consts)
-/*[clinic end generated code: output=5412aeafca683c8b input=7e8a3de86ebdd0f9]*/
+ PyObject *consts, int nlocals)
+/*[clinic end generated code: output=57c53c3a3dfd1df0 input=6a96d1926d58d7e5]*/
{
- return _PyCompile_OptimizeCfg(instructions, consts);
+ return _PyCompile_OptimizeCfg(instructions, consts, nlocals);
}
static int
diff --git a/Modules/clinic/_testinternalcapi.c.h b/Modules/clinic/_testinternalcapi.c.h
index 41dd50437956..f51241258745 100644
--- a/Modules/clinic/_testinternalcapi.c.h
+++ b/Modules/clinic/_testinternalcapi.c.h
@@ -83,7 +83,7 @@ _testinternalcapi_compiler_codegen(PyObject *module, PyObject *const *args, Py_s
}
PyDoc_STRVAR(_testinternalcapi_optimize_cfg__doc__,
-"optimize_cfg($module, /, instructions, consts)\n"
+"optimize_cfg($module, /, instructions, consts, nlocals)\n"
"--\n"
"\n"
"Apply compiler optimizations to an instruction list.");
@@ -93,7 +93,7 @@ PyDoc_STRVAR(_testinternalcapi_optimize_cfg__doc__,
static PyObject *
_testinternalcapi_optimize_cfg_impl(PyObject *module, PyObject *instructions,
- PyObject *consts);
+ PyObject *consts, int nlocals);
static PyObject *
_testinternalcapi_optimize_cfg(PyObject *module, PyObject *const *args, Py_ssize_t nargs, PyObject *kwnames)
@@ -101,14 +101,14 @@ _testinternalcapi_optimize_cfg(PyObject *module, PyObject *const *args, Py_ssize
PyObject *return_value = NULL;
#if defined(Py_BUILD_CORE) && !defined(Py_BUILD_CORE_MODULE)
- #define NUM_KEYWORDS 2
+ #define NUM_KEYWORDS 3
static struct {
PyGC_Head _this_is_not_used;
PyObject_VAR_HEAD
PyObject *ob_item[NUM_KEYWORDS];
} _kwtuple = {
.ob_base = PyVarObject_HEAD_INIT(&PyTuple_Type, NUM_KEYWORDS)
- .ob_item = { &_Py_ID(instructions), &_Py_ID(consts), },
+ .ob_item = { &_Py_ID(instructions), &_Py_ID(consts), &_Py_ID(nlocals), },
};
#undef NUM_KEYWORDS
#define KWTUPLE (&_kwtuple.ob_base.ob_base)
@@ -117,24 +117,29 @@ _testinternalcapi_optimize_cfg(PyObject *module, PyObject *const *args, Py_ssize
# define KWTUPLE NULL
#endif // !Py_BUILD_CORE
- static const char * const _keywords[] = {"instructions", "consts", NULL};
+ static const char * const _keywords[] = {"instructions", "consts", "nlocals", NULL};
static _PyArg_Parser _parser = {
.keywords = _keywords,
.fname = "optimize_cfg",
.kwtuple = KWTUPLE,
};
#undef KWTUPLE
- PyObject *argsbuf[2];
+ PyObject *argsbuf[3];
PyObject *instructions;
PyObject *consts;
+ int nlocals;
- args = _PyArg_UnpackKeywords(args, nargs, NULL, kwnames, &_parser, 2, 2, 0, argsbuf);
+ args = _PyArg_UnpackKeywords(args, nargs, NULL, kwnames, &_parser, 3, 3, 0, argsbuf);
if (!args) {
goto exit;
}
instructions = args[0];
consts = args[1];
- return_value = _testinternalcapi_optimize_cfg_impl(module, instructions, consts);
+ nlocals = _PyLong_AsInt(args[2]);
+ if (nlocals == -1 && PyErr_Occurred()) {
+ goto exit;
+ }
+ return_value = _testinternalcapi_optimize_cfg_impl(module, instructions, consts, nlocals);
exit:
return return_value;
@@ -201,4 +206,4 @@ _testinternalcapi_assemble_code_object(PyObject *module, PyObject *const *args,
exit:
return return_value;
}
-/*[clinic end generated code: output=ab661d56a14b1a1c input=a9049054013a1b77]*/
+/*[clinic end generated code: output=2965f1578b986218 input=a9049054013a1b77]*/
diff --git a/Python/compile.c b/Python/compile.c
index 07f8d6684770..96cd6daa29fa 100644
--- a/Python/compile.c
+++ b/Python/compile.c
@@ -7996,7 +7996,7 @@ _PyCompile_CodeGen(PyObject *ast, PyObject *filename, PyCompilerFlags *pflags,
}
PyObject *
-_PyCompile_OptimizeCfg(PyObject *instructions, PyObject *consts)
+_PyCompile_OptimizeCfg(PyObject *instructions, PyObject *consts, int nlocals)
{
PyObject *res = NULL;
PyObject *const_cache = PyDict_New();
@@ -8008,7 +8008,7 @@ _PyCompile_OptimizeCfg(PyObject *instructions, PyObject *consts)
if (instructions_to_cfg(instructions, &g) < 0) {
goto error;
}
- int code_flags = 0, nlocals = 0, nparams = 0, firstlineno = 1;
+ int code_flags = 0, nparams = 0, firstlineno = 1;
if (_PyCfg_OptimizeCodeUnit(&g, consts, const_cache, code_flags, nlocals,
nparams, firstlineno) < 0) {
goto error;
diff --git a/Python/flowgraph.c b/Python/flowgraph.c
index 7f790b79d284..f8039a4985d9 100644
--- a/Python/flowgraph.c
+++ b/Python/flowgraph.c
@@ -1293,6 +1293,11 @@ swaptimize(basicblock *block, int *ix)
(opcode) == STORE_FAST_MAYBE_NULL || \
(opcode) == POP_TOP)
+#define STORES_TO(instr) \
+ (((instr).i_opcode == STORE_FAST || \
+ (instr).i_opcode == STORE_FAST_MAYBE_NULL) \
+ ? (instr).i_oparg : -1)
+
static int
next_swappable_instruction(basicblock *block, int i, int lineno)
{
@@ -1344,6 +1349,23 @@ apply_static_swaps(basicblock *block, int i)
return;
}
}
+ // The reordering is not safe if the two instructions to be swapped
+ // store to the same location, or if any intervening instruction stores
+ // to the same location as either of them.
+ int store_j = STORES_TO(block->b_instr[j]);
+ int store_k = STORES_TO(block->b_instr[k]);
+ if (store_j >= 0 || store_k >= 0) {
+ if (store_j == store_k) {
+ return;
+ }
+ for (int idx = j + 1; idx < k; idx++) {
+ int store_idx = STORES_TO(block->b_instr[idx]);
+ if (store_idx >= 0 && (store_idx == store_j || store_idx == store_k)) {
+ return;
+ }
+ }
+ }
+
// Success!
INSTR_SET_OP0(swap, NOP);
cfg_instr temp = block->b_instr[j];
More information about the Python-checkins
mailing list