Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Properly check for builtin derived code #108128

Merged
merged 1 commit into from
Feb 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions compiler/rustc_builtin_macros/src/deriving/generic/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -761,8 +761,7 @@ impl<'a> TraitDef<'a> {
let path = cx.path_all(self.span, false, vec![type_ident], self_params);
let self_type = cx.ty_path(path);

let attr = cx.attr_word(sym::automatically_derived, self.span);
let attrs = thin_vec![attr];
let attrs = thin_vec![cx.attr_word(sym::automatically_derived, self.span),];
let opt_trait_ref = Some(trait_ref);

cx.item(
Expand Down
21 changes: 18 additions & 3 deletions compiler/rustc_middle/src/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ use rustc_serialize::{Decodable, Encodable};
use rustc_session::cstore::Untracked;
use rustc_span::hygiene::MacroKind;
use rustc_span::symbol::{kw, sym, Ident, Symbol};
use rustc_span::{ExpnId, Span};
use rustc_span::{ExpnId, ExpnKind, Span};
use rustc_target::abi::{Align, Integer, IntegerType, VariantIdx};
pub use rustc_target::abi::{ReprFlags, ReprOptions};
use rustc_type_ir::WithCachedTypeInfo;
Expand Down Expand Up @@ -2436,8 +2436,23 @@ impl<'tcx> TyCtxt<'tcx> {
None
}

/// If the given `DefId` belongs to a trait that was automatically derived, returns `true`.
pub fn is_builtin_derive(self, def_id: DefId) -> bool {
/// Check if the given `DefId` is `#\[automatically_derived\], *and*
/// whether it was produced by expanding a builtin derive macro.
pub fn is_builtin_derived(self, def_id: DefId) -> bool {
if self.is_automatically_derived(def_id)
&& let Some(def_id) = def_id.as_local()
&& let outer = self.def_span(def_id).ctxt().outer_expn_data()
&& matches!(outer.kind, ExpnKind::Macro(MacroKind::Derive, _))
&& self.has_attr(outer.macro_def_id.unwrap(), sym::rustc_builtin_macro)
{
true
} else {
false
}
}

/// Check if the given `DefId` is `#\[automatically_derived\]`.
pub fn is_automatically_derived(self, def_id: DefId) -> bool {
self.has_attr(def_id, sym::automatically_derived)
}

Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_mir_transform/src/check_packed_ref.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,12 +42,12 @@ impl<'tcx> Visitor<'tcx> for PackedRefChecker<'_, 'tcx> {
if util::is_disaligned(self.tcx, self.body, self.param_env, *place) {
let def_id = self.body.source.instance.def_id();
if let Some(impl_def_id) = self.tcx.impl_of_method(def_id)
&& self.tcx.is_builtin_derive(impl_def_id)
&& self.tcx.is_builtin_derived(impl_def_id)
{
// If we ever reach here it means that the generated derive
// code is somehow doing an unaligned reference, which it
// shouldn't do.
unreachable!();
span_bug!(self.source_info.span, "builtin derive created an unaligned reference");
} else {
struct_span_err!(
self.tcx.sess,
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_passes/src/dead.rs
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ impl<'tcx> MarkSymbolVisitor<'tcx> {
/// for discussion).
fn should_ignore_item(&mut self, def_id: DefId) -> bool {
if let Some(impl_of) = self.tcx.impl_of_method(def_id) {
if !self.tcx.has_attr(impl_of, sym::automatically_derived) {
if !self.tcx.is_automatically_derived(impl_of) {
return false;
}

Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_passes/src/stability.rs
Original file line number Diff line number Diff line change
Expand Up @@ -537,7 +537,7 @@ impl<'tcx> MissingStabilityAnnotations<'tcx> {
// then it would be "stable" at least for the impl.
// We gate usages of it using `feature(const_trait_impl)` anyways
// so there is no unstable leakage
if self.tcx.is_builtin_derive(def_id.to_def_id()) {
if self.tcx.is_automatically_derived(def_id.to_def_id()) {
return;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2097,7 +2097,7 @@ impl<'tcx> InferCtxtPrivExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
// Ignore automatically derived impls and `!Trait` impls.
.filter(|&def_id| {
self.tcx.impl_polarity(def_id) != ty::ImplPolarity::Negative
|| self.tcx.is_builtin_derive(def_id)
|| self.tcx.is_automatically_derived(def_id)
})
.filter_map(|def_id| self.tcx.impl_trait_ref(def_id))
.map(ty::EarlyBinder::subst_identity)
Expand Down
31 changes: 17 additions & 14 deletions tests/ui/lint/unaligned_references.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,20 @@ pub struct Packed2 {
z: u8,
}

trait Foo {
fn evil(&self);
}

// Test for #108122
#[automatically_derived]
impl Foo for Packed2 {
fn evil(&self) {
unsafe {
&self.x; //~ ERROR reference to packed field
}
}
}

fn main() {
unsafe {
let good = Good { data: 0, ptr: &0, data2: [0, 0], aligned: [0; 32] };
Expand All @@ -37,6 +51,7 @@ fn main() {
let _ = &packed2.x; //~ ERROR reference to packed field
let _ = &packed2.y; // ok, has align 2 in packed(2) struct
let _ = &packed2.z; // ok, has align 1
packed2.evil();
}

unsafe {
Expand Down Expand Up @@ -71,22 +86,10 @@ fn main() {
#[repr(packed)]
struct Misalign<T>(u8, T);

let m1 = Misalign(
0,
Wrapper {
a: U16(10),
b: HasDrop,
},
);
let m1 = Misalign(0, Wrapper { a: U16(10), b: HasDrop });
let _ref = &m1.1.a; //~ ERROR reference to packed field

let m2 = Misalign(
0,
Wrapper2 {
a: U16(10),
b: HasDrop,
},
);
let m2 = Misalign(0, Wrapper2 { a: U16(10), b: HasDrop });
let _ref = &m2.1.a; //~ ERROR reference to packed field
}
}
29 changes: 19 additions & 10 deletions tests/ui/lint/unaligned_references.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,14 @@
error[E0793]: reference to packed field is unaligned
--> $DIR/unaligned_references.rs:20:17
--> $DIR/unaligned_references.rs:25:13
|
LL | &self.x;
| ^^^^^^^
|
= note: fields of packed structs are not properly aligned, and creating a misaligned reference is undefined behavior (even if that reference is never dereferenced)
= help: copy the field contents to a local variable, or replace the reference with a raw pointer and use `read_unaligned`/`write_unaligned` (loads and stores via `*p` must be properly aligned even when using raw pointers)

error[E0793]: reference to packed field is unaligned
--> $DIR/unaligned_references.rs:34:17
|
LL | let _ = &good.ptr;
| ^^^^^^^^^
Expand All @@ -8,7 +17,7 @@ LL | let _ = &good.ptr;
= help: copy the field contents to a local variable, or replace the reference with a raw pointer and use `read_unaligned`/`write_unaligned` (loads and stores via `*p` must be properly aligned even when using raw pointers)

error[E0793]: reference to packed field is unaligned
--> $DIR/unaligned_references.rs:21:17
--> $DIR/unaligned_references.rs:35:17
|
LL | let _ = &good.data;
| ^^^^^^^^^^
Expand All @@ -17,7 +26,7 @@ LL | let _ = &good.data;
= help: copy the field contents to a local variable, or replace the reference with a raw pointer and use `read_unaligned`/`write_unaligned` (loads and stores via `*p` must be properly aligned even when using raw pointers)

error[E0793]: reference to packed field is unaligned
--> $DIR/unaligned_references.rs:23:17
--> $DIR/unaligned_references.rs:37:17
|
LL | let _ = &good.data as *const _;
| ^^^^^^^^^^
Expand All @@ -26,7 +35,7 @@ LL | let _ = &good.data as *const _;
= help: copy the field contents to a local variable, or replace the reference with a raw pointer and use `read_unaligned`/`write_unaligned` (loads and stores via `*p` must be properly aligned even when using raw pointers)

error[E0793]: reference to packed field is unaligned
--> $DIR/unaligned_references.rs:24:27
--> $DIR/unaligned_references.rs:38:27
|
LL | let _: *const _ = &good.data;
| ^^^^^^^^^^
Expand All @@ -35,7 +44,7 @@ LL | let _: *const _ = &good.data;
= help: copy the field contents to a local variable, or replace the reference with a raw pointer and use `read_unaligned`/`write_unaligned` (loads and stores via `*p` must be properly aligned even when using raw pointers)

error[E0793]: reference to packed field is unaligned
--> $DIR/unaligned_references.rs:26:17
--> $DIR/unaligned_references.rs:40:17
|
LL | let _ = good.data.clone();
| ^^^^^^^^^^^^^^^^^
Expand All @@ -44,7 +53,7 @@ LL | let _ = good.data.clone();
= help: copy the field contents to a local variable, or replace the reference with a raw pointer and use `read_unaligned`/`write_unaligned` (loads and stores via `*p` must be properly aligned even when using raw pointers)

error[E0793]: reference to packed field is unaligned
--> $DIR/unaligned_references.rs:28:17
--> $DIR/unaligned_references.rs:42:17
|
LL | let _ = &good.data2[0];
| ^^^^^^^^^^^^^^
Expand All @@ -53,7 +62,7 @@ LL | let _ = &good.data2[0];
= help: copy the field contents to a local variable, or replace the reference with a raw pointer and use `read_unaligned`/`write_unaligned` (loads and stores via `*p` must be properly aligned even when using raw pointers)

error[E0793]: reference to packed field is unaligned
--> $DIR/unaligned_references.rs:37:17
--> $DIR/unaligned_references.rs:51:17
|
LL | let _ = &packed2.x;
| ^^^^^^^^^^
Expand All @@ -62,7 +71,7 @@ LL | let _ = &packed2.x;
= help: copy the field contents to a local variable, or replace the reference with a raw pointer and use `read_unaligned`/`write_unaligned` (loads and stores via `*p` must be properly aligned even when using raw pointers)

error[E0793]: reference to packed field is unaligned
--> $DIR/unaligned_references.rs:81:20
--> $DIR/unaligned_references.rs:90:20
|
LL | let _ref = &m1.1.a;
| ^^^^^^^
Expand All @@ -71,14 +80,14 @@ LL | let _ref = &m1.1.a;
= help: copy the field contents to a local variable, or replace the reference with a raw pointer and use `read_unaligned`/`write_unaligned` (loads and stores via `*p` must be properly aligned even when using raw pointers)

error[E0793]: reference to packed field is unaligned
--> $DIR/unaligned_references.rs:90:20
--> $DIR/unaligned_references.rs:93:20
|
LL | let _ref = &m2.1.a;
| ^^^^^^^
|
= note: fields of packed structs are not properly aligned, and creating a misaligned reference is undefined behavior (even if that reference is never dereferenced)
= help: copy the field contents to a local variable, or replace the reference with a raw pointer and use `read_unaligned`/`write_unaligned` (loads and stores via `*p` must be properly aligned even when using raw pointers)

error: aborting due to 9 previous errors
error: aborting due to 10 previous errors

For more information about this error, try `rustc --explain E0793`.