Skip to content

Commit 63f3ab3

Browse files
committed
Auto merge of #118324 - RalfJung:ctfe-read-only-pointers, r=saethlin
compile-time evaluation: detect writes through immutable pointers This has two motivations: - it unblocks rust-lang/rust#116745 (and therefore takes a big step towards `const_mut_refs` stabilization), because we can now detect if the memory that we find in `const` can be interned as "immutable" - it would detect the UB that was uncovered in rust-lang/rust#117905, which was caused by accidental stabilization of `copy` functions in `const` that can only be called with UB When UB is detected, we emit a future-compat warn-by-default lint. This is not a breaking change, so completely in line with [the const-UB RFC](https://rust-lang.github.io/rfcs/3016-const-ub.html), meaning we don't need t-lang FCP here. I made the lint immediately show up for dependencies since it is nearly impossible to even trigger this lint without `const_mut_refs` -- the accidentally stabilized `copy` functions are the only way this can happen, so the crates that popped up in #117905 are the only causes of such UB (in the code that crater covers), and the three cases of UB that we know about have all been fixed in their respective crates already. The way this is implemented is by making use of the fact that our interpreter is already generic over the notion of provenance. For CTFE we now use the new `CtfeProvenance` type which is conceptually an `AllocId` plus a boolean `immutable` flag (but packed for a more efficient representation). This means we can mark a pointer as immutable when it is created as a shared reference. The flag will be propagated to all pointers derived from this one. We can then check the immutable flag on each write to reject writes through immutable pointers. I just hope perf works out.
2 parents 1fb4056 + 786aeb2 commit 63f3ab3

File tree

3 files changed

+23
-9
lines changed

3 files changed

+23
-9
lines changed

src/diagnostics.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ pub enum TerminationInfo {
4444
},
4545
DataRace {
4646
involves_non_atomic: bool,
47-
ptr: Pointer,
47+
ptr: Pointer<AllocId>,
4848
op1: RacingOp,
4949
op2: RacingOp,
5050
extra: Option<&'static str>,

src/intptrcast.rs

+3-2
Original file line numberDiff line numberDiff line change
@@ -256,12 +256,13 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
256256
/// Convert a relative (tcx) pointer to a Miri pointer.
257257
fn ptr_from_rel_ptr(
258258
&self,
259-
ptr: Pointer<AllocId>,
259+
ptr: Pointer<CtfeProvenance>,
260260
tag: BorTag,
261261
) -> InterpResult<'tcx, Pointer<Provenance>> {
262262
let ecx = self.eval_context_ref();
263263

264-
let (alloc_id, offset) = ptr.into_parts(); // offset is relative (AllocId provenance)
264+
let (prov, offset) = ptr.into_parts(); // offset is relative (AllocId provenance)
265+
let alloc_id = prov.alloc_id();
265266
let base_addr = ecx.addr_from_alloc_id(alloc_id)?;
266267

267268
// Add offset with the right kind of pointer-overflowing arithmetic.

src/machine.rs

+19-6
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ use rustc_data_structures::fx::{FxHashMap, FxHashSet};
1717
use rustc_data_structures::static_assert_size;
1818
use rustc_middle::{
1919
mir,
20+
query::TyCtxtAt,
2021
ty::{
2122
self,
2223
layout::{LayoutCx, LayoutError, LayoutOf, TyAndLayout},
@@ -259,6 +260,17 @@ impl interpret::Provenance for Provenance {
259260
}
260261
}
261262

263+
fn fmt(ptr: &Pointer<Self>, f: &mut fmt::Formatter<'_>) -> fmt::Result {
264+
let (prov, addr) = ptr.into_parts(); // address is absolute
265+
write!(f, "{:#x}", addr.bytes())?;
266+
if f.alternate() {
267+
write!(f, "{prov:#?}")?;
268+
} else {
269+
write!(f, "{prov:?}")?;
270+
}
271+
Ok(())
272+
}
273+
262274
fn join(left: Option<Self>, right: Option<Self>) -> Option<Self> {
263275
match (left, right) {
264276
// If both are the *same* concrete tag, that is the result.
@@ -1170,11 +1182,11 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> {
11701182

11711183
fn adjust_alloc_base_pointer(
11721184
ecx: &MiriInterpCx<'mir, 'tcx>,
1173-
ptr: Pointer<AllocId>,
1185+
ptr: Pointer<CtfeProvenance>,
11741186
) -> InterpResult<'tcx, Pointer<Provenance>> {
1187+
let alloc_id = ptr.provenance.alloc_id();
11751188
if cfg!(debug_assertions) {
11761189
// The machine promises to never call us on thread-local or extern statics.
1177-
let alloc_id = ptr.provenance;
11781190
match ecx.tcx.try_get_global_alloc(alloc_id) {
11791191
Some(GlobalAlloc::Static(def_id)) if ecx.tcx.is_thread_local_static(def_id) => {
11801192
panic!("adjust_alloc_base_pointer called on thread-local static")
@@ -1185,8 +1197,9 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> {
11851197
_ => {}
11861198
}
11871199
}
1200+
// FIXME: can we somehow preserve the immutability of `ptr`?
11881201
let tag = if let Some(borrow_tracker) = &ecx.machine.borrow_tracker {
1189-
borrow_tracker.borrow_mut().base_ptr_tag(ptr.provenance, &ecx.machine)
1202+
borrow_tracker.borrow_mut().base_ptr_tag(alloc_id, &ecx.machine)
11901203
} else {
11911204
// Value does not matter, SB is disabled
11921205
BorTag::default()
@@ -1245,7 +1258,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> {
12451258

12461259
#[inline(always)]
12471260
fn before_memory_read(
1248-
_tcx: TyCtxt<'tcx>,
1261+
_tcx: TyCtxtAt<'tcx>,
12491262
machine: &Self,
12501263
alloc_extra: &AllocExtra<'tcx>,
12511264
(alloc_id, prov_extra): (AllocId, Self::ProvenanceExtra),
@@ -1265,7 +1278,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> {
12651278

12661279
#[inline(always)]
12671280
fn before_memory_write(
1268-
_tcx: TyCtxt<'tcx>,
1281+
_tcx: TyCtxtAt<'tcx>,
12691282
machine: &mut Self,
12701283
alloc_extra: &mut AllocExtra<'tcx>,
12711284
(alloc_id, prov_extra): (AllocId, Self::ProvenanceExtra),
@@ -1285,7 +1298,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> {
12851298

12861299
#[inline(always)]
12871300
fn before_memory_deallocation(
1288-
_tcx: TyCtxt<'tcx>,
1301+
_tcx: TyCtxtAt<'tcx>,
12891302
machine: &mut Self,
12901303
alloc_extra: &mut AllocExtra<'tcx>,
12911304
(alloc_id, prove_extra): (AllocId, Self::ProvenanceExtra),

0 commit comments

Comments
 (0)