Skip to content

Commit aee26a8

Browse files
RalfJungRalfJung
committed
validation: descend from consts into statics
1 parent 60b12fd commit aee26a8

25 files changed

+327
-210
lines changed

Diff for: ‎compiler/rustc_const_eval/messages.ftl

+3-3
Original file line numberDiff line numberDiff line change
@@ -215,9 +215,6 @@ const_eval_modified_global =
215215
const_eval_mut_deref =
216216
mutation through a reference is not allowed in {const_eval_const_context}s
217217
218-
const_eval_mutable_data_in_const =
219-
constant refers to mutable data
220-
221218
const_eval_mutable_ptr_in_final = encountered mutable pointer in final value of {const_eval_intern_kind}
222219
223220
const_eval_non_const_fmt_macro_call =
@@ -414,6 +411,9 @@ const_eval_upcast_mismatch =
414411
## (We'd love to sort this differently to make that more clear but tidy won't let us...)
415412
const_eval_validation_box_to_static = {$front_matter}: encountered a box pointing to a static variable in a constant
416413
const_eval_validation_box_to_uninhabited = {$front_matter}: encountered a box pointing to uninhabited type {$ty}
414+
415+
const_eval_validation_const_ref_to_mutable = {$front_matter}: encountered reference to mutable memory in `const`
416+
417417
const_eval_validation_dangling_box_no_provenance = {$front_matter}: encountered a dangling box ({$pointer} has no provenance)
418418
const_eval_validation_dangling_box_out_of_bounds = {$front_matter}: encountered a dangling box (going beyond the bounds of its allocation)
419419
const_eval_validation_dangling_box_use_after_free = {$front_matter}: encountered a dangling box (use-after-free)

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

+9-15
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,12 @@
11
// Not in interpret to make sure we do not use private implementation details
22

3-
use crate::interpret::InterpCx;
43
use rustc_middle::mir;
5-
use rustc_middle::mir::interpret::{InterpError, InterpErrorInfo};
4+
use rustc_middle::mir::interpret::InterpErrorInfo;
65
use rustc_middle::query::TyCtxtAt;
76
use rustc_middle::ty::{self, Ty};
87

8+
use crate::interpret::{format_interp_error, InterpCx};
9+
910
mod error;
1011
mod eval_queries;
1112
mod fn_queries;
@@ -25,24 +26,17 @@ pub(crate) enum ValTreeCreationError {
2526
NodesOverflow,
2627
/// Values of this type, or this particular value, are not supported as valtrees.
2728
NonSupportedType,
28-
/// The value pointed to non-read-only memory, so we cannot make it a valtree.
29-
NotReadOnly,
30-
Other,
3129
}
3230
pub(crate) type ValTreeCreationResult<'tcx> = Result<ty::ValTree<'tcx>, ValTreeCreationError>;
3331

3432
impl From<InterpErrorInfo<'_>> for ValTreeCreationError {
3533
fn from(err: InterpErrorInfo<'_>) -> Self {
36-
match err.kind() {
37-
InterpError::MachineStop(err) => {
38-
let err = err.downcast_ref::<ConstEvalErrKind>().unwrap();
39-
match err {
40-
ConstEvalErrKind::ConstAccessesMutGlobal => ValTreeCreationError::NotReadOnly,
41-
_ => ValTreeCreationError::Other,
42-
}
43-
}
44-
_ => ValTreeCreationError::Other,
45-
}
34+
ty::tls::with(|tcx| {
35+
bug!(
36+
"Unexpected Undefined Behavior error during valtree construction: {}",
37+
format_interp_error(tcx.dcx(), err),
38+
)
39+
})
4640
}
4741
}
4842

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

+1-13
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ use super::eval_queries::{mk_eval_cx, op_to_const};
99
use super::machine::CompileTimeEvalContext;
1010
use super::{ValTreeCreationError, ValTreeCreationResult, VALTREE_MAX_NODES};
1111
use crate::const_eval::CanAccessMutGlobal;
12-
use crate::errors::{MaxNumNodesInConstErr, MutableDataInConstErr};
12+
use crate::errors::MaxNumNodesInConstErr;
1313
use crate::interpret::MPlaceTy;
1414
use crate::interpret::{
1515
intern_const_alloc_recursive, ImmTy, Immediate, InternKind, MemPlaceMeta, MemoryKind, PlaceTy,
@@ -248,18 +248,6 @@ pub(crate) fn eval_to_valtree<'tcx>(
248248
tcx.dcx().emit_err(MaxNumNodesInConstErr { span, global_const_id });
249249
Err(handled.into())
250250
}
251-
ValTreeCreationError::NotReadOnly => {
252-
let handled =
253-
tcx.dcx().emit_err(MutableDataInConstErr { span, global_const_id });
254-
Err(handled.into())
255-
}
256-
ValTreeCreationError::Other => {
257-
let handled = tcx.dcx().span_delayed_bug(
258-
span.unwrap_or(DUMMY_SP),
259-
"unexpected error during valtree construction",
260-
);
261-
Err(handled.into())
262-
}
263251
ValTreeCreationError::NonSupportedType => Ok(None),
264252
}
265253
}

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

+2-8
Original file line numberDiff line numberDiff line change
@@ -117,14 +117,6 @@ pub(crate) struct MaxNumNodesInConstErr {
117117
pub global_const_id: String,
118118
}
119119

120-
#[derive(Diagnostic)]
121-
#[diag(const_eval_mutable_data_in_const)]
122-
pub(crate) struct MutableDataInConstErr {
123-
#[primary_span]
124-
pub span: Option<Span>,
125-
pub global_const_id: String,
126-
}
127-
128120
#[derive(Diagnostic)]
129121
#[diag(const_eval_unallowed_fn_pointer_call)]
130122
pub(crate) struct UnallowedFnPointerCall {
@@ -619,6 +611,7 @@ impl<'tcx> ReportErrorExt for ValidationErrorInfo<'tcx> {
619611

620612
PointerAsInt { .. } => const_eval_validation_pointer_as_int,
621613
PartialPointer => const_eval_validation_partial_pointer,
614+
ConstRefToMutable => const_eval_validation_const_ref_to_mutable,
622615
MutableRefInConst => const_eval_validation_mutable_ref_in_const,
623616
MutableRefToImmutable => const_eval_validation_mutable_ref_to_immutable,
624617
NullFnPtr => const_eval_validation_null_fn_ptr,
@@ -773,6 +766,7 @@ impl<'tcx> ReportErrorExt for ValidationErrorInfo<'tcx> {
773766
NullPtr { .. }
774767
| PtrToStatic { .. }
775768
| MutableRefInConst
769+
| ConstRefToMutable
776770
| MutableRefToImmutable
777771
| NullFnPtr
778772
| NeverVal

Diff for: ‎compiler/rustc_const_eval/src/interpret/eval_context.rs

+21-21
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ use std::{fmt, mem};
44
use either::{Either, Left, Right};
55

66
use hir::CRATE_HIR_ID;
7+
use rustc_errors::DiagCtxt;
78
use rustc_hir::{self as hir, def_id::DefId, definitions::DefPathData};
89
use rustc_index::IndexVec;
910
use rustc_middle::mir;
@@ -430,6 +431,26 @@ pub(super) fn from_known_layout<'tcx>(
430431
}
431432
}
432433

434+
/// Turn the given error into a human-readable string. Expects the string to be printed, so if
435+
/// `RUSTC_CTFE_BACKTRACE` is set this will show a backtrace of the rustc internals that
436+
/// triggered the error.
437+
///
438+
/// This is NOT the preferred way to render an error; use `report` from `const_eval` instead.
439+
/// However, this is useful when error messages appear in ICEs.
440+
pub fn format_interp_error<'tcx>(dcx: &DiagCtxt, e: InterpErrorInfo<'tcx>) -> String {
441+
let (e, backtrace) = e.into_parts();
442+
backtrace.print_backtrace();
443+
// FIXME(fee1-dead), HACK: we want to use the error as title therefore we can just extract the
444+
// label and arguments from the InterpError.
445+
#[allow(rustc::untranslatable_diagnostic)]
446+
let mut diag = dcx.struct_allow("");
447+
let msg = e.diagnostic_message();
448+
e.add_args(dcx, &mut diag);
449+
let s = dcx.eagerly_translate_to_string(msg, diag.args());
450+
diag.cancel();
451+
s
452+
}
453+
433454
impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
434455
pub fn new(
435456
tcx: TyCtxt<'tcx>,
@@ -462,27 +483,6 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
462483
.map_or(CRATE_HIR_ID, |def_id| self.tcx.local_def_id_to_hir_id(def_id))
463484
}
464485

465-
/// Turn the given error into a human-readable string. Expects the string to be printed, so if
466-
/// `RUSTC_CTFE_BACKTRACE` is set this will show a backtrace of the rustc internals that
467-
/// triggered the error.
468-
///
469-
/// This is NOT the preferred way to render an error; use `report` from `const_eval` instead.
470-
/// However, this is useful when error messages appear in ICEs.
471-
pub fn format_error(&self, e: InterpErrorInfo<'tcx>) -> String {
472-
let (e, backtrace) = e.into_parts();
473-
backtrace.print_backtrace();
474-
// FIXME(fee1-dead), HACK: we want to use the error as title therefore we can just extract the
475-
// label and arguments from the InterpError.
476-
let dcx = self.tcx.dcx();
477-
#[allow(rustc::untranslatable_diagnostic)]
478-
let mut diag = dcx.struct_allow("");
479-
let msg = e.diagnostic_message();
480-
e.add_args(dcx, &mut diag);
481-
let s = dcx.eagerly_translate_to_string(msg, diag.args());
482-
diag.cancel();
483-
s
484-
}
485-
486486
#[inline(always)]
487487
pub(crate) fn stack(&self) -> &[Frame<'mir, 'tcx, M::Provenance, M::FrameExtra>] {
488488
M::stack(self)

Diff for: ‎compiler/rustc_const_eval/src/interpret/mod.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ mod visitor;
2020

2121
pub use rustc_middle::mir::interpret::*; // have all the `interpret` symbols in one place: here
2222

23-
pub use self::eval_context::{Frame, FrameInfo, InterpCx, StackPopCleanup};
23+
pub use self::eval_context::{format_interp_error, Frame, FrameInfo, InterpCx, StackPopCleanup};
2424
pub use self::intern::{
2525
intern_const_alloc_for_constprop, intern_const_alloc_recursive, InternKind,
2626
};

Diff for: ‎compiler/rustc_const_eval/src/interpret/validity.rs

+37-33
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,9 @@ use rustc_target::abi::{
2727
use std::hash::Hash;
2828

2929
use super::{
30-
AllocId, CheckInAllocMsg, GlobalAlloc, ImmTy, Immediate, InterpCx, InterpResult, MPlaceTy,
31-
Machine, MemPlaceMeta, OpTy, Pointer, Projectable, Scalar, ValueVisitor,
30+
format_interp_error, AllocId, CheckInAllocMsg, GlobalAlloc, ImmTy, Immediate, InterpCx,
31+
InterpResult, MPlaceTy, Machine, MemPlaceMeta, OpTy, Pointer, Projectable, Scalar,
32+
ValueVisitor,
3233
};
3334

3435
// for the validation errors
@@ -460,46 +461,49 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, '
460461
// Special handling for pointers to statics (irrespective of their type).
461462
assert!(!self.ecx.tcx.is_thread_local_static(did));
462463
assert!(self.ecx.tcx.is_static(did));
464+
let is_mut =
465+
matches!(self.ecx.tcx.def_kind(did), DefKind::Static(Mutability::Mut))
466+
|| !self
467+
.ecx
468+
.tcx
469+
.type_of(did)
470+
.no_bound_vars()
471+
.expect("statics should not have generic parameters")
472+
.is_freeze(*self.ecx.tcx, ty::ParamEnv::reveal_all());
463473
// Mutability check.
464474
if ptr_expected_mutbl == Mutability::Mut {
465-
if matches!(
466-
self.ecx.tcx.def_kind(did),
467-
DefKind::Static(Mutability::Not)
468-
) && self
469-
.ecx
470-
.tcx
471-
.type_of(did)
472-
.no_bound_vars()
473-
.expect("statics should not have generic parameters")
474-
.is_freeze(*self.ecx.tcx, ty::ParamEnv::reveal_all())
475-
{
475+
if !is_mut {
476476
throw_validation_failure!(self.path, MutableRefToImmutable);
477477
}
478478
}
479-
// We skip recursively checking other statics. These statics must be sound by
480-
// themselves, and the only way to get broken statics here is by using
481-
// unsafe code.
482-
// The reasons we don't check other statics is twofold. For one, in all
483-
// sound cases, the static was already validated on its own, and second, we
484-
// trigger cycle errors if we try to compute the value of the other static
485-
// and that static refers back to us.
486-
// We might miss const-invalid data,
487-
// but things are still sound otherwise (in particular re: consts
488-
// referring to statics).
489-
return Ok(());
479+
match self.ctfe_mode {
480+
Some(CtfeValidationMode::Static { .. }) => {
481+
// We skip recursively checking other statics. These statics must be sound by
482+
// themselves, and the only way to get broken statics here is by using
483+
// unsafe code.
484+
// The reasons we don't check other statics is twofold. For one, in all
485+
// sound cases, the static was already validated on its own, and second, we
486+
// trigger cycle errors if we try to compute the value of the other static
487+
// and that static refers back to us.
488+
// This could miss some UB, but that's fine.
489+
return Ok(());
490+
}
491+
Some(CtfeValidationMode::Const { .. }) => {
492+
// For consts on the other hand we have to recursively check;
493+
// pattern matching assumes a valid value. However we better make
494+
// sure this is not mutable.
495+
if is_mut {
496+
throw_validation_failure!(self.path, ConstRefToMutable);
497+
}
498+
}
499+
None => {}
500+
}
490501
}
491502
GlobalAlloc::Memory(alloc) => {
492503
if alloc.inner().mutability == Mutability::Mut
493504
&& matches!(self.ctfe_mode, Some(CtfeValidationMode::Const { .. }))
494505
{
495-
// This is impossible: this can only be some inner allocation of a
496-
// `static mut` (everything else either hits the `GlobalAlloc::Static`
497-
// case or is interned immutably). To get such a pointer we'd have to
498-
// load it from a static, but such loads lead to a CTFE error.
499-
span_bug!(
500-
self.ecx.tcx.span,
501-
"encountered reference to mutable memory inside a `const`"
502-
);
506+
throw_validation_failure!(self.path, ConstRefToMutable);
503507
}
504508
if ptr_expected_mutbl == Mutability::Mut
505509
&& alloc.inner().mutability == Mutability::Not
@@ -979,7 +983,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
979983
Err(err) => {
980984
bug!(
981985
"Unexpected Undefined Behavior error during validation: {}",
982-
self.format_error(err)
986+
format_interp_error(self.tcx.dcx(), err)
983987
);
984988
}
985989
}

Diff for: ‎compiler/rustc_middle/src/mir/interpret/error.rs

+1
Original file line numberDiff line numberDiff line change
@@ -425,6 +425,7 @@ pub enum ValidationErrorKind<'tcx> {
425425
PtrToUninhabited { ptr_kind: PointerKind, ty: Ty<'tcx> },
426426
PtrToStatic { ptr_kind: PointerKind },
427427
MutableRefInConst,
428+
ConstRefToMutable,
428429
MutableRefToImmutable,
429430
UnsafeCellInImmutable,
430431
NullFnPtr,

Diff for: ‎compiler/rustc_mir_transform/src/const_prop_lint.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use either::Left;
77

88
use rustc_const_eval::interpret::Immediate;
99
use rustc_const_eval::interpret::{
10-
InterpCx, InterpResult, MemoryKind, OpTy, Scalar, StackPopCleanup,
10+
format_interp_error, InterpCx, InterpResult, MemoryKind, OpTy, Scalar, StackPopCleanup,
1111
};
1212
use rustc_const_eval::ReportErrorExt;
1313
use rustc_hir::def::DefKind;
@@ -236,7 +236,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
236236
assert!(
237237
!error.kind().formatted_string(),
238238
"const-prop encountered formatting error: {}",
239-
self.ecx.format_error(error),
239+
format_interp_error(self.ecx.tcx.dcx(), error),
240240
);
241241
None
242242
}

Diff for: ‎src/tools/miri/src/diagnostics.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -290,7 +290,7 @@ pub fn report_error<'tcx, 'mir>(
290290
) =>
291291
{
292292
ecx.handle_ice(); // print interpreter backtrace
293-
bug!("This validation error should be impossible in Miri: {}", ecx.format_error(e));
293+
bug!("This validation error should be impossible in Miri: {}", format_interp_error(ecx.tcx.dcx(), e));
294294
}
295295
UndefinedBehavior(_) => "Undefined Behavior",
296296
ResourceExhaustion(_) => "resource exhaustion",
@@ -304,7 +304,7 @@ pub fn report_error<'tcx, 'mir>(
304304
) => "post-monomorphization error",
305305
_ => {
306306
ecx.handle_ice(); // print interpreter backtrace
307-
bug!("This error should be impossible in Miri: {}", ecx.format_error(e));
307+
bug!("This error should be impossible in Miri: {}", format_interp_error(ecx.tcx.dcx(), e));
308308
}
309309
};
310310
#[rustfmt::skip]
@@ -370,7 +370,7 @@ pub fn report_error<'tcx, 'mir>(
370370
_ => {}
371371
}
372372

373-
msg.insert(0, ecx.format_error(e));
373+
msg.insert(0, format_interp_error(ecx.tcx.dcx(), e));
374374

375375
report_msg(
376376
DiagLevel::Error,

Diff for: ‎tests/ui/consts/const_refs_to_static_fail.rs

+5-3
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,15 @@
1+
// normalize-stderr-test "(the raw bytes of the constant) \(size: [0-9]*, align: [0-9]*\)" -> "$1 (size: $$SIZE, align: $$ALIGN)"
2+
// normalize-stderr-test "([0-9a-f][0-9a-f] |╾─*ALLOC[0-9]+(\+[a-z0-9]+)?(<imm>)?─*╼ )+ *│.*" -> "HEX_DUMP"
13
#![feature(const_refs_to_static, const_mut_refs, sync_unsafe_cell)]
24
use std::cell::SyncUnsafeCell;
35

46
static S: SyncUnsafeCell<i32> = SyncUnsafeCell::new(0);
57
static mut S_MUT: i32 = 0;
68

7-
const C1: &SyncUnsafeCell<i32> = &S;
9+
const C1: &SyncUnsafeCell<i32> = &S; //~ERROR undefined behavior
10+
//~| encountered reference to mutable memory
811
const C1_READ: () = unsafe {
9-
assert!(*C1.get() == 0); //~ERROR evaluation of constant value failed
10-
//~^ constant accesses mutable global memory
12+
assert!(*C1.get() == 0);
1113
};
1214
const C2: *const i32 = unsafe { std::ptr::addr_of!(S_MUT) };
1315
const C2_READ: () = unsafe {

Diff for: ‎tests/ui/consts/const_refs_to_static_fail.stderr

+15-4
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,22 @@
1-
error[E0080]: evaluation of constant value failed
2-
--> $DIR/const_refs_to_static_fail.rs:9:13
1+
error[E0080]: it is undefined behavior to use this value
2+
--> $DIR/const_refs_to_static_fail.rs:9:1
3+
|
4+
LL | const C1: &SyncUnsafeCell<i32> = &S;
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ constructing invalid value: encountered reference to mutable memory in `const`
6+
|
7+
= 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.
8+
= note: the raw bytes of the constant (size: $SIZE, align: $ALIGN) {
9+
HEX_DUMP
10+
}
11+
12+
note: erroneous constant encountered
13+
--> $DIR/const_refs_to_static_fail.rs:12:14
314
|
415
LL | assert!(*C1.get() == 0);
5-
| ^^^^^^^^^ constant accesses mutable global memory
16+
| ^^
617

718
error[E0080]: evaluation of constant value failed
8-
--> $DIR/const_refs_to_static_fail.rs:14:13
19+
--> $DIR/const_refs_to_static_fail.rs:16:13
920
|
1021
LL | assert!(*C2 == 0);
1122
| ^^^ constant accesses mutable global memory

0 commit comments

Comments
 (0)