Skip to content

Commit 55c938d

Browse files
authored
Unrolled build for #146324
Rollup merge of #146324 - RalfJung:no-ptr-fragment, r=oli-obk const-eval: disable pointer fragment support This fixes #146291 by disabling pointer fragment support for const-eval. I want to properly fix this eventually, but won't get to it in the next few weeks, so this is an emergency patch to prevent the buggy implementation from landing on stable. The beta cutoff is on Sep 12th so if this PR lands after that, we'll need a backport.
2 parents fefce3c + aed0ed4 commit 55c938d

File tree

10 files changed

+119
-9
lines changed

10 files changed

+119
-9
lines changed

compiler/rustc_const_eval/src/interpret/memory.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1501,8 +1501,10 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
15011501
// `get_bytes_mut` will clear the provenance, which is correct,
15021502
// since we don't want to keep any provenance at the target.
15031503
// This will also error if copying partial provenance is not supported.
1504-
let provenance =
1505-
src_alloc.provenance().prepare_copy(src_range, dest_offset, num_copies, self);
1504+
let provenance = src_alloc
1505+
.provenance()
1506+
.prepare_copy(src_range, dest_offset, num_copies, self)
1507+
.map_err(|e| e.to_interp_error(src_alloc_id))?;
15061508
// Prepare a copy of the initialization mask.
15071509
let init = src_alloc.init_mask().prepare_copy(src_range);
15081510

compiler/rustc_middle/src/mir/interpret/allocation.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -724,6 +724,11 @@ impl<Prov: Provenance, Extra, Bytes: AllocBytes> Allocation<Prov, Extra, Bytes>
724724
}
725725
// If we get here, we have to check per-byte provenance, and join them together.
726726
let prov = 'prov: {
727+
if !Prov::OFFSET_IS_ADDR {
728+
// FIXME(#146291): We need to ensure that we don't mix different pointers with
729+
// the same provenance.
730+
return Err(AllocError::ReadPartialPointer(range.start));
731+
}
727732
// Initialize with first fragment. Must have index 0.
728733
let Some((mut joint_prov, 0)) = self.provenance.get_byte(range.start, cx) else {
729734
break 'prov None;

compiler/rustc_middle/src/mir/interpret/allocation/provenance_map.rs

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ use rustc_serialize::{Decodable, Decoder, Encodable, Encoder};
1111
use tracing::trace;
1212

1313
use super::{AllocRange, CtfeProvenance, Provenance, alloc_range};
14+
use crate::mir::interpret::{AllocError, AllocResult};
1415

1516
/// Stores the provenance information of pointers stored in memory.
1617
#[derive(Clone, PartialEq, Eq, Hash, Debug)]
@@ -137,6 +138,11 @@ impl<Prov: Provenance> ProvenanceMap<Prov> {
137138
let Some(bytes) = self.bytes.as_deref_mut() else {
138139
return true;
139140
};
141+
if !Prov::OFFSET_IS_ADDR {
142+
// FIXME(#146291): We need to ensure that we don't mix different pointers with
143+
// the same provenance.
144+
return false;
145+
}
140146
let ptr_size = cx.data_layout().pointer_size();
141147
while let Some((offset, (prov, _))) = bytes.iter().next().copied() {
142148
// Check if this fragment starts a pointer.
@@ -285,7 +291,7 @@ impl<Prov: Provenance> ProvenanceMap<Prov> {
285291
dest: Size,
286292
count: u64,
287293
cx: &impl HasDataLayout,
288-
) -> ProvenanceCopy<Prov> {
294+
) -> AllocResult<ProvenanceCopy<Prov>> {
289295
let shift_offset = move |idx, offset| {
290296
// compute offset for current repetition
291297
let dest_offset = dest + src.size * idx; // `Size` operations
@@ -363,6 +369,12 @@ impl<Prov: Provenance> ProvenanceMap<Prov> {
363369
}
364370
trace!("byte provenances: {bytes:?}");
365371

372+
if !bytes.is_empty() && !Prov::OFFSET_IS_ADDR {
373+
// FIXME(#146291): We need to ensure that we don't mix different pointers with
374+
// the same provenance.
375+
return Err(AllocError::ReadPartialPointer(src.start));
376+
}
377+
366378
// And again a buffer for the new list on the target side.
367379
let mut dest_bytes = Vec::with_capacity(bytes.len() * (count as usize));
368380
for i in 0..count {
@@ -373,7 +385,7 @@ impl<Prov: Provenance> ProvenanceMap<Prov> {
373385
dest_bytes_box = Some(dest_bytes.into_boxed_slice());
374386
}
375387

376-
ProvenanceCopy { dest_ptrs: dest_ptrs_box, dest_bytes: dest_bytes_box }
388+
Ok(ProvenanceCopy { dest_ptrs: dest_ptrs_box, dest_bytes: dest_bytes_box })
377389
}
378390

379391
/// Applies a provenance copy.

library/core/src/ptr/mod.rs

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1348,6 +1348,40 @@ pub const unsafe fn swap<T>(x: *mut T, y: *mut T) {
13481348
/// assert_eq!(x, [7, 8, 3, 4]);
13491349
/// assert_eq!(y, [1, 2, 9]);
13501350
/// ```
1351+
///
1352+
/// # Const evaluation limitations
1353+
///
1354+
/// If this function is invoked during const-evaluation, the current implementation has a small (and
1355+
/// rarely relevant) limitation: if `count` is at least 2 and the data pointed to by `x` or `y`
1356+
/// contains a pointer that crosses the boundary of two `T`-sized chunks of memory, the function may
1357+
/// fail to evaluate (similar to a panic during const-evaluation). This behavior may change in the
1358+
/// future.
1359+
///
1360+
/// The limitation is illustrated by the following example:
1361+
///
1362+
/// ```
1363+
/// use std::mem::size_of;
1364+
/// use std::ptr;
1365+
///
1366+
/// const { unsafe {
1367+
/// const PTR_SIZE: usize = size_of::<*const i32>();
1368+
/// let mut data1 = [0u8; PTR_SIZE];
1369+
/// let mut data2 = [0u8; PTR_SIZE];
1370+
/// // Store a pointer in `data1`.
1371+
/// data1.as_mut_ptr().cast::<*const i32>().write_unaligned(&42);
1372+
/// // Swap the contents of `data1` and `data2` by swapping `PTR_SIZE` many `u8`-sized chunks.
1373+
/// // This call will fail, because the pointer in `data1` crosses the boundary
1374+
/// // between several of the 1-byte chunks that are being swapped here.
1375+
/// //ptr::swap_nonoverlapping(data1.as_mut_ptr(), data2.as_mut_ptr(), PTR_SIZE);
1376+
/// // Swap the contents of `data1` and `data2` by swapping a single chunk of size
1377+
/// // `[u8; PTR_SIZE]`. That works, as there is no pointer crossing the boundary between
1378+
/// // two chunks.
1379+
/// ptr::swap_nonoverlapping(&mut data1, &mut data2, 1);
1380+
/// // Read the pointer from `data2` and dereference it.
1381+
/// let ptr = data2.as_ptr().cast::<*const i32>().read_unaligned();
1382+
/// assert!(*ptr == 42);
1383+
/// } }
1384+
/// ```
13511385
#[inline]
13521386
#[stable(feature = "swap_nonoverlapping", since = "1.27.0")]
13531387
#[rustc_const_stable(feature = "const_swap_nonoverlapping", since = "1.88.0")]
@@ -1376,7 +1410,9 @@ pub const unsafe fn swap_nonoverlapping<T>(x: *mut T, y: *mut T, count: usize) {
13761410
const_eval_select!(
13771411
@capture[T] { x: *mut T, y: *mut T, count: usize }:
13781412
if const {
1379-
// At compile-time we don't need all the special code below.
1413+
// At compile-time we want to always copy this in chunks of `T`, to ensure that if there
1414+
// are pointers inside `T` we will copy them in one go rather than trying to copy a part
1415+
// of a pointer (which would not work).
13801416
// SAFETY: Same preconditions as this function
13811417
unsafe { swap_nonoverlapping_const(x, y, count) }
13821418
} else {

library/coretests/tests/ptr.rs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -936,12 +936,13 @@ fn test_const_swap_ptr() {
936936
assert!(*s1.0.ptr == 666);
937937
assert!(*s2.0.ptr == 1);
938938

939-
// Swap them back, byte-for-byte
939+
// Swap them back, again as an array.
940+
// FIXME(#146291): we should be swapping back at type `u8` but that currently does not work.
940941
unsafe {
941942
ptr::swap_nonoverlapping(
942-
ptr::from_mut(&mut s1).cast::<u8>(),
943-
ptr::from_mut(&mut s2).cast::<u8>(),
944-
size_of::<A>(),
943+
ptr::from_mut(&mut s1).cast::<T>(),
944+
ptr::from_mut(&mut s2).cast::<T>(),
945+
1,
945946
);
946947
}
947948

tests/ui/consts/const-eval/ptr_fragments.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
//! Test that various operations involving pointer fragments work as expected.
22
//@ run-pass
3+
//@ ignore-test: disabled due to <https://github.com/rust-lang/rust/issues/146291>
34

45
use std::mem::{self, MaybeUninit, transmute};
56
use std::ptr;

tests/ui/consts/const-eval/ptr_fragments_in_final.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
//! Test that we properly error when there is a pointer fragment in the final value.
2+
//@ ignore-test: disabled due to <https://github.com/rust-lang/rust/issues/146291>
23

34
use std::{mem::{self, MaybeUninit}, ptr};
45

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
//! This mixes fragments from different pointers to the same allocarion, in a way
2+
//! that we should not accept. See <https://github.com/rust-lang/rust/issues/146291>.
3+
static A: u8 = 123;
4+
5+
const HALF_PTR: usize = std::mem::size_of::<*const ()>() / 2;
6+
7+
const fn mix_ptr() -> *const u8 {
8+
unsafe {
9+
let x: *const u8 = &raw const A;
10+
let mut y = x.wrapping_add(usize::MAX / 4);
11+
core::ptr::copy_nonoverlapping(
12+
(&raw const x).cast::<u8>(),
13+
(&raw mut y).cast::<u8>(),
14+
HALF_PTR,
15+
);
16+
y
17+
}
18+
}
19+
20+
const APTR: *const u8 = mix_ptr(); //~ERROR: unable to read parts of a pointer
21+
22+
fn main() {
23+
let a = APTR;
24+
println!("{a:p}");
25+
let b = mix_ptr();
26+
println!("{b:p}");
27+
assert_eq!(a, b);
28+
}
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
error[E0080]: unable to read parts of a pointer from memory at ALLOC0
2+
--> $DIR/ptr_fragments_mixed.rs:20:25
3+
|
4+
LL | const APTR: *const u8 = mix_ptr();
5+
| ^^^^^^^^^ evaluation of `APTR` failed inside this call
6+
|
7+
= help: this code performed an operation that depends on the underlying bytes representing a pointer
8+
= help: the absolute address of a pointer is not known at compile-time, so such operations are not supported
9+
note: inside `mix_ptr`
10+
--> $DIR/ptr_fragments_mixed.rs:11:9
11+
|
12+
LL | / core::ptr::copy_nonoverlapping(
13+
LL | | (&raw const x).cast::<u8>(),
14+
LL | | (&raw mut y).cast::<u8>(),
15+
LL | | HALF_PTR,
16+
LL | | );
17+
| |_________^
18+
note: inside `std::ptr::copy_nonoverlapping::<u8>`
19+
--> $SRC_DIR/core/src/ptr/mod.rs:LL:COL
20+
21+
error: aborting due to 1 previous error
22+
23+
For more information about this error, try `rustc --explain E0080`.

tests/ui/consts/const-eval/read_partial_ptr.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
//! Ensure we error when trying to load from a pointer whose provenance has been messed with.
2+
//@ ignore-test: disabled due to <https://github.com/rust-lang/rust/issues/146291>
23

34
const PARTIAL_OVERWRITE: () = {
45
let mut p = &42;

0 commit comments

Comments
 (0)