diff --git a/rust-version b/rust-version index 3482331f59..143d077e3c 100644 --- a/rust-version +++ b/rust-version @@ -1 +1 @@ -3e99439f4dacc8ba0d2ca48d221694362d587927 +3e827cc21e0734edd26170e8d1481f0d66a1426b diff --git a/src/data_race.rs b/src/data_race.rs index e8071845c7..bcc2e17654 100644 --- a/src/data_race.rs +++ b/src/data_race.rs @@ -720,7 +720,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: MiriEvalContextExt<'mir, 'tcx> { if let Some(data_race) = &mut this.memory.extra.data_race { if data_race.multi_threaded.get() { let alloc_meta = - this.memory.get_raw_mut(ptr.alloc_id)?.extra.data_race.as_mut().unwrap(); + this.memory.get_alloc_extra_mut(ptr.alloc_id)?.data_race.as_mut().unwrap(); alloc_meta.reset_clocks(ptr.offset, size); } } @@ -1024,7 +1024,7 @@ trait EvalContextPrivExt<'mir, 'tcx: 'mir>: MiriEvalContextExt<'mir, 'tcx> { let place_ptr = place.ptr.assert_ptr(); let size = place.layout.size; let alloc_meta = - &this.memory.get_raw(place_ptr.alloc_id)?.extra.data_race.as_ref().unwrap(); + &this.memory.get_alloc_extra(place_ptr.alloc_id)?.data_race.as_ref().unwrap(); log::trace!( "Atomic op({}) with ordering {:?} on memory({:?}, offset={}, size={})", description, diff --git a/src/diagnostics.rs b/src/diagnostics.rs index f074cfd491..ae50b50860 100644 --- a/src/diagnostics.rs +++ b/src/diagnostics.rs @@ -136,13 +136,13 @@ pub fn report_error<'tcx, 'mir>( // Extra output to help debug specific issues. match e.kind() { - UndefinedBehavior(UndefinedBehaviorInfo::InvalidUninitBytes(Some(access))) => { + UndefinedBehavior(UndefinedBehaviorInfo::InvalidUninitBytes(Some((alloc_id, access)))) => { eprintln!( "Uninitialized read occurred at offsets 0x{:x}..0x{:x} into this allocation:", - access.uninit_ptr.offset.bytes(), - access.uninit_ptr.offset.bytes() + access.uninit_size.bytes(), + access.uninit_offset.bytes(), + access.uninit_offset.bytes() + access.uninit_size.bytes(), ); - eprintln!("{:?}", ecx.memory.dump_alloc(access.uninit_ptr.alloc_id)); + eprintln!("{:?}", ecx.memory.dump_alloc(*alloc_id)); } _ => {} } diff --git a/src/helpers.rs b/src/helpers.rs index ef5ea94478..45a5a8d417 100644 --- a/src/helpers.rs +++ b/src/helpers.rs @@ -8,7 +8,7 @@ use log::trace; use rustc_hir::def_id::{DefId, CRATE_DEF_INDEX}; use rustc_middle::mir; use rustc_middle::ty::{self, layout::TyAndLayout, List, TyCtxt}; -use rustc_target::abi::{FieldsShape, LayoutOf, Size, Variants}; +use rustc_target::abi::{Align, FieldsShape, LayoutOf, Size, Variants}; use rustc_target::spec::abi::Abi; use rand::RngCore; @@ -577,10 +577,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx let ptr = this.force_ptr(sptr)?; // We need to read at least 1 byte, so we can eagerly get a ptr. // Step 1: determine the length. - let alloc = this.memory.get_raw(ptr.alloc_id)?; let mut len = Size::ZERO; loop { - let byte = alloc.read_scalar(this, ptr.offset(len, this)?, size1)?.to_u8()?; + // FIXME: We are re-getting the allocation each time around the loop. + // Would be nice if we could somehow "extend" an existing AllocRange. + let alloc = this.memory.get(ptr.offset(len, this)?.into(), size1, Align::ONE)?.unwrap(); // not a ZST, so we will get a result + let byte = alloc.read_scalar(alloc_range(Size::ZERO, size1))?.to_u8()?; if byte == 0 { break; } else { @@ -595,12 +597,15 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx fn read_wide_str(&self, sptr: Scalar) -> InterpResult<'tcx, Vec> { let this = self.eval_context_ref(); let size2 = Size::from_bytes(2); + let align2 = Align::from_bytes(2).unwrap(); let mut ptr = this.force_ptr(sptr)?; // We need to read at least 1 wchar, so we can eagerly get a ptr. let mut wchars = Vec::new(); - let alloc = this.memory.get_raw(ptr.alloc_id)?; loop { - let wchar = alloc.read_scalar(this, ptr, size2)?.to_u16()?; + // FIXME: We are re-getting the allocation each time around the loop. + // Would be nice if we could somehow "extend" an existing AllocRange. + let alloc = this.memory.get(ptr.into(), size2, align2)?.unwrap(); // not a ZST, so we will get a result + let wchar = alloc.read_scalar(alloc_range(Size::ZERO, size2))?.to_u16()?; if wchar == 0 { break; } else { diff --git a/src/machine.rs b/src/machine.rs index 51e0d8f6a6..7ed8147753 100644 --- a/src/machine.rs +++ b/src/machine.rs @@ -506,15 +506,57 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'mir, 'tcx> { } #[inline(always)] - fn before_deallocation( - memory_extra: &mut Self::MemoryExtra, - id: AllocId, + fn memory_read( + _memory_extra: &Self::MemoryExtra, + alloc: &Allocation, + ptr: Pointer, + size: Size, ) -> InterpResult<'tcx> { - if Some(id) == memory_extra.tracked_alloc_id { - register_diagnostic(NonHaltingDiagnostic::FreedAlloc(id)); + if let Some(data_race) = &alloc.extra.data_race { + data_race.read(ptr, size)?; } + if let Some(stacked_borrows) = &alloc.extra.stacked_borrows { + stacked_borrows.memory_read(ptr, size) + } else { + Ok(()) + } + } - Ok(()) + #[inline(always)] + fn memory_written( + _memory_extra: &mut Self::MemoryExtra, + alloc: &mut Allocation, + ptr: Pointer, + size: Size, + ) -> InterpResult<'tcx> { + if let Some(data_race) = &mut alloc.extra.data_race { + data_race.write(ptr, size)?; + } + if let Some(stacked_borrows) = &mut alloc.extra.stacked_borrows { + stacked_borrows.memory_written(ptr, size) + } else { + Ok(()) + } + } + + #[inline(always)] + fn memory_deallocated( + memory_extra: &mut Self::MemoryExtra, + alloc: &mut Allocation, + ptr: Pointer, + ) -> InterpResult<'tcx> { + let size = alloc.size(); + if Some(ptr.alloc_id) == memory_extra.tracked_alloc_id { + register_diagnostic(NonHaltingDiagnostic::FreedAlloc(ptr.alloc_id)); + } + if let Some(data_race) = &mut alloc.extra.data_race { + data_race.deallocate(ptr, size)?; + } + if let Some(stacked_borrows) = &mut alloc.extra.stacked_borrows { + stacked_borrows.memory_deallocated(ptr, size) + } else { + Ok(()) + } } fn after_static_mem_initialized( @@ -601,53 +643,3 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'mir, 'tcx> { intptrcast::GlobalState::ptr_to_int(ptr, memory) } } - -impl AllocationExtra for AllocExtra { - #[inline(always)] - fn memory_read<'tcx>( - alloc: &Allocation, - ptr: Pointer, - size: Size, - ) -> InterpResult<'tcx> { - if let Some(data_race) = &alloc.extra.data_race { - data_race.read(ptr, size)?; - } - if let Some(stacked_borrows) = &alloc.extra.stacked_borrows { - stacked_borrows.memory_read(ptr, size) - } else { - Ok(()) - } - } - - #[inline(always)] - fn memory_written<'tcx>( - alloc: &mut Allocation, - ptr: Pointer, - size: Size, - ) -> InterpResult<'tcx> { - if let Some(data_race) = &mut alloc.extra.data_race { - data_race.write(ptr, size)?; - } - if let Some(stacked_borrows) = &mut alloc.extra.stacked_borrows { - stacked_borrows.memory_written(ptr, size) - } else { - Ok(()) - } - } - - #[inline(always)] - fn memory_deallocated<'tcx>( - alloc: &mut Allocation, - ptr: Pointer, - size: Size, - ) -> InterpResult<'tcx> { - if let Some(data_race) = &mut alloc.extra.data_race { - data_race.deallocate(ptr, size)?; - } - if let Some(stacked_borrows) = &mut alloc.extra.stacked_borrows { - stacked_borrows.memory_deallocated(ptr, size) - } else { - Ok(()) - } - } -} diff --git a/src/shims/intrinsics.rs b/src/shims/intrinsics.rs index ee0e833f9e..f2979a3c69 100644 --- a/src/shims/intrinsics.rs +++ b/src/shims/intrinsics.rs @@ -574,7 +574,13 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx // even if the type they wrap would be less aligned (e.g. AtomicU64 on 32bit must // be 8-aligned). let align = Align::from_bytes(place.layout.size.bytes()).unwrap(); - this.memory.check_ptr_access(place.ptr, place.layout.size, align)?; + this.memory.check_ptr_access_align( + place.ptr, + place.layout.size, + align, + CheckInAllocMsg::MemoryAccessTest, + )?; + // Perform regular access. this.write_scalar(val, dest)?; Ok(()) } @@ -594,7 +600,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx // even if the type they wrap would be less aligned (e.g. AtomicU64 on 32bit must // be 8-aligned). let align = Align::from_bytes(place.layout.size.bytes()).unwrap(); - this.memory.check_ptr_access(place.ptr, place.layout.size, align)?; + this.memory.check_ptr_access_align( + place.ptr, + place.layout.size, + align, + CheckInAllocMsg::MemoryAccessTest, + )?; // Perform atomic store this.write_scalar_atomic(val, &place, atomic)?; @@ -644,7 +655,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx // even if the type they wrap would be less aligned (e.g. AtomicU64 on 32bit must // be 8-aligned). let align = Align::from_bytes(place.layout.size.bytes()).unwrap(); - this.memory.check_ptr_access(place.ptr, place.layout.size, align)?; + this.memory.check_ptr_access_align( + place.ptr, + place.layout.size, + align, + CheckInAllocMsg::MemoryAccessTest, + )?; match atomic_op { AtomicOp::Min => { @@ -681,7 +697,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx // even if the type they wrap would be less aligned (e.g. AtomicU64 on 32bit must // be 8-aligned). let align = Align::from_bytes(place.layout.size.bytes()).unwrap(); - this.memory.check_ptr_access(place.ptr, place.layout.size, align)?; + this.memory.check_ptr_access_align( + place.ptr, + place.layout.size, + align, + CheckInAllocMsg::MemoryAccessTest, + )?; let old = this.atomic_exchange_scalar(&place, new, atomic)?; this.write_scalar(old, dest)?; // old value is returned @@ -707,7 +728,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx // even if the type they wrap would be less aligned (e.g. AtomicU64 on 32bit must // be 8-aligned). let align = Align::from_bytes(place.layout.size.bytes()).unwrap(); - this.memory.check_ptr_access(place.ptr, place.layout.size, align)?; + this.memory.check_ptr_access_align( + place.ptr, + place.layout.size, + align, + CheckInAllocMsg::MemoryAccessTest, + )?; let old = this.atomic_compare_exchange_scalar( &place, diff --git a/src/shims/os_str.rs b/src/shims/os_str.rs index f1f14fa828..8a3f567770 100644 --- a/src/shims/os_str.rs +++ b/src/shims/os_str.rs @@ -9,7 +9,7 @@ use std::os::unix::ffi::{OsStrExt, OsStringExt}; #[cfg(windows)] use std::os::windows::ffi::{OsStrExt, OsStringExt}; -use rustc_target::abi::{LayoutOf, Size}; +use rustc_target::abi::{Align, LayoutOf, Size}; use crate::*; @@ -144,17 +144,14 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx // Store the UTF-16 string. let size2 = Size::from_bytes(2); let this = self.eval_context_mut(); - let tcx = &*this.tcx; - let ptr = this.force_ptr(sptr)?; // we need to write at least the 0 terminator - let alloc = this.memory.get_raw_mut(ptr.alloc_id)?; + let mut alloc = this + .memory + .get_mut(sptr, size2 * string_length, Align::from_bytes(2).unwrap())? + .unwrap(); // not a ZST, so we will get a result for (offset, wchar) in u16_vec.into_iter().chain(iter::once(0x0000)).enumerate() { let offset = u64::try_from(offset).unwrap(); - alloc.write_scalar( - tcx, - ptr.offset(size2 * offset, tcx)?, - Scalar::from_u16(wchar).into(), - size2, - )?; + alloc + .write_scalar(alloc_range(size2 * offset, size2), Scalar::from_u16(wchar).into())?; } Ok((true, string_length - 1)) } diff --git a/src/shims/posix/fs.rs b/src/shims/posix/fs.rs index 234f03ff46..06594e5ca1 100644 --- a/src/shims/posix/fs.rs +++ b/src/shims/posix/fs.rs @@ -677,10 +677,11 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx trace!("Reading from FD {}, size {}", fd, count); // Check that the *entire* buffer is actually valid memory. - this.memory.check_ptr_access( + this.memory.check_ptr_access_align( buf, Size::from_bytes(count), - Align::from_bytes(1).unwrap(), + Align::ONE, + CheckInAllocMsg::MemoryAccessTest, )?; // We cap the number of read bytes to the largest value that we are able to fit in both the @@ -722,10 +723,11 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx // Isolation check is done via `FileDescriptor` trait. // Check that the *entire* buffer is actually valid memory. - this.memory.check_ptr_access( + this.memory.check_ptr_access_align( buf, Size::from_bytes(count), - Align::from_bytes(1).unwrap(), + Align::ONE, + CheckInAllocMsg::MemoryAccessTest, )?; // We cap the number of written bytes to the largest value that we are able to fit in both the diff --git a/src/shims/posix/linux/sync.rs b/src/shims/posix/linux/sync.rs index c5101203eb..fda70d815d 100644 --- a/src/shims/posix/linux/sync.rs +++ b/src/shims/posix/linux/sync.rs @@ -82,10 +82,11 @@ pub fn futex<'tcx>( // Check the pointer for alignment and validity. // The API requires `addr` to be a 4-byte aligned pointer, and will // use the 4 bytes at the given address as an (atomic) i32. - this.memory.check_ptr_access( + this.memory.check_ptr_access_align( addr.to_scalar()?, Size::from_bytes(4), Align::from_bytes(4).unwrap(), + CheckInAllocMsg::MemoryAccessTest, )?; // Read an `i32` through the pointer, regardless of any wrapper types. // It's not uncommon for `addr` to be passed as another type than `*mut i32`, such as `*const AtomicI32`. diff --git a/src/stacked_borrows.rs b/src/stacked_borrows.rs index 88f42efd13..1139e21e0f 100644 --- a/src/stacked_borrows.rs +++ b/src/stacked_borrows.rs @@ -561,7 +561,7 @@ trait EvalContextPrivExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx ); // Get the allocation. It might not be mutable, so we cannot use `get_mut`. - let extra = &this.memory.get_raw(ptr.alloc_id)?.extra; + let extra = this.memory.get_alloc_extra(ptr.alloc_id)?; let stacked_borrows = extra.stacked_borrows.as_ref().expect("we should have Stacked Borrows data"); // Update the stacks. diff --git a/tests/compile-fail/data_race/alloc_read_race.rs b/tests/compile-fail/data_race/alloc_read_race.rs index fc1e9d30e6..093c9024f2 100644 --- a/tests/compile-fail/data_race/alloc_read_race.rs +++ b/tests/compile-fail/data_race/alloc_read_race.rs @@ -1,4 +1,5 @@ // ignore-windows: Concurrency on Windows is not supported yet. +#![feature(new_uninit)] use std::thread::spawn; use std::ptr::null_mut; @@ -29,7 +30,7 @@ pub fn main() { // Uses relaxed semantics to not generate // a release sequence. let pointer = &*ptr.0; - pointer.store(Box::into_raw(Box::new(MaybeUninit::uninit())), Ordering::Relaxed); + pointer.store(Box::into_raw(Box::new_uninit()), Ordering::Relaxed); }); let j2 = spawn(move || { diff --git a/tests/compile-fail/data_race/alloc_write_race.rs b/tests/compile-fail/data_race/alloc_write_race.rs index d9f5af396a..becebe6a12 100644 --- a/tests/compile-fail/data_race/alloc_write_race.rs +++ b/tests/compile-fail/data_race/alloc_write_race.rs @@ -1,4 +1,5 @@ // ignore-windows: Concurrency on Windows is not supported yet. +#![feature(new_uninit)] use std::thread::spawn; use std::ptr::null_mut; @@ -10,11 +11,6 @@ struct EvilSend(pub T); unsafe impl Send for EvilSend {} unsafe impl Sync for EvilSend {} -extern "C" { - fn malloc(size: usize) -> *mut u8; - fn free(ptr: *mut u8); -} - pub fn main() { // Shared atomic pointer let pointer = AtomicPtr::new(null_mut::()); @@ -33,7 +29,7 @@ pub fn main() { // Uses relaxed semantics to not generate // a release sequence. let pointer = &*ptr.0; - pointer.store(malloc(std::mem::size_of::()) as *mut usize, Ordering::Relaxed); + pointer.store(Box::into_raw(Box::::new_uninit()) as *mut usize, Ordering::Relaxed); }); let j2 = spawn(move || { @@ -45,6 +41,6 @@ pub fn main() { j2.join().unwrap(); // Clean up memory, will never be executed - free(pointer.load(Ordering::Relaxed) as *mut _); + drop(Box::from_raw(pointer.load(Ordering::Relaxed))); } }