Cranelift DFG: make inst clone deep-clone varargs lists. (#5727)
When investigating #5716, I found that rematerialization of a `call`, in addition to blowing up for other reasons, caused aliasing of the varargs list (the `EntityList` in the `ListPool`), such that editing the args of the second copy of the call instruction inadvertently updated the first as well. This PR modifies `DataFlowGraph::clone_inst` so that it always clones the varargs list if present. This shouldn't have any functional impact on Cranelift today, because we don't rematerialize any instructions with varargs; but it's important to get it right to avoid a bug later!
This commit is contained in:
@@ -907,6 +907,10 @@ impl DataFlowGraph {
|
||||
pub fn clone_inst(&mut self, inst: Inst) -> Inst {
|
||||
// First, add a clone of the InstructionData.
|
||||
let inst_data = self.insts[inst].clone();
|
||||
// If the `inst_data` has a reference to a ValueList, clone it
|
||||
// as well, because we can't share these (otherwise mutating
|
||||
// one would affect the other).
|
||||
let inst_data = inst_data.deep_clone(&mut self.value_lists);
|
||||
let new_inst = self.make_inst(inst_data);
|
||||
// Get the controlling type variable.
|
||||
let ctrl_typevar = self.ctrl_typevar(inst);
|
||||
@@ -1615,4 +1619,25 @@ mod tests {
|
||||
assert_eq!(pos.func.dfg.resolve_aliases(c2), c2);
|
||||
assert_eq!(pos.func.dfg.resolve_aliases(c), c2);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn cloning() {
|
||||
use crate::ir::InstBuilder;
|
||||
|
||||
let mut func = Function::new();
|
||||
let mut sig = Signature::new(crate::isa::CallConv::SystemV);
|
||||
sig.params.push(ir::AbiParam::new(types::I32));
|
||||
let sig = func.import_signature(sig);
|
||||
let block0 = func.dfg.make_block();
|
||||
let mut pos = FuncCursor::new(&mut func);
|
||||
pos.insert_block(block0);
|
||||
let v1 = pos.ins().iconst(types::I32, 0);
|
||||
let v2 = pos.ins().iconst(types::I32, 1);
|
||||
let call_inst = pos.ins().call_indirect(sig, v1, &[v1]);
|
||||
let func = pos.func;
|
||||
|
||||
let call_inst_dup = func.dfg.clone_inst(call_inst);
|
||||
func.dfg.inst_args_mut(call_inst)[0] = v2;
|
||||
assert_eq!(v1, func.dfg.inst_args(call_inst_dup)[0]);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -123,6 +123,15 @@ impl BlockCall {
|
||||
pub fn display<'a>(&self, pool: &'a ValueListPool) -> DisplayBlockCall<'a> {
|
||||
DisplayBlockCall { block: *self, pool }
|
||||
}
|
||||
|
||||
/// Deep-clone the underlying list in the same pool. The returned
|
||||
/// list will have identical contents but changes to this list
|
||||
/// will not change its contents or vice-versa.
|
||||
pub fn deep_clone(&self, pool: &mut ValueListPool) -> Self {
|
||||
Self {
|
||||
values: self.values.deep_clone(pool),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Wrapper for the context needed to display a [BlockCall] value.
|
||||
|
||||
Reference in New Issue
Block a user