From eb83a264882a44ec08ee50eb9b2bff477c2812f1 Mon Sep 17 00:00:00 2001 From: Carl Meyer <carl@oddbird.net> Date: Thu, 18 May 2023 17:10:18 -0700 Subject: [PATCH] [3.11] gh-104615: don't make unsafe swaps in apply_static_swaps (GH-104620). (cherry picked from commit 0589c6a4d3d822cace42050198cb9a5e99c879ad) Co-authored-by: Carl Meyer <carl@oddbird.net> --- Lib/test/test_compile.py | 18 +++++++++++++++++ ...-05-18-13-00-21.gh-issue-104615.h_rtw2.rst | 2 ++ Python/compile.c | 20 +++++++++++++++++++ 3 files changed, 40 insertions(+) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2023-05-18-13-00-21.gh-issue-104615.h_rtw2.rst diff --git a/Lib/test/test_compile.py b/Lib/test/test_compile.py index c96ae4375df883..c756d43a3cf3b8 100644 --- a/Lib/test/test_compile.py +++ b/Lib/test/test_compile.py @@ -1064,6 +1064,24 @@ def test_compare_positions(self): with self.subTest(source): self.assertEqual(actual_positions, expected_positions) + 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/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 00000000000000..447291a619dd67 --- /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/Python/compile.c b/Python/compile.c index f87a423acd1f02..1c712fba31573e 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -8690,6 +8690,9 @@ swaptimize(basicblock *block, int *ix) #define SWAPPABLE(opcode) \ ((opcode) == STORE_FAST || (opcode) == POP_TOP) +#define STORES_TO(instr) \ + (((instr).i_opcode == STORE_FAST) ? (instr).i_oparg : -1) + static int next_swappable_instruction(basicblock *block, int i, int lineno) { @@ -8741,6 +8744,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! swap->i_opcode = NOP; struct instr temp = block->b_instr[j];