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

Rollup of 6 pull requests #128117

Closed
wants to merge 14 commits into from
Closed
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
2 changes: 2 additions & 0 deletions compiler/rustc_driver_impl/src/signal_handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ macro raw_errln($tokens:tt) {
}

/// Signal handler installed for SIGSEGV
// FIXME(obeis): Use `SyncUnsafeCell` instead of allowing `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
Expand Down
20 changes: 8 additions & 12 deletions compiler/rustc_hir_analysis/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -463,24 +463,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 = 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
}
hir_analysis_static_mut_ref = creating a {$shared_label}reference to a mutable static
.label = {$shared_label}reference to mutable static
.shared_note = shared references to mutable statics are dangerous since if there's any kind of mutation of, or mutable reference created for, that static while the reference lives, that's undefined behavior
.mut_note = mutable references to mutable statics are dangerous since if there's any other pointer used or reference created for that static while the reference lives, that's 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
hir_analysis_static_mut_refs_lint = creating a {$shared_label}reference to mutable static is discouraged
.label = {$shared_label}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
}
.shared_note = shared references to mutable statics are dangerous since if there's any kind of mutation of, or mutable reference created for, that static while the reference lives, that's undefined behavior
.mut_note = mutable references to mutable statics are dangerous since if there's any other pointer used or reference created for that static while the reference lives, that's undefined behavior

hir_analysis_static_specialize = cannot specialize on `'static` lifetime

Expand Down
145 changes: 104 additions & 41 deletions compiler/rustc_hir_analysis/src/check/errs.rs
Original file line number Diff line number Diff line change
@@ -1,60 +1,113 @@
use rustc_hir as hir;
use rustc_lint_defs::builtin::STATIC_MUT_REFS;
use rustc_middle::ty::{Mutability, TyCtxt};
use rustc_middle::ty::{Mutability, TyCtxt, TyKind};
use rustc_span::Span;

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)
&& path_if_static_mut(expr)
{
handle_static_mut_ref(
tcx,
span,
span.with_hi(expr.span.lo()),
span.shrink_to_hi(),
span.edition().at_least_rust_2024(),
m,
hir_id,
);
pub fn maybe_expr_static_mut(tcx: TyCtxt<'_>, expr: &hir::Expr<'_>) {
let err_span = expr.span;
let lint_level_hir_id = expr.hir_id;
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) =>
{
handle_static_mut_ref(
tcx,
err_span,
err_span.with_hi(ex.span.lo()),
err_span.shrink_to_hi(),
err_span.edition().at_least_rust_2024(),
Some(m),
lint_level_hir_id,
!expr.span.from_expansion(),
);
}
hir::ExprKind::Index(expr, _, _)
if let Some(err_span) = path_is_static_mut(expr, err_span) =>
{
handle_static_mut_ref(
tcx,
err_span,
err_span.with_hi(expr.span.lo()),
err_span.shrink_to_hi(),
err_span.edition().at_least_rust_2024(),
None,
lint_level_hir_id,
false,
);
}
_ => {}
}
}

/// Check for shared or mutable references of `static mut` inside statement
pub fn maybe_stmt_static_mut(tcx: TyCtxt<'_>, stmt: hir::Stmt<'_>) {
pub fn maybe_stmt_static_mut(tcx: TyCtxt<'_>, stmt: &hir::Stmt<'_>) {
if let hir::StmtKind::Let(loc) = stmt.kind
&& let hir::PatKind::Binding(ba, _, _, _) = loc.pat.kind
&& let hir::ByRef::Yes(rmutbl) = ba.0
&& let Some(init) = loc.init
&& path_if_static_mut(init)
&& let Some(err_span) = path_is_static_mut(init, init.span)
{
handle_static_mut_ref(
tcx,
init.span,
init.span.shrink_to_lo(),
init.span.shrink_to_hi(),
err_span,
err_span.shrink_to_lo(),
err_span.shrink_to_hi(),
loc.span.edition().at_least_rust_2024(),
rmutbl,
Some(rmutbl),
stmt.hir_id,
false,
);
}
}

/// Check if method call takes shared or mutable references of `static mut`
#[allow(rustc::usage_of_ty_tykind)]
pub fn maybe_method_static_mut(tcx: TyCtxt<'_>, expr: &hir::Expr<'_>) {
if let hir::ExprKind::MethodCall(_, e, _, _) = expr.kind
&& let Some(err_span) = path_is_static_mut(e, expr.span)
&& let typeck = tcx.typeck(expr.hir_id.owner)
&& let Some(method_def_id) = typeck.type_dependent_def_id(expr.hir_id)
&& let inputs = tcx.fn_sig(method_def_id).skip_binder().inputs().skip_binder()
&& !inputs.is_empty()
&& inputs[0].is_ref()
{
let m = if let TyKind::Ref(_, _, m) = inputs[0].kind() { *m } else { Mutability::Not };

handle_static_mut_ref(
tcx,
err_span,
err_span.shrink_to_lo(),
err_span.shrink_to_hi(),
err_span.edition().at_least_rust_2024(),
Some(m),
expr.hir_id,
false,
);
}
}

fn path_if_static_mut(expr: &hir::Expr<'_>) -> bool {
fn path_is_static_mut(mut expr: &hir::Expr<'_>, mut err_span: Span) -> Option<Span> {
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 true;
return Some(err_span);
}
false
None
}

fn handle_static_mut_ref(
Expand All @@ -63,27 +116,37 @@ fn handle_static_mut_ref(
lo: Span,
hi: Span,
e2024: bool,
mutable: Mutability,
hir_id: hir::HirId,
mutable: Option<Mutability>,
lint_level_hir_id: hir::HirId,
suggest_addr_of: bool,
) {
let (shared_label, shared_note, mut_note, sugg) = match mutable {
Some(Mutability::Mut) => {
let sugg =
if suggest_addr_of { Some(errors::MutRefSugg::Mut { lo, hi }) } else { None };
("mutable ", false, true, sugg)
}
Some(Mutability::Not) => {
let sugg =
if suggest_addr_of { Some(errors::MutRefSugg::Shared { lo, hi }) } else { None };
("shared ", true, false, sugg)
}
None => ("", true, true, None),
};
if e2024 {
let (sugg, shared) = if mutable == Mutability::Mut {
(errors::MutRefSugg::Mut { lo, hi }, "mutable")
} else {
(errors::MutRefSugg::Shared { lo, hi }, "shared")
};
tcx.dcx().emit_err(errors::StaticMutRef { span, sugg, shared });
tcx.dcx().emit_err(errors::StaticMutRef {
span,
sugg,
shared_label,
shared_note,
mut_note,
});
} else {
let (sugg, shared) = if mutable == Mutability::Mut {
(errors::MutRefSugg::Mut { lo, hi }, "mutable")
} else {
(errors::MutRefSugg::Shared { lo, hi }, "shared")
};
tcx.emit_node_span_lint(
STATIC_MUT_REFS,
hir_id,
lint_level_hir_id,
span,
errors::RefOfMutStatic { span, sugg, shared },
errors::RefOfMutStatic { span, sugg, shared_label, shared_note, mut_note },
);
}
}
2 changes: 1 addition & 1 deletion compiler/rustc_hir_analysis/src/check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ mod check;
mod compare_impl_item;
pub mod dropck;
mod entry;
mod errs;
pub mod errs;
pub mod intrinsic;
pub mod intrinsicck;
mod region;
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_hir_analysis/src/check/region.rs
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ 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);
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
Expand All @@ -248,7 +248,7 @@ 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);
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
2 changes: 2 additions & 0 deletions compiler/rustc_hir_analysis/src/collect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ use std::cell::Cell;
use std::iter;
use std::ops::Bound;

use crate::check::errs::maybe_method_static_mut;
use crate::check::intrinsic::intrinsic_operation_unsafety;
use crate::errors;
use crate::hir_ty_lowering::{HirTyLowerer, RegionInferReason};
Expand Down Expand Up @@ -324,6 +325,7 @@ impl<'tcx> Visitor<'tcx> for CollectItemTypesVisitor<'tcx> {
// depends on typecheck and would therefore hide
// any further errors in case one typeck fails.
}
maybe_method_static_mut(self.tcx, expr);
intravisit::walk_expr(self, expr);
}

Expand Down
20 changes: 13 additions & 7 deletions compiler/rustc_hir_analysis/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1502,14 +1502,17 @@ pub struct OnlyCurrentTraitsPointerSugg<'a> {

#[derive(Diagnostic)]
#[diag(hir_analysis_static_mut_ref, code = E0796)]
#[note]
pub struct StaticMutRef<'a> {
#[primary_span]
#[label]
pub span: Span,
#[subdiagnostic]
pub sugg: MutRefSugg,
pub shared: &'a str,
pub sugg: Option<MutRefSugg>,
pub shared_label: &'a str,
#[note(hir_analysis_shared_note)]
pub shared_note: bool,
#[note(hir_analysis_mut_note)]
pub mut_note: bool,
}

#[derive(Subdiagnostic)]
Expand Down Expand Up @@ -1538,17 +1541,20 @@ pub enum MutRefSugg {
},
}

// STATIC_MUT_REF lint
// `STATIC_MUT_REF` lint
#[derive(LintDiagnostic)]
#[diag(hir_analysis_static_mut_refs_lint)]
#[note]
#[note(hir_analysis_why_note)]
pub struct RefOfMutStatic<'a> {
#[label]
pub span: Span,
#[subdiagnostic]
pub sugg: MutRefSugg,
pub shared: &'a str,
pub sugg: Option<MutRefSugg>,
pub shared_label: &'a str,
#[note(hir_analysis_shared_note)]
pub shared_note: bool,
#[note(hir_analysis_mut_note)]
pub mut_note: bool,
}

#[derive(Diagnostic)]
Expand Down
2 changes: 2 additions & 0 deletions library/alloc/tests/vec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1285,6 +1285,8 @@ fn test_from_iter_specialization_panic_during_iteration_drops() {

#[test]
#[cfg_attr(not(panic = "unwind"), ignore = "test requires unwinding support")]
// FIXME(obeis): Use `SyncUnsafeCell` instead of allowing `static_mut_ref` lint
#[cfg_attr(not(bootstrap), allow(static_mut_refs))]
fn test_from_iter_specialization_panic_during_drop_doesnt_leak() {
static mut DROP_COUNTER_OLD: [usize; 5] = [0; 5];
static mut DROP_COUNTER_NEW: [usize; 2] = [0; 2];
Expand Down
12 changes: 9 additions & 3 deletions library/core/src/primitive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
//! const SOME_PROPERTY: bool = true;
//! }
//!
//! # trait QueryId { const SOME_PROPERTY: core::primitive::bool; }
//! # trait QueryId { const SOME_PROPERTY: ::core::primitive::bool; }
//! ```
//!
//! Note that the `SOME_PROPERTY` associated constant would not compile, as its
Expand All @@ -25,11 +25,17 @@
//! pub struct bool;
//!
//! impl QueryId for bool {
//! const SOME_PROPERTY: core::primitive::bool = true;
//! const SOME_PROPERTY: ::core::primitive::bool = true;
//! }
//!
//! # trait QueryId { const SOME_PROPERTY: core::primitive::bool; }
//! # trait QueryId { const SOME_PROPERTY: ::core::primitive::bool; }
//! ```
//!
//! We also used `::core` instead of `core`, because `core` can be
//! shadowed, too. Paths, starting with `::`, are searched in
//! the [extern prelude] since Edition 2018.
//!
//! [extern prelude]: https://doc.rust-lang.org/nightly/reference/names/preludes.html#extern-prelude

#[stable(feature = "core_primitive", since = "1.43.0")]
pub use bool;
Expand Down
2 changes: 2 additions & 0 deletions library/core/tests/atomic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,8 @@ fn static_init() {
}

#[test]
// FIXME(obeis): Use `SyncUnsafeCell` instead of allowing `static_mut_ref` lint
#[cfg_attr(not(bootstrap), allow(static_mut_refs))]
fn atomic_access_bool() {
static mut ATOMIC: AtomicBool = AtomicBool::new(false);

Expand Down
2 changes: 2 additions & 0 deletions library/panic_unwind/src/seh.rs
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,8 @@ cfg_if::cfg_if! {
}
}

// FIXME(obeis): Do not allow `static_mut_refs` lint
#[allow(static_mut_refs)]
pub unsafe fn panic(data: Box<dyn Any + Send>) -> u32 {
use core::intrinsics::atomic_store_seqcst;

Expand Down
3 changes: 3 additions & 0 deletions library/std/src/sync/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@
//! Consider the following code, operating on some global static variables:
//!
//! ```rust
//! // FIXME(obeis): Do not allow `static_mut_refs` lint
//! #![allow(static_mut_refs)]
//!
//! static mut A: u32 = 0;
//! static mut B: u32 = 0;
//! static mut C: u32 = 0;
Expand Down
Loading
Loading