Skip to content

Commit

Permalink
Rollup merge of #121782 - RalfJung:mutable-ref-in-static, r=oli-obk
Browse files Browse the repository at this point in the history
allow statics pointing to mutable statics

Fixes #120450 for good. We can even simplify our checks: no need to specifically go looking for mutable references in const, we can just reject any reference that points to something mutable.

r? `@oli-obk`
  • Loading branch information
matthiaskrgr authored Feb 29, 2024
2 parents 2fdcdd9 + a9596fb commit 255fdcc
Show file tree
Hide file tree
Showing 12 changed files with 64 additions and 307 deletions.
1 change: 0 additions & 1 deletion compiler/rustc_const_eval/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -453,7 +453,6 @@ 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_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
2 changes: 0 additions & 2 deletions compiler/rustc_const_eval/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -613,7 +613,6 @@ impl<'tcx> ReportErrorExt for ValidationErrorInfo<'tcx> {
PartialPointer => const_eval_validation_partial_pointer,
ConstRefToMutable => const_eval_validation_const_ref_to_mutable,
ConstRefToExtern => const_eval_validation_const_ref_to_extern,
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 Down Expand Up @@ -767,7 +766,6 @@ impl<'tcx> ReportErrorExt for ValidationErrorInfo<'tcx> {
}
NullPtr { .. }
| PtrToStatic { .. }
| MutableRefInConstOrStatic
| ConstRefToMutable
| ConstRefToExtern
| MutableRefToImmutable
Expand Down
25 changes: 8 additions & 17 deletions compiler/rustc_const_eval/src/interpret/validity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,14 +148,6 @@ impl CtfeValidationMode {
}
}
}

fn may_contain_mutable_ref(self) -> bool {
match self {
CtfeValidationMode::Static { mutbl } => mutbl == Mutability::Mut,
CtfeValidationMode::Promoted { .. } => false,
CtfeValidationMode::Const { .. } => false,
}
}
}

/// State for tracking recursive validation of references
Expand Down Expand Up @@ -511,20 +503,19 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, '
// 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 {
// Mutable pointer to immutable memory is no good.
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);
// In a const, everything must be completely immutable.
if matches!(self.ctfe_mode, Some(CtfeValidationMode::Const { .. })) {
if ptr_expected_mutbl == Mutability::Mut
|| alloc_actual_mutbl == Mutability::Mut
{
throw_validation_failure!(self.path, ConstRefToMutable);
}
}
}
// Potentially skip recursive check.
Expand Down
1 change: 0 additions & 1 deletion compiler/rustc_middle/src/mir/interpret/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -420,7 +420,6 @@ pub enum ValidationErrorKind<'tcx> {
PartialPointer,
PtrToUninhabited { ptr_kind: PointerKind, ty: Ty<'tcx> },
PtrToStatic { ptr_kind: PointerKind },
MutableRefInConstOrStatic,
ConstRefToMutable,
ConstRefToExtern,
MutableRefToImmutable,
Expand Down
5 changes: 5 additions & 0 deletions tests/ui/consts/const-mut-refs/const_mut_refs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,11 @@ const fn bazz(foo: &mut Foo) -> usize {
// Empty slices get promoted so this passes the static checks.
// Make sure it also passes the dynamic checks.
static MUTABLE_REFERENCE_HOLDER: Mutex<&mut [u8]> = Mutex::new(&mut []);
// This variant with a non-empty slice also seems entirely reasonable.
static MUTABLE_REFERENCE_HOLDER2: Mutex<&mut [u8]> = unsafe {
static mut FOO: [u8; 1] = [42]; // a private static that we are sure nobody else will reference
Mutex::new(&mut *std::ptr::addr_of_mut!(FOO))
};

fn main() {
let _: [(); foo().bar()] = [(); 1];
Expand Down
12 changes: 4 additions & 8 deletions tests/ui/consts/const-mut-refs/mut_ref_in_final_dynamic_check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,7 @@ const fn helper() -> Option<&'static mut i32> { unsafe {
Some(&mut *std::ptr::addr_of_mut!(BUFFER))
} }
const MUT: Option<&mut i32> = helper(); //~ ERROR it is undefined behavior to use this value
//~^ encountered mutable reference
static MUT_STATIC: Option<&mut i32> = helper(); //~ ERROR it is undefined behavior to use this value
//~^ encountered mutable reference
//~^ encountered reference to mutable

const fn helper_int2ptr() -> Option<&'static mut i32> { unsafe {
// Undefined behaviour (integer as pointer), who doesn't love tests like this.
Expand All @@ -38,11 +36,9 @@ const fn helper_dangling() -> Option<&'static mut i32> { unsafe {
const DANGLING: Option<&mut i32> = helper_dangling(); //~ ERROR encountered dangling pointer
static DANGLING_STATIC: Option<&mut i32> = helper_dangling(); //~ ERROR encountered dangling pointer

// Variant of the real-world case in <https://github.com/rust-lang/rust/issues/120450>.
// Maybe we should allow this in the future (then the rest should move to `const_mut_refs.rs`),
// but for now we reject it.
// These are fine! Just statics pointing to mutable statics, nothing fundamentally wrong with this.
static MUT_STATIC: Option<&mut i32> = helper();
static mut MUT_ARRAY: &mut [u8] = &mut [42];
static MUTEX: Mutex<&mut [u8]> = Mutex::new(unsafe { &mut *MUT_ARRAY }); //~ ERROR it is undefined behavior to use this value
//~^ encountered mutable reference
static MUTEX: Mutex<&mut [u8]> = Mutex::new(unsafe { &mut *MUT_ARRAY });

fn main() {}
Original file line number Diff line number Diff line change
Expand Up @@ -2,26 +2,15 @@ error[E0080]: it is undefined behavior to use this value
--> $DIR/mut_ref_in_final_dynamic_check.rs:20:1
|
LL | const MUT: Option<&mut i32> = helper();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ constructing invalid value at .<enum-variant(Some)>.0: encountered mutable reference in a `const` or `static`
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ constructing invalid value at .<enum-variant(Some)>.0: encountered reference to mutable memory in `const`
|
= 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.
= note: the raw bytes of the constant (size: $SIZE, align: $ALIGN) {
HEX_DUMP
}

error[E0080]: it is undefined behavior to use this value
--> $DIR/mut_ref_in_final_dynamic_check.rs:22:1
|
LL | static MUT_STATIC: Option<&mut i32> = helper();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ constructing invalid value at .<enum-variant(Some)>.0: encountered mutable reference in a `const` or `static`
|
= 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.
= note: the raw bytes of the constant (size: $SIZE, align: $ALIGN) {
HEX_DUMP
}

error[E0080]: it is undefined behavior to use this value
--> $DIR/mut_ref_in_final_dynamic_check.rs:29:1
--> $DIR/mut_ref_in_final_dynamic_check.rs:27:1
|
LL | const INT2PTR: Option<&mut i32> = helper_int2ptr();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ constructing invalid value at .<enum-variant(Some)>.0: encountered a dangling reference (0x2a[noalloc] has no provenance)
Expand All @@ -32,7 +21,7 @@ LL | const INT2PTR: Option<&mut i32> = helper_int2ptr();
}

error[E0080]: it is undefined behavior to use this value
--> $DIR/mut_ref_in_final_dynamic_check.rs:31:1
--> $DIR/mut_ref_in_final_dynamic_check.rs:29:1
|
LL | static INT2PTR_STATIC: Option<&mut i32> = helper_int2ptr();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ constructing invalid value at .<enum-variant(Some)>.0: encountered a dangling reference (0x2a[noalloc] has no provenance)
Expand All @@ -43,28 +32,17 @@ LL | static INT2PTR_STATIC: Option<&mut i32> = helper_int2ptr();
}

error: encountered dangling pointer in final value of constant
--> $DIR/mut_ref_in_final_dynamic_check.rs:38:1
--> $DIR/mut_ref_in_final_dynamic_check.rs:36:1
|
LL | const DANGLING: Option<&mut i32> = helper_dangling();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: encountered dangling pointer in final value of static
--> $DIR/mut_ref_in_final_dynamic_check.rs:39:1
--> $DIR/mut_ref_in_final_dynamic_check.rs:37:1
|
LL | static DANGLING_STATIC: Option<&mut i32> = helper_dangling();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error[E0080]: it is undefined behavior to use this value
--> $DIR/mut_ref_in_final_dynamic_check.rs:45:1
|
LL | static MUTEX: Mutex<&mut [u8]> = Mutex::new(unsafe { &mut *MUT_ARRAY });
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ constructing invalid value at .data.value: encountered mutable reference in a `const` or `static`
|
= 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.
= note: the raw bytes of the constant (size: $SIZE, align: $ALIGN) {
HEX_DUMP
}

error: aborting due to 7 previous errors
error: aborting due to 5 previous errors

For more information about this error, try `rustc --explain E0080`.
180 changes: 0 additions & 180 deletions tests/ui/consts/miri_unleashed/mutable_references_err.64bit.stderr

This file was deleted.

3 changes: 2 additions & 1 deletion tests/ui/consts/miri_unleashed/mutable_references_err.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
//@ stderr-per-bitwidth
//@ compile-flags: -Zunleash-the-miri-inside-of-you
//@ normalize-stderr-test "(the raw bytes of the constant) \(size: [0-9]*, align: [0-9]*\)" -> "$1 (size: $$SIZE, align: $$ALIGN)"
//@ normalize-stderr-test "([0-9a-f][0-9a-f] |╾─*ALLOC[0-9]+(\+[a-z0-9]+)?(<imm>)?─*╼ )+ *│.*" -> "HEX_DUMP"
#![allow(invalid_reference_casting, static_mut_refs)]

use std::cell::UnsafeCell;
Expand Down
Loading

0 comments on commit 255fdcc

Please sign in to comment.