Fix write_value of ByVal into a ByRef.

Previously, you could perform the following, if you assume we could make
`Cell<i32>` into a primitive. (Alternately, you could achieve this with
unsafe code):

    x = Cell::new(12);
    y = &x;

    // Miri locals array:
    //   x = ByRef(alloc123);
    //   y = ByVal(Ptr(alloc123));
    //
    // Miri allocations:
    //   alloc123: [12, 0, 0, 0]

    x.set(42);

    // Miri locals array:
    //   x = ByVal(I32(42));
    //   y = ByVal(Ptr(alloc123));
    //
    // Miri allocations:
    //   alloc123: [12, 0, 0, 0]

Notice how `y` still refers to the allocation that used to represent
`x`. But now `x` was changed to `42` and `y` is still looking at memory
containing `12`.

Now, instead, we keep `x` as a `ByRef` and write the `42` constant into
it.

Unit test to follow in the next commit.
This commit is contained in:
Scott Olson 2016-10-18 21:02:37 -06:00
parent f5c0a24bb0
commit 39bb1254d1

View File

@ -1156,29 +1156,50 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
fn write_value(
&mut self,
value: Value,
src_val: Value,
dest: Lvalue,
dest_ty: Ty<'tcx>,
) -> EvalResult<'tcx, ()> {
match dest {
Lvalue::Ptr { ptr, extra } => {
assert_eq!(extra, LvalueExtra::None);
self.write_value_to_ptr(value, ptr, dest_ty)?;
self.write_value_to_ptr(src_val, ptr, dest_ty)?;
}
// The cases here can be a bit subtle. Read carefully!
Lvalue::Local { frame, local } => {
if let Value::ByRef(src_ptr) = value {
let dest_val = self.stack[frame].get_local(local);
let dest_ptr = if let Some(Value::ByRef(ptr)) = dest_val {
ptr
} else {
let ptr = self.alloc_ptr(dest_ty)?;
self.stack[frame].set_local(local, Value::ByRef(ptr));
ptr
};
let dest_val = self.stack[frame].get_local(local);
if let Some(Value::ByRef(dest_ptr)) = dest_val {
// If the local value is already `ByRef` (that is, backed by an `Allocation`),
// then we must write the new value into this allocation, because there may be
// other pointers into the allocation. These other pointers are logically
// pointers into the local variable, and must be able to observe the change.
//
// Thus, it would be an error to replace the `ByRef` with a `ByVal`, unless we
// knew for certain that there were no outstanding pointers to this local.
self.write_value_to_ptr(src_val, dest_ptr, dest_ty)?;
} else if let Value::ByRef(src_ptr) = src_val {
// If the local value is not `ByRef`, then we know there are no pointers to it
// and we can simply overwrite the `Value` in the locals array directly.
//
// In this specific case, where the source value is `ByRef`, we must duplicate
// the allocation, because this is a by-value operation. It would be incorrect
// if they referred to the same allocation, since then a change to one would
// implicitly change the other.
//
// TODO(solson): It would be valid to attempt reading a primitive value out of
// the source and writing that into the destination without making an
// allocation. This would be a pure optimization.
let dest_ptr = self.alloc_ptr(dest_ty)?;
self.copy(src_ptr, dest_ptr, dest_ty)?;
self.stack[frame].set_local(local, Value::ByRef(dest_ptr));
} else {
// FIXME(solson): Is it safe to free the existing local here?
self.stack[frame].set_local(local, value);
// Finally, we have the simple case where neither source nor destination are
// `ByRef`. We may simply copy the source value over the the destintion local.
self.stack[frame].set_local(local, src_val);
}
}
}