Skip to content

Commit

Permalink
Auto merge of #128117 - matthiaskrgr:rollup-7z2pp6o, r=matthiaskrgr
Browse files Browse the repository at this point in the history
Rollup of 6 pull requests

Successful merges:

 - #124895 (Disallow hidden references to mutable static)
 - #128043 (Docs for core::primitive: mention that "core" can be shadowed, too, so we should write "::core")
 - #128092 (Remove wrapper functions from c.rs)
 - #128100 (Allow to pass a full path for `run-make` tests)
 - #128106 (Fix return type of FileAttr methods on AIX target)
 - #128108 (ensure std step before preparing sysroot)

r? `@ghost`
`@rustbot` modify labels: rollup
  • Loading branch information
bors committed Jul 23, 2024
2 parents 8bfcae7 + b4a0ce6 commit 31c9479
Show file tree
Hide file tree
Showing 109 changed files with 1,411 additions and 672 deletions.
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

0 comments on commit 31c9479

Please sign in to comment.