Skip to content

Commit 6696cbd

Browse files
sfwclaude
andcommitted
Fix copy-prop soundness: invalidate mappings on IndexAssign writes
IndexAssign(local, ..) mutates the local in-place but was not invalidating copy propagation mappings involving that local, which could lead to unsound substitutions for aggregate values. Add retain-based invalidation and a regression test. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 5ef9b75 commit 6696cbd

1 file changed

Lines changed: 64 additions & 16 deletions

File tree

src/mir/optimize.rs

Lines changed: 64 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -381,24 +381,33 @@ fn copy_propagate(func: &mut Function, stats: &mut OptStats) {
381381
// Step 1: Substitute operands in this statement using the current map
382382
total_count += substitute_stmt(stmt, &subst);
383383

384-
// Step 2: If this statement assigns to a local, update the map
385-
if let StatementKind::Assign(dest, rvalue) = &stmt.kind {
386-
let dest_local = *dest;
387-
388-
// Invalidate any mapping where dest or src equals the assigned local
389-
subst.retain(|d, s| *d != dest_local && *s != dest_local);
390-
391-
// If this is a simple copy to a compiler temp, add to map
392-
if let Rvalue::Use(operand) = rvalue {
393-
let dest_idx = dest_local.0 as usize;
394-
let is_temp = dest_idx < locals.len() && locals[dest_idx].name.is_none();
395-
if is_temp
396-
&& let Operand::Copy(src) | Operand::Local(src) | Operand::Move(src) =
397-
operand
398-
{
399-
subst.insert(dest_local, *src);
384+
// Step 2: If this statement writes to a local, update the map
385+
match &stmt.kind {
386+
StatementKind::Assign(dest, rvalue) => {
387+
let dest_local = *dest;
388+
389+
// Invalidate any mapping where dest or src equals the assigned local
390+
subst.retain(|d, s| *d != dest_local && *s != dest_local);
391+
392+
// If this is a simple copy to a compiler temp, add to map
393+
if let Rvalue::Use(operand) = rvalue {
394+
let dest_idx = dest_local.0 as usize;
395+
let is_temp = dest_idx < locals.len() && locals[dest_idx].name.is_none();
396+
if is_temp
397+
&& let Operand::Copy(src) | Operand::Local(src) | Operand::Move(src) =
398+
operand
399+
{
400+
subst.insert(dest_local, *src);
401+
}
400402
}
401403
}
404+
StatementKind::IndexAssign(local, _, _) => {
405+
// Mutating an element of `local` — invalidate any mapping
406+
// involving this local to prevent unsound substitution.
407+
let written = *local;
408+
subst.retain(|d, s| *d != written && *s != written);
409+
}
410+
StatementKind::Nop => {}
402411
}
403412
}
404413

@@ -1383,6 +1392,45 @@ mod tests {
13831392
);
13841393
}
13851394

1395+
#[test]
1396+
fn test_copy_prop_index_assign_invalidation() {
1397+
// _1 = Copy(_0); IndexAssign(_0, idx, val); return Copy(_1)
1398+
// The IndexAssign mutates _0, so _1 → _0 must be invalidated.
1399+
let locals = vec![
1400+
make_local(Some("arr")), // _0 = user var (array)
1401+
make_local(None), // _1 = temp
1402+
];
1403+
let stmts = vec![
1404+
assign(1, Rvalue::Use(Operand::Copy(Local(0)))),
1405+
Statement {
1406+
kind: StatementKind::IndexAssign(
1407+
Local(0),
1408+
Operand::Constant(Constant::Int(0)),
1409+
Operand::Constant(Constant::Int(999)),
1410+
),
1411+
},
1412+
];
1413+
let block = make_block(0, stmts, Terminator::Return(Some(Operand::Copy(Local(1)))));
1414+
let mut func = make_function(locals, vec![block]);
1415+
1416+
let mut stats = OptStats::default();
1417+
copy_propagate(&mut func, &mut stats);
1418+
1419+
// _1 → _0 should have been invalidated by IndexAssign on _0
1420+
if let Some(Terminator::Return(Some(Operand::Copy(local)))) = &func.blocks[0].terminator {
1421+
assert_eq!(
1422+
local.0, 1,
1423+
"Return should still reference _1, not _0 (IndexAssign mutated _0)"
1424+
);
1425+
} else {
1426+
panic!("Expected Return(Copy(_1))");
1427+
}
1428+
assert_eq!(
1429+
stats.copies_propagated, 0,
1430+
"No substitutions should occur after IndexAssign invalidation"
1431+
);
1432+
}
1433+
13861434
// ---- Dead Block Elimination ----
13871435

13881436
#[test]

0 commit comments

Comments
 (0)