From 9342d1e73e28d496d88f24beaf5b629981fdb09d Mon Sep 17 00:00:00 2001 From: Gary Guo Date: Thu, 1 Dec 2022 18:34:59 +0000 Subject: [PATCH 1/7] Allow unsafe through inline const This is handled similar to closures --- .../rustc_mir_transform/src/check_unsafety.rs | 35 +++++++++++++++++-- 1 file changed, 32 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_mir_transform/src/check_unsafety.rs b/compiler/rustc_mir_transform/src/check_unsafety.rs index e783d189137..782abd7804d 100644 --- a/compiler/rustc_mir_transform/src/check_unsafety.rs +++ b/compiler/rustc_mir_transform/src/check_unsafety.rs @@ -1,6 +1,7 @@ use rustc_data_structures::fx::FxHashSet; use rustc_errors::struct_span_err; use rustc_hir as hir; +use rustc_hir::def::DefKind; use rustc_hir::def_id::{DefId, LocalDefId}; use rustc_hir::hir_id::HirId; use rustc_hir::intravisit; @@ -134,6 +135,28 @@ fn visit_rvalue(&mut self, rvalue: &Rvalue<'tcx>, location: Location) { self.super_rvalue(rvalue, location); } + fn visit_operand(&mut self, op: &Operand<'tcx>, location: Location) { + if let Operand::Constant(constant) = op { + let maybe_uneval = match constant.literal { + ConstantKind::Val(..) | ConstantKind::Ty(_) => None, + ConstantKind::Unevaluated(uv, _) => Some(uv), + }; + + if let Some(uv) = maybe_uneval { + if uv.promoted.is_none() { + let def_id = uv.def.def_id_for_type_of(); + if self.tcx.def_kind(def_id) == DefKind::InlineConst { + let local_def_id = def_id.expect_local(); + let UnsafetyCheckResult { violations, used_unsafe_blocks, .. } = + self.tcx.unsafety_check_result(local_def_id); + self.register_violations(violations, used_unsafe_blocks.iter().copied()); + } + } + } + } + self.super_operand(op, location); + } + fn visit_place(&mut self, place: &Place<'tcx>, context: PlaceContext, _location: Location) { // On types with `scalar_valid_range`, prevent // * `&mut x.field` @@ -410,6 +433,12 @@ fn visit_block(&mut self, block: &'tcx hir::Block<'tcx>) { intravisit::walk_block(self, block); } + fn visit_anon_const(&mut self, c: &'tcx hir::AnonConst) { + if matches!(self.tcx.def_kind(c.def_id), DefKind::InlineConst) { + self.visit_body(self.tcx.hir().body(c.body)) + } + } + fn visit_fn( &mut self, fk: intravisit::FnKind<'tcx>, @@ -484,7 +513,7 @@ fn unsafety_check_result<'tcx>( let mut checker = UnsafetyChecker::new(body, def.did, tcx, param_env); checker.visit_body(&body); - let unused_unsafes = (!tcx.is_closure(def.did.to_def_id())) + let unused_unsafes = (!tcx.is_typeck_child(def.did.to_def_id())) .then(|| check_unused_unsafe(tcx, def.did, &checker.used_unsafe_blocks)); tcx.arena.alloc(UnsafetyCheckResult { @@ -516,8 +545,8 @@ fn report_unused_unsafe(tcx: TyCtxt<'_>, kind: UnusedUnsafe, id: HirId) { pub fn check_unsafety(tcx: TyCtxt<'_>, def_id: LocalDefId) { debug!("check_unsafety({:?})", def_id); - // closures are handled by their parent fn. - if tcx.is_closure(def_id.to_def_id()) { + // closures and inline consts are handled by their parent fn. + if tcx.is_typeck_child(def_id.to_def_id()) { return; } From aa5af2a0034b211631204218caa8f2f17ea0b0e6 Mon Sep 17 00:00:00 2001 From: Gary Guo Date: Thu, 1 Dec 2022 19:05:49 +0000 Subject: [PATCH 2/7] Allow unsafe through inline const for THIR unsafety checker The closure handling code is changed slightly to avoid allocation when THIR building failed. --- .../rustc_mir_build/src/check_unsafety.rs | 37 +++++++++++++------ 1 file changed, 25 insertions(+), 12 deletions(-) diff --git a/compiler/rustc_mir_build/src/check_unsafety.rs b/compiler/rustc_mir_build/src/check_unsafety.rs index fb1ea9ed300..dc7f37c2566 100644 --- a/compiler/rustc_mir_build/src/check_unsafety.rs +++ b/compiler/rustc_mir_build/src/check_unsafety.rs @@ -408,16 +408,29 @@ fn visit_expr(&mut self, expr: &Expr<'tcx>) { } else { ty::WithOptConstParam::unknown(closure_id) }; - let (closure_thir, expr) = self.tcx.thir_body(closure_def).unwrap_or_else(|_| { - (self.tcx.alloc_steal_thir(Thir::new()), ExprId::from_u32(0)) - }); - let closure_thir = &closure_thir.borrow(); - let hir_context = self.tcx.hir().local_def_id_to_hir_id(closure_id); - let mut closure_visitor = - UnsafetyVisitor { thir: closure_thir, hir_context, ..*self }; - closure_visitor.visit_expr(&closure_thir[expr]); - // Unsafe blocks can be used in closures, make sure to take it into account - self.safety_context = closure_visitor.safety_context; + if let Ok((closure_thir, expr)) = self.tcx.thir_body(closure_def) { + let closure_thir = &closure_thir.borrow(); + let hir_context = self.tcx.hir().local_def_id_to_hir_id(closure_id); + let mut closure_visitor = + UnsafetyVisitor { thir: closure_thir, hir_context, ..*self }; + closure_visitor.visit_expr(&closure_thir[expr]); + // Unsafe blocks can be used in closures, make sure to take it into account + self.safety_context = closure_visitor.safety_context; + } + } + ExprKind::ConstBlock { did, substs: _ } => { + let def_id = did.expect_local(); + if let Ok((inner_thir, expr)) = + self.tcx.thir_body(ty::WithOptConstParam::unknown(def_id)) + { + let inner_thir = &inner_thir.borrow(); + let hir_context = self.tcx.hir().local_def_id_to_hir_id(def_id); + let mut inner_visitor = + UnsafetyVisitor { thir: inner_thir, hir_context, ..*self }; + inner_visitor.visit_expr(&inner_thir[expr]); + // Unsafe blocks can be used in inline consts, make sure to take it into account + self.safety_context = inner_visitor.safety_context; + } } ExprKind::Field { lhs, .. } => { let lhs = &self.thir[lhs]; @@ -612,8 +625,8 @@ pub fn check_unsafety<'tcx>(tcx: TyCtxt<'tcx>, def: ty::WithOptConstParam Date: Thu, 1 Dec 2022 19:13:18 +0000 Subject: [PATCH 3/7] Add tests --- .../ui/inline-const/expr-unsafe-err.mir.stderr | 11 +++++++++++ src/test/ui/inline-const/expr-unsafe-err.rs | 11 +++++++++++ .../ui/inline-const/expr-unsafe-err.thir.stderr | 11 +++++++++++ src/test/ui/inline-const/expr-unsafe.mir.stderr | 14 ++++++++++++++ src/test/ui/inline-const/expr-unsafe.rs | 16 ++++++++++++++++ .../ui/inline-const/expr-unsafe.thir.stderr | 17 +++++++++++++++++ 6 files changed, 80 insertions(+) create mode 100644 src/test/ui/inline-const/expr-unsafe-err.mir.stderr create mode 100644 src/test/ui/inline-const/expr-unsafe-err.rs create mode 100644 src/test/ui/inline-const/expr-unsafe-err.thir.stderr create mode 100644 src/test/ui/inline-const/expr-unsafe.mir.stderr create mode 100644 src/test/ui/inline-const/expr-unsafe.rs create mode 100644 src/test/ui/inline-const/expr-unsafe.thir.stderr diff --git a/src/test/ui/inline-const/expr-unsafe-err.mir.stderr b/src/test/ui/inline-const/expr-unsafe-err.mir.stderr new file mode 100644 index 00000000000..1bec41e2efa --- /dev/null +++ b/src/test/ui/inline-const/expr-unsafe-err.mir.stderr @@ -0,0 +1,11 @@ +error[E0133]: call to unsafe function is unsafe and requires unsafe function or block + --> $DIR/expr-unsafe-err.rs:8:9 + | +LL | require_unsafe(); + | ^^^^^^^^^^^^^^^^ call to unsafe function + | + = note: consult the function's documentation for information on how to avoid undefined behavior + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0133`. diff --git a/src/test/ui/inline-const/expr-unsafe-err.rs b/src/test/ui/inline-const/expr-unsafe-err.rs new file mode 100644 index 00000000000..adf05d352ea --- /dev/null +++ b/src/test/ui/inline-const/expr-unsafe-err.rs @@ -0,0 +1,11 @@ +// revisions: mir thir +// [thir]compile-flags: -Z thir-unsafeck +#![feature(inline_const)] +const unsafe fn require_unsafe() -> usize { 1 } + +fn main() { + const { + require_unsafe(); + //~^ ERROR [E0133] + } +} diff --git a/src/test/ui/inline-const/expr-unsafe-err.thir.stderr b/src/test/ui/inline-const/expr-unsafe-err.thir.stderr new file mode 100644 index 00000000000..c971e8afb35 --- /dev/null +++ b/src/test/ui/inline-const/expr-unsafe-err.thir.stderr @@ -0,0 +1,11 @@ +error[E0133]: call to unsafe function `require_unsafe` is unsafe and requires unsafe function or block + --> $DIR/expr-unsafe-err.rs:8:9 + | +LL | require_unsafe(); + | ^^^^^^^^^^^^^^^^ call to unsafe function + | + = note: consult the function's documentation for information on how to avoid undefined behavior + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0133`. diff --git a/src/test/ui/inline-const/expr-unsafe.mir.stderr b/src/test/ui/inline-const/expr-unsafe.mir.stderr new file mode 100644 index 00000000000..1ab6e42fba0 --- /dev/null +++ b/src/test/ui/inline-const/expr-unsafe.mir.stderr @@ -0,0 +1,14 @@ +warning: unnecessary `unsafe` block + --> $DIR/expr-unsafe.rs:12:13 + | +LL | unsafe {} + | ^^^^^^ unnecessary `unsafe` block + | +note: the lint level is defined here + --> $DIR/expr-unsafe.rs:4:9 + | +LL | #![warn(unused_unsafe)] + | ^^^^^^^^^^^^^ + +warning: 1 warning emitted + diff --git a/src/test/ui/inline-const/expr-unsafe.rs b/src/test/ui/inline-const/expr-unsafe.rs new file mode 100644 index 00000000000..d71efd33db1 --- /dev/null +++ b/src/test/ui/inline-const/expr-unsafe.rs @@ -0,0 +1,16 @@ +// check-pass +// revisions: mir thir +// [thir]compile-flags: -Z thir-unsafeck +#![warn(unused_unsafe)] +#![feature(inline_const)] +const unsafe fn require_unsafe() -> usize { 1 } + +fn main() { + unsafe { + const { + require_unsafe(); + unsafe {} + //~^ WARNING unnecessary `unsafe` block + } + } +} diff --git a/src/test/ui/inline-const/expr-unsafe.thir.stderr b/src/test/ui/inline-const/expr-unsafe.thir.stderr new file mode 100644 index 00000000000..4737444fb61 --- /dev/null +++ b/src/test/ui/inline-const/expr-unsafe.thir.stderr @@ -0,0 +1,17 @@ +warning: unnecessary `unsafe` block + --> $DIR/expr-unsafe.rs:12:13 + | +LL | unsafe { + | ------ because it's nested under this `unsafe` block +... +LL | unsafe {} + | ^^^^^^ unnecessary `unsafe` block + | +note: the lint level is defined here + --> $DIR/expr-unsafe.rs:4:9 + | +LL | #![warn(unused_unsafe)] + | ^^^^^^^^^^^^^ + +warning: 1 warning emitted + From adf171721952b8793ca50a97210c71deb3034b9b Mon Sep 17 00:00:00 2001 From: Gary Guo Date: Thu, 1 Dec 2022 23:41:45 +0000 Subject: [PATCH 4/7] Ensure valid local_data is set for custom mir building MIR unsafety checking requires this to be valid --- compiler/rustc_mir_build/src/build/custom/mod.rs | 7 ++++++- compiler/rustc_mir_build/src/build/mod.rs | 1 + 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_mir_build/src/build/custom/mod.rs b/compiler/rustc_mir_build/src/build/custom/mod.rs index eb021f47757..1fd2e40c187 100644 --- a/compiler/rustc_mir_build/src/build/custom/mod.rs +++ b/compiler/rustc_mir_build/src/build/custom/mod.rs @@ -20,6 +20,7 @@ use rustc_ast::Attribute; use rustc_data_structures::fx::FxHashMap; use rustc_hir::def_id::DefId; +use rustc_hir::HirId; use rustc_index::vec::IndexVec; use rustc_middle::{ mir::*, @@ -33,6 +34,7 @@ pub(super) fn build_custom_mir<'tcx>( tcx: TyCtxt<'tcx>, did: DefId, + hir_id: HirId, thir: &Thir<'tcx>, expr: ExprId, params: &IndexVec>, @@ -67,7 +69,10 @@ pub(super) fn build_custom_mir<'tcx>( parent_scope: None, inlined: None, inlined_parent_scope: None, - local_data: ClearCrossCrate::Clear, + local_data: ClearCrossCrate::Set(SourceScopeLocalData { + lint_root: hir_id, + safety: Safety::Safe, + }), }); body.injection_phase = Some(parse_attribute(attr)); diff --git a/compiler/rustc_mir_build/src/build/mod.rs b/compiler/rustc_mir_build/src/build/mod.rs index 007f3b55ec8..7af89dd472f 100644 --- a/compiler/rustc_mir_build/src/build/mod.rs +++ b/compiler/rustc_mir_build/src/build/mod.rs @@ -487,6 +487,7 @@ fn construct_fn<'tcx>( return custom::build_custom_mir( tcx, fn_def.did.to_def_id(), + fn_id, thir, expr, arguments, From 5c58a1b0031d57d199376c9ed761bad7ed9e3396 Mon Sep 17 00:00:00 2001 From: Gary Guo Date: Thu, 1 Dec 2022 23:51:03 +0000 Subject: [PATCH 5/7] Remove unnecessary recursive call to parent unsafeck All bodies are unsafe checked anyway. Current MIR unsafeck also just returns for closures. --- compiler/rustc_mir_build/src/check_unsafety.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/compiler/rustc_mir_build/src/check_unsafety.rs b/compiler/rustc_mir_build/src/check_unsafety.rs index dc7f37c2566..bd9535eedda 100644 --- a/compiler/rustc_mir_build/src/check_unsafety.rs +++ b/compiler/rustc_mir_build/src/check_unsafety.rs @@ -627,9 +627,6 @@ pub fn check_unsafety<'tcx>(tcx: TyCtxt<'tcx>, def: ty::WithOptConstParam Date: Thu, 1 Dec 2022 23:54:06 +0000 Subject: [PATCH 6/7] Add tests (currently broken) for unsafe + inline const pat --- src/test/ui/inline-const/pat-unsafe-err.rs | 17 +++++++++++++++++ src/test/ui/inline-const/pat-unsafe.rs | 22 ++++++++++++++++++++++ 2 files changed, 39 insertions(+) create mode 100644 src/test/ui/inline-const/pat-unsafe-err.rs create mode 100644 src/test/ui/inline-const/pat-unsafe.rs diff --git a/src/test/ui/inline-const/pat-unsafe-err.rs b/src/test/ui/inline-const/pat-unsafe-err.rs new file mode 100644 index 00000000000..e290b438c51 --- /dev/null +++ b/src/test/ui/inline-const/pat-unsafe-err.rs @@ -0,0 +1,17 @@ +// ignore-test This is currently broken +// revisions: mir thir +// [thir]compile-flags: -Z thir-unsafeck + +#![allow(incomplete_features)] +#![feature(inline_const_pat)] + +const unsafe fn require_unsafe() -> usize { 1 } + +fn main() { + match () { + const { + require_unsafe(); + //~^ ERROR [E0133] + } => (), + } +} diff --git a/src/test/ui/inline-const/pat-unsafe.rs b/src/test/ui/inline-const/pat-unsafe.rs new file mode 100644 index 00000000000..bcf7f6e0180 --- /dev/null +++ b/src/test/ui/inline-const/pat-unsafe.rs @@ -0,0 +1,22 @@ +// ignore-test This is currently broken +// check-pass +// revisions: mir thir +// [thir]compile-flags: -Z thir-unsafeck + +#![allow(incomplete_features)] +#![warn(unused_unsafe)] +#![feature(inline_const_pat)] + +const unsafe fn require_unsafe() -> usize { 1 } + +fn main() { + unsafe { + match () { + const { + require_unsafe(); + unsafe {} + //~^ WARNING unnecessary `unsafe` block + } => (), + } + } +} From d6dc9124b7b814c2c5c35aa6295ea5e54071daac Mon Sep 17 00:00:00 2001 From: Gary Guo Date: Tue, 13 Dec 2022 02:34:43 +0000 Subject: [PATCH 7/7] Extract shared logic into a new function --- .../rustc_mir_build/src/check_unsafety.rs | 34 ++++++++----------- 1 file changed, 14 insertions(+), 20 deletions(-) diff --git a/compiler/rustc_mir_build/src/check_unsafety.rs b/compiler/rustc_mir_build/src/check_unsafety.rs index bd9535eedda..3bb1f51650a 100644 --- a/compiler/rustc_mir_build/src/check_unsafety.rs +++ b/compiler/rustc_mir_build/src/check_unsafety.rs @@ -132,6 +132,18 @@ fn warn_unused_unsafe( fn unsafe_op_in_unsafe_fn_allowed(&self) -> bool { self.tcx.lint_level_at_node(UNSAFE_OP_IN_UNSAFE_FN, self.hir_context).0 == Level::Allow } + + /// Handle closures/generators/inline-consts, which is unsafecked with their parent body. + fn visit_inner_body(&mut self, def: ty::WithOptConstParam) { + if let Ok((inner_thir, expr)) = self.tcx.thir_body(def) { + let inner_thir = &inner_thir.borrow(); + let hir_context = self.tcx.hir().local_def_id_to_hir_id(def.did); + let mut inner_visitor = UnsafetyVisitor { thir: inner_thir, hir_context, ..*self }; + inner_visitor.visit_expr(&inner_thir[expr]); + // Unsafe blocks can be used in the inner body, make sure to take it into account + self.safety_context = inner_visitor.safety_context; + } + } } // Searches for accesses to layout constrained fields. @@ -408,29 +420,11 @@ fn visit_expr(&mut self, expr: &Expr<'tcx>) { } else { ty::WithOptConstParam::unknown(closure_id) }; - if let Ok((closure_thir, expr)) = self.tcx.thir_body(closure_def) { - let closure_thir = &closure_thir.borrow(); - let hir_context = self.tcx.hir().local_def_id_to_hir_id(closure_id); - let mut closure_visitor = - UnsafetyVisitor { thir: closure_thir, hir_context, ..*self }; - closure_visitor.visit_expr(&closure_thir[expr]); - // Unsafe blocks can be used in closures, make sure to take it into account - self.safety_context = closure_visitor.safety_context; - } + self.visit_inner_body(closure_def); } ExprKind::ConstBlock { did, substs: _ } => { let def_id = did.expect_local(); - if let Ok((inner_thir, expr)) = - self.tcx.thir_body(ty::WithOptConstParam::unknown(def_id)) - { - let inner_thir = &inner_thir.borrow(); - let hir_context = self.tcx.hir().local_def_id_to_hir_id(def_id); - let mut inner_visitor = - UnsafetyVisitor { thir: inner_thir, hir_context, ..*self }; - inner_visitor.visit_expr(&inner_thir[expr]); - // Unsafe blocks can be used in inline consts, make sure to take it into account - self.safety_context = inner_visitor.safety_context; - } + self.visit_inner_body(ty::WithOptConstParam::unknown(def_id)); } ExprKind::Field { lhs, .. } => { let lhs = &self.thir[lhs];