diff --git a/compiler/rustc_const_eval/src/interpret/memory.rs b/compiler/rustc_const_eval/src/interpret/memory.rs index 323e1cefd5869..bac3a9da48d99 100644 --- a/compiler/rustc_const_eval/src/interpret/memory.rs +++ b/compiler/rustc_const_eval/src/interpret/memory.rs @@ -1498,14 +1498,10 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> { // Prepare getting source provenance. let src_bytes = src_alloc.get_bytes_unchecked(src_range).as_ptr(); // raw ptr, so we can also get a ptr to the destination allocation - // first copy the provenance to a temporary buffer, because - // `get_bytes_mut` will clear the provenance, which is correct, - // since we don't want to keep any provenance at the target. - // This will also error if copying partial provenance is not supported. - let provenance = src_alloc - .provenance() - .prepare_copy(src_range, self) - .map_err(|e| e.to_interp_error(src_alloc_id))?; + // First copy the provenance to a temporary buffer, because + // `get_bytes_unchecked_for_overwrite_ptr` will clear the provenance (in preparation for + // inserting the new provenance), and that can overlap with the source range. + let provenance = src_alloc.provenance_prepare_copy(src_range, self); // Prepare a copy of the initialization mask. let init = src_alloc.init_mask().prepare_copy(src_range); diff --git a/compiler/rustc_middle/src/lib.rs b/compiler/rustc_middle/src/lib.rs index 754a258eef93d..d276f158cdf85 100644 --- a/compiler/rustc_middle/src/lib.rs +++ b/compiler/rustc_middle/src/lib.rs @@ -51,6 +51,7 @@ #![feature(negative_impls)] #![feature(never_type)] #![feature(ptr_alignment_type)] +#![feature(range_bounds_is_empty)] #![feature(rustc_attrs)] #![feature(rustdoc_internals)] #![feature(sized_hierarchy)] diff --git a/compiler/rustc_middle/src/mir/interpret/allocation.rs b/compiler/rustc_middle/src/mir/interpret/allocation.rs index 8e603ce1b911a..8a720a21a5acf 100644 --- a/compiler/rustc_middle/src/mir/interpret/allocation.rs +++ b/compiler/rustc_middle/src/mir/interpret/allocation.rs @@ -630,7 +630,7 @@ impl Allocation range: AllocRange, ) -> &mut [u8] { self.mark_init(range, true); - self.provenance.clear(range, cx); + self.provenance.clear(range, &self.bytes, cx); &mut self.bytes[range.start.bytes_usize()..range.end().bytes_usize()] } @@ -643,7 +643,7 @@ impl Allocation range: AllocRange, ) -> *mut [u8] { self.mark_init(range, true); - self.provenance.clear(range, cx); + self.provenance.clear(range, &self.bytes, cx); assert!(range.end().bytes_usize() <= self.bytes.len()); // need to do our own bounds-check // Crucially, we go via `AllocBytes::as_mut_ptr`, not `AllocBytes::deref_mut`. @@ -722,37 +722,49 @@ impl Allocation if self.provenance.range_empty(range, cx) { return Ok(Scalar::from_uint(bits, range.size)); } - // If we get here, we have to check per-byte provenance, and join them together. + // If we get here, we have to check whether we can merge per-byte provenance. let prov = 'prov: { - if !Prov::OFFSET_IS_ADDR { - // FIXME(#146291): We need to ensure that we don't mix different pointers with - // the same provenance. - return Err(AllocError::ReadPartialPointer(range.start)); - } - // Initialize with first fragment. Must have index 0. - let Some((mut joint_prov, 0)) = self.provenance.get_byte(range.start, cx) else { + // If there is any ptr-sized provenance overlapping with this range, + // this is definitely mixing multiple pointers and we can bail. + if !self.provenance.range_ptrs_is_empty(range, cx) { break 'prov None; - }; - // Update with the remaining fragments. - for offset in Size::from_bytes(1)..range.size { - // Ensure there is provenance here and it has the right index. - let Some((frag_prov, frag_idx)) = - self.provenance.get_byte(range.start + offset, cx) - else { + } + // Scan all fragments, and ensure their indices, provenance, and bytes match. + // However, we have to ignore wildcard fragments for this (this is needed for Miri's + // native-lib mode). Therefore, we will only know the expected provenance and bytes + // once we find the first non-wildcard fragment. + let mut expected = None; + for idx in Size::ZERO..range.size { + // Ensure there is provenance here. + let Some(frag) = self.provenance.get_byte(range.start + idx, cx) else { break 'prov None; }; - // Wildcard provenance is allowed to come with any index (this is needed - // for Miri's native-lib mode to work). - if u64::from(frag_idx) != offset.bytes() && Some(frag_prov) != Prov::WILDCARD { + // If this is wildcard provenance, ignore this fragment. + if Some(frag.prov) == Prov::WILDCARD { + continue; + } + // For non-wildcard fragments, the index must match. + if u64::from(frag.idx) != idx.bytes() { break 'prov None; } - // Merge this byte's provenance with the previous ones. - joint_prov = match Prov::join(joint_prov, frag_prov) { - Some(prov) => prov, - None => break 'prov None, - }; + // If there are expectations registered, check them. + // If not, record this fragment as setting the expectations. + match expected { + Some(expected) => { + if (frag.prov, frag.bytes) != expected { + break 'prov None; + } + } + None => { + expected = Some((frag.prov, frag.bytes)); + } + } } - break 'prov Some(joint_prov); + // The final provenance is the expected one we found along the way, or wildcard if + // we didn't find any. + break 'prov Some( + expected.map(|(prov, _addr)| prov).or_else(|| Prov::WILDCARD).unwrap(), + ); }; if prov.is_none() && !Prov::OFFSET_IS_ADDR { // There are some bytes with provenance here but overall the provenance does not add up. @@ -816,7 +828,7 @@ impl Allocation /// Write "uninit" to the given memory range. pub fn write_uninit(&mut self, cx: &impl HasDataLayout, range: AllocRange) { self.mark_init(range, false); - self.provenance.clear(range, cx); + self.provenance.clear(range, &self.bytes, cx); } /// Mark all bytes in the given range as initialised and reset the provenance @@ -831,21 +843,28 @@ impl Allocation size: Size::from_bytes(self.len()), }); self.mark_init(range, true); - self.provenance.write_wildcards(cx, range); + self.provenance.write_wildcards(cx, &self.bytes, range); } /// Remove all provenance in the given memory range. pub fn clear_provenance(&mut self, cx: &impl HasDataLayout, range: AllocRange) { - self.provenance.clear(range, cx); + self.provenance.clear(range, &self.bytes, cx); } pub fn provenance_merge_bytes(&mut self, cx: &impl HasDataLayout) -> bool { self.provenance.merge_bytes(cx) } + pub fn provenance_prepare_copy( + &self, + range: AllocRange, + cx: &impl HasDataLayout, + ) -> ProvenanceCopy { + self.provenance.prepare_copy(range, &self.bytes, cx) + } + /// Applies a previously prepared provenance copy. - /// The affected range, as defined in the parameters to `provenance().prepare_copy` is expected - /// to be clear of provenance. + /// The affected range is expected to be clear of provenance. /// /// This is dangerous to use as it can violate internal `Allocation` invariants! /// It only exists to support an efficient implementation of `mem_copy_repeatedly`. diff --git a/compiler/rustc_middle/src/mir/interpret/allocation/provenance_map.rs b/compiler/rustc_middle/src/mir/interpret/allocation/provenance_map.rs index 67baf63bbfadd..24ece24d99b1a 100644 --- a/compiler/rustc_middle/src/mir/interpret/allocation/provenance_map.rs +++ b/compiler/rustc_middle/src/mir/interpret/allocation/provenance_map.rs @@ -2,7 +2,7 @@ //! representation for the common case where PTR_SIZE consecutive bytes have the same provenance. use std::cmp; -use std::ops::Range; +use std::ops::{Range, RangeBounds}; use rustc_abi::{HasDataLayout, Size}; use rustc_data_structures::sorted_map::SortedMap; @@ -11,7 +11,22 @@ use rustc_serialize::{Decodable, Decoder, Encodable, Encoder}; use tracing::trace; use super::{AllocRange, CtfeProvenance, Provenance, alloc_range}; -use crate::mir::interpret::{AllocError, AllocResult}; + +/// A pointer fragment represents one byte of a pointer. +/// If the bytes are re-assembled in their original order, the pointer can be used again. +/// Wildcard provenance is allowed to have index 0 everywhere. +#[derive(Clone, PartialEq, Eq, Hash, Debug)] +#[derive(HashStable)] +pub struct PointerFrag { + /// The position of this fragment inside the pointer (in `0..8`). + pub idx: u8, + /// The pointer this is a fragment of: provenance and the "real" raw bytes. + pub prov: Prov, + /// This is taken as a direct subslice of the raw allocation data, so we don't have to worry + /// about endianness. If the pointer size is less than 8, only the first N bytes of this are + /// ever non-zero. + pub bytes: [u8; 8], +} /// Stores the provenance information of pointers stored in memory. #[derive(Clone, PartialEq, Eq, Hash, Debug)] @@ -21,10 +36,7 @@ pub struct ProvenanceMap { /// bytes. Two entries in this map are always at least a pointer size apart. ptrs: SortedMap, /// This stores byte-sized provenance fragments. - /// The `u8` indicates the position of this byte inside its original pointer. - /// If the bytes are re-assembled in their original order, the pointer can be used again. - /// Wildcard provenance is allowed to have index 0 everywhere. - bytes: Option>>, + bytes: Option>>>, } // These impls are generic over `Prov` since `CtfeProvenance` is only decodable/encodable @@ -49,7 +61,7 @@ impl ProvenanceMap { } /// The caller must guarantee that the given provenance list is already sorted - /// by address and contain no duplicates. + /// by offset and contain no duplicates. pub fn from_presorted_ptrs(r: Vec<(Size, Prov)>) -> Self { ProvenanceMap { ptrs: SortedMap::from_presorted_elements(r), bytes: None } } @@ -89,12 +101,12 @@ impl ProvenanceMap { } /// `pm.range_ptrs_is_empty(r, cx)` == `pm.range_ptrs_get(r, cx).is_empty()`, but is faster. - fn range_ptrs_is_empty(&self, range: AllocRange, cx: &impl HasDataLayout) -> bool { + pub(super) fn range_ptrs_is_empty(&self, range: AllocRange, cx: &impl HasDataLayout) -> bool { self.ptrs.range_is_empty(Self::adjusted_range_ptrs(range, cx)) } /// Returns all byte-wise provenance in the given range. - fn range_bytes_get(&self, range: AllocRange) -> &[(Size, (Prov, u8))] { + fn range_bytes_get(&self, range: AllocRange) -> &[(Size, PointerFrag)] { if let Some(bytes) = self.bytes.as_ref() { bytes.range(range.start..range.end()) } else { @@ -107,18 +119,11 @@ impl ProvenanceMap { self.bytes.as_ref().is_none_or(|bytes| bytes.range_is_empty(range.start..range.end())) } - /// Get the provenance of a single byte. - pub fn get_byte(&self, offset: Size, cx: &impl HasDataLayout) -> Option<(Prov, u8)> { - let prov = self.range_ptrs_get(alloc_range(offset, Size::from_bytes(1)), cx); - debug_assert!(prov.len() <= 1); - if let Some(entry) = prov.first() { - // If it overlaps with this byte, it is on this byte. - debug_assert!(self.bytes.as_ref().is_none_or(|b| !b.contains_key(&offset))); - Some((entry.1, (offset - entry.0).bytes() as u8)) - } else { - // Look up per-byte provenance. - self.bytes.as_ref().and_then(|b| b.get(&offset).copied()) - } + /// Get the provenance of a single byte. Must only be called if there is no + /// pointer-sized provenance here. + pub fn get_byte(&self, offset: Size, cx: &impl HasDataLayout) -> Option<&PointerFrag> { + debug_assert!(self.range_ptrs_is_empty(alloc_range(offset, Size::from_bytes(1)), cx)); + self.bytes.as_ref().and_then(|b| b.get(&offset)) } /// Gets the provenances of all bytes (including from pointers) in a range. @@ -128,37 +133,37 @@ impl ProvenanceMap { range: AllocRange, ) -> impl Iterator { let ptr_provs = self.range_ptrs_get(range, cx).iter().map(|(_, p)| *p); - let byte_provs = self.range_bytes_get(range).iter().map(|(_, (p, _))| *p); + let byte_provs = self.range_bytes_get(range).iter().map(|(_, frag)| frag.prov); ptr_provs.chain(byte_provs) } /// Attempt to merge per-byte provenance back into ptr chunks, if the right fragments - /// sit next to each other. Return `false` is that is not possible due to partial pointers. + /// sit next to each other. Return `false` if that is not possible due to partial pointers. pub fn merge_bytes(&mut self, cx: &impl HasDataLayout) -> bool { let Some(bytes) = self.bytes.as_deref_mut() else { return true; }; - if !Prov::OFFSET_IS_ADDR { - // FIXME(#146291): We need to ensure that we don't mix different pointers with - // the same provenance. - return false; - } let ptr_size = cx.data_layout().pointer_size(); - while let Some((offset, (prov, _))) = bytes.iter().next().copied() { + while let Some((offset, first_frag)) = bytes.iter().next() { + let offset = *offset; // Check if this fragment starts a pointer. let range = offset..offset + ptr_size; let frags = bytes.range(range.clone()); if frags.len() != ptr_size.bytes_usize() { + // We can't merge this one, no point in trying to merge the rest. return false; } - for (idx, (_offset, (frag_prov, frag_idx))) in frags.iter().copied().enumerate() { - if frag_prov != prov || frag_idx != idx as u8 { + for (idx, (_offset, frag)) in frags.iter().enumerate() { + if !(frag.prov == first_frag.prov + && frag.bytes == first_frag.bytes + && frag.idx == idx as u8) + { return false; } } // Looks like a pointer! Move it over to the ptr provenance map. + self.ptrs.insert(offset, first_frag.prov); bytes.remove_range(range); - self.ptrs.insert(offset, prov); } // We managed to convert everything into whole pointers. self.bytes = None; @@ -182,8 +187,8 @@ impl ProvenanceMap { /// Yields all the provenances stored in this map. pub fn provenances(&self) -> impl Iterator { - let bytes = self.bytes.iter().flat_map(|b| b.values().map(|(p, _i)| p)); - self.ptrs.values().chain(bytes).copied() + let bytes = self.bytes.iter().flat_map(|b| b.values().map(|frag| frag.prov)); + self.ptrs.values().copied().chain(bytes) } pub fn insert_ptr(&mut self, offset: Size, prov: Prov, cx: &impl HasDataLayout) { @@ -191,9 +196,34 @@ impl ProvenanceMap { self.ptrs.insert(offset, prov); } + /// Returns an iterator that yields the fragments of this pointer whose absolute positions are + /// inside `pos_range`. + fn ptr_fragments( + pos_range: impl RangeBounds, + ptr_pos: Size, + prov: Prov, + data_bytes: &[u8], + ptr_size: Size, + ) -> impl Iterator)> { + if pos_range.is_empty() { + return either::Left(std::iter::empty()); + } + // Read ptr_size many bytes starting at ptr_pos. + let mut bytes = [0u8; 8]; + (&mut bytes[..ptr_size.bytes_usize()]) + .copy_from_slice(&data_bytes[ptr_pos.bytes_usize()..][..ptr_size.bytes_usize()]); + // Yield the fragments of this pointer. + either::Right( + (ptr_pos..ptr_pos + ptr_size).filter(move |pos| pos_range.contains(pos)).map( + move |pos| (pos, PointerFrag { idx: (pos - ptr_pos).bytes() as u8, bytes, prov }), + ), + ) + } + /// Removes all provenance inside the given range. /// If there is provenance overlapping with the edges, might result in an error. - pub fn clear(&mut self, range: AllocRange, cx: &impl HasDataLayout) { + #[allow(irrefutable_let_patterns)] // these actually make the code more clear + pub fn clear(&mut self, range: AllocRange, data_bytes: &[u8], cx: &impl HasDataLayout) { let start = range.start; let end = range.end(); // Clear the bytewise part -- this is easy. @@ -201,46 +231,42 @@ impl ProvenanceMap { bytes.remove_range(start..end); } + // Find all provenance overlapping the given range. + let ptrs_range = Self::adjusted_range_ptrs(range, cx); + if self.ptrs.range_is_empty(ptrs_range.clone()) { + // No provenance in this range, we are done. This is the common case. + return; + } let pointer_size = cx.data_layout().pointer_size(); - // For the ptr-sized part, find the first (inclusive) and last (exclusive) byte of - // provenance that overlaps with the given range. - let (first, last) = { - // Find all provenance overlapping the given range. - if self.range_ptrs_is_empty(range, cx) { - // No provenance in this range, we are done. This is the common case. - return; - } - - // This redoes some of the work of `range_get_ptrs_is_empty`, but this path is much - // colder than the early return above, so it's worth it. - let provenance = self.range_ptrs_get(range, cx); - (provenance.first().unwrap().0, provenance.last().unwrap().0 + pointer_size) - }; + // This redoes some of the work of `range_is_empty`, but this path is much + // colder than the early return above, so it's worth it. + let ptrs = self.ptrs.range(ptrs_range.clone()); // We need to handle clearing the provenance from parts of a pointer. - if first < start { + if let &(first, prov) = ptrs.first().unwrap() + && first < start + { // Insert the remaining part in the bytewise provenance. - let prov = self.ptrs[&first]; let bytes = self.bytes.get_or_insert_with(Box::default); - for offset in first..start { - bytes.insert(offset, (prov, (offset - first).bytes() as u8)); + for (pos, frag) in Self::ptr_fragments(..start, first, prov, data_bytes, pointer_size) { + bytes.insert(pos, frag); } } - if last > end { - let begin_of_last = last - pointer_size; + if let &(last, prov) = ptrs.last().unwrap() + && last + pointer_size > end + { // Insert the remaining part in the bytewise provenance. - let prov = self.ptrs[&begin_of_last]; let bytes = self.bytes.get_or_insert_with(Box::default); - for offset in end..last { - bytes.insert(offset, (prov, (offset - begin_of_last).bytes() as u8)); + for (pos, frag) in Self::ptr_fragments(end.., last, prov, data_bytes, pointer_size) { + bytes.insert(pos, frag); } } // Forget all the provenance. // Since provenance do not overlap, we know that removing until `last` (exclusive) is fine, // i.e., this will not remove any other provenance just after the ones we care about. - self.ptrs.remove_range(first..last); + self.ptrs.remove_range(ptrs_range); } /// Overwrites all provenance in the given range with wildcard provenance. @@ -248,30 +274,25 @@ impl ProvenanceMap { /// bytewise on their remaining bytes. /// /// Provided for usage in Miri and panics otherwise. - pub fn write_wildcards(&mut self, cx: &impl HasDataLayout, range: AllocRange) { + pub fn write_wildcards( + &mut self, + cx: &impl HasDataLayout, + data_bytes: &[u8], + range: AllocRange, + ) { let wildcard = Prov::WILDCARD.unwrap(); - let bytes = self.bytes.get_or_insert_with(Box::default); + // Clear existing provenance in this range. + self.clear(range, data_bytes, cx); - // Remove pointer provenances that overlap with the range, then readd the edge ones bytewise. - let ptr_range = Self::adjusted_range_ptrs(range, cx); - let ptrs = self.ptrs.range(ptr_range.clone()); - if let Some((offset, prov)) = ptrs.first().copied() { - for byte_ofs in offset..range.start { - bytes.insert(byte_ofs, (prov, (byte_ofs - offset).bytes() as u8)); - } - } - if let Some((offset, prov)) = ptrs.last().copied() { - for byte_ofs in range.end()..offset + cx.data_layout().pointer_size() { - bytes.insert(byte_ofs, (prov, (byte_ofs - offset).bytes() as u8)); - } - } - self.ptrs.remove_range(ptr_range); - - // Overwrite bytewise provenance. + // Make everything in the range wildcards. + let bytes = self.bytes.get_or_insert_with(Box::default); for offset in range.start..range.end() { - // The fragment index does not matter for wildcard provenance. - bytes.insert(offset, (wildcard, 0)); + // The fragment index and bytes do not matter for wildcard provenance. + bytes.insert( + offset, + PointerFrag { prov: wildcard, idx: Default::default(), bytes: Default::default() }, + ); } } } @@ -281,15 +302,16 @@ impl ProvenanceMap { /// Offsets are relative to the beginning of the copied range. pub struct ProvenanceCopy { ptrs: Box<[(Size, Prov)]>, - bytes: Box<[(Size, (Prov, u8))]>, + bytes: Box<[(Size, PointerFrag)]>, } impl ProvenanceMap { pub fn prepare_copy( &self, range: AllocRange, + data_bytes: &[u8], cx: &impl HasDataLayout, - ) -> AllocResult> { + ) -> ProvenanceCopy { let shift_offset = move |offset| offset - range.start; let ptr_size = cx.data_layout().pointer_size(); @@ -312,14 +334,15 @@ impl ProvenanceMap { let end_overlap = self.range_ptrs_get(alloc_range(range.end(), Size::ZERO), cx).first(); // We only need to go here if there is some overlap or some bytewise provenance. if begin_overlap.is_some() || end_overlap.is_some() || self.bytes.is_some() { - let mut bytes: Vec<(Size, (Prov, u8))> = Vec::new(); + let mut bytes: Vec<(Size, PointerFrag)> = Vec::new(); // First, if there is a part of a pointer at the start, add that. - if let Some(entry) = begin_overlap { - trace!("start overlapping entry: {entry:?}"); + if let Some(&(pos, prov)) = begin_overlap { // For really small copies, make sure we don't run off the end of the range. - let entry_end = cmp::min(entry.0 + ptr_size, range.end()); - for offset in range.start..entry_end { - bytes.push((shift_offset(offset), (entry.1, (offset - entry.0).bytes() as u8))); + let end = cmp::min(pos + ptr_size, range.end()); + for (pos, frag) in + Self::ptr_fragments(range.start..end, pos, prov, data_bytes, ptr_size) + { + bytes.push((shift_offset(pos), frag)); } } else { trace!("no start overlapping entry"); @@ -329,45 +352,35 @@ impl ProvenanceMap { bytes.extend( self.range_bytes_get(range) .iter() - .map(|&(offset, reloc)| (shift_offset(offset), reloc)), + .map(|(offset, frag)| (shift_offset(*offset), frag.clone())), ); // And finally possibly parts of a pointer at the end. - if let Some(entry) = end_overlap { - trace!("end overlapping entry: {entry:?}"); - // For really small copies, make sure we don't start before `range` does. - let entry_start = cmp::max(entry.0, range.start); - for offset in entry_start..range.end() { - if bytes.last().is_none_or(|bytes_entry| bytes_entry.0 < offset) { - // The last entry, if it exists, has a lower offset than us, so we - // can add it at the end and remain sorted. - bytes.push(( - shift_offset(offset), - (entry.1, (offset - entry.0).bytes() as u8), - )); - } else { - // There already is an entry for this offset in there! This can happen when the - // start and end range checks actually end up hitting the same pointer, so we - // already added this in the "pointer at the start" part above. - assert!(entry.0 <= range.start); - } + // We only have to go here if this is actually different than the begin_overlap. + if let Some(&(pos, prov)) = end_overlap + && begin_overlap.is_none_or(|(begin, _)| *begin != pos) + { + // If this was a really small copy, we'd have handled this in begin_overlap. + assert!(pos >= range.start); + for (pos, frag) in + Self::ptr_fragments(pos..range.end(), pos, prov, data_bytes, ptr_size) + { + let pos = shift_offset(pos); + // The last entry, if it exists, has a lower offset than us, so we + // can add it at the end and remain sorted. + debug_assert!(bytes.last().is_none_or(|bytes_entry| bytes_entry.0 < pos)); + bytes.push((pos, frag)); } } else { trace!("no end overlapping entry"); } trace!("byte provenances: {bytes:?}"); - if !bytes.is_empty() && !Prov::OFFSET_IS_ADDR { - // FIXME(#146291): We need to ensure that we don't mix different pointers with - // the same provenance. - return Err(AllocError::ReadPartialPointer(range.start)); - } - // And again a buffer for the new list on the target side. bytes_box = bytes.into_boxed_slice(); } - Ok(ProvenanceCopy { ptrs: ptrs_box, bytes: bytes_box }) + ProvenanceCopy { ptrs: ptrs_box, bytes: bytes_box } } /// Applies a provenance copy. @@ -381,8 +394,8 @@ impl ProvenanceMap { let chunk_len = copy.ptrs.len() as u64; self.ptrs.insert_presorted((0..chunk_len * repeat).map(|i| { let chunk = i / chunk_len; - let (offset, reloc) = copy.ptrs[(i % chunk_len) as usize]; - (shift_offset(chunk, offset), reloc) + let (offset, prov) = copy.ptrs[(i % chunk_len) as usize]; + (shift_offset(chunk, offset), prov) })); } if !copy.bytes.is_empty() { @@ -390,8 +403,8 @@ impl ProvenanceMap { self.bytes.get_or_insert_with(Box::default).insert_presorted( (0..chunk_len * repeat).map(|i| { let chunk = i / chunk_len; - let (offset, reloc) = copy.bytes[(i % chunk_len) as usize]; - (shift_offset(chunk, offset), reloc) + let (offset, frag) = ©.bytes[(i % chunk_len) as usize]; + (shift_offset(chunk, *offset), frag.clone()) }), ); } diff --git a/compiler/rustc_middle/src/mir/interpret/pointer.rs b/compiler/rustc_middle/src/mir/interpret/pointer.rs index 2aac8852b7e84..b6bcc87ff0413 100644 --- a/compiler/rustc_middle/src/mir/interpret/pointer.rs +++ b/compiler/rustc_middle/src/mir/interpret/pointer.rs @@ -77,9 +77,6 @@ pub trait Provenance: Copy + PartialEq + fmt::Debug + 'static { /// Otherwise this function is best-effort (but must agree with `Machine::ptr_get_alloc`). /// (Identifying the offset in that allocation, however, is harder -- use `Memory::ptr_get_alloc` for that.) fn get_alloc_id(self) -> Option; - - /// Defines the 'join' of provenance: what happens when doing a pointer load and different bytes have different provenance. - fn join(left: Self, right: Self) -> Option; } /// The type of provenance in the compile-time interpreter. @@ -191,10 +188,6 @@ impl Provenance for CtfeProvenance { fn get_alloc_id(self) -> Option { Some(self.alloc_id()) } - - fn join(left: Self, right: Self) -> Option { - if left == right { Some(left) } else { None } - } } // We also need this impl so that one can debug-print `Pointer` @@ -223,10 +216,6 @@ impl Provenance for AllocId { fn get_alloc_id(self) -> Option { Some(self) } - - fn join(_left: Self, _right: Self) -> Option { - unreachable!() - } } /// Represents a pointer in the Miri engine. diff --git a/compiler/rustc_middle/src/mir/pretty.rs b/compiler/rustc_middle/src/mir/pretty.rs index 929513c0dced5..0949d9e27e30b 100644 --- a/compiler/rustc_middle/src/mir/pretty.rs +++ b/compiler/rustc_middle/src/mir/pretty.rs @@ -1786,7 +1786,7 @@ pub fn write_allocation_bytes<'tcx, Prov: Provenance, Extra, Bytes: AllocBytes>( ascii.push('╼'); i += ptr_size; } - } else if let Some((prov, idx)) = alloc.provenance().get_byte(i, &tcx) { + } else if let Some(frag) = alloc.provenance().get_byte(i, &tcx) { // Memory with provenance must be defined assert!( alloc.init_mask().is_range_initialized(alloc_range(i, Size::from_bytes(1))).is_ok() @@ -1796,7 +1796,8 @@ pub fn write_allocation_bytes<'tcx, Prov: Provenance, Extra, Bytes: AllocBytes>( // Format is similar to "oversized" above. let j = i.bytes_usize(); let c = alloc.inspect_with_uninit_and_ptr_outside_interpreter(j..j + 1)[0]; - write!(w, "╾{c:02x}{prov:#?} (ptr fragment {idx})╼")?; + // FIXME: Find a way to print `frag.offset` that does not look terrible... + write!(w, "╾{c:02x}{prov:#?} (ptr fragment {idx})╼", prov = frag.prov, idx = frag.idx)?; i += Size::from_bytes(1); } else if alloc .init_mask() diff --git a/library/core/src/ptr/mod.rs b/library/core/src/ptr/mod.rs index fd067d19fcd98..ea0514f405f1e 100644 --- a/library/core/src/ptr/mod.rs +++ b/library/core/src/ptr/mod.rs @@ -1352,40 +1352,6 @@ pub const unsafe fn swap(x: *mut T, y: *mut T) { /// assert_eq!(x, [7, 8, 3, 4]); /// assert_eq!(y, [1, 2, 9]); /// ``` -/// -/// # Const evaluation limitations -/// -/// If this function is invoked during const-evaluation, the current implementation has a small (and -/// rarely relevant) limitation: if `count` is at least 2 and the data pointed to by `x` or `y` -/// contains a pointer that crosses the boundary of two `T`-sized chunks of memory, the function may -/// fail to evaluate (similar to a panic during const-evaluation). This behavior may change in the -/// future. -/// -/// The limitation is illustrated by the following example: -/// -/// ``` -/// use std::mem::size_of; -/// use std::ptr; -/// -/// const { unsafe { -/// const PTR_SIZE: usize = size_of::<*const i32>(); -/// let mut data1 = [0u8; PTR_SIZE]; -/// let mut data2 = [0u8; PTR_SIZE]; -/// // Store a pointer in `data1`. -/// data1.as_mut_ptr().cast::<*const i32>().write_unaligned(&42); -/// // Swap the contents of `data1` and `data2` by swapping `PTR_SIZE` many `u8`-sized chunks. -/// // This call will fail, because the pointer in `data1` crosses the boundary -/// // between several of the 1-byte chunks that are being swapped here. -/// //ptr::swap_nonoverlapping(data1.as_mut_ptr(), data2.as_mut_ptr(), PTR_SIZE); -/// // Swap the contents of `data1` and `data2` by swapping a single chunk of size -/// // `[u8; PTR_SIZE]`. That works, as there is no pointer crossing the boundary between -/// // two chunks. -/// ptr::swap_nonoverlapping(&mut data1, &mut data2, 1); -/// // Read the pointer from `data2` and dereference it. -/// let ptr = data2.as_ptr().cast::<*const i32>().read_unaligned(); -/// assert!(*ptr == 42); -/// } } -/// ``` #[inline] #[stable(feature = "swap_nonoverlapping", since = "1.27.0")] #[rustc_const_stable(feature = "const_swap_nonoverlapping", since = "1.88.0")] @@ -1414,9 +1380,7 @@ pub const unsafe fn swap_nonoverlapping(x: *mut T, y: *mut T, count: usize) { const_eval_select!( @capture[T] { x: *mut T, y: *mut T, count: usize }: if const { - // At compile-time we want to always copy this in chunks of `T`, to ensure that if there - // are pointers inside `T` we will copy them in one go rather than trying to copy a part - // of a pointer (which would not work). + // At compile-time we don't need all the special code below. // SAFETY: Same preconditions as this function unsafe { swap_nonoverlapping_const(x, y, count) } } else { diff --git a/library/coretests/tests/ptr.rs b/library/coretests/tests/ptr.rs index 4d5138d539b95..b0c691195a930 100644 --- a/library/coretests/tests/ptr.rs +++ b/library/coretests/tests/ptr.rs @@ -944,13 +944,12 @@ fn test_const_swap_ptr() { assert!(*s1.0.ptr == 666); assert!(*s2.0.ptr == 1); - // Swap them back, again as an array. - // FIXME(#146291): we should be swapping back at type `u8` but that currently does not work. + // Swap them back, byte-for-byte unsafe { ptr::swap_nonoverlapping( - ptr::from_mut(&mut s1).cast::(), - ptr::from_mut(&mut s2).cast::(), - 1, + ptr::from_mut(&mut s1).cast::(), + ptr::from_mut(&mut s2).cast::(), + size_of::(), ); } diff --git a/src/tools/miri/src/machine.rs b/src/tools/miri/src/machine.rs index fadbdf5cea999..38efc9562e05a 100644 --- a/src/tools/miri/src/machine.rs +++ b/src/tools/miri/src/machine.rs @@ -355,21 +355,6 @@ impl interpret::Provenance for Provenance { } Ok(()) } - - fn join(left: Self, right: Self) -> Option { - match (left, right) { - // If both are the *same* concrete tag, that is the result. - ( - Provenance::Concrete { alloc_id: left_alloc, tag: left_tag }, - Provenance::Concrete { alloc_id: right_alloc, tag: right_tag }, - ) if left_alloc == right_alloc && left_tag == right_tag => Some(left), - // If one side is a wildcard, the best possible outcome is that it is equal to the other - // one, and we use that. - (Provenance::Wildcard, o) | (o, Provenance::Wildcard) => Some(o), - // Otherwise, fall back to `None`. - _ => None, - } - } } impl fmt::Debug for ProvenanceExtra { diff --git a/tests/ui/consts/const-eval/ptr_fragments.rs b/tests/ui/consts/const-eval/ptr_fragments.rs index 7dc5870f89e2b..45564b596ef58 100644 --- a/tests/ui/consts/const-eval/ptr_fragments.rs +++ b/tests/ui/consts/const-eval/ptr_fragments.rs @@ -1,6 +1,5 @@ //! Test that various operations involving pointer fragments work as expected. //@ run-pass -//@ ignore-test: disabled due to use std::mem::{self, MaybeUninit, transmute}; use std::ptr; @@ -59,6 +58,15 @@ fn reassemble_ptr_fragments_in_static() { }; } +const _PARTIAL_OVERWRITE: () = { + // The result in `p` is not a valid pointer, but we never use it again so that's fine. + let mut p = &42; + unsafe { + let ptr: *mut _ = &mut p; + *(ptr as *mut u8) = 123; + } +}; + fn main() { assert_eq!(unsafe { MEMCPY_RET.assume_init().read() }, 42); } diff --git a/tests/ui/consts/const-eval/ptr_fragments_in_final.rs b/tests/ui/consts/const-eval/ptr_fragments_in_final.rs index aed33d7ed8d31..4037a2d237221 100644 --- a/tests/ui/consts/const-eval/ptr_fragments_in_final.rs +++ b/tests/ui/consts/const-eval/ptr_fragments_in_final.rs @@ -1,5 +1,4 @@ //! Test that we properly error when there is a pointer fragment in the final value. -//@ ignore-test: disabled due to use std::{mem::{self, MaybeUninit}, ptr}; @@ -21,6 +20,21 @@ const MEMCPY_RET: MaybeUninit<*const i32> = unsafe { //~ERROR: partial pointer i ptr2 }; -fn main() { - assert_eq!(unsafe { MEMCPY_RET.assume_init().read() }, 42); -} +// Mixing two different pointers that have the same provenance. +const MIXED_PTR: MaybeUninit<*const u8> = { //~ERROR: partial pointer in final value + static A: u8 = 123; + const HALF_PTR: usize = std::mem::size_of::<*const ()>() / 2; + + unsafe { + let x: *const u8 = &raw const A; + let mut y = MaybeUninit::new(x.wrapping_add(usize::MAX / 4)); + core::ptr::copy_nonoverlapping( + (&raw const x).cast::(), + (&raw mut y).cast::(), + HALF_PTR, + ); + y + } +}; + +fn main() {} diff --git a/tests/ui/consts/const-eval/ptr_fragments_in_final.stderr b/tests/ui/consts/const-eval/ptr_fragments_in_final.stderr index 628bf2566e592..41a8224165811 100644 --- a/tests/ui/consts/const-eval/ptr_fragments_in_final.stderr +++ b/tests/ui/consts/const-eval/ptr_fragments_in_final.stderr @@ -6,5 +6,13 @@ LL | const MEMCPY_RET: MaybeUninit<*const i32> = unsafe { | = note: while pointers can be broken apart into individual bytes during const-evaluation, only complete pointers (with all their bytes in the right order) are supported in the final value -error: aborting due to 1 previous error +error: encountered partial pointer in final value of constant + --> $DIR/ptr_fragments_in_final.rs:24:1 + | +LL | const MIXED_PTR: MaybeUninit<*const u8> = { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: while pointers can be broken apart into individual bytes during const-evaluation, only complete pointers (with all their bytes in the right order) are supported in the final value + +error: aborting due to 2 previous errors diff --git a/tests/ui/consts/const-eval/ptr_fragments_mixed.rs b/tests/ui/consts/const-eval/ptr_fragments_mixed.rs index 79a42820f501e..24169eac4780d 100644 --- a/tests/ui/consts/const-eval/ptr_fragments_mixed.rs +++ b/tests/ui/consts/const-eval/ptr_fragments_mixed.rs @@ -1,10 +1,13 @@ -//! This mixes fragments from different pointers to the same allocarion, in a way -//! that we should not accept. See . +//! This mixes fragments from different pointers, in a way that we should not accept. +//! See . + static A: u8 = 123; +static B: u8 = 123; const HALF_PTR: usize = std::mem::size_of::<*const ()>() / 2; -const fn mix_ptr() -> *const u8 { +// All fragments have the same provenance, but they did not all come from the same pointer. +const APTR: *const u8 = { unsafe { let x: *const u8 = &raw const A; let mut y = x.wrapping_add(usize::MAX / 4); @@ -13,16 +16,22 @@ const fn mix_ptr() -> *const u8 { (&raw mut y).cast::(), HALF_PTR, ); - y + y //~ERROR: unable to read parts of a pointer } -} +}; -const APTR: *const u8 = mix_ptr(); //~ERROR: unable to read parts of a pointer +// All fragments have the same relative offset, but not all the same provenance. +const BPTR: *const u8 = { + unsafe { + let x: *const u8 = &raw const A; + let mut y = &raw const B; + core::ptr::copy_nonoverlapping( + (&raw const x).cast::(), + (&raw mut y).cast::(), + HALF_PTR, + ); + y //~ERROR: unable to read parts of a pointer + } +}; -fn main() { - let a = APTR; - println!("{a:p}"); - let b = mix_ptr(); - println!("{b:p}"); - assert_eq!(a, b); -} +fn main() {} diff --git a/tests/ui/consts/const-eval/ptr_fragments_mixed.stderr b/tests/ui/consts/const-eval/ptr_fragments_mixed.stderr index 9e991ab7a7d38..608f09d24bf42 100644 --- a/tests/ui/consts/const-eval/ptr_fragments_mixed.stderr +++ b/tests/ui/consts/const-eval/ptr_fragments_mixed.stderr @@ -1,23 +1,21 @@ error[E0080]: unable to read parts of a pointer from memory at ALLOC0 - --> $DIR/ptr_fragments_mixed.rs:20:25 + --> $DIR/ptr_fragments_mixed.rs:19:9 | -LL | const APTR: *const u8 = mix_ptr(); - | ^^^^^^^^^ evaluation of `APTR` failed inside this call +LL | y + | ^ evaluation of `APTR` failed here | = help: this code performed an operation that depends on the underlying bytes representing a pointer = help: the absolute address of a pointer is not known at compile-time, so such operations are not supported -note: inside `mix_ptr` - --> $DIR/ptr_fragments_mixed.rs:11:9 + +error[E0080]: unable to read parts of a pointer from memory at ALLOC1 + --> $DIR/ptr_fragments_mixed.rs:33:9 + | +LL | y + | ^ evaluation of `BPTR` failed here | -LL | / core::ptr::copy_nonoverlapping( -LL | | (&raw const x).cast::(), -LL | | (&raw mut y).cast::(), -LL | | HALF_PTR, -LL | | ); - | |_________^ -note: inside `std::ptr::copy_nonoverlapping::` - --> $SRC_DIR/core/src/ptr/mod.rs:LL:COL + = help: this code performed an operation that depends on the underlying bytes representing a pointer + = help: the absolute address of a pointer is not known at compile-time, so such operations are not supported -error: aborting due to 1 previous error +error: aborting due to 2 previous errors For more information about this error, try `rustc --explain E0080`. diff --git a/tests/ui/consts/const-eval/read_partial_ptr.rs b/tests/ui/consts/const-eval/read_partial_ptr.rs index c153e274e41f6..bccef9c0bc6ca 100644 --- a/tests/ui/consts/const-eval/read_partial_ptr.rs +++ b/tests/ui/consts/const-eval/read_partial_ptr.rs @@ -1,5 +1,4 @@ //! Ensure we error when trying to load from a pointer whose provenance has been messed with. -//@ ignore-test: disabled due to const PARTIAL_OVERWRITE: () = { let mut p = &42;