diff --git a/compiler/rustc_middle/src/mir/mod.rs b/compiler/rustc_middle/src/mir/mod.rs index 4a7d4dab5079..2bd30001a915 100644 --- a/compiler/rustc_middle/src/mir/mod.rs +++ b/compiler/rustc_middle/src/mir/mod.rs @@ -1744,7 +1744,9 @@ impl<'tcx> Place<'tcx> { /// Iterate over the projections in evaluation order, i.e., the first element is the base with /// its projection and then subsequently more projections are added. - pub fn iter_projections(self) -> impl Iterator, PlaceElem<'tcx>)> + DoubleEndedIterator { + pub fn iter_projections( + self, + ) -> impl Iterator, PlaceElem<'tcx>)> + DoubleEndedIterator { self.projection.iter().enumerate().map(move |(i, proj)| { let base = PlaceRef { local: self.local, projection: &self.projection[..i] }; (base, proj) diff --git a/compiler/rustc_mir/src/transform/check_unsafety.rs b/compiler/rustc_mir/src/transform/check_unsafety.rs index e90d5149c2f1..e64955c4986c 100644 --- a/compiler/rustc_mir/src/transform/check_unsafety.rs +++ b/compiler/rustc_mir/src/transform/check_unsafety.rs @@ -181,6 +181,9 @@ 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( @@ -190,53 +193,69 @@ impl<'a, 'tcx> Visitor<'tcx> for UnsafetyChecker<'a, 'tcx> { } } - for (i, _elem) in place.projection.iter().enumerate() { - let proj_base = &place.projection[..i]; - if context.is_borrow() { - if util::is_disaligned(self.tcx, self.body, self.param_env, *place) { + // Some checks below need the extra metainfo of the local declaration. + let decl = &self.body.local_decls[place.local]; + + // Check the base local: it might be an unsafe-to-access static. We only check derefs of the + // temporary holding the static pointer to avoid duplicate errors + // . + if decl.internal && place.projection.first() == Some(&ProjectionElem::Deref) { + // If the projection root is an artifical local that we introduced when + // desugaring `static`, give a more specific error message + // (avoid the general "raw pointer" clause below, that would only be confusing). + if let Some(box LocalInfo::StaticRef { def_id, .. }) = decl.local_info { + if self.tcx.is_mutable_static(def_id) { self.require_unsafe( - UnsafetyViolationKind::BorrowPacked, - UnsafetyViolationDetails::BorrowOfPackedField, + UnsafetyViolationKind::General, + UnsafetyViolationDetails::UseOfMutableStatic, ); + return; + } else if self.tcx.is_foreign_item(def_id) { + self.require_unsafe( + UnsafetyViolationKind::General, + UnsafetyViolationDetails::UseOfExternStatic, + ); + return; } } - let source_info = self.source_info; - if let [] = proj_base { - let decl = &self.body.local_decls[place.local]; - if decl.internal { - // If the projection root is an artifical local that we introduced when - // desugaring `static`, give a more specific error message - // (avoid the general "raw pointer" clause below, that would only be confusing). - if let Some(box LocalInfo::StaticRef { def_id, .. }) = decl.local_info { - if self.tcx.is_mutable_static(def_id) { - self.require_unsafe( - UnsafetyViolationKind::General, - UnsafetyViolationDetails::UseOfMutableStatic, - ); - return; - } else if self.tcx.is_foreign_item(def_id) { - self.require_unsafe( - UnsafetyViolationKind::General, - UnsafetyViolationDetails::UseOfExternStatic, - ); - return; - } - } else { - // Internal locals are used in the `move_val_init` desugaring. - // We want to check unsafety against the source info of the - // desugaring, rather than the source info of the RHS. - self.source_info = self.body.local_decls[place.local].source_info; - } + } + + // Check for raw pointer `Deref`. + for (base, proj) in place.iter_projections() { + if proj == ProjectionElem::Deref { + let source_info = self.source_info; // Backup source_info so we can restore it later. + if base.projection.is_empty() && decl.internal { + // Internal locals are used in the `move_val_init` desugaring. + // We want to check unsafety against the source info of the + // desugaring, rather than the source info of the RHS. + self.source_info = self.body.local_decls[place.local].source_info; } + let base_ty = base.ty(self.body, self.tcx).ty; + if base_ty.is_unsafe_ptr() { + self.require_unsafe( + UnsafetyViolationKind::GeneralAndConstFn, + UnsafetyViolationDetails::DerefOfRawPointer, + ) + } + self.source_info = source_info; // Restore backed-up source_info. } - let base_ty = Place::ty_from(place.local, proj_base, self.body, self.tcx).ty; - match base_ty.kind() { - ty::RawPtr(..) => self.require_unsafe( - UnsafetyViolationKind::GeneralAndConstFn, - UnsafetyViolationDetails::DerefOfRawPointer, - ), - ty::Adt(adt, _) if adt.is_union() => { - let assign_to_field = matches!( + } + + // Check for union fields. For this we traverse right-to-left, as the last `Deref` changes + // whether we *read* the union field or potentially *write* to it (if this place is being assigned to). + let mut saw_deref = false; + for (base, proj) in place.iter_projections().rev() { + if proj == ProjectionElem::Deref { + saw_deref = true; + continue; + } + + let base_ty = base.ty(self.body, self.tcx).ty; + if base_ty.ty_adt_def().map_or(false, |adt| adt.is_union()) { + // If we did not hit a `Deref` yet and the overall place use is an assignment, the + // rules are different. + let assign_to_field = !saw_deref + && matches!( context, PlaceContext::MutatingUse( MutatingUseContext::Store @@ -244,45 +263,35 @@ impl<'a, 'tcx> Visitor<'tcx> for UnsafetyChecker<'a, 'tcx> { | MutatingUseContext::AsmOutput ) ); - // If there is a `Deref` further along the projection chain, this is *not* an - // assignment to a union field. In that case the union field is just read to - // obtain the pointer/reference. - let assign_to_field = assign_to_field - && !place.projection[i..] - .iter() - .any(|elem| matches!(elem, ProjectionElem::Deref)); - // If this is just an assignment, determine if the assigned type needs dropping. - if assign_to_field { - // We have to check the actual type of the assignment, as that determines if the - // old value is being dropped. - let assigned_ty = place.ty(&self.body.local_decls, self.tcx).ty; - // To avoid semver hazard, we only consider `Copy` and `ManuallyDrop` non-dropping. - let manually_drop = assigned_ty - .ty_adt_def() - .map_or(false, |adt_def| adt_def.is_manually_drop()); - let nodrop = manually_drop - || assigned_ty.is_copy_modulo_regions( - self.tcx.at(self.source_info.span), - self.param_env, - ); - if !nodrop { - self.require_unsafe( - UnsafetyViolationKind::GeneralAndConstFn, - UnsafetyViolationDetails::AssignToDroppingUnionField, - ); - } else { - // write to non-drop union field, safe - } - } else { + // If this is just an assignment, determine if the assigned type needs dropping. + if assign_to_field { + // We have to check the actual type of the assignment, as that determines if the + // old value is being dropped. + let assigned_ty = place.ty(&self.body.local_decls, self.tcx).ty; + // To avoid semver hazard, we only consider `Copy` and `ManuallyDrop` non-dropping. + let manually_drop = assigned_ty + .ty_adt_def() + .map_or(false, |adt_def| adt_def.is_manually_drop()); + let nodrop = manually_drop + || assigned_ty.is_copy_modulo_regions( + self.tcx.at(self.source_info.span), + self.param_env, + ); + if !nodrop { self.require_unsafe( UnsafetyViolationKind::GeneralAndConstFn, - UnsafetyViolationDetails::AccessToUnionField, - ) + UnsafetyViolationDetails::AssignToDroppingUnionField, + ); + } else { + // write to non-drop union field, safe } + } else { + self.require_unsafe( + UnsafetyViolationKind::GeneralAndConstFn, + UnsafetyViolationDetails::AccessToUnionField, + ) } - _ => {} } - self.source_info = source_info; } } }