diff options
Diffstat (limited to 'compiler')
| -rw-r--r-- | compiler/rustc_lint/src/lib.rs | 1 | ||||
| -rw-r--r-- | compiler/rustc_lint_defs/src/builtin.rs | 65 | ||||
| -rw-r--r-- | compiler/rustc_middle/src/mir/query.rs | 6 | ||||
| -rw-r--r-- | compiler/rustc_mir/src/transform/check_packed_ref.rs | 89 | ||||
| -rw-r--r-- | compiler/rustc_mir/src/transform/check_unsafety.rs | 130 | ||||
| -rw-r--r-- | compiler/rustc_mir/src/transform/mod.rs | 1 |
6 files changed, 89 insertions, 203 deletions
diff --git a/compiler/rustc_lint/src/lib.rs b/compiler/rustc_lint/src/lib.rs index cacdf260548..b547ee89387 100644 --- a/compiler/rustc_lint/src/lib.rs +++ b/compiler/rustc_lint/src/lib.rs @@ -325,6 +325,7 @@ fn register_builtins(store: &mut LintStore, no_interleave_lints: bool) { store.register_renamed("exceeding_bitshifts", "arithmetic_overflow"); store.register_renamed("redundant_semicolon", "redundant_semicolons"); store.register_renamed("overlapping_patterns", "overlapping_range_endpoints"); + store.register_renamed("safe_packed_borrows", "unaligned_references"); // These were moved to tool lints, but rustc still sees them when compiling normally, before // tool lints are registered, so `check_tool_name_for_backwards_compat` doesn't work. Use diff --git a/compiler/rustc_lint_defs/src/builtin.rs b/compiler/rustc_lint_defs/src/builtin.rs index cd4d01ddc05..3baafee4612 100644 --- a/compiler/rustc_lint_defs/src/builtin.rs +++ b/compiler/rustc_lint_defs/src/builtin.rs @@ -1057,6 +1057,7 @@ declare_lint! { /// unsafe { /// let foo = Foo { field1: 0, field2: 0 }; /// let _ = &foo.field1; + /// println!("{}", foo.field1); // An implicit `&` is added here, triggering the lint. /// } /// } /// ``` @@ -1065,20 +1066,20 @@ declare_lint! { /// /// ### Explanation /// - /// Creating a reference to an insufficiently aligned packed field is - /// [undefined behavior] and should be disallowed. - /// - /// This lint is "allow" by default because there is no stable - /// alternative, and it is not yet certain how widespread existing code - /// will trigger this lint. - /// - /// See [issue #27060] for more discussion. + /// Creating a reference to an insufficiently aligned packed field is [undefined behavior] and + /// should be disallowed. Using an `unsafe` block does not change anything about this. Instead, + /// the code should do a copy of the data in the packed field or use raw pointers and unaligned + /// accesses. See [issue #82523] for more information. /// /// [undefined behavior]: https://doc.rust-lang.org/reference/behavior-considered-undefined.html - /// [issue #27060]: https://github.com/rust-lang/rust/issues/27060 + /// [issue #82523]: https://github.com/rust-lang/rust/issues/82523 pub UNALIGNED_REFERENCES, - Allow, + Warn, "detects unaligned references to fields of packed structs", + @future_incompatible = FutureIncompatibleInfo { + reference: "issue #82523 <https://github.com/rust-lang/rust/issues/82523>", + edition: None, + }; report_in_external_macro } @@ -1151,49 +1152,6 @@ declare_lint! { } declare_lint! { - /// The `safe_packed_borrows` lint detects borrowing a field in the - /// interior of a packed structure with alignment other than 1. - /// - /// ### Example - /// - /// ```rust - /// #[repr(packed)] - /// pub struct Unaligned<T>(pub T); - /// - /// pub struct Foo { - /// start: u8, - /// data: Unaligned<u32>, - /// } - /// - /// fn main() { - /// let x = Foo { start: 0, data: Unaligned(1) }; - /// let y = &x.data.0; - /// } - /// ``` - /// - /// {{produces}} - /// - /// ### Explanation - /// - /// This type of borrow is unsafe and can cause errors on some platforms - /// and violates some assumptions made by the compiler. This was - /// previously allowed unintentionally. This is a [future-incompatible] - /// lint to transition this to a hard error in the future. See [issue - /// #46043] for more details, including guidance on how to solve the - /// problem. - /// - /// [issue #46043]: https://github.com/rust-lang/rust/issues/46043 - /// [future-incompatible]: ../index.md#future-incompatible-lints - pub SAFE_PACKED_BORROWS, - Warn, - "safe borrows of fields of packed structs were erroneously allowed", - @future_incompatible = FutureIncompatibleInfo { - reference: "issue #46043 <https://github.com/rust-lang/rust/issues/46043>", - edition: None, - }; -} - -declare_lint! { /// The `patterns_in_fns_without_body` lint detects `mut` identifier /// patterns as a parameter in functions without a body. /// @@ -2953,7 +2911,6 @@ declare_lint_pass! { RENAMED_AND_REMOVED_LINTS, UNALIGNED_REFERENCES, CONST_ITEM_MUTATION, - SAFE_PACKED_BORROWS, PATTERNS_IN_FNS_WITHOUT_BODY, MISSING_FRAGMENT_SPECIFIER, LATE_BOUND_LIFETIME_ARGUMENTS, diff --git a/compiler/rustc_middle/src/mir/query.rs b/compiler/rustc_middle/src/mir/query.rs index bde48018013..ad3baccf154 100644 --- a/compiler/rustc_middle/src/mir/query.rs +++ b/compiler/rustc_middle/src/mir/query.rs @@ -23,15 +23,9 @@ pub enum UnsafetyViolationKind { General, /// Permitted both in `const fn`s and regular `fn`s. GeneralAndConstFn, - /// Borrow of packed field. - /// Has to be handled as a lint for backwards compatibility. - BorrowPacked, /// Unsafe operation in an `unsafe fn` but outside an `unsafe` block. /// Has to be handled as a lint for backwards compatibility. UnsafeFn, - /// Borrow of packed field in an `unsafe fn` but outside an `unsafe` block. - /// Has to be handled as a lint for backwards compatibility. - UnsafeFnBorrowPacked, } #[derive(Copy, Clone, PartialEq, TyEncodable, TyDecodable, HashStable, Debug)] diff --git a/compiler/rustc_mir/src/transform/check_packed_ref.rs b/compiler/rustc_mir/src/transform/check_packed_ref.rs index ee88daa83e7..13b7221046b 100644 --- a/compiler/rustc_mir/src/transform/check_packed_ref.rs +++ b/compiler/rustc_mir/src/transform/check_packed_ref.rs @@ -1,11 +1,18 @@ +use rustc_hir::def_id::{DefId, LocalDefId}; use rustc_middle::mir::visit::{PlaceContext, Visitor}; use rustc_middle::mir::*; +use rustc_middle::ty::query::Providers; use rustc_middle::ty::{self, TyCtxt}; use rustc_session::lint::builtin::UNALIGNED_REFERENCES; +use rustc_span::symbol::sym; use crate::transform::MirPass; use crate::util; +pub(crate) fn provide(providers: &mut Providers) { + *providers = Providers { unsafe_derive_on_repr_packed, ..*providers }; +} + pub struct CheckPackedRef; impl<'tcx> MirPass<'tcx> for CheckPackedRef { @@ -24,6 +31,41 @@ struct PackedRefChecker<'a, 'tcx> { source_info: SourceInfo, } +fn unsafe_derive_on_repr_packed(tcx: TyCtxt<'_>, def_id: LocalDefId) { + let lint_hir_id = tcx.hir().local_def_id_to_hir_id(def_id); + + tcx.struct_span_lint_hir(UNALIGNED_REFERENCES, lint_hir_id, tcx.def_span(def_id), |lint| { + // FIXME: when we make this a hard error, this should have its + // own error code. + let message = if tcx.generics_of(def_id).own_requires_monomorphization() { + "`#[derive]` can't be used on a `#[repr(packed)]` struct with \ + type or const parameters (error E0133)" + .to_string() + } else { + "`#[derive]` can't be used on a `#[repr(packed)]` struct that \ + does not derive Copy (error E0133)" + .to_string() + }; + lint.build(&message).emit() + }); +} + +fn builtin_derive_def_id(tcx: TyCtxt<'_>, def_id: DefId) -> Option<DefId> { + debug!("builtin_derive_def_id({:?})", def_id); + if let Some(impl_def_id) = tcx.impl_of_method(def_id) { + if tcx.has_attr(impl_def_id, sym::automatically_derived) { + debug!("builtin_derive_def_id({:?}) - is {:?}", def_id, impl_def_id); + Some(impl_def_id) + } else { + debug!("builtin_derive_def_id({:?}) - not automatically derived", def_id); + None + } + } else { + debug!("builtin_derive_def_id({:?}) - not a method", def_id); + None + } +} + impl<'a, 'tcx> Visitor<'tcx> for PackedRefChecker<'a, 'tcx> { fn visit_terminator(&mut self, terminator: &Terminator<'tcx>, location: Location) { // Make sure we know where in the MIR we are. @@ -40,26 +82,33 @@ impl<'a, 'tcx> Visitor<'tcx> for PackedRefChecker<'a, 'tcx> { fn visit_place(&mut self, place: &Place<'tcx>, context: PlaceContext, _location: Location) { if context.is_borrow() { if util::is_disaligned(self.tcx, self.body, self.param_env, *place) { - let source_info = self.source_info; - let lint_root = self.body.source_scopes[source_info.scope] - .local_data - .as_ref() - .assert_crate_local() - .lint_root; - self.tcx.struct_span_lint_hir( - UNALIGNED_REFERENCES, - lint_root, - source_info.span, - |lint| { - lint.build("reference to packed field is unaligned") - .note( - "fields of packed structs are not properly aligned, and creating \ - a misaligned reference is undefined behavior (even if that \ - reference is never dereferenced)", - ) - .emit() - }, - ); + let def_id = self.body.source.instance.def_id(); + if let Some(impl_def_id) = builtin_derive_def_id(self.tcx, def_id) { + // If a method is defined in the local crate, + // the impl containing that method should also be. + self.tcx.ensure().unsafe_derive_on_repr_packed(impl_def_id.expect_local()); + } else { + let source_info = self.source_info; + let lint_root = self.body.source_scopes[source_info.scope] + .local_data + .as_ref() + .assert_crate_local() + .lint_root; + self.tcx.struct_span_lint_hir( + UNALIGNED_REFERENCES, + lint_root, + source_info.span, + |lint| { + lint.build("reference to packed field is unaligned") + .note( + "fields of packed structs are not properly aligned, and creating \ + a misaligned reference is undefined behavior (even if that \ + reference is never dereferenced)", + ) + .emit() + }, + ); + } } } } diff --git a/compiler/rustc_mir/src/transform/check_unsafety.rs b/compiler/rustc_mir/src/transform/check_unsafety.rs index 532d201e056..09da9b2e4d6 100644 --- a/compiler/rustc_mir/src/transform/check_unsafety.rs +++ b/compiler/rustc_mir/src/transform/check_unsafety.rs @@ -10,14 +10,12 @@ use rustc_middle::mir::*; use rustc_middle::ty::cast::CastTy; use rustc_middle::ty::query::Providers; use rustc_middle::ty::{self, TyCtxt}; -use rustc_session::lint::builtin::{SAFE_PACKED_BORROWS, UNSAFE_OP_IN_UNSAFE_FN, UNUSED_UNSAFE}; +use rustc_session::lint::builtin::{UNSAFE_OP_IN_UNSAFE_FN, UNUSED_UNSAFE}; use rustc_session::lint::Level; -use rustc_span::symbol::sym; use std::ops::Bound; use crate::const_eval::is_min_const_fn; -use crate::util; pub struct UnsafetyChecker<'a, 'tcx> { body: &'a Body<'tcx>, @@ -182,18 +180,6 @@ impl<'a, 'tcx> Visitor<'tcx> for UnsafetyChecker<'a, 'tcx> { self.check_mut_borrowing_layout_constrained_field(*place, context.is_mutating_use()); } - // Check for borrows to packed fields. - // `is_disaligned` already traverses the place to consider all projections after the last - // `Deref`, so this only needs to be called once at the top level. - if context.is_borrow() { - if util::is_disaligned(self.tcx, self.body, self.param_env, *place) { - self.require_unsafe( - UnsafetyViolationKind::BorrowPacked, - UnsafetyViolationDetails::BorrowOfPackedField, - ); - } - } - // Some checks below need the extra metainfo of the local declaration. let decl = &self.body.local_decls[place.local]; @@ -317,25 +303,15 @@ impl<'a, 'tcx> UnsafetyChecker<'a, 'tcx> { // `unsafe` blocks are required in safe code Safety::Safe => { for violation in violations { - let mut violation = *violation; match violation.kind { UnsafetyViolationKind::GeneralAndConstFn | UnsafetyViolationKind::General => {} - UnsafetyViolationKind::BorrowPacked => { - if self.min_const_fn { - // const fns don't need to be backwards compatible and can - // emit these violations as a hard error instead of a backwards - // compat lint - violation.kind = UnsafetyViolationKind::General; - } - } - UnsafetyViolationKind::UnsafeFn - | UnsafetyViolationKind::UnsafeFnBorrowPacked => { + UnsafetyViolationKind::UnsafeFn => { bug!("`UnsafetyViolationKind::UnsafeFn` in an `Safe` context") } } - if !self.violations.contains(&violation) { - self.violations.push(violation) + if !self.violations.contains(violation) { + self.violations.push(*violation) } } false @@ -345,11 +321,7 @@ impl<'a, 'tcx> UnsafetyChecker<'a, 'tcx> { for violation in violations { let mut violation = *violation; - if violation.kind == UnsafetyViolationKind::BorrowPacked { - violation.kind = UnsafetyViolationKind::UnsafeFnBorrowPacked; - } else { - violation.kind = UnsafetyViolationKind::UnsafeFn; - } + violation.kind = UnsafetyViolationKind::UnsafeFn; if !self.violations.contains(&violation) { self.violations.push(violation) } @@ -369,8 +341,7 @@ impl<'a, 'tcx> UnsafetyChecker<'a, 'tcx> { // these unsafe things are stable in const fn UnsafetyViolationKind::GeneralAndConstFn => {} // these things are forbidden in const fns - UnsafetyViolationKind::General - | UnsafetyViolationKind::BorrowPacked => { + UnsafetyViolationKind::General => { let mut violation = *violation; // const fns don't need to be backwards compatible and can // emit these violations as a hard error instead of a backwards @@ -380,8 +351,7 @@ impl<'a, 'tcx> UnsafetyChecker<'a, 'tcx> { self.violations.push(violation) } } - UnsafetyViolationKind::UnsafeFn - | UnsafetyViolationKind::UnsafeFnBorrowPacked => bug!( + UnsafetyViolationKind::UnsafeFn => bug!( "`UnsafetyViolationKind::UnsafeFn` in an `ExplicitUnsafe` context" ), } @@ -464,7 +434,6 @@ pub(crate) fn provide(providers: &mut Providers) { ty::WithOptConstParam { did, const_param_did: Some(param_did) }, ) }, - unsafe_derive_on_repr_packed, ..*providers }; } @@ -544,25 +513,6 @@ fn unsafety_check_result<'tcx>( }) } -fn unsafe_derive_on_repr_packed(tcx: TyCtxt<'_>, def_id: LocalDefId) { - let lint_hir_id = tcx.hir().local_def_id_to_hir_id(def_id); - - tcx.struct_span_lint_hir(SAFE_PACKED_BORROWS, lint_hir_id, tcx.def_span(def_id), |lint| { - // FIXME: when we make this a hard error, this should have its - // own error code. - let message = if tcx.generics_of(def_id).own_requires_monomorphization() { - "`#[derive]` can't be used on a `#[repr(packed)]` struct with \ - type or const parameters (error E0133)" - .to_string() - } else { - "`#[derive]` can't be used on a `#[repr(packed)]` struct that \ - does not derive Copy (error E0133)" - .to_string() - }; - lint.build(&message).emit() - }); -} - /// Returns the `HirId` for an enclosing scope that is also `unsafe`. fn is_enclosed( tcx: TyCtxt<'_>, @@ -609,22 +559,6 @@ fn report_unused_unsafe(tcx: TyCtxt<'_>, used_unsafe: &FxHashSet<hir::HirId>, id }); } -fn builtin_derive_def_id(tcx: TyCtxt<'_>, def_id: DefId) -> Option<DefId> { - debug!("builtin_derive_def_id({:?})", def_id); - if let Some(impl_def_id) = tcx.impl_of_method(def_id) { - if tcx.has_attr(impl_def_id, sym::automatically_derived) { - debug!("builtin_derive_def_id({:?}) - is {:?}", def_id, impl_def_id); - Some(impl_def_id) - } else { - debug!("builtin_derive_def_id({:?}) - not automatically derived", def_id); - None - } - } else { - debug!("builtin_derive_def_id({:?}) - not a method", def_id); - None - } -} - pub fn check_unsafety(tcx: TyCtxt<'_>, def_id: LocalDefId) { debug!("check_unsafety({:?})", def_id); @@ -657,27 +591,6 @@ pub fn check_unsafety(tcx: TyCtxt<'_>, def_id: LocalDefId) { .note(note) .emit(); } - UnsafetyViolationKind::BorrowPacked => { - if let Some(impl_def_id) = builtin_derive_def_id(tcx, def_id.to_def_id()) { - // If a method is defined in the local crate, - // the impl containing that method should also be. - tcx.ensure().unsafe_derive_on_repr_packed(impl_def_id.expect_local()); - } else { - tcx.struct_span_lint_hir( - SAFE_PACKED_BORROWS, - lint_root, - source_info.span, - |lint| { - lint.build(&format!( - "{} is unsafe and requires unsafe{} block (error E0133)", - description, unsafe_fn_msg, - )) - .note(note) - .emit() - }, - ) - } - } UnsafetyViolationKind::UnsafeFn => tcx.struct_span_lint_hir( UNSAFE_OP_IN_UNSAFE_FN, lint_root, @@ -692,35 +605,6 @@ pub fn check_unsafety(tcx: TyCtxt<'_>, def_id: LocalDefId) { .emit(); }, ), - UnsafetyViolationKind::UnsafeFnBorrowPacked => { - // When `unsafe_op_in_unsafe_fn` is disallowed, the behavior of safe and unsafe functions - // should be the same in terms of warnings and errors. Therefore, with `#[warn(safe_packed_borrows)]`, - // a safe packed borrow should emit a warning *but not an error* in an unsafe function, - // just like in a safe function, even if `unsafe_op_in_unsafe_fn` is `deny`. - // - // Also, `#[warn(unsafe_op_in_unsafe_fn)]` can't cause any new errors. Therefore, with - // `#[deny(safe_packed_borrows)]` and `#[warn(unsafe_op_in_unsafe_fn)]`, a packed borrow - // should only issue a warning for the sake of backwards compatibility. - // - // The solution those 2 expectations is to always take the minimum of both lints. - // This prevent any new errors (unless both lints are explicitly set to `deny`). - let lint = if tcx.lint_level_at_node(SAFE_PACKED_BORROWS, lint_root).0 - <= tcx.lint_level_at_node(UNSAFE_OP_IN_UNSAFE_FN, lint_root).0 - { - SAFE_PACKED_BORROWS - } else { - UNSAFE_OP_IN_UNSAFE_FN - }; - tcx.struct_span_lint_hir(&lint, lint_root, source_info.span, |lint| { - lint.build(&format!( - "{} is unsafe and requires unsafe block (error E0133)", - description, - )) - .span_label(source_info.span, description) - .note(note) - .emit(); - }) - } } } diff --git a/compiler/rustc_mir/src/transform/mod.rs b/compiler/rustc_mir/src/transform/mod.rs index 66d2ab8a3f1..5c49ee69edc 100644 --- a/compiler/rustc_mir/src/transform/mod.rs +++ b/compiler/rustc_mir/src/transform/mod.rs @@ -59,6 +59,7 @@ pub use rustc_middle::mir::MirSource; pub(crate) fn provide(providers: &mut Providers) { self::check_unsafety::provide(providers); + self::check_packed_ref::provide(providers); *providers = Providers { mir_keys, mir_const, |