Skip to content

Commit b5c17f8

Browse files
borsbors
committed
Auto merge of #118324 - RalfJung:ctfe-read-only-pointers, r=<try>
compile-time evaluation: detect writes through immutable pointers This has two motivations: - it unblocks #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 #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 uncovered have all been fixed in their respective crates already. I just hope perf works out.
2 parents 0b8a61b + f25c190 commit b5c17f8

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

48 files changed

+635
-324
lines changed

Diff for: ‎compiler/rustc_codegen_cranelift/src/constant.rs

+4-2
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,8 @@ pub(crate) fn codegen_const_value<'tcx>(
126126
}
127127
}
128128
Scalar::Ptr(ptr, _size) => {
129-
let (alloc_id, offset) = ptr.into_parts(); // we know the `offset` is relative
129+
let (prov, offset) = ptr.into_parts(); // we know the `offset` is relative
130+
let alloc_id = prov.alloc_id();
130131
let base_addr = match fx.tcx.global_alloc(alloc_id) {
131132
GlobalAlloc::Memory(alloc) => {
132133
let data_id = data_id_for_alloc_id(
@@ -374,7 +375,8 @@ fn define_all_allocs(tcx: TyCtxt<'_>, module: &mut dyn Module, cx: &mut Constant
374375
let bytes = alloc.inspect_with_uninit_and_ptr_outside_interpreter(0..alloc.len()).to_vec();
375376
data.define(bytes.into_boxed_slice());
376377

377-
for &(offset, alloc_id) in alloc.provenance().ptrs().iter() {
378+
for &(offset, prov) in alloc.provenance().ptrs().iter() {
379+
let alloc_id = prov.alloc_id();
378380
let addend = {
379381
let endianness = tcx.data_layout.endian;
380382
let offset = offset.bytes() as usize;

Diff for: ‎compiler/rustc_codegen_gcc/src/common.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,8 @@ impl<'gcc, 'tcx> ConstMethods<'tcx> for CodegenCx<'gcc, 'tcx> {
199199
}
200200
}
201201
Scalar::Ptr(ptr, _size) => {
202-
let (alloc_id, offset) = ptr.into_parts();
202+
let (prov, offset) = ptr.into_parts(); // we know the `offset` is relative
203+
let alloc_id = prov.alloc_id();
203204
let base_addr =
204205
match self.tcx.global_alloc(alloc_id) {
205206
GlobalAlloc::Memory(alloc) => {

Diff for: ‎compiler/rustc_codegen_gcc/src/consts.rs

+3-2
Original file line numberDiff line numberDiff line change
@@ -285,7 +285,8 @@ pub fn const_alloc_to_gcc<'gcc, 'tcx>(cx: &CodegenCx<'gcc, 'tcx>, alloc: ConstAl
285285
let pointer_size = dl.pointer_size.bytes() as usize;
286286

287287
let mut next_offset = 0;
288-
for &(offset, alloc_id) in alloc.provenance().ptrs().iter() {
288+
for &(offset, prov) in alloc.provenance().ptrs().iter() {
289+
let alloc_id = prov.alloc_id();
289290
let offset = offset.bytes();
290291
assert_eq!(offset as usize as u64, offset);
291292
let offset = offset as usize;
@@ -313,7 +314,7 @@ pub fn const_alloc_to_gcc<'gcc, 'tcx>(cx: &CodegenCx<'gcc, 'tcx>, alloc: ConstAl
313314

314315
llvals.push(cx.scalar_to_backend(
315316
InterpScalar::from_pointer(
316-
interpret::Pointer::new(alloc_id, Size::from_bytes(ptr_offset)),
317+
interpret::Pointer::new(prov, Size::from_bytes(ptr_offset)),
317318
&cx.tcx,
318319
),
319320
abi::Scalar::Initialized { value: Primitive::Pointer(address_space), valid_range: WrappingRange::full(dl.pointer_size) },

Diff for: ‎compiler/rustc_codegen_llvm/src/common.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -246,8 +246,8 @@ impl<'ll, 'tcx> ConstMethods<'tcx> for CodegenCx<'ll, 'tcx> {
246246
}
247247
}
248248
Scalar::Ptr(ptr, _size) => {
249-
let (alloc_id, offset) = ptr.into_parts();
250-
let (base_addr, base_addr_space) = match self.tcx.global_alloc(alloc_id) {
249+
let (prov, offset) = ptr.into_parts();
250+
let (base_addr, base_addr_space) = match self.tcx.global_alloc(prov.alloc_id()) {
251251
GlobalAlloc::Memory(alloc) => {
252252
let init = const_alloc_to_llvm(self, alloc);
253253
let alloc = alloc.inner();

Diff for: ‎compiler/rustc_codegen_llvm/src/consts.rs

+3-6
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ pub fn const_alloc_to_llvm<'ll>(cx: &CodegenCx<'ll, '_>, alloc: ConstAllocation<
7272
}
7373

7474
let mut next_offset = 0;
75-
for &(offset, alloc_id) in alloc.provenance().ptrs().iter() {
75+
for &(offset, prov) in alloc.provenance().ptrs().iter() {
7676
let offset = offset.bytes();
7777
assert_eq!(offset as usize as u64, offset);
7878
let offset = offset as usize;
@@ -92,13 +92,10 @@ pub fn const_alloc_to_llvm<'ll>(cx: &CodegenCx<'ll, '_>, alloc: ConstAllocation<
9292
.expect("const_alloc_to_llvm: could not read relocation pointer")
9393
as u64;
9494

95-
let address_space = cx.tcx.global_alloc(alloc_id).address_space(cx);
95+
let address_space = cx.tcx.global_alloc(prov.alloc_id()).address_space(cx);
9696

9797
llvals.push(cx.scalar_to_backend(
98-
InterpScalar::from_pointer(
99-
Pointer::new(alloc_id, Size::from_bytes(ptr_offset)),
100-
&cx.tcx,
101-
),
98+
InterpScalar::from_pointer(Pointer::new(prov, Size::from_bytes(ptr_offset)), &cx.tcx),
10299
Scalar::Initialized {
103100
value: Primitive::Pointer(address_space),
104101
valid_range: WrappingRange::full(dl.pointer_size),

Diff for: ‎compiler/rustc_codegen_ssa/src/mir/operand.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ impl<'a, 'tcx, V: CodegenObject> OperandRef<'tcx, V> {
105105
bug!("from_const: invalid ScalarPair layout: {:#?}", layout);
106106
};
107107
let a = Scalar::from_pointer(
108-
Pointer::new(bx.tcx().reserve_and_set_memory_alloc(data), Size::ZERO),
108+
Pointer::new(bx.tcx().reserve_and_set_memory_alloc(data).into(), Size::ZERO),
109109
&bx.tcx(),
110110
);
111111
let a_llval = bx.scalar_to_backend(

Diff for: ‎compiler/rustc_const_eval/messages.ftl

+3
Original file line numberDiff line numberDiff line change
@@ -461,6 +461,9 @@ const_eval_validation_uninhabited_val = {$front_matter}: encountered a value of
461461
const_eval_validation_uninit = {$front_matter}: encountered uninitialized memory, but {$expected}
462462
const_eval_validation_unsafe_cell = {$front_matter}: encountered `UnsafeCell` in a `const`
463463
464+
const_eval_write_through_immutable_pointer =
465+
writing through a pointer that was derived from a shared (immutable) reference
466+
464467
const_eval_write_to_read_only =
465468
writing to {$allocation} which is read-only
466469
const_eval_zst_pointer_out_of_bounds =

Diff for: ‎compiler/rustc_const_eval/src/const_eval/error.rs

+36-8
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,16 @@
11
use std::mem;
22

33
use rustc_errors::{DiagnosticArgValue, DiagnosticMessage, IntoDiagnostic, IntoDiagnosticArg};
4+
use rustc_hir::CRATE_HIR_ID;
45
use rustc_middle::mir::AssertKind;
6+
use rustc_middle::query::TyCtxtAt;
57
use rustc_middle::ty::TyCtxt;
68
use rustc_middle::ty::{layout::LayoutError, ConstInt};
79
use rustc_span::{ErrorGuaranteed, Span, Symbol, DUMMY_SP};
810

9-
use super::InterpCx;
11+
use super::{CompileTimeInterpreter, InterpCx};
1012
use crate::errors::{self, FrameNote, ReportErrorExt};
11-
use crate::interpret::{ErrorHandled, InterpError, InterpErrorInfo, Machine, MachineStopType};
13+
use crate::interpret::{ErrorHandled, InterpError, InterpErrorInfo, MachineStopType};
1214

1315
/// The CTFE machine has some custom error kinds.
1416
#[derive(Clone, Debug)]
@@ -57,16 +59,20 @@ impl<'tcx> Into<InterpErrorInfo<'tcx>> for ConstEvalErrKind {
5759
}
5860
}
5961

60-
pub fn get_span_and_frames<'tcx, 'mir, M: Machine<'mir, 'tcx>>(
61-
ecx: &InterpCx<'mir, 'tcx, M>,
62+
pub fn get_span_and_frames<'tcx, 'mir>(
63+
tcx: TyCtxtAt<'tcx>,
64+
machine: &CompileTimeInterpreter<'mir, 'tcx>,
6265
) -> (Span, Vec<errors::FrameNote>)
6366
where
6467
'tcx: 'mir,
6568
{
66-
let mut stacktrace = ecx.generate_stacktrace();
69+
let mut stacktrace =
70+
InterpCx::<CompileTimeInterpreter<'mir, 'tcx>>::generate_stacktrace_from_stack(
71+
&machine.stack,
72+
);
6773
// Filter out `requires_caller_location` frames.
68-
stacktrace.retain(|frame| !frame.instance.def.requires_caller_location(*ecx.tcx));
69-
let span = stacktrace.first().map(|f| f.span).unwrap_or(ecx.tcx.span);
74+
stacktrace.retain(|frame| !frame.instance.def.requires_caller_location(*tcx));
75+
let span = stacktrace.first().map(|f| f.span).unwrap_or(tcx.span);
7076

7177
let mut frames = Vec::new();
7278

@@ -87,7 +93,7 @@ where
8793

8894
let mut last_frame: Option<errors::FrameNote> = None;
8995
for frame_info in &stacktrace {
90-
let frame = frame_info.as_note(*ecx.tcx);
96+
let frame = frame_info.as_note(*tcx);
9197
match last_frame.as_mut() {
9298
Some(last_frame)
9399
if last_frame.span == frame.span
@@ -156,3 +162,25 @@ where
156162
}
157163
}
158164
}
165+
166+
/// Emit a lint from a const-eval situation.
167+
// Even if this is unused, please don't remove it -- chances are we will need to emit a lint during const-eval again in the future!
168+
pub(super) fn lint<'tcx, 'mir, L>(
169+
tcx: TyCtxtAt<'tcx>,
170+
machine: &CompileTimeInterpreter<'mir, 'tcx>,
171+
lint: &'static rustc_session::lint::Lint,
172+
decorator: impl FnOnce(Vec<errors::FrameNote>) -> L,
173+
) where
174+
L: for<'a> rustc_errors::DecorateLint<'a, ()>,
175+
{
176+
let (span, frames) = get_span_and_frames(tcx, machine);
177+
178+
tcx.emit_spanned_lint(
179+
lint,
180+
// We use the root frame for this so the crate that defines the const defines whether the
181+
// lint is emitted.
182+
machine.stack.first().and_then(|frame| frame.lint_root()).unwrap_or(CRATE_HIR_ID),
183+
span,
184+
decorator(frames),
185+
);
186+
}

Diff for: ‎compiler/rustc_const_eval/src/const_eval/eval_queries.rs

+7-7
Original file line numberDiff line numberDiff line change
@@ -155,8 +155,8 @@ pub(super) fn op_to_const<'tcx>(
155155
match immediate {
156156
Left(ref mplace) => {
157157
// We know `offset` is relative to the allocation, so we can use `into_parts`.
158-
let (alloc_id, offset) = mplace.ptr().into_parts();
159-
let alloc_id = alloc_id.expect("cannot have `fake` place fot non-ZST type");
158+
let (prov, offset) = mplace.ptr().into_parts();
159+
let alloc_id = prov.expect("cannot have `fake` place for non-ZST type").alloc_id();
160160
ConstValue::Indirect { alloc_id, offset }
161161
}
162162
// see comment on `let force_as_immediate` above
@@ -178,8 +178,8 @@ pub(super) fn op_to_const<'tcx>(
178178
);
179179
let msg = "`op_to_const` on an immediate scalar pair must only be used on slice references to the beginning of an actual allocation";
180180
// We know `offset` is relative to the allocation, so we can use `into_parts`.
181-
let (alloc_id, offset) = a.to_pointer(ecx).expect(msg).into_parts();
182-
let alloc_id = alloc_id.expect(msg);
181+
let (prov, offset) = a.to_pointer(ecx).expect(msg).into_parts();
182+
let alloc_id = prov.expect(msg).alloc_id();
183183
let data = ecx.tcx.global_alloc(alloc_id).unwrap_memory();
184184
assert!(offset == abi::Size::ZERO, "{}", msg);
185185
let meta = b.to_target_usize(ecx).expect(msg);
@@ -338,7 +338,7 @@ pub fn eval_in_interpreter<'mir, 'tcx>(
338338
*ecx.tcx,
339339
error,
340340
None,
341-
|| super::get_span_and_frames(&ecx),
341+
|| super::get_span_and_frames(ecx.tcx, &ecx.machine),
342342
|span, frames| ConstEvalError {
343343
span,
344344
error_kind: kind,
@@ -353,7 +353,7 @@ pub fn eval_in_interpreter<'mir, 'tcx>(
353353
let validation =
354354
const_validate_mplace(&ecx, &mplace, is_static, cid.promoted.is_some());
355355

356-
let alloc_id = mplace.ptr().provenance.unwrap();
356+
let alloc_id = mplace.ptr().provenance.unwrap().alloc_id();
357357

358358
// Validation failed, report an error.
359359
if let Err(error) = validation {
@@ -419,7 +419,7 @@ pub fn const_report_error<'mir, 'tcx>(
419419
*ecx.tcx,
420420
error,
421421
None,
422-
|| crate::const_eval::get_span_and_frames(ecx),
422+
|| crate::const_eval::get_span_and_frames(ecx.tcx, &ecx.machine),
423423
move |span, frames| errors::UndefinedBehavior { span, ub_note, frames, raw_bytes },
424424
)
425425
}

Diff for: ‎compiler/rustc_const_eval/src/const_eval/machine.rs

+54-18
Original file line numberDiff line numberDiff line change
@@ -1,30 +1,30 @@
1-
use rustc_hir::def::DefKind;
2-
use rustc_hir::LangItem;
3-
use rustc_middle::mir;
4-
use rustc_middle::mir::interpret::PointerArithmetic;
5-
use rustc_middle::ty::layout::{FnAbiOf, TyAndLayout};
6-
use rustc_middle::ty::{self, TyCtxt};
7-
use rustc_span::Span;
81
use std::borrow::Borrow;
2+
use std::fmt;
93
use std::hash::Hash;
104
use std::ops::ControlFlow;
115

6+
use rustc_ast::Mutability;
127
use rustc_data_structures::fx::FxIndexMap;
138
use rustc_data_structures::fx::IndexEntry;
14-
use std::fmt;
15-
16-
use rustc_ast::Mutability;
9+
use rustc_hir::def::DefKind;
1710
use rustc_hir::def_id::DefId;
11+
use rustc_hir::LangItem;
12+
use rustc_middle::mir;
1813
use rustc_middle::mir::AssertMessage;
14+
use rustc_middle::query::TyCtxtAt;
15+
use rustc_middle::ty;
16+
use rustc_middle::ty::layout::{FnAbiOf, TyAndLayout};
17+
use rustc_session::lint::builtin::WRITES_THROUGH_IMMUTABLE_POINTER;
1918
use rustc_span::symbol::{sym, Symbol};
19+
use rustc_span::Span;
2020
use rustc_target::abi::{Align, Size};
2121
use rustc_target::spec::abi::Abi as CallAbi;
2222

2323
use crate::errors::{LongRunning, LongRunningWarn};
2424
use crate::fluent_generated as fluent;
2525
use crate::interpret::{
26-
self, compile_time_machine, AllocId, ConstAllocation, FnArg, FnVal, Frame, ImmTy, InterpCx,
27-
InterpResult, OpTy, PlaceTy, Pointer, Scalar,
26+
self, compile_time_machine, AllocId, AllocRange, ConstAllocation, CtfeProvenance, FnArg, FnVal,
27+
Frame, ImmTy, InterpCx, InterpResult, OpTy, PlaceTy, Pointer, PointerArithmetic, Scalar,
2828
};
2929

3030
use super::error::*;
@@ -49,7 +49,7 @@ pub struct CompileTimeInterpreter<'mir, 'tcx> {
4949
pub(super) num_evaluated_steps: usize,
5050

5151
/// The virtual call stack.
52-
pub(super) stack: Vec<Frame<'mir, 'tcx, AllocId, ()>>,
52+
pub(super) stack: Vec<Frame<'mir, 'tcx>>,
5353

5454
/// We need to make sure consts never point to anything mutable, even recursively. That is
5555
/// relied on for pattern matching on consts with references.
@@ -638,10 +638,7 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir,
638638
}
639639

640640
#[inline(always)]
641-
fn expose_ptr(
642-
_ecx: &mut InterpCx<'mir, 'tcx, Self>,
643-
_ptr: Pointer<AllocId>,
644-
) -> InterpResult<'tcx> {
641+
fn expose_ptr(_ecx: &mut InterpCx<'mir, 'tcx, Self>, _ptr: Pointer) -> InterpResult<'tcx> {
645642
// This is only reachable with -Zunleash-the-miri-inside-of-you.
646643
throw_unsup_format!("exposing pointers is not possible at compile-time")
647644
}
@@ -674,7 +671,7 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir,
674671
}
675672

676673
fn before_access_global(
677-
_tcx: TyCtxt<'tcx>,
674+
_tcx: TyCtxtAt<'tcx>,
678675
machine: &Self,
679676
alloc_id: AllocId,
680677
alloc: ConstAllocation<'tcx>,
@@ -711,6 +708,45 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir,
711708
}
712709
}
713710
}
711+
712+
fn retag_ptr_value(
713+
ecx: &mut InterpCx<'mir, 'tcx, Self>,
714+
_kind: mir::RetagKind,
715+
val: &ImmTy<'tcx, CtfeProvenance>,
716+
) -> InterpResult<'tcx, ImmTy<'tcx, CtfeProvenance>> {
717+
if let ty::Ref(_, ty, mutbl) = val.layout.ty.kind()
718+
&& *mutbl == Mutability::Not
719+
&& ty.is_freeze(*ecx.tcx, ecx.param_env)
720+
{
721+
// This is a frozen shared reference, mark it immutable.
722+
let place = ecx.ref_to_mplace(val)?;
723+
let new_place = place.map_provenance(|p| p.map(CtfeProvenance::as_immutable));
724+
Ok(ImmTy::from_immediate(new_place.to_ref(ecx), val.layout))
725+
} else {
726+
Ok(val.clone())
727+
}
728+
}
729+
730+
fn before_memory_write(
731+
tcx: TyCtxtAt<'tcx>,
732+
machine: &mut Self,
733+
_alloc_extra: &mut Self::AllocExtra,
734+
(_alloc_id, immutable): (AllocId, bool),
735+
range: AllocRange,
736+
) -> InterpResult<'tcx> {
737+
if range.size == Size::ZERO {
738+
// Nothing to check.
739+
return Ok(());
740+
}
741+
// Reject writes through immutable pointers.
742+
if immutable {
743+
super::lint(tcx, machine, WRITES_THROUGH_IMMUTABLE_POINTER, |frames| {
744+
crate::errors::WriteThroughImmutablePointer { frames }
745+
});
746+
}
747+
// Everything else is fine.
748+
Ok(())
749+
}
714750
}
715751

716752
// Please do not add any code below the above `Machine` trait impl. I (oli-obk) plan more cleanups

Diff for: ‎compiler/rustc_const_eval/src/errors.rs

+7
Original file line numberDiff line numberDiff line change
@@ -402,6 +402,13 @@ pub struct ConstEvalError {
402402
pub frame_notes: Vec<FrameNote>,
403403
}
404404

405+
#[derive(LintDiagnostic)]
406+
#[diag(const_eval_write_through_immutable_pointer)]
407+
pub struct WriteThroughImmutablePointer {
408+
#[subdiagnostic]
409+
pub frames: Vec<FrameNote>,
410+
}
411+
405412
#[derive(Diagnostic)]
406413
#[diag(const_eval_nullary_intrinsic_fail)]
407414
pub struct NullaryIntrinsicError {

0 commit comments

Comments
 (0)