Skip to content

Commit d147d5b

Browse files
RalfJungRalfJung
committed
const validation: ensure we don't have &mut to read-only memory or UnsafeCell in read-only memory
1 parent 341a7e2 commit d147d5b

15 files changed

+488
-72
lines changed

‎compiler/rustc_const_eval/messages.ftl

+3-2
Original file line numberDiff line numberDiff line change
@@ -441,7 +441,8 @@ const_eval_validation_invalid_fn_ptr = {$front_matter}: encountered {$value}, bu
441441
const_eval_validation_invalid_ref_meta = {$front_matter}: encountered invalid reference metadata: total size is bigger than largest supported object
442442
const_eval_validation_invalid_ref_slice_meta = {$front_matter}: encountered invalid reference metadata: slice is bigger than largest supported object
443443
const_eval_validation_invalid_vtable_ptr = {$front_matter}: encountered {$value}, but expected a vtable pointer
444-
const_eval_validation_mutable_ref_in_const = {$front_matter}: encountered mutable reference in a `const`
444+
const_eval_validation_mutable_ref_in_const = {$front_matter}: encountered mutable reference in a `const` or `static`
445+
const_eval_validation_mutable_ref_to_immutable = {$front_matter}: encountered mutable reference or box pointing to read-only memory
445446
const_eval_validation_never_val = {$front_matter}: encountered a value of the never type `!`
446447
const_eval_validation_null_box = {$front_matter}: encountered a null box
447448
const_eval_validation_null_fn_ptr = {$front_matter}: encountered a null function pointer
@@ -459,7 +460,7 @@ const_eval_validation_unaligned_ref = {$front_matter}: encountered an unaligned
459460
const_eval_validation_uninhabited_enum_variant = {$front_matter}: encountered an uninhabited enum variant
460461
const_eval_validation_uninhabited_val = {$front_matter}: encountered a value of uninhabited type `{$ty}`
461462
const_eval_validation_uninit = {$front_matter}: encountered uninitialized memory, but {$expected}
462-
const_eval_validation_unsafe_cell = {$front_matter}: encountered `UnsafeCell` in a `const`
463+
const_eval_validation_unsafe_cell = {$front_matter}: encountered `UnsafeCell` in read-only memory
463464
464465
const_eval_write_to_read_only =
465466
writing to {$allocation} which is read-only

‎compiler/rustc_const_eval/src/const_eval/eval_queries.rs

+22-13
Original file line numberDiff line numberDiff line change
@@ -301,6 +301,12 @@ pub fn eval_in_interpreter<'mir, 'tcx>(
301301
cid: GlobalId<'tcx>,
302302
is_static: bool,
303303
) -> ::rustc_middle::mir::interpret::EvalToAllocationRawResult<'tcx> {
304+
// `is_static` just means "in static", it could still be a promoted!
305+
debug_assert_eq!(
306+
is_static,
307+
ecx.tcx.static_mutability(cid.instance.def_id()).is_some()
308+
);
309+
304310
let res = ecx.load_mir(cid.instance.def, cid.promoted);
305311
match res.and_then(|body| eval_body_using_ecx(&mut ecx, cid, &body)) {
306312
Err(error) => {
@@ -338,8 +344,7 @@ pub fn eval_in_interpreter<'mir, 'tcx>(
338344
Ok(mplace) => {
339345
// Since evaluation had no errors, validate the resulting constant.
340346
// This is a separate `try` block to provide more targeted error reporting.
341-
let validation =
342-
const_validate_mplace(&ecx, &mplace, is_static, cid.promoted.is_some());
347+
let validation = const_validate_mplace(&ecx, &mplace, cid);
343348

344349
let alloc_id = mplace.ptr().provenance.unwrap();
345350

@@ -358,22 +363,26 @@ pub fn eval_in_interpreter<'mir, 'tcx>(
358363
pub fn const_validate_mplace<'mir, 'tcx>(
359364
ecx: &InterpCx<'mir, 'tcx, CompileTimeInterpreter<'mir, 'tcx>>,
360365
mplace: &MPlaceTy<'tcx>,
361-
is_static: bool,
362-
is_promoted: bool,
366+
cid: GlobalId<'tcx>,
363367
) -> InterpResult<'tcx> {
364368
let mut ref_tracking = RefTracking::new(mplace.clone());
365369
let mut inner = false;
366370
while let Some((mplace, path)) = ref_tracking.todo.pop() {
367-
let mode = if is_static {
368-
if is_promoted {
369-
// Promoteds in statics are allowed to point to statics.
370-
CtfeValidationMode::Const { inner, allow_static_ptrs: true }
371-
} else {
372-
// a `static`
373-
CtfeValidationMode::Regular
371+
let mode = match ecx.tcx.static_mutability(cid.instance.def_id()) {
372+
Some(_) if cid.promoted.is_some() => {
373+
// Promoteds in statics are consts that re allowed to point to statics.
374+
CtfeValidationMode::Const {
375+
allow_immutable_unsafe_cell: false,
376+
allow_static_ptrs: true,
377+
}
378+
}
379+
Some(mutbl) => CtfeValidationMode::Static { mutbl }, // a `static`
380+
None => {
381+
// In normal `const` (not promoted), the outermost allocation is always only copied,
382+
// so having `UnsafeCell` in there is okay despite them being in immutable memory.
383+
let allow_immutable_unsafe_cell = cid.promoted.is_none() && !inner;
384+
CtfeValidationMode::Const { allow_immutable_unsafe_cell, allow_static_ptrs: false }
374385
}
375-
} else {
376-
CtfeValidationMode::Const { inner, allow_static_ptrs: false }
377386
};
378387
ecx.const_validate_operand(&mplace.into(), path, &mut ref_tracking, mode)?;
379388
inner = true;

‎compiler/rustc_const_eval/src/errors.rs

+4-2
Original file line numberDiff line numberDiff line change
@@ -618,12 +618,13 @@ impl<'tcx> ReportErrorExt for ValidationErrorInfo<'tcx> {
618618
PointerAsInt { .. } => const_eval_validation_pointer_as_int,
619619
PartialPointer => const_eval_validation_partial_pointer,
620620
MutableRefInConst => const_eval_validation_mutable_ref_in_const,
621+
MutableRefToImmutable => const_eval_validation_mutable_ref_to_immutable,
621622
NullFnPtr => const_eval_validation_null_fn_ptr,
622623
NeverVal => const_eval_validation_never_val,
623624
NullablePtrOutOfRange { .. } => const_eval_validation_nullable_ptr_out_of_range,
624625
PtrOutOfRange { .. } => const_eval_validation_ptr_out_of_range,
625626
OutOfRange { .. } => const_eval_validation_out_of_range,
626-
UnsafeCell => const_eval_validation_unsafe_cell,
627+
UnsafeCellInImmutable => const_eval_validation_unsafe_cell,
627628
UninhabitedVal { .. } => const_eval_validation_uninhabited_val,
628629
InvalidEnumTag { .. } => const_eval_validation_invalid_enum_tag,
629630
UninhabitedEnumVariant => const_eval_validation_uninhabited_enum_variant,
@@ -771,9 +772,10 @@ impl<'tcx> ReportErrorExt for ValidationErrorInfo<'tcx> {
771772
| PtrToStatic { .. }
772773
| PtrToMut { .. }
773774
| MutableRefInConst
775+
| MutableRefToImmutable
774776
| NullFnPtr
775777
| NeverVal
776-
| UnsafeCell
778+
| UnsafeCellInImmutable
777779
| InvalidMetaSliceTooLarge { .. }
778780
| InvalidMetaTooLarge { .. }
779781
| DanglingPtrUseAfterFree { .. }

‎compiler/rustc_const_eval/src/interpret/validity.rs

+107-27
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,13 @@ use std::num::NonZeroUsize;
99

1010
use either::{Left, Right};
1111

12+
use hir::def::DefKind;
1213
use rustc_ast::Mutability;
1314
use rustc_data_structures::fx::FxHashSet;
1415
use rustc_hir as hir;
1516
use rustc_middle::mir::interpret::{
16-
ExpectedKind, InterpError, InvalidMetaKind, Misalignment, PointerKind, ValidationErrorInfo,
17-
ValidationErrorKind, ValidationErrorKind::*,
17+
ExpectedKind, InterpError, InvalidMetaKind, Misalignment, PointerKind, Provenance,
18+
ValidationErrorInfo, ValidationErrorKind, ValidationErrorKind::*,
1819
};
1920
use rustc_middle::ty;
2021
use rustc_middle::ty::layout::{LayoutOf, TyAndLayout};
@@ -123,15 +124,41 @@ pub enum PathElem {
123124
}
124125

125126
/// Extra things to check for during validation of CTFE results.
127+
#[derive(Copy, Clone)]
126128
pub enum CtfeValidationMode {
127-
/// Regular validation, nothing special happening.
128-
Regular,
129-
/// Validation of a `const`.
130-
/// `inner` says if this is an inner, indirect allocation (as opposed to the top-level const
131-
/// allocation). Being an inner allocation makes a difference because the top-level allocation
132-
/// of a `const` is copied for each use, but the inner allocations are implicitly shared.
129+
/// Validation of a `static`
130+
Static { mutbl: Mutability },
131+
/// Validation of a `const` (including promoteds).
132+
/// `allow_immutable_unsafe_cell` says whether we allow `UnsafeCell` in immutable memory (which is the
133+
/// case for the top-level allocation of a `const`, where this is fine because the allocation will be
134+
/// copied at each use site).
133135
/// `allow_static_ptrs` says if pointers to statics are permitted (which is the case for promoteds in statics).
134-
Const { inner: bool, allow_static_ptrs: bool },
136+
Const { allow_immutable_unsafe_cell: bool, allow_static_ptrs: bool },
137+
}
138+
139+
impl CtfeValidationMode {
140+
fn allow_immutable_unsafe_cell(self) -> bool {
141+
match self {
142+
CtfeValidationMode::Static { .. } => false,
143+
CtfeValidationMode::Const { allow_immutable_unsafe_cell, .. } => {
144+
allow_immutable_unsafe_cell
145+
}
146+
}
147+
}
148+
149+
fn allow_static_ptrs(self) -> bool {
150+
match self {
151+
CtfeValidationMode::Static { .. } => true, // statics can point to statics
152+
CtfeValidationMode::Const { allow_static_ptrs, .. } => allow_static_ptrs,
153+
}
154+
}
155+
156+
fn may_contain_mutable_ref(self) -> bool {
157+
match self {
158+
CtfeValidationMode::Static { mutbl } => mutbl == Mutability::Mut,
159+
CtfeValidationMode::Const { .. } => false,
160+
}
161+
}
135162
}
136163

137164
/// State for tracking recursive validation of references
@@ -418,6 +445,22 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, '
418445
}
419446
// Recursive checking
420447
if let Some(ref_tracking) = self.ref_tracking.as_deref_mut() {
448+
// Determine whether this pointer expects to be pointing to something mutable.
449+
let ptr_expected_mutbl = match ptr_kind {
450+
PointerKind::Box => Mutability::Mut,
451+
PointerKind::Ref => {
452+
let tam = value.layout.ty.builtin_deref(false).unwrap();
453+
// ZST never require mutability. We do not take into account interior mutability
454+
// here since we cannot know if there really is an `UnsafeCell` inside
455+
// `Option<UnsafeCell>` -- so we check that in the recursive descent behind this
456+
// reference.
457+
if size == Size::ZERO || tam.mutbl == Mutability::Not {
458+
Mutability::Not
459+
} else {
460+
Mutability::Mut
461+
}
462+
}
463+
};
421464
// Proceed recursively even for ZST, no reason to skip them!
422465
// `!` is a ZST and we want to validate it.
423466
if let Ok((alloc_id, _offset, _prov)) = self.ecx.ptr_try_get_alloc_id(place.ptr()) {
@@ -428,16 +471,29 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, '
428471
// Special handling for pointers to statics (irrespective of their type).
429472
assert!(!self.ecx.tcx.is_thread_local_static(did));
430473
assert!(self.ecx.tcx.is_static(did));
431-
if matches!(
432-
self.ctfe_mode,
433-
Some(CtfeValidationMode::Const { allow_static_ptrs: false, .. })
434-
) {
474+
if self.ctfe_mode.is_some_and(|c| !c.allow_static_ptrs()) {
435475
// See const_eval::machine::MemoryExtra::can_access_statics for why
436476
// this check is so important.
437477
// This check is reachable when the const just referenced the static,
438478
// but never read it (so we never entered `before_access_global`).
439479
throw_validation_failure!(self.path, PtrToStatic { ptr_kind });
440480
}
481+
// Mutability check.
482+
if ptr_expected_mutbl == Mutability::Mut {
483+
if matches!(
484+
self.ecx.tcx.def_kind(did),
485+
DefKind::Static(Mutability::Not)
486+
) && self
487+
.ecx
488+
.tcx
489+
.type_of(did)
490+
.no_bound_vars()
491+
.expect("statics should not have generic parameters")
492+
.is_freeze(*self.ecx.tcx, ty::ParamEnv::reveal_all())
493+
{
494+
throw_validation_failure!(self.path, MutableRefToImmutable);
495+
}
496+
}
441497
// We skip recursively checking other statics. These statics must be sound by
442498
// themselves, and the only way to get broken statics here is by using
443499
// unsafe code.
@@ -459,9 +515,20 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, '
459515
// and we would catch that here.
460516
throw_validation_failure!(self.path, PtrToMut { ptr_kind });
461517
}
518+
if ptr_expected_mutbl == Mutability::Mut
519+
&& alloc.inner().mutability == Mutability::Not
520+
{
521+
throw_validation_failure!(self.path, MutableRefToImmutable);
522+
}
462523
}
463-
// Nothing to check for these.
464-
None | Some(GlobalAlloc::Function(..) | GlobalAlloc::VTable(..)) => {}
524+
Some(GlobalAlloc::Function(..) | GlobalAlloc::VTable(..)) => {
525+
// These are immutable, we better don't allow mutable pointers here.
526+
if ptr_expected_mutbl == Mutability::Mut {
527+
throw_validation_failure!(self.path, MutableRefToImmutable);
528+
}
529+
}
530+
// Dangling, already handled.
531+
None => bug!(),
465532
}
466533
}
467534
let path = &self.path;
@@ -532,11 +599,9 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, '
532599
Ok(true)
533600
}
534601
ty::Ref(_, ty, mutbl) => {
535-
if matches!(self.ctfe_mode, Some(CtfeValidationMode::Const { .. }))
602+
if self.ctfe_mode.is_some_and(|c| !c.may_contain_mutable_ref())
536603
&& *mutbl == Mutability::Mut
537604
{
538-
// A mutable reference inside a const? That does not seem right (except if it is
539-
// a ZST).
540605
let layout = self.ecx.layout_of(*ty)?;
541606
if !layout.is_zst() {
542607
throw_validation_failure!(self.path, MutableRefInConst);
@@ -642,6 +707,19 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, '
642707
)
643708
}
644709
}
710+
711+
fn in_mutable_memory(&self, op: &OpTy<'tcx, M::Provenance>) -> bool {
712+
if let Some(mplace) = op.as_mplace_or_imm().left() {
713+
if let Some(alloc_id) = mplace.ptr().provenance.and_then(|p| p.get_alloc_id()) {
714+
if self.ecx.tcx.global_alloc(alloc_id).unwrap_memory().inner().mutability
715+
== Mutability::Mut
716+
{
717+
return true;
718+
}
719+
}
720+
}
721+
false
722+
}
645723
}
646724

647725
impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx, M>
@@ -705,10 +783,12 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx, M>
705783
op: &OpTy<'tcx, M::Provenance>,
706784
_fields: NonZeroUsize,
707785
) -> InterpResult<'tcx> {
708-
// Special check preventing `UnsafeCell` inside unions in the inner part of constants.
709-
if matches!(self.ctfe_mode, Some(CtfeValidationMode::Const { inner: true, .. })) {
710-
if !op.layout.ty.is_freeze(*self.ecx.tcx, self.ecx.param_env) {
711-
throw_validation_failure!(self.path, UnsafeCell);
786+
// Special check for CTFE validation, preventing `UnsafeCell` inside unions in immutable memory.
787+
if self.ctfe_mode.is_some_and(|c| !c.allow_immutable_unsafe_cell()) {
788+
if !op.layout.is_zst() && !op.layout.ty.is_freeze(*self.ecx.tcx, self.ecx.param_env) {
789+
if !self.in_mutable_memory(op) {
790+
throw_validation_failure!(self.path, UnsafeCellInImmutable);
791+
}
712792
}
713793
}
714794
Ok(())
@@ -730,11 +810,11 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx, M>
730810
}
731811

732812
// Special check preventing `UnsafeCell` in the inner part of constants
733-
if let Some(def) = op.layout.ty.ty_adt_def() {
734-
if matches!(self.ctfe_mode, Some(CtfeValidationMode::Const { inner: true, .. }))
735-
&& def.is_unsafe_cell()
736-
{
737-
throw_validation_failure!(self.path, UnsafeCell);
813+
if self.ctfe_mode.is_some_and(|c| !c.allow_immutable_unsafe_cell()) {
814+
if !op.layout.is_zst() && let Some(def) = op.layout.ty.ty_adt_def() && def.is_unsafe_cell() {
815+
if !self.in_mutable_memory(op) {
816+
throw_validation_failure!(self.path, UnsafeCellInImmutable);
817+
}
738818
}
739819
}
740820

‎compiler/rustc_middle/src/mir/interpret/error.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -429,12 +429,13 @@ pub enum ValidationErrorKind<'tcx> {
429429
PtrToStatic { ptr_kind: PointerKind },
430430
PtrToMut { ptr_kind: PointerKind },
431431
MutableRefInConst,
432+
MutableRefToImmutable,
433+
UnsafeCellInImmutable,
432434
NullFnPtr,
433435
NeverVal,
434436
NullablePtrOutOfRange { range: WrappingRange, max_value: u128 },
435437
PtrOutOfRange { range: WrappingRange, max_value: u128 },
436438
OutOfRange { value: String, range: WrappingRange, max_value: u128 },
437-
UnsafeCell,
438439
UninhabitedVal { ty: Ty<'tcx> },
439440
InvalidEnumTag { value: String },
440441
UninhabitedEnumVariant,

‎tests/ui/consts/const-mut-refs/mut_ref_in_final_dynamic_check.32bit.stderr

+1-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ error[E0080]: it is undefined behavior to use this value
22
--> $DIR/mut_ref_in_final_dynamic_check.rs:17:1
33
|
44
LL | const A: Option<&mut i32> = helper();
5-
| ^^^^^^^^^^^^^^^^^^^^^^^^^ constructing invalid value at .<enum-variant(Some)>.0: encountered mutable reference in a `const`
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^ constructing invalid value at .<enum-variant(Some)>.0: encountered mutable reference in a `const` or `static`
66
|
77
= note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior.
88
= note: the raw bytes of the constant (size: 4, align: 4) {

‎tests/ui/consts/const-mut-refs/mut_ref_in_final_dynamic_check.64bit.stderr

+1-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ error[E0080]: it is undefined behavior to use this value
22
--> $DIR/mut_ref_in_final_dynamic_check.rs:17:1
33
|
44
LL | const A: Option<&mut i32> = helper();
5-
| ^^^^^^^^^^^^^^^^^^^^^^^^^ constructing invalid value at .<enum-variant(Some)>.0: encountered mutable reference in a `const`
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^ constructing invalid value at .<enum-variant(Some)>.0: encountered mutable reference in a `const` or `static`
66
|
77
= note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior.
88
= note: the raw bytes of the constant (size: 8, align: 8) {

‎tests/ui/consts/invalid-union.32bit.stderr

+1-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ error[E0080]: it is undefined behavior to use this value
22
--> $DIR/invalid-union.rs:41:1
33
|
44
LL | fn main() {
5-
| ^^^^^^^^^ constructing invalid value at .<deref>.y.<enum-variant(B)>.0: encountered `UnsafeCell` in a `const`
5+
| ^^^^^^^^^ constructing invalid value at .<deref>.y.<enum-variant(B)>.0: encountered `UnsafeCell` in read-only memory
66
|
77
= note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior.
88
= note: the raw bytes of the constant (size: 4, align: 4) {

0 commit comments

Comments
 (0)