Skip to content

Commit

Permalink
pythongh-104615: don't make unsafe swaps in apply_static_swaps
Browse files Browse the repository at this point in the history
  • Loading branch information
carljm committed May 18, 2023
1 parent dcdc90d commit 5028eda
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 0 deletions.
18 changes: 18 additions & 0 deletions Lib/test/test_compile.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
6 changes: 6 additions & 0 deletions Lib/test/test_listcomps.py
Original file line number Diff line number Diff line change
Expand Up @@ -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}

Expand Down
22 changes: 22 additions & 0 deletions Python/flowgraph.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down Expand Up @@ -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; idx < k; idx++) {
int store_idx = STORES_TO(block->b_instr[idx]);
if (store_idx == store_j || store_idx == store_k) {
return;
}
}
}

// Success!
INSTR_SET_OP0(swap, NOP);
cfg_instr temp = block->b_instr[j];
Expand Down

0 comments on commit 5028eda

Please sign in to comment.