Skip to content

allow statics pointing to mutable statics #121782

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Mar 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -614,7 +614,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 @@ -768,7 +767,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 { .. })) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So we don't have any tests testing the promoted code path? Or is that rejected elsewhere even in unleash mode?

Copy link
Member Author

@RalfJung RalfJung Feb 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With promoteds we have to accept this or else we fail this recently added test.

We know from the internet that a promoted will never add new mutable memory, so all references to mutable memory must be shared refs as otherwise we'd hit the error above about a mutable ref to immutable memory. (But the shared refs may have interior mutability. We do complain if we find an UnsafeCell in an immutable location though.)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤣

We know from the internet

(sorry, I know what you meant, but I literally lolled)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😂

(For anyone else reading along, I meant "interner".)

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