Skip to content

Commit

Permalink
Rollup merge of #121179 - RalfJung:zst-mutable-refs, r=oli-obk
Browse files Browse the repository at this point in the history
allow mutable references in const values when they point to no memory

Fixes #120450

The second commit is just some drive-by test suite cleanup.

r? `@oli-obk`
  • Loading branch information
GuillaumeGomez committed Feb 16, 2024
2 parents f60576b + d3fc69a commit 0c92146
Show file tree
Hide file tree
Showing 21 changed files with 209 additions and 387 deletions.
2 changes: 1 addition & 1 deletion compiler/rustc_const_eval/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -453,7 +453,7 @@ const_eval_validation_invalid_fn_ptr = {$front_matter}: encountered {$value}, bu
const_eval_validation_invalid_ref_meta = {$front_matter}: encountered invalid reference metadata: total size is bigger than largest supported object
const_eval_validation_invalid_ref_slice_meta = {$front_matter}: encountered invalid reference metadata: slice is bigger than largest supported object
const_eval_validation_invalid_vtable_ptr = {$front_matter}: encountered {$value}, but expected a vtable pointer
const_eval_validation_mutable_ref_in_const = {$front_matter}: encountered mutable reference in a `const` or `static`
const_eval_validation_mutable_ref_in_const_or_static = {$front_matter}: encountered mutable reference in a `const` or `static`
const_eval_validation_mutable_ref_to_immutable = {$front_matter}: encountered mutable reference or box pointing to read-only memory
const_eval_validation_never_val = {$front_matter}: encountered a value of the never type `!`
const_eval_validation_null_box = {$front_matter}: encountered a null box
Expand Down
24 changes: 13 additions & 11 deletions compiler/rustc_const_eval/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -603,18 +603,18 @@ impl<'tcx> ReportErrorExt for ValidationErrorInfo<'tcx> {
PtrToUninhabited { ptr_kind: PointerKind::Box, .. } => {
const_eval_validation_box_to_uninhabited
}
PtrToUninhabited { ptr_kind: PointerKind::Ref, .. } => {
PtrToUninhabited { ptr_kind: PointerKind::Ref(_), .. } => {
const_eval_validation_ref_to_uninhabited
}

PtrToStatic { ptr_kind: PointerKind::Box } => const_eval_validation_box_to_static,
PtrToStatic { ptr_kind: PointerKind::Ref } => const_eval_validation_ref_to_static,
PtrToStatic { ptr_kind: PointerKind::Ref(_) } => const_eval_validation_ref_to_static,

PointerAsInt { .. } => const_eval_validation_pointer_as_int,
PartialPointer => const_eval_validation_partial_pointer,
ConstRefToMutable => const_eval_validation_const_ref_to_mutable,
ConstRefToExtern => const_eval_validation_const_ref_to_extern,
MutableRefInConst => const_eval_validation_mutable_ref_in_const,
MutableRefInConstOrStatic => const_eval_validation_mutable_ref_in_const_or_static,
MutableRefToImmutable => const_eval_validation_mutable_ref_to_immutable,
NullFnPtr => const_eval_validation_null_fn_ptr,
NeverVal => const_eval_validation_never_val,
Expand All @@ -630,37 +630,39 @@ impl<'tcx> ReportErrorExt for ValidationErrorInfo<'tcx> {
InvalidMetaSliceTooLarge { ptr_kind: PointerKind::Box } => {
const_eval_validation_invalid_box_slice_meta
}
InvalidMetaSliceTooLarge { ptr_kind: PointerKind::Ref } => {
InvalidMetaSliceTooLarge { ptr_kind: PointerKind::Ref(_) } => {
const_eval_validation_invalid_ref_slice_meta
}

InvalidMetaTooLarge { ptr_kind: PointerKind::Box } => {
const_eval_validation_invalid_box_meta
}
InvalidMetaTooLarge { ptr_kind: PointerKind::Ref } => {
InvalidMetaTooLarge { ptr_kind: PointerKind::Ref(_) } => {
const_eval_validation_invalid_ref_meta
}
UnalignedPtr { ptr_kind: PointerKind::Ref, .. } => const_eval_validation_unaligned_ref,
UnalignedPtr { ptr_kind: PointerKind::Ref(_), .. } => {
const_eval_validation_unaligned_ref
}
UnalignedPtr { ptr_kind: PointerKind::Box, .. } => const_eval_validation_unaligned_box,

NullPtr { ptr_kind: PointerKind::Box } => const_eval_validation_null_box,
NullPtr { ptr_kind: PointerKind::Ref } => const_eval_validation_null_ref,
NullPtr { ptr_kind: PointerKind::Ref(_) } => const_eval_validation_null_ref,
DanglingPtrNoProvenance { ptr_kind: PointerKind::Box, .. } => {
const_eval_validation_dangling_box_no_provenance
}
DanglingPtrNoProvenance { ptr_kind: PointerKind::Ref, .. } => {
DanglingPtrNoProvenance { ptr_kind: PointerKind::Ref(_), .. } => {
const_eval_validation_dangling_ref_no_provenance
}
DanglingPtrOutOfBounds { ptr_kind: PointerKind::Box } => {
const_eval_validation_dangling_box_out_of_bounds
}
DanglingPtrOutOfBounds { ptr_kind: PointerKind::Ref } => {
DanglingPtrOutOfBounds { ptr_kind: PointerKind::Ref(_) } => {
const_eval_validation_dangling_ref_out_of_bounds
}
DanglingPtrUseAfterFree { ptr_kind: PointerKind::Box } => {
const_eval_validation_dangling_box_use_after_free
}
DanglingPtrUseAfterFree { ptr_kind: PointerKind::Ref } => {
DanglingPtrUseAfterFree { ptr_kind: PointerKind::Ref(_) } => {
const_eval_validation_dangling_ref_use_after_free
}
InvalidBool { .. } => const_eval_validation_invalid_bool,
Expand Down Expand Up @@ -766,7 +768,7 @@ impl<'tcx> ReportErrorExt for ValidationErrorInfo<'tcx> {
}
NullPtr { .. }
| PtrToStatic { .. }
| MutableRefInConst
| MutableRefInConstOrStatic
| ConstRefToMutable
| ConstRefToExtern
| MutableRefToImmutable
Expand Down
85 changes: 39 additions & 46 deletions compiler/rustc_const_eval/src/interpret/validity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -445,22 +445,22 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, '
// Determine whether this pointer expects to be pointing to something mutable.
let ptr_expected_mutbl = match ptr_kind {
PointerKind::Box => Mutability::Mut,
PointerKind::Ref => {
let tam = value.layout.ty.builtin_deref(false).unwrap();
// ZST never require mutability. We do not take into account interior mutability
// here since we cannot know if there really is an `UnsafeCell` inside
// `Option<UnsafeCell>` -- so we check that in the recursive descent behind this
// reference.
if size == Size::ZERO { Mutability::Not } else { tam.mutbl }
PointerKind::Ref(mutbl) => {
// We do not take into account interior mutability here since we cannot know if
// there really is an `UnsafeCell` inside `Option<UnsafeCell>` -- so we check
// that in the recursive descent behind this reference (controlled by
// `allow_immutable_unsafe_cell`).
mutbl
}
};
// Proceed recursively even for ZST, no reason to skip them!
// `!` is a ZST and we want to validate it.
if let Ok((alloc_id, _offset, _prov)) = self.ecx.ptr_try_get_alloc_id(place.ptr()) {
let mut skip_recursive_check = false;
// Let's see what kind of memory this points to.
// `unwrap` since dangling pointers have already been handled.
let alloc_kind = self.ecx.tcx.try_get_global_alloc(alloc_id).unwrap();
match alloc_kind {
let alloc_actual_mutbl = match alloc_kind {
GlobalAlloc::Static(did) => {
// Special handling for pointers to statics (irrespective of their type).
assert!(!self.ecx.tcx.is_thread_local_static(did));
Expand All @@ -474,12 +474,6 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, '
.no_bound_vars()
.expect("statics should not have generic parameters")
.is_freeze(*self.ecx.tcx, ty::ParamEnv::reveal_all());
// Mutability check.
if ptr_expected_mutbl == Mutability::Mut {
if !is_mut {
throw_validation_failure!(self.path, MutableRefToImmutable);
}
}
// Mode-specific checks
match self.ctfe_mode {
Some(
Expand All @@ -494,42 +488,49 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, '
// trigger cycle errors if we try to compute the value of the other static
// and that static refers back to us (potentially through a promoted).
// This could miss some UB, but that's fine.
return Ok(());
skip_recursive_check = true;
}
Some(CtfeValidationMode::Const { .. }) => {
// For consts on the other hand we have to recursively check;
// pattern matching assumes a valid value. However we better make
// sure this is not mutable.
if is_mut {
throw_validation_failure!(self.path, ConstRefToMutable);
}
// We can't recursively validate `extern static`, so we better reject them.
if self.ecx.tcx.is_foreign_item(did) {
throw_validation_failure!(self.path, ConstRefToExtern);
}
}
None => {}
}
// Return alloc mutability
if is_mut { Mutability::Mut } else { Mutability::Not }
}
GlobalAlloc::Memory(alloc) => {
if alloc.inner().mutability == Mutability::Mut
&& matches!(self.ctfe_mode, Some(CtfeValidationMode::Const { .. }))
{
throw_validation_failure!(self.path, ConstRefToMutable);
}
if ptr_expected_mutbl == Mutability::Mut
&& alloc.inner().mutability == Mutability::Not
{
throw_validation_failure!(self.path, MutableRefToImmutable);
}
}
GlobalAlloc::Memory(alloc) => alloc.inner().mutability,
GlobalAlloc::Function(..) | GlobalAlloc::VTable(..) => {
// These are immutable, we better don't allow mutable pointers here.
if ptr_expected_mutbl == Mutability::Mut {
throw_validation_failure!(self.path, MutableRefToImmutable);
}
Mutability::Not
}
};
// Mutability check.
// If this allocation has size zero, there is no actual mutability here.
let (size, _align, _alloc_kind) = self.ecx.get_alloc_info(alloc_id);
if size != Size::ZERO {
if ptr_expected_mutbl == Mutability::Mut
&& alloc_actual_mutbl == Mutability::Not
{
throw_validation_failure!(self.path, MutableRefToImmutable);
}
if ptr_expected_mutbl == Mutability::Mut
&& self.ctfe_mode.is_some_and(|c| !c.may_contain_mutable_ref())
{
throw_validation_failure!(self.path, MutableRefInConstOrStatic);
}
if alloc_actual_mutbl == Mutability::Mut
&& matches!(self.ctfe_mode, Some(CtfeValidationMode::Const { .. }))
{
throw_validation_failure!(self.path, ConstRefToMutable);
}
}
// Potentially skip recursive check.
if skip_recursive_check {
return Ok(());
}
}
let path = &self.path;
ref_tracking.track(place, || {
Expand Down Expand Up @@ -598,16 +599,8 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, '
}
Ok(true)
}
ty::Ref(_, ty, mutbl) => {
if self.ctfe_mode.is_some_and(|c| !c.may_contain_mutable_ref())
&& *mutbl == Mutability::Mut
{
let layout = self.ecx.layout_of(*ty)?;
if !layout.is_zst() {
throw_validation_failure!(self.path, MutableRefInConst);
}
}
self.check_safe_pointer(value, PointerKind::Ref)?;
ty::Ref(_, _ty, mutbl) => {
self.check_safe_pointer(value, PointerKind::Ref(*mutbl))?;
Ok(true)
}
ty::FnPtr(_sig) => {
Expand Down
9 changes: 5 additions & 4 deletions compiler/rustc_middle/src/mir/interpret/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use rustc_macros::HashStable;
use rustc_session::CtfeBacktrace;
use rustc_span::{def_id::DefId, Span, DUMMY_SP};
use rustc_target::abi::{call, Align, Size, VariantIdx, WrappingRange};
use rustc_type_ir::Mutability;

use std::borrow::Cow;
use std::{any::Any, backtrace::Backtrace, fmt};
Expand Down Expand Up @@ -367,15 +368,15 @@ pub enum UndefinedBehaviorInfo<'tcx> {

#[derive(Debug, Clone, Copy)]
pub enum PointerKind {
Ref,
Ref(Mutability),
Box,
}

impl IntoDiagnosticArg for PointerKind {
fn into_diagnostic_arg(self) -> DiagnosticArgValue {
DiagnosticArgValue::Str(
match self {
Self::Ref => "ref",
Self::Ref(_) => "ref",
Self::Box => "box",
}
.into(),
Expand Down Expand Up @@ -408,7 +409,7 @@ impl From<PointerKind> for ExpectedKind {
fn from(x: PointerKind) -> ExpectedKind {
match x {
PointerKind::Box => ExpectedKind::Box,
PointerKind::Ref => ExpectedKind::Reference,
PointerKind::Ref(_) => ExpectedKind::Reference,
}
}
}
Expand All @@ -419,7 +420,7 @@ pub enum ValidationErrorKind<'tcx> {
PartialPointer,
PtrToUninhabited { ptr_kind: PointerKind, ty: Ty<'tcx> },
PtrToStatic { ptr_kind: PointerKind },
MutableRefInConst,
MutableRefInConstOrStatic,
ConstRefToMutable,
ConstRefToExtern,
MutableRefToImmutable,
Expand Down
52 changes: 0 additions & 52 deletions tests/ui/consts/const-eval/validate_uninhabited_zsts.64bit.stderr

This file was deleted.

2 changes: 0 additions & 2 deletions tests/ui/consts/const-eval/validate_uninhabited_zsts.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
// stderr-per-bitwidth

const fn foo() -> ! {
unsafe { std::mem::transmute(()) }
//~^ ERROR evaluation of constant value failed
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
warning: the type `!` does not permit zero-initialization
--> $DIR/validate_uninhabited_zsts.rs:4:14
--> $DIR/validate_uninhabited_zsts.rs:2:14
|
LL | unsafe { std::mem::transmute(()) }
| ^^^^^^^^^^^^^^^^^^^^^^^ this code causes undefined behavior when executed
Expand All @@ -8,41 +8,41 @@ LL | unsafe { std::mem::transmute(()) }
= note: `#[warn(invalid_value)]` on by default

error[E0080]: evaluation of constant value failed
--> $DIR/validate_uninhabited_zsts.rs:4:14
--> $DIR/validate_uninhabited_zsts.rs:2:14
|
LL | unsafe { std::mem::transmute(()) }
| ^^^^^^^^^^^^^^^^^^^^^^^ constructing invalid value: encountered a value of the never type `!`
|
note: inside `foo`
--> $DIR/validate_uninhabited_zsts.rs:4:14
--> $DIR/validate_uninhabited_zsts.rs:2:14
|
LL | unsafe { std::mem::transmute(()) }
| ^^^^^^^^^^^^^^^^^^^^^^^
note: inside `FOO`
--> $DIR/validate_uninhabited_zsts.rs:19:33
--> $DIR/validate_uninhabited_zsts.rs:17:33
|
LL | const FOO: [empty::Empty; 3] = [foo(); 3];
| ^^^^^

error[E0080]: evaluation of constant value failed
--> $DIR/validate_uninhabited_zsts.rs:21:42
--> $DIR/validate_uninhabited_zsts.rs:19:42
|
LL | const BAR: [empty::Empty; 3] = [unsafe { std::mem::transmute(()) }; 3];
| ^^^^^^^^^^^^^^^^^^^^^^^ constructing invalid value at .0: encountered a value of uninhabited type `Void`

warning: the type `empty::Empty` does not permit zero-initialization
--> $DIR/validate_uninhabited_zsts.rs:21:42
--> $DIR/validate_uninhabited_zsts.rs:19:42
|
LL | const BAR: [empty::Empty; 3] = [unsafe { std::mem::transmute(()) }; 3];
| ^^^^^^^^^^^^^^^^^^^^^^^ this code causes undefined behavior when executed
|
note: in this struct field
--> $DIR/validate_uninhabited_zsts.rs:16:22
--> $DIR/validate_uninhabited_zsts.rs:14:22
|
LL | pub struct Empty(Void);
| ^^^^
note: enums with no inhabited variants have no valid value
--> $DIR/validate_uninhabited_zsts.rs:13:5
--> $DIR/validate_uninhabited_zsts.rs:11:5
|
LL | enum Void {}
| ^^^^^^^^^
Expand Down
Loading

0 comments on commit 0c92146

Please sign in to comment.