From 74cab947f79045e34eb973199274ee5f3c132bd8 Mon Sep 17 00:00:00 2001 From: Obei Sideg Date: Sat, 24 Aug 2024 06:47:43 +0300 Subject: [PATCH] Disallow hidden references to mutable static --- .../rustc_driver_impl/src/signal_handler.rs | 2 + .../src/error_codes/E0796.md | 6 +- compiler/rustc_error_codes/src/lib.rs | 1 + compiler/rustc_hir_analysis/messages.ftl | 19 --- compiler/rustc_hir_analysis/src/check/mod.rs | 1 - .../rustc_hir_analysis/src/check/region.rs | 6 - compiler/rustc_hir_analysis/src/errors.rs | 51 ------ compiler/rustc_lint/messages.ftl | 7 + compiler/rustc_lint/src/lib.rs | 3 + compiler/rustc_lint/src/lints.rs | 32 ++++ compiler/rustc_lint/src/static_mut_refs.rs | 154 ++++++++++++++++++ compiler/rustc_lint_defs/src/builtin.rs | 52 ------ compiler/rustc_lint_defs/src/lib.rs | 2 +- 13 files changed, 203 insertions(+), 133 deletions(-) create mode 100644 compiler/rustc_lint/src/static_mut_refs.rs diff --git a/compiler/rustc_driver_impl/src/signal_handler.rs b/compiler/rustc_driver_impl/src/signal_handler.rs index e1f868c2522..411661824f0 100644 --- a/compiler/rustc_driver_impl/src/signal_handler.rs +++ b/compiler/rustc_driver_impl/src/signal_handler.rs @@ -35,6 +35,8 @@ macro raw_errln($tokens:tt) { } /// Signal handler installed for SIGSEGV +// FIXME(static_mut_refs): Do not allow `static_mut_refs` lint +#[allow(static_mut_refs)] extern "C" fn print_stack_trace(_: libc::c_int) { const MAX_FRAMES: usize = 256; // Reserve data segment so we don't have to malloc in a signal handler, which might fail diff --git a/compiler/rustc_error_codes/src/error_codes/E0796.md b/compiler/rustc_error_codes/src/error_codes/E0796.md index 7ac429e5215..ced163e98f7 100644 --- a/compiler/rustc_error_codes/src/error_codes/E0796.md +++ b/compiler/rustc_error_codes/src/error_codes/E0796.md @@ -1,14 +1,14 @@ +#### Note: this error code is no longer emitted by the compiler. + You have created a reference to a mutable static. Erroneous code example: -```compile_fail,edition2024,E0796 +``` static mut X: i32 = 23; - fn work() { let _val = unsafe { X }; } - let x_ref = unsafe { &mut X }; work(); // The next line has Undefined Behavior! diff --git a/compiler/rustc_error_codes/src/lib.rs b/compiler/rustc_error_codes/src/lib.rs index 150f99a3ee7..6c887e31924 100644 --- a/compiler/rustc_error_codes/src/lib.rs +++ b/compiler/rustc_error_codes/src/lib.rs @@ -679,3 +679,4 @@ E0798: 0798, // E0723, // unstable feature in `const` context // E0738, // Removed; errored on `#[track_caller] fn`s in `extern "Rust" { ... }`. // E0744, // merged into E0728 +// E0796, // unused error code. We use `static_mut_refs` lint instead. diff --git a/compiler/rustc_hir_analysis/messages.ftl b/compiler/rustc_hir_analysis/messages.ftl index bde94be6f51..633ccacedfc 100644 --- a/compiler/rustc_hir_analysis/messages.ftl +++ b/compiler/rustc_hir_analysis/messages.ftl @@ -467,25 +467,6 @@ 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 = 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 hir_analysis_tait_forward_compat = item constrains opaque type that is not in its signature diff --git a/compiler/rustc_hir_analysis/src/check/mod.rs b/compiler/rustc_hir_analysis/src/check/mod.rs index dbdad2eb41d..3a7366ef78a 100644 --- a/compiler/rustc_hir_analysis/src/check/mod.rs +++ b/compiler/rustc_hir_analysis/src/check/mod.rs @@ -66,7 +66,6 @@ mod check; mod compare_impl_item; pub mod dropck; mod entry; -mod errs; pub mod intrinsic; pub mod intrinsicck; mod region; diff --git a/compiler/rustc_hir_analysis/src/check/region.rs b/compiler/rustc_hir_analysis/src/check/region.rs index 33e58a92e37..1a5f4659812 100644 --- a/compiler/rustc_hir_analysis/src/check/region.rs +++ b/compiler/rustc_hir_analysis/src/check/region.rs @@ -20,8 +20,6 @@ use rustc_middle::ty::TyCtxt; use rustc_span::source_map; use tracing::debug; -use super::errs::{maybe_expr_static_mut, maybe_stmt_static_mut}; - #[derive(Debug, Copy, Clone)] struct Context { /// The scope that contains any new variables declared, plus its depth in @@ -229,8 +227,6 @@ 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); - // 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 @@ -249,8 +245,6 @@ 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); diff --git a/compiler/rustc_hir_analysis/src/errors.rs b/compiler/rustc_hir_analysis/src/errors.rs index 39df18ff658..e4e5f76ae32 100644 --- a/compiler/rustc_hir_analysis/src/errors.rs +++ b/compiler/rustc_hir_analysis/src/errors.rs @@ -1522,57 +1522,6 @@ pub(crate) struct OnlyCurrentTraitsPointerSugg<'a> { pub ptr_ty: Ty<'a>, } -#[derive(Diagnostic)] -#[diag(hir_analysis_static_mut_ref, code = E0796)] -#[note] -pub(crate) struct StaticMutRef<'a> { - #[primary_span] - #[label] - pub span: Span, - #[subdiagnostic] - pub sugg: MutRefSugg, - pub shared: &'a str, -} - -#[derive(Subdiagnostic)] -pub(crate) enum MutRefSugg { - #[multipart_suggestion( - hir_analysis_suggestion, - style = "verbose", - applicability = "maybe-incorrect" - )] - Shared { - #[suggestion_part(code = "addr_of!(")] - lo: Span, - #[suggestion_part(code = ")")] - hi: Span, - }, - #[multipart_suggestion( - hir_analysis_suggestion_mut, - style = "verbose", - applicability = "maybe-incorrect" - )] - Mut { - #[suggestion_part(code = "addr_of_mut!(")] - lo: Span, - #[suggestion_part(code = ")")] - hi: Span, - }, -} - -// STATIC_MUT_REF lint -#[derive(LintDiagnostic)] -#[diag(hir_analysis_static_mut_refs_lint)] -#[note] -#[note(hir_analysis_why_note)] -pub(crate) struct RefOfMutStatic<'a> { - #[label] - pub span: Span, - #[subdiagnostic] - pub sugg: MutRefSugg, - pub shared: &'a str, -} - #[derive(Diagnostic)] #[diag(hir_analysis_not_supported_delegation)] pub(crate) struct UnsupportedDelegation<'a> { diff --git a/compiler/rustc_lint/messages.ftl b/compiler/rustc_lint/messages.ftl index 4aa86944a0b..0e3d34355a1 100644 --- a/compiler/rustc_lint/messages.ftl +++ b/compiler/rustc_lint/messages.ftl @@ -769,6 +769,13 @@ lint_single_use_lifetime = lifetime parameter `{$ident}` only used once lint_span_use_eq_ctxt = use `.eq_ctxt()` instead of `.ctxt() == .ctxt()` +lint_static_mut_refs_lint = creating a {$shared_label}reference to mutable static is discouraged + .label = {$shared_label}reference to mutable static + .suggestion = use `&raw const` instead to create a raw pointer + .suggestion_mut = use `&raw mut` instead to create a raw pointer + .shared_note = shared references to mutable statics are dangerous; it's undefined behavior if the static is mutated or if a mutable reference is created for it while the shared reference lives + .mut_note = mutable references to mutable statics are dangerous; it's undefined behavior if any other pointer to the static is used or if any other reference is created for the static while the mutable reference lives + lint_supertrait_as_deref_target = this `Deref` implementation is covered by an implicit supertrait coercion .label = `{$self_ty}` implements `Deref` which conflicts with supertrait `{$supertrait_principal}` .label2 = target type is a supertrait of `{$self_ty}` diff --git a/compiler/rustc_lint/src/lib.rs b/compiler/rustc_lint/src/lib.rs index 0c32157758c..84dbebfc3b5 100644 --- a/compiler/rustc_lint/src/lib.rs +++ b/compiler/rustc_lint/src/lib.rs @@ -81,6 +81,7 @@ mod ptr_nulls; mod redundant_semicolon; mod reference_casting; mod shadowed_into_iter; +mod static_mut_refs; mod tail_expr_drop_order; mod traits; mod types; @@ -120,6 +121,7 @@ use rustc_middle::query::Providers; use rustc_middle::ty::TyCtxt; use shadowed_into_iter::ShadowedIntoIter; pub use shadowed_into_iter::{ARRAY_INTO_ITER, BOXED_SLICE_INTO_ITER}; +use static_mut_refs::*; use tail_expr_drop_order::TailExprDropOrder; use traits::*; use types::*; @@ -246,6 +248,7 @@ late_lint_methods!( ImplTraitOvercaptures: ImplTraitOvercaptures, TailExprDropOrder: TailExprDropOrder, IfLetRescope: IfLetRescope::default(), + StaticMutRefs: StaticMutRefs, ] ] ); diff --git a/compiler/rustc_lint/src/lints.rs b/compiler/rustc_lint/src/lints.rs index e49b102cb39..27e18bc5469 100644 --- a/compiler/rustc_lint/src/lints.rs +++ b/compiler/rustc_lint/src/lints.rs @@ -3060,3 +3060,35 @@ pub(crate) struct UnsafeAttrOutsideUnsafeSuggestion { pub(crate) struct OutOfScopeMacroCalls { pub path: String, } + +#[derive(LintDiagnostic)] +#[diag(lint_static_mut_refs_lint)] +pub(crate) struct RefOfMutStatic<'a> { + #[label] + pub span: Span, + #[subdiagnostic] + pub sugg: Option, + pub shared_label: &'a str, + #[note(lint_shared_note)] + pub shared_note: bool, + #[note(lint_mut_note)] + pub mut_note: bool, +} + +#[derive(Subdiagnostic)] +pub(crate) enum MutRefSugg { + #[multipart_suggestion(lint_suggestion, style = "verbose", applicability = "maybe-incorrect")] + Shared { + #[suggestion_part(code = "&raw const ")] + span: Span, + }, + #[multipart_suggestion( + lint_suggestion_mut, + style = "verbose", + applicability = "maybe-incorrect" + )] + Mut { + #[suggestion_part(code = "&raw mut ")] + span: Span, + }, +} diff --git a/compiler/rustc_lint/src/static_mut_refs.rs b/compiler/rustc_lint/src/static_mut_refs.rs new file mode 100644 index 00000000000..3dd26fb9c53 --- /dev/null +++ b/compiler/rustc_lint/src/static_mut_refs.rs @@ -0,0 +1,154 @@ +use rustc_hir as hir; +use rustc_hir::{Expr, Stmt}; +use rustc_middle::ty::{Mutability, TyKind}; +use rustc_session::lint::FutureIncompatibilityReason; +use rustc_session::{declare_lint, declare_lint_pass}; +use rustc_span::edition::Edition; +use rustc_span::Span; + +use crate::lints::{MutRefSugg, RefOfMutStatic}; +use crate::{LateContext, LateLintPass, LintContext}; + +declare_lint! { + /// The `static_mut_refs` 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, in 2024 is "deny". + pub STATIC_MUT_REFS, + Warn, + "shared references or mutable references of mutable static is discouraged", + @future_incompatible = FutureIncompatibleInfo { + reason: FutureIncompatibilityReason::EditionError(Edition::Edition2024), + reference: "", + explain_reason: false, + }; + @edition Edition2024 => Deny; +} + +declare_lint_pass!(StaticMutRefs => [STATIC_MUT_REFS]); + +impl<'tcx> LateLintPass<'tcx> for StaticMutRefs { + #[allow(rustc::usage_of_ty_tykind)] + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &Expr<'_>) { + let err_span = expr.span; + match expr.kind { + hir::ExprKind::AddrOf(borrow_kind, m, ex) + if matches!(borrow_kind, hir::BorrowKind::Ref) + && let Some(err_span) = path_is_static_mut(ex, err_span) => + { + emit_static_mut_refs( + cx, + err_span, + err_span.with_hi(ex.span.lo()), + m, + !expr.span.from_expansion(), + ); + } + hir::ExprKind::MethodCall(_, e, _, _) + if let Some(err_span) = path_is_static_mut(e, expr.span) + && let typeck = cx.typeck_results() + && let Some(method_def_id) = typeck.type_dependent_def_id(expr.hir_id) + && let inputs = + cx.tcx.fn_sig(method_def_id).skip_binder().inputs().skip_binder() + && let Some(receiver) = inputs.get(0) + && let TyKind::Ref(_, _, m) = receiver.kind() => + { + emit_static_mut_refs(cx, err_span, err_span.shrink_to_lo(), *m, false); + } + _ => {} + } + } + + fn check_stmt(&mut self, cx: &LateContext<'tcx>, stmt: &Stmt<'_>) { + if let hir::StmtKind::Let(loc) = stmt.kind + && let hir::PatKind::Binding(ba, _, _, _) = loc.pat.kind + && let hir::ByRef::Yes(m) = ba.0 + && let Some(init) = loc.init + && let Some(err_span) = path_is_static_mut(init, init.span) + { + emit_static_mut_refs(cx, err_span, err_span.shrink_to_lo(), m, false); + } + } +} + +fn path_is_static_mut(mut expr: &hir::Expr<'_>, mut err_span: Span) -> Option { + if err_span.from_expansion() { + err_span = expr.span; + } + + while let hir::ExprKind::Field(e, _) = expr.kind { + expr = e; + } + + 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 { safety: _, mutability: Mutability::Mut, nested: false } = + def_kind + { + return Some(err_span); + } + None +} + +fn emit_static_mut_refs( + cx: &LateContext<'_>, + span: Span, + sugg_span: Span, + mutable: Mutability, + suggest_addr_of: bool, +) { + let (shared_label, shared_note, mut_note, sugg) = match mutable { + Mutability::Mut => { + let sugg = + if suggest_addr_of { Some(MutRefSugg::Mut { span: sugg_span }) } else { None }; + ("mutable ", false, true, sugg) + } + Mutability::Not => { + let sugg = + if suggest_addr_of { Some(MutRefSugg::Shared { span: sugg_span }) } else { None }; + ("shared ", true, false, sugg) + } + }; + + cx.emit_span_lint( + STATIC_MUT_REFS, + span, + RefOfMutStatic { span, sugg, shared_label, shared_note, mut_note }, + ); +} diff --git a/compiler/rustc_lint_defs/src/builtin.rs b/compiler/rustc_lint_defs/src/builtin.rs index 25d33126754..73a36a6f0c1 100644 --- a/compiler/rustc_lint_defs/src/builtin.rs +++ b/compiler/rustc_lint_defs/src/builtin.rs @@ -100,7 +100,6 @@ declare_lint_pass! { SINGLE_USE_LIFETIMES, SOFT_UNSTABLE, STABLE_FEATURES, - STATIC_MUT_REFS, TEST_UNSTABLE_LINT, TEXT_DIRECTION_CODEPOINT_IN_COMMENT, TRIVIAL_CASTS, @@ -1927,57 +1926,6 @@ declare_lint! { }; } -declare_lint! { - /// The `static_mut_refs` 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, in 2024 there is - /// a hard error instead. - pub STATIC_MUT_REFS, - Warn, - "shared references or mutable references of mutable static is discouraged", - @future_incompatible = FutureIncompatibleInfo { - reason: FutureIncompatibilityReason::EditionError(Edition::Edition2024), - reference: "issue #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`, diff --git a/compiler/rustc_lint_defs/src/lib.rs b/compiler/rustc_lint_defs/src/lib.rs index 5c4b7e5664d..f5ed69658ce 100644 --- a/compiler/rustc_lint_defs/src/lib.rs +++ b/compiler/rustc_lint_defs/src/lib.rs @@ -12,7 +12,7 @@ use rustc_error_messages::{DiagMessage, MultiSpan}; use rustc_hir::def::Namespace; use rustc_hir::{HashStableContext, HirId, MissingLifetimeKind}; use rustc_macros::{Decodable, Encodable, HashStable_Generic}; -use rustc_span::edition::Edition; +pub use rustc_span::edition::Edition; use rustc_span::symbol::{Ident, MacroRulesNormalizedIdent}; use rustc_span::{sym, Span, Symbol}; use rustc_target::spec::abi::Abi;