Skip to content

Commit

Permalink
Auto merge of #121034 - obeis:improve-static-mut-ref, r=RalfJung
Browse files Browse the repository at this point in the history
Improve wording of `static_mut_ref`

Close #120964
  • Loading branch information
bors committed Feb 18, 2024
2 parents 23a3d77 + 408eeae commit bcb3545
Show file tree
Hide file tree
Showing 73 changed files with 784 additions and 461 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -112,8 +112,8 @@ fn start<T: Termination + 'static>(

static mut NUM: u8 = 6 * 7;

// FIXME: Use `SyncUnsafeCell` instead of allowing `static_mut_ref` lint
#[allow(static_mut_ref)]
// FIXME: Use `SyncUnsafeCell` instead of allowing `static_mut_refs` lint
#[allow(static_mut_refs)]
static NUM_REF: &'static u8 = unsafe { &NUM };

unsafe fn zeroed<T>() -> T {
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_codegen_gcc/example/mini_core_hello_world.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,8 @@ fn start<T: Termination + 'static>(

static mut NUM: u8 = 6 * 7;

// FIXME: Use `SyncUnsafeCell` instead of allowing `static_mut_ref` lint
#[allow(static_mut_ref)]
// FIXME: Use `SyncUnsafeCell` instead of allowing `static_mut_refs` lint
#[allow(static_mut_refs)]
static NUM_REF: &'static u8 = unsafe { &NUM };

macro_rules! assert {
Expand Down
26 changes: 15 additions & 11 deletions compiler/rustc_error_codes/src/error_codes/E0796.md
Original file line number Diff line number Diff line change
@@ -1,22 +1,26 @@
Reference of mutable static.
You have created a reference to a mutable static.

Erroneous code example:

```compile_fail,edition2024,E0796
static mut X: i32 = 23;
static mut Y: i32 = 24;
unsafe {
let y = &X;
let ref x = X;
let (x, y) = (&X, &Y);
foo(&X);
fn work() {
let _val = unsafe { X };
}
fn foo<'a>(_x: &'a i32) {}
let x_ref = unsafe { &mut X };
work();
// The next line has Undefined Behavior!
// `x_ref` is a mutable reference and allows no aliases,
// but `work` has been reading the reference between
// the moment `x_ref` was created and when it was used.
// This violates the uniqueness of `x_ref`.
*x_ref = 42;
```

Mutable statics can be written to by multiple threads: aliasing violations or
data races will cause undefined behavior.
A reference to a mutable static has lifetime `'static`. This is very dangerous
as it is easy to accidentally overlap the lifetime of that reference with
other, conflicting accesses to the same static.

Reference of mutable static is a hard error from 2024 edition.
References to mutable statics are a hard error in the 2024 edition.
31 changes: 18 additions & 13 deletions compiler/rustc_hir_analysis/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -375,19 +375,24 @@ hir_analysis_start_not_target_feature = `#[start]` function is not allowed to ha
hir_analysis_start_not_track_caller = `#[start]` function is not allowed to be `#[track_caller]`
.label = `#[start]` function is not allowed to be `#[track_caller]`
hir_analysis_static_mut_ref = reference of mutable static is disallowed
.label = reference of mutable static
.note = mutable statics can be written to by multiple threads: aliasing violations or data races will cause undefined behavior
.suggestion = shared references are dangerous since if there's any kind of mutation of that static while the reference lives, that's UB; use `addr_of!` instead to create a raw pointer
.suggestion_mut = mutable references are dangerous since if there's any other pointer or reference used for that static while the reference lives, that's UB; use `addr_of_mut!` instead to create a raw pointer
hir_analysis_static_mut_ref_lint = {$shared}reference of mutable static is discouraged
.label = shared reference of mutable static
.label_mut = mutable reference of mutable static
.suggestion = shared references are dangerous since if there's any kind of mutation of that static while the reference lives, that's UB; use `addr_of!` instead to create a raw pointer
.suggestion_mut = mutable references are dangerous since if there's any other pointer or reference used for that static while the reference lives, that's UB; use `addr_of_mut!` instead to create a raw pointer
.note = reference of mutable static is a hard error from 2024 edition
.why_note = mutable statics can be written to by multiple threads: aliasing violations or data races will cause undefined behavior
hir_analysis_static_mut_ref = creating a {$shared} reference to a mutable static
.label = {$shared} reference to mutable static
.note = {$shared ->
[shared] this shared reference has lifetime `'static`, but if the static ever gets mutated, or a mutable reference is created, then any further use of this shared reference is Undefined Behavior
*[mutable] this mutable reference has lifetime `'static`, but if the static gets accessed (read or written) by any other means, or any other reference is created, then any further use of this mutable reference is Undefined Behavior
}
.suggestion = use `addr_of!` instead to create a raw pointer
.suggestion_mut = use `addr_of_mut!` instead to create a raw pointer
hir_analysis_static_mut_refs_lint = creating a {$shared} reference to mutable static is discouraged
.label = {$shared} reference to mutable static
.suggestion = use `addr_of!` instead to create a raw pointer
.suggestion_mut = use `addr_of_mut!` instead to create a raw pointer
.note = this will be a hard error in the 2024 edition
.why_note = {$shared ->
[shared] this shared reference has lifetime `'static`, but if the static ever gets mutated, or a mutable reference is created, then any further use of this shared reference is Undefined Behavior
*[mutable] this mutable reference has lifetime `'static`, but if the static gets accessed (read or written) by any other means, or any other reference is created, then any further use of this mutable reference is Undefined Behavior
}
hir_analysis_static_specialize = cannot specialize on `'static` lifetime
Expand Down
28 changes: 10 additions & 18 deletions compiler/rustc_hir_analysis/src/check/errs.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use rustc_hir as hir;
use rustc_hir_pretty::qpath_to_string;
use rustc_lint_defs::builtin::STATIC_MUT_REF;
use rustc_lint_defs::builtin::STATIC_MUT_REFS;
use rustc_middle::ty::TyCtxt;
use rustc_span::Span;
use rustc_type_ir::Mutability;
Expand Down Expand Up @@ -66,32 +66,24 @@ fn handle_static_mut_ref(
hir_id: hir::HirId,
) {
if e2024 {
let sugg = if mutable {
errors::StaticMutRefSugg::Mut { span, var }
let (sugg, shared) = if mutable {
(errors::StaticMutRefSugg::Mut { span, var }, "mutable")
} else {
errors::StaticMutRefSugg::Shared { span, var }
(errors::StaticMutRefSugg::Shared { span, var }, "shared")
};
tcx.sess.parse_sess.dcx.emit_err(errors::StaticMutRef { span, sugg });
tcx.sess.parse_sess.dcx.emit_err(errors::StaticMutRef { span, sugg, shared });
return;
}

let (label, sugg, shared) = if mutable {
(
errors::RefOfMutStaticLabel::Mut { span },
errors::RefOfMutStaticSugg::Mut { span, var },
"mutable ",
)
let (sugg, shared) = if mutable {
(errors::RefOfMutStaticSugg::Mut { span, var }, "mutable")
} else {
(
errors::RefOfMutStaticLabel::Shared { span },
errors::RefOfMutStaticSugg::Shared { span, var },
"shared ",
)
(errors::RefOfMutStaticSugg::Shared { span, var }, "shared")
};
tcx.emit_node_span_lint(
STATIC_MUT_REF,
STATIC_MUT_REFS,
hir_id,
span,
errors::RefOfMutStatic { shared, why_note: (), label, sugg },
errors::RefOfMutStatic { span, sugg, shared },
);
}
28 changes: 7 additions & 21 deletions compiler/rustc_hir_analysis/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1462,12 +1462,13 @@ pub struct OnlyCurrentTraitsPointerSugg<'a> {
#[derive(Diagnostic)]
#[diag(hir_analysis_static_mut_ref, code = E0796)]
#[note]
pub struct StaticMutRef {
pub struct StaticMutRef<'a> {
#[primary_span]
#[label]
pub span: Span,
#[subdiagnostic]
pub sugg: StaticMutRefSugg,
pub shared: &'a str,
}

#[derive(Subdiagnostic)]
Expand Down Expand Up @@ -1498,30 +1499,15 @@ pub enum StaticMutRefSugg {

// STATIC_MUT_REF lint
#[derive(LintDiagnostic)]
#[diag(hir_analysis_static_mut_ref_lint)]
#[diag(hir_analysis_static_mut_refs_lint)]
#[note]
#[note(hir_analysis_why_note)]
pub struct RefOfMutStatic<'a> {
pub shared: &'a str,
#[note(hir_analysis_why_note)]
pub why_note: (),
#[subdiagnostic]
pub label: RefOfMutStaticLabel,
#[label]
pub span: Span,
#[subdiagnostic]
pub sugg: RefOfMutStaticSugg,
}

#[derive(Subdiagnostic)]
pub enum RefOfMutStaticLabel {
#[label(hir_analysis_label)]
Shared {
#[primary_span]
span: Span,
},
#[label(hir_analysis_label_mut)]
Mut {
#[primary_span]
span: Span,
},
pub shared: &'a str,
}

#[derive(Subdiagnostic)]
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_lint/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,7 @@ fn register_builtins(store: &mut LintStore) {
store.register_renamed("or_patterns_back_compat", "rust_2021_incompatible_or_patterns");
store.register_renamed("non_fmt_panic", "non_fmt_panics");
store.register_renamed("unused_tuple_struct_fields", "dead_code");
store.register_renamed("static_mut_ref", "static_mut_refs");

// These were moved to tool lints, but rustc still sees them when compiling normally, before
// tool lints are registered, so `check_tool_name_for_backwards_compat` doesn't work. Use
Expand Down
8 changes: 4 additions & 4 deletions compiler/rustc_lint_defs/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ declare_lint_pass! {
SINGLE_USE_LIFETIMES,
SOFT_UNSTABLE,
STABLE_FEATURES,
STATIC_MUT_REF,
STATIC_MUT_REFS,
SUSPICIOUS_AUTO_TRAIT_IMPLS,
TEST_UNSTABLE_LINT,
TEXT_DIRECTION_CODEPOINT_IN_COMMENT,
Expand Down Expand Up @@ -1769,7 +1769,7 @@ declare_lint! {
}

declare_lint! {
/// The `static_mut_ref` lint checks for shared or mutable references
/// The `static_mut_refs` lint checks for shared or mutable references
/// of mutable static inside `unsafe` blocks and `unsafe` functions.
///
/// ### Example
Expand Down Expand Up @@ -1807,9 +1807,9 @@ declare_lint! {
/// Shared or mutable references of mutable static are almost always a mistake and
/// can lead to undefined behavior and various other problems in your code.
///
/// This lint is "warn" by default on editions up to 2021, from 2024 there is
/// This lint is "warn" by default on editions up to 2021, in 2024 there is
/// a hard error instead.
pub STATIC_MUT_REF,
pub STATIC_MUT_REFS,
Warn,
"shared references or mutable references of mutable static is discouraged",
@future_incompatible = FutureIncompatibleInfo {
Expand Down
10 changes: 6 additions & 4 deletions library/panic_unwind/src/seh.rs
Original file line number Diff line number Diff line change
Expand Up @@ -261,8 +261,9 @@ cfg_if::cfg_if! {
}
}

// FIXME: Use `SyncUnsafeCell` instead of allowing `static_mut_ref` lint
#[allow(static_mut_ref)]
// FIXME: Use `SyncUnsafeCell` instead of allowing `static_mut_refs` lint
#[cfg_attr(bootstrap, allow(static_mut_ref))]
#[cfg_attr(not(bootstrap), allow(static_mut_refs))]
pub unsafe fn panic(data: Box<dyn Any + Send>) -> u32 {
use core::intrinsics::atomic_store_seqcst;

Expand Down Expand Up @@ -324,8 +325,9 @@ pub unsafe fn panic(data: Box<dyn Any + Send>) -> u32 {
_CxxThrowException(throw_ptr, &mut THROW_INFO as *mut _ as *mut _);
}

// FIXME: Use `SyncUnsafeCell` instead of allowing `static_mut_ref` lint
#[allow(static_mut_ref)]
// FIXME: Use `SyncUnsafeCell` instead of allowing `static_mut_refs` lint
#[cfg_attr(bootstrap, allow(static_mut_ref))]
#[cfg_attr(not(bootstrap), allow(static_mut_refs))]
pub unsafe fn cleanup(payload: *mut u8) -> Box<dyn Any + Send> {
// A null payload here means that we got here from the catch (...) of
// __rust_try. This happens when a non-Rust foreign exception is caught.
Expand Down
5 changes: 3 additions & 2 deletions library/std/src/panicking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -337,8 +337,9 @@ pub mod panic_count {
#[doc(hidden)]
#[cfg(not(feature = "panic_immediate_abort"))]
#[unstable(feature = "update_panic_count", issue = "none")]
// FIXME: Use `SyncUnsafeCell` instead of allowing `static_mut_ref` lint
#[allow(static_mut_ref)]
// FIXME: Use `SyncUnsafeCell` instead of allowing `static_mut_refs` lint
#[cfg_attr(bootstrap, allow(static_mut_ref))]
#[cfg_attr(not(bootstrap), allow(static_mut_refs))]
pub mod panic_count {
use crate::cell::Cell;
use crate::sync::atomic::{AtomicUsize, Ordering};
Expand Down
5 changes: 3 additions & 2 deletions library/std/src/sys/pal/common/thread_local/fast_local.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,9 @@ pub macro thread_local_inner {
(@key $t:ty, const $init:expr) => {{
#[inline]
#[deny(unsafe_op_in_unsafe_fn)]
// FIXME: Use `SyncUnsafeCell` instead of allowing `static_mut_ref` lint
#[allow(static_mut_ref)]
// FIXME: Use `SyncUnsafeCell` instead of allowing `static_mut_refs` lint
#[cfg_attr(bootstrap, allow(static_mut_ref))]
#[cfg_attr(not(bootstrap), allow(static_mut_refs))]
unsafe fn __getit(
_init: $crate::option::Option<&mut $crate::option::Option<$t>>,
) -> $crate::option::Option<&'static $t> {
Expand Down
5 changes: 3 additions & 2 deletions library/std/src/sys/pal/common/thread_local/static_local.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,9 @@ pub macro thread_local_inner {
(@key $t:ty, const $init:expr) => {{
#[inline] // see comments below
#[deny(unsafe_op_in_unsafe_fn)]
// FIXME: Use `SyncUnsafeCell` instead of allowing `static_mut_ref` lint
#[allow(static_mut_ref)]
// FIXME: Use `SyncUnsafeCell` instead of allowing `static_mut_refs` lint
#[cfg_attr(bootstrap, allow(static_mut_ref))]
#[cfg_attr(not(bootstrap), allow(static_mut_refs))]
unsafe fn __getit(
_init: $crate::option::Option<&mut $crate::option::Option<$t>>,
) -> $crate::option::Option<&'static $t> {
Expand Down
4 changes: 2 additions & 2 deletions library/std/src/thread/local.rs
Original file line number Diff line number Diff line change
Expand Up @@ -180,8 +180,8 @@ impl<T: 'static> fmt::Debug for LocalKey<T> {
#[stable(feature = "rust1", since = "1.0.0")]
#[cfg_attr(not(test), rustc_diagnostic_item = "thread_local_macro")]
#[allow_internal_unstable(thread_local_internals)]
// FIXME: Use `SyncUnsafeCell` instead of allowing `static_mut_ref` lint
#[allow(static_mut_ref)]
// FIXME: Use `SyncUnsafeCell` instead of allowing `static_mut_refs` lint
#[cfg_attr(not(bootstrap), allow(static_mut_refs))]
macro_rules! thread_local {
// empty (base case for the recursion)
() => {};
Expand Down
4 changes: 2 additions & 2 deletions src/tools/miri/tests/fail/tls/tls_static_dealloc.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
//! Ensure that thread-local statics get deallocated when the thread dies.

#![feature(thread_local)]
// FIXME: Use `SyncUnsafeCell` instead of allowing `static_mut_ref` lint
#![allow(static_mut_ref)]
// FIXME: Use `SyncUnsafeCell` instead of allowing `static_mut_refs` lint
#![allow(static_mut_refs)]

#[thread_local]
static mut TLS: u8 = 0;
Expand Down
4 changes: 2 additions & 2 deletions src/tools/miri/tests/pass/static_mut.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
static mut FOO: i32 = 42;

// FIXME: Use `SyncUnsafeCell` instead of allowing `static_mut_ref` lint
#[allow(static_mut_ref)]
// FIXME: Use `SyncUnsafeCell` instead of allowing `static_mut_refs` lint
#[allow(static_mut_refs)]
static BAR: Foo = Foo(unsafe { &FOO as *const _ });

#[allow(dead_code)]
Expand Down
4 changes: 2 additions & 2 deletions src/tools/miri/tests/pass/tls/tls_static.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@
//! test, we also check that thread-locals act as per-thread statics.

#![feature(thread_local)]
// FIXME: Use `SyncUnsafeCell` instead of allowing `static_mut_ref` lint
#![allow(static_mut_ref)]
// FIXME: Use `SyncUnsafeCell` instead of allowing `static_mut_refs` lint
#![allow(static_mut_refs)]

use std::thread;

Expand Down
4 changes: 2 additions & 2 deletions tests/ui/abi/statics/static-mut-foreign.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,9 @@ unsafe fn run() {
rust_dbg_static_mut = -3;
assert_eq!(rust_dbg_static_mut, -3);
static_bound(&rust_dbg_static_mut);
//~^ WARN shared reference of mutable static is discouraged [static_mut_ref]
//~^ WARN shared reference to mutable static is discouraged [static_mut_refs]
static_bound_set(&mut rust_dbg_static_mut);
//~^ WARN mutable reference of mutable static is discouraged [static_mut_ref]
//~^ WARN mutable reference to mutable static is discouraged [static_mut_refs]
}

pub fn main() {
Expand Down
22 changes: 11 additions & 11 deletions tests/ui/abi/statics/static-mut-foreign.stderr
Original file line number Diff line number Diff line change
@@ -1,28 +1,28 @@
warning: shared reference of mutable static is discouraged
warning: creating a shared reference to mutable static is discouraged
--> $DIR/static-mut-foreign.rs:35:18
|
LL | static_bound(&rust_dbg_static_mut);
| ^^^^^^^^^^^^^^^^^^^^ shared reference of mutable static
| ^^^^^^^^^^^^^^^^^^^^ shared reference to mutable static
|
= note: for more information, see issue #114447 <https://github.com/rust-lang/rust/issues/114447>
= note: reference of mutable static is a hard error from 2024 edition
= note: mutable statics can be written to by multiple threads: aliasing violations or data races will cause undefined behavior
= note: `#[warn(static_mut_ref)]` on by default
help: shared references are dangerous since if there's any kind of mutation of that static while the reference lives, that's UB; use `addr_of!` instead to create a raw pointer
= note: this will be a hard error in the 2024 edition
= note: this shared reference has lifetime `'static`, but if the static ever gets mutated, or a mutable reference is created, then any further use of this shared reference is Undefined Behavior
= note: `#[warn(static_mut_refs)]` on by default
help: use `addr_of!` instead to create a raw pointer
|
LL | static_bound(addr_of!(rust_dbg_static_mut));
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

warning: mutable reference of mutable static is discouraged
warning: creating a mutable reference to mutable static is discouraged
--> $DIR/static-mut-foreign.rs:37:22
|
LL | static_bound_set(&mut rust_dbg_static_mut);
| ^^^^^^^^^^^^^^^^^^^^^^^^ mutable reference of mutable static
| ^^^^^^^^^^^^^^^^^^^^^^^^ mutable reference to mutable static
|
= note: for more information, see issue #114447 <https://github.com/rust-lang/rust/issues/114447>
= note: reference of mutable static is a hard error from 2024 edition
= note: mutable statics can be written to by multiple threads: aliasing violations or data races will cause undefined behavior
help: mutable references are dangerous since if there's any other pointer or reference used for that static while the reference lives, that's UB; use `addr_of_mut!` instead to create a raw pointer
= note: this will be a hard error in the 2024 edition
= note: this mutable reference has lifetime `'static`, but if the static gets accessed (read or written) by any other means, or any other reference is created, then any further use of this mutable reference is Undefined Behavior
help: use `addr_of_mut!` instead to create a raw pointer
|
LL | static_bound_set(addr_of_mut!(rust_dbg_static_mut));
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Expand Down
Loading

0 comments on commit bcb3545

Please sign in to comment.