bjoernager/rust
bjoernager/rust
1
Fork 0
Code Issues Pull requests Activity

miri: treat non-memory local variables properly for data race detection

Browse source
This commit is contained in:
Ralf Jung 2024-08-31 17:03:03 +02:00
parent 304b7f801b
commit a888905226
16 changed files with 433 additions and 18 deletions
Download patch file Download diff file Expand all files Collapse all files

21
compiler/rustc_const_eval/src/interpret/machine.rs
View file

@ -540,10 +540,29 @@ pub trait Machine<'tcx>: Sized {
Ok(ReturnAction::Normal)
}
/// Called immediately after an "immediate" local variable is read
/// (i.e., this is called for reads that do not end up accessing addressable memory).
#[inline(always)]
fn after_local_read(_ecx: &InterpCx<'tcx, Self>, _local: mir::Local) -> InterpResult<'tcx> {
Ok(())
}
/// Called immediately after an "immediate" local variable is assigned a new value
/// (i.e., this is called for writes that do not end up in memory).
/// `storage_live` indicates whether this is the initial write upon `StorageLive`.
#[inline(always)]
fn after_local_write(
_ecx: &mut InterpCx<'tcx, Self>,
_local: mir::Local,
_storage_live: bool,
) -> InterpResult<'tcx> {
Ok(())
}
/// Called immediately after actual memory was allocated for a local
/// but before the local's stack frame is updated to point to that memory.
#[inline(always)]
fn after_local_allocated(
fn after_local_moved_to_memory(
_ecx: &mut InterpCx<'tcx, Self>,
_local: mir::Local,
_mplace: &MPlaceTy<'tcx, Self::Provenance>,

4
compiler/rustc_const_eval/src/interpret/memory.rs
View file

@ -1030,6 +1030,10 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
);
res
}
pub(super) fn validation_in_progress(&self) -> bool {
self.memory.validation_in_progress
}
}
#[doc(hidden)]

1
compiler/rustc_const_eval/src/interpret/operand.rs
View file

@ -719,6 +719,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
if matches!(op, Operand::Immediate(_)) {
assert!(!layout.is_unsized());
}
M::after_local_read(self, local)?;
Ok(OpTy { op, layout })
}

31
compiler/rustc_const_eval/src/interpret/place.rs
View file

@ -504,15 +504,13 @@ where
&self,
local: mir::Local,
) -> InterpResult<'tcx, PlaceTy<'tcx, M::Provenance>> {
// Other parts of the system rely on `Place::Local` never being unsized.
// So we eagerly check here if this local has an MPlace, and if yes we use it.
let frame = self.frame();
let layout = self.layout_of_local(frame, local, None)?;
let place = if layout.is_sized() {
// We can just always use the `Local` for sized values.
Place::Local { local, offset: None, locals_addr: frame.locals_addr() }
} else {
// Unsized `Local` isn't okay (we cannot store the metadata).
// Other parts of the system rely on `Place::Local` never being unsized.
match frame.locals[local].access()? {
Operand::Immediate(_) => bug!(),
Operand::Indirect(mplace) => Place::Ptr(*mplace),
@ -565,7 +563,10 @@ where
place: &PlaceTy<'tcx, M::Provenance>,
) -> InterpResult<
'tcx,
Either<MPlaceTy<'tcx, M::Provenance>, (&mut Immediate<M::Provenance>, TyAndLayout<'tcx>)>,
Either<
MPlaceTy<'tcx, M::Provenance>,
(&mut Immediate<M::Provenance>, TyAndLayout<'tcx>, mir::Local),
>,
> {
Ok(match place.to_place().as_mplace_or_local() {
Left(mplace) => Left(mplace),
@ -584,7 +585,7 @@ where
}
Operand::Immediate(local_val) => {
// The local still has the optimized representation.
Right((local_val, layout))
Right((local_val, layout, local))
}
}
}
@ -646,9 +647,13 @@ where
assert!(dest.layout().is_sized(), "Cannot write unsized immediate data");
match self.as_mplace_or_mutable_local(&dest.to_place())? {
Right((local_val, local_layout)) => {
Right((local_val, local_layout, local)) => {
// Local can be updated in-place.
*local_val = src;
// Call the machine hook (the data race detector needs to know about this write).
if !self.validation_in_progress() {
M::after_local_write(self, local, /*storage_live*/ false)?;
}
// Double-check that the value we are storing and the local fit to each other.
if cfg!(debug_assertions) {
src.assert_matches_abi(local_layout.abi, self);
@ -717,8 +722,12 @@ where
dest: &impl Writeable<'tcx, M::Provenance>,
) -> InterpResult<'tcx> {
match self.as_mplace_or_mutable_local(&dest.to_place())? {
Right((local_val, _local_layout)) => {
Right((local_val, _local_layout, local)) => {
*local_val = Immediate::Uninit;
// Call the machine hook (the data race detector needs to know about this write).
if !self.validation_in_progress() {
M::after_local_write(self, local, /*storage_live*/ false)?;
}
}
Left(mplace) => {
let Some(mut alloc) = self.get_place_alloc_mut(&mplace)? else {
@ -737,8 +746,12 @@ where
dest: &impl Writeable<'tcx, M::Provenance>,
) -> InterpResult<'tcx> {
match self.as_mplace_or_mutable_local(&dest.to_place())? {
Right((local_val, _local_layout)) => {
Right((local_val, _local_layout, local)) => {
local_val.clear_provenance()?;
// Call the machine hook (the data race detector needs to know about this write).
if !self.validation_in_progress() {
M::after_local_write(self, local, /*storage_live*/ false)?;
}
}
Left(mplace) => {
let Some(mut alloc) = self.get_place_alloc_mut(&mplace)? else {
@ -944,7 +957,7 @@ where
mplace.mplace,
)?;
}
M::after_local_allocated(self, local, &mplace)?;
M::after_local_moved_to_memory(self, local, &mplace)?;
// Now we can call `access_mut` again, asserting it goes well, and actually
// overwrite things. This points to the entire allocation, not just the part
// the place refers to, i.e. we do this before we apply `offset`.

5
compiler/rustc_const_eval/src/interpret/stack.rs
View file

@ -534,8 +534,11 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
let dest_place = self.allocate_dyn(layout, MemoryKind::Stack, meta)?;
Operand::Indirect(*dest_place.mplace())
} else {
assert!(!meta.has_meta()); // we're dropping the metadata
// Just make this an efficient immediate.
assert!(!meta.has_meta()); // we're dropping the metadata
// Make sure the machine knows this "write" is happening. (This is important so that
// races involving local variable allocation can be detected by Miri.)
M::after_local_write(self, local, /*storage_live*/ true)?;
// Note that not calling `layout_of` here does have one real consequence:
// if the type is too big, we'll only notice this when the local is actually initialized,
// which is a bit too late -- we should ideally notice this already here, when the memory