Skip to content
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

Disallow reference to static mut and adding static_mut_ref lint #117556

Merged
merged 6 commits into from
Jan 9, 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: 1 addition & 0 deletions Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -3877,6 +3877,7 @@ dependencies = [
"rustc_feature",
"rustc_fluent_macro",
"rustc_hir",
"rustc_hir_pretty",
"rustc_index",
"rustc_infer",
"rustc_lint_defs",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,9 @@ 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)]
obeis marked this conversation as resolved.
Show resolved Hide resolved
static NUM_REF: &'static u8 = unsafe { &NUM };

unsafe fn zeroed<T>() -> T {
Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_codegen_gcc/example/mini_core_hello_world.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,9 @@ 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)]
static NUM_REF: &'static u8 = unsafe { &NUM };

macro_rules! assert {
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_error_codes/src/error_codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -515,6 +515,7 @@ E0792: include_str!("./error_codes/E0792.md"),
E0793: include_str!("./error_codes/E0793.md"),
E0794: include_str!("./error_codes/E0794.md"),
E0795: include_str!("./error_codes/E0795.md"),
E0796: include_str!("./error_codes/E0796.md"),
}

// Undocumented removed error codes. Note that many removed error codes are kept in the list above
Expand Down
22 changes: 22 additions & 0 deletions compiler/rustc_error_codes/src/error_codes/E0796.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
Reference of 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 foo<'a>(_x: &'a i32) {}
```

Mutable statics can be written to by multiple threads: aliasing violations or
data races will cause undefined behavior.

Reference of mutable static is a hard error from 2024 edition.
1 change: 1 addition & 0 deletions compiler/rustc_hir_analysis/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ rustc_errors = { path = "../rustc_errors" }
rustc_feature = { path = "../rustc_feature" }
rustc_fluent_macro = { path = "../rustc_fluent_macro" }
rustc_hir = { path = "../rustc_hir" }
rustc_hir_pretty = { path = "../rustc_hir_pretty" }
rustc_index = { path = "../rustc_index" }
rustc_infer = { path = "../rustc_infer" }
rustc_lint_defs = { path = "../rustc_lint_defs" }
Expand Down
14 changes: 14 additions & 0 deletions compiler/rustc_hir_analysis/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -346,6 +346,20 @@ 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_specialize = cannot specialize on `'static` lifetime

hir_analysis_substs_on_overridden_impl = could not resolve substs on overridden impl
Expand Down
97 changes: 97 additions & 0 deletions compiler/rustc_hir_analysis/src/check/errs.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
use rustc_hir as hir;
use rustc_hir_pretty::qpath_to_string;
use rustc_lint_defs::builtin::STATIC_MUT_REF;
use rustc_middle::ty::TyCtxt;
use rustc_span::Span;
use rustc_type_ir::Mutability;

use crate::errors;

/// Check for shared or mutable references of `static mut` inside expression
pub fn maybe_expr_static_mut(tcx: TyCtxt<'_>, expr: hir::Expr<'_>) {
let span = expr.span;
let hir_id = expr.hir_id;
if let hir::ExprKind::AddrOf(borrow_kind, m, expr) = expr.kind
&& matches!(borrow_kind, hir::BorrowKind::Ref)
&& let Some(var) = is_path_static_mut(*expr)
{
handle_static_mut_ref(
tcx,
span,
var,
span.edition().at_least_rust_2024(),
matches!(m, Mutability::Mut),
hir_id,
);
}
}

/// Check for shared or mutable references of `static mut` inside statement
pub fn maybe_stmt_static_mut(tcx: TyCtxt<'_>, stmt: hir::Stmt<'_>) {
if let hir::StmtKind::Local(loc) = stmt.kind
&& let hir::PatKind::Binding(ba, _, _, _) = loc.pat.kind
&& matches!(ba.0, rustc_ast::ByRef::Yes)
&& let Some(init) = loc.init
&& let Some(var) = is_path_static_mut(*init)
{
handle_static_mut_ref(
tcx,
init.span,
var,
loc.span.edition().at_least_rust_2024(),
matches!(ba.1, Mutability::Mut),
stmt.hir_id,
);
}
}

fn is_path_static_mut(expr: hir::Expr<'_>) -> Option<String> {
if let hir::ExprKind::Path(qpath) = expr.kind
&& let hir::QPath::Resolved(_, path) = qpath
&& let hir::def::Res::Def(def_kind, _) = path.res
&& let hir::def::DefKind::Static(mt) = def_kind
&& matches!(mt, Mutability::Mut)
{
return Some(qpath_to_string(&qpath));
}
None
}

fn handle_static_mut_ref(
tcx: TyCtxt<'_>,
span: Span,
var: String,
e2024: bool,
mutable: bool,
hir_id: hir::HirId,
) {
if e2024 {
let sugg = if mutable {
errors::StaticMutRefSugg::Mut { span, var }
} else {
errors::StaticMutRefSugg::Shared { span, var }
};
tcx.sess.parse_sess.dcx.emit_err(errors::StaticMutRef { span, sugg });
return;
}

let (label, sugg, shared) = if mutable {
(
errors::RefOfMutStaticLabel::Mut { span },
errors::RefOfMutStaticSugg::Mut { span, var },
"mutable ",
)
} else {
(
errors::RefOfMutStaticLabel::Shared { span },
errors::RefOfMutStaticSugg::Shared { span, var },
"shared ",
)
};
tcx.emit_spanned_lint(
STATIC_MUT_REF,
hir_id,
span,
errors::RefOfMutStatic { shared, why_note: (), label, sugg },
);
}
1 change: 1 addition & 0 deletions compiler/rustc_hir_analysis/src/check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ mod check;
mod compare_impl_item;
pub mod dropck;
mod entry;
mod errs;
pub mod intrinsic;
pub mod intrinsicck;
mod region;
Expand Down
6 changes: 6 additions & 0 deletions compiler/rustc_hir_analysis/src/check/region.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ use rustc_middle::ty::TyCtxt;
use rustc_span::source_map;
use rustc_span::Span;

use super::errs::{maybe_expr_static_mut, maybe_stmt_static_mut};

use std::mem;

#[derive(Debug, Copy, Clone)]
Expand Down Expand Up @@ -224,6 +226,8 @@ fn resolve_stmt<'tcx>(visitor: &mut RegionResolutionVisitor<'tcx>, stmt: &'tcx h
let stmt_id = stmt.hir_id.local_id;
debug!("resolve_stmt(stmt.id={:?})", stmt_id);

maybe_stmt_static_mut(visitor.tcx, *stmt);
est31 marked this conversation as resolved.
Show resolved Hide resolved

// Every statement will clean up the temporaries created during
// execution of that statement. Therefore each statement has an
// associated destruction scope that represents the scope of the
Expand All @@ -242,6 +246,8 @@ fn resolve_stmt<'tcx>(visitor: &mut RegionResolutionVisitor<'tcx>, stmt: &'tcx h
fn resolve_expr<'tcx>(visitor: &mut RegionResolutionVisitor<'tcx>, expr: &'tcx hir::Expr<'tcx>) {
debug!("resolve_expr - pre-increment {} expr = {:?}", visitor.expr_and_pat_count, expr);

maybe_expr_static_mut(visitor.tcx, *expr);

let prev_cx = visitor.cx;
visitor.enter_node_scope_with_dtor(expr.hir_id.local_id);

Expand Down
91 changes: 91 additions & 0 deletions compiler/rustc_hir_analysis/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1410,3 +1410,94 @@ pub struct OnlyCurrentTraitsPointerSugg<'a> {
pub mut_key: &'a str,
pub ptr_ty: Ty<'a>,
}

#[derive(Diagnostic)]
#[diag(hir_analysis_static_mut_ref, code = "E0796")]
#[note]
pub struct StaticMutRef {
#[primary_span]
#[label]
pub span: Span,
#[subdiagnostic]
pub sugg: StaticMutRefSugg,
}

#[derive(Subdiagnostic)]
pub enum StaticMutRefSugg {
#[suggestion(
hir_analysis_suggestion,
style = "verbose",
code = "addr_of!({var})",
applicability = "maybe-incorrect"
)]
Shared {
#[primary_span]
span: Span,
var: String,
},
#[suggestion(
hir_analysis_suggestion_mut,
style = "verbose",
code = "addr_of_mut!({var})",
applicability = "maybe-incorrect"
)]
Mut {
#[primary_span]
span: Span,
var: String,
},
}

// STATIC_MUT_REF lint
#[derive(LintDiagnostic)]
#[diag(hir_analysis_static_mut_ref_lint)]
#[note]
pub struct RefOfMutStatic<'a> {
pub shared: &'a str,
#[note(hir_analysis_why_note)]
pub why_note: (),
#[subdiagnostic]
pub label: RefOfMutStaticLabel,
#[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,
},
}

#[derive(Subdiagnostic)]
pub enum RefOfMutStaticSugg {
#[suggestion(
hir_analysis_suggestion,
style = "verbose",
code = "addr_of!({var})",
applicability = "maybe-incorrect"
)]
Shared {
#[primary_span]
span: Span,
var: String,
},
#[suggestion(
hir_analysis_suggestion_mut,
style = "verbose",
code = "addr_of_mut!({var})",
applicability = "maybe-incorrect"
)]
Mut {
#[primary_span]
span: Span,
var: String,
},
}
52 changes: 52 additions & 0 deletions compiler/rustc_lint_defs/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ declare_lint_pass! {
SINGLE_USE_LIFETIMES,
SOFT_UNSTABLE,
STABLE_FEATURES,
STATIC_MUT_REF,
SUSPICIOUS_AUTO_TRAIT_IMPLS,
TEST_UNSTABLE_LINT,
TEXT_DIRECTION_CODEPOINT_IN_COMMENT,
Expand Down Expand Up @@ -1767,6 +1768,57 @@ declare_lint! {
};
}

declare_lint! {
/// The `static_mut_ref` lint checks for shared or mutable references
/// of mutable static inside `unsafe` blocks and `unsafe` functions.
///
/// ### Example
///
/// ```rust,edition2021
/// fn main() {
/// 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);
/// }
/// }
///
/// unsafe fn _foo() {
/// static mut X: i32 = 23;
/// static mut Y: i32 = 24;
///
/// let y = &X;
/// let ref x = X;
/// let (x, y) = (&X, &Y);
/// foo(&X);
/// }
///
/// fn foo<'a>(_x: &'a i32) {}
/// ```
///
/// {{produces}}
///
/// ### Explanation
///
/// 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
/// a hard error instead.
pub STATIC_MUT_REF,
obeis marked this conversation as resolved.
Show resolved Hide resolved
Warn,
"shared references or mutable references of mutable static is discouraged",
@future_incompatible = FutureIncompatibleInfo {
reason: FutureIncompatibilityReason::EditionError(Edition::Edition2024),
reference: "issue #114447 <https://github.com/rust-lang/rust/issues/114447>",
explain_reason: false,
};
}

declare_lint! {
/// The `absolute_paths_not_starting_with_crate` lint detects fully
/// qualified paths that start with a module name instead of `crate`,
Expand Down
4 changes: 4 additions & 0 deletions library/panic_unwind/src/seh.rs
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,8 @@ cfg_if::cfg_if! {
}
}

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

Expand Down Expand Up @@ -322,6 +324,8 @@ 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
#[cfg_attr(not(bootstrap), allow(static_mut_ref))]
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
Loading
Loading