Skip to content

Commit

Permalink
make unaligned_refereces future-incompat lint warn-by-default, and re…
Browse files Browse the repository at this point in the history
…move the safe_packed_borrows lint that it replaces
  • Loading branch information
RalfJung committed Mar 27, 2021
1 parent f811f14 commit fb4f48e
Show file tree
Hide file tree
Showing 25 changed files with 251 additions and 415 deletions.
1 change: 1 addition & 0 deletions compiler/rustc_lint/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
65 changes: 11 additions & 54 deletions compiler/rustc_lint_defs/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
/// }
/// }
/// ```
Expand All @@ -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
}

Expand Down Expand Up @@ -1150,49 +1151,6 @@ declare_lint! {
"detects attempts to mutate a `const` item",
}

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.
Expand Down Expand Up @@ -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,
Expand Down
6 changes: 0 additions & 6 deletions compiler/rustc_middle/src/mir/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down
89 changes: 69 additions & 20 deletions compiler/rustc_mir/src/transform/check_packed_ref.rs
Original file line number Diff line number Diff line change
@@ -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 {
Expand All @@ -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.
Expand All @@ -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()
},
);
}
}
}
}
Expand Down
Loading

0 comments on commit fb4f48e

Please sign in to comment.