From b27e649537770ba6631f3347974e3ae7e082adfe Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 16 May 2020 16:17:07 +0200 Subject: [PATCH 1/4] add a lint against references to packed fields --- .../transform/check_packed_ref.rs | 66 +++++++++++++++++++ src/librustc_mir/transform/mod.rs | 6 +- src/librustc_session/lint/builtin.rs | 9 ++- src/test/ui/lint/packed_reference.rs | 25 +++++++ src/test/ui/lint/packed_reference.stderr | 23 +++++++ 5 files changed, 127 insertions(+), 2 deletions(-) create mode 100644 src/librustc_mir/transform/check_packed_ref.rs create mode 100644 src/test/ui/lint/packed_reference.rs create mode 100644 src/test/ui/lint/packed_reference.stderr diff --git a/src/librustc_mir/transform/check_packed_ref.rs b/src/librustc_mir/transform/check_packed_ref.rs new file mode 100644 index 00000000000..9e07a4599f6 --- /dev/null +++ b/src/librustc_mir/transform/check_packed_ref.rs @@ -0,0 +1,66 @@ +use rustc_middle::mir::visit::{PlaceContext, Visitor}; +use rustc_middle::mir::*; +use rustc_middle::ty::{self, TyCtxt}; +use rustc_session::lint::builtin::PACKED_REFERENCES; + +use crate::transform::{MirPass, MirSource}; +use crate::util; + +pub struct CheckPackedRef; + +impl<'tcx> MirPass<'tcx> for CheckPackedRef { + fn run_pass(&self, tcx: TyCtxt<'tcx>, src: MirSource<'tcx>, body: &mut Body<'tcx>) { + let param_env = tcx.param_env(src.instance.def_id()); + let source_info = SourceInfo::outermost(body.span); + let mut checker = PackedRefChecker { body, tcx, param_env, source_info }; + checker.visit_body(&body); + } +} + +struct PackedRefChecker<'a, 'tcx> { + body: &'a Body<'tcx>, + tcx: TyCtxt<'tcx>, + param_env: ty::ParamEnv<'tcx>, + source_info: SourceInfo, +} + +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. + self.source_info = terminator.source_info; + self.super_terminator(terminator, location); + } + + fn visit_statement(&mut self, statement: &Statement<'tcx>, location: Location) { + // Make sure we know where in the MIR we are. + self.source_info = statement.source_info; + self.super_statement(statement, location); + } + + 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( + PACKED_REFERENCES, + lint_root, + source_info.span, + |lint| { + lint.build(&format!("reference to packed field is not allowed",)) + .note( + "fields of packed structs might be misaligned, and creating \ + a misaligned reference is undefined behavior (even if that \ + reference is never dereferenced)", + ) + .emit() + }, + ); + } + } + } +} diff --git a/src/librustc_mir/transform/mod.rs b/src/librustc_mir/transform/mod.rs index 0551ed5a15d..7d2f8903624 100644 --- a/src/librustc_mir/transform/mod.rs +++ b/src/librustc_mir/transform/mod.rs @@ -17,6 +17,7 @@ pub mod add_call_guards; pub mod add_moves_for_packed_drops; pub mod add_retag; pub mod check_consts; +pub mod check_packed_ref; pub mod check_unsafety; pub mod cleanup_post_borrowck; pub mod const_prop; @@ -211,10 +212,11 @@ fn mir_const_qualif(tcx: TyCtxt<'_>, def_id: DefId) -> ConstQualifs { validator.qualifs_in_return_place() } +/// Make MIR ready for const evaluation. This is run on all MIR, not just on consts! fn mir_const(tcx: TyCtxt<'_>, def_id: DefId) -> Steal> { let def_id = def_id.expect_local(); - // Unsafety check uses the raw mir, so make sure it is run + // Unsafety check uses the raw mir, so make sure it is run. let _ = tcx.unsafety_check_result(def_id); let mut body = tcx.mir_built(def_id).steal(); @@ -230,6 +232,8 @@ fn mir_const(tcx: TyCtxt<'_>, def_id: DefId) -> Steal> { None, MirPhase::Const, &[&[ + // MIR-level lints. + &check_packed_ref::CheckPackedRef, // What we need to do constant evaluation. &simplify::SimplifyCfg::new("initial"), &rustc_peek::SanityCheck, diff --git a/src/librustc_session/lint/builtin.rs b/src/librustc_session/lint/builtin.rs index 3d03e46683e..0753e9e4b73 100644 --- a/src/librustc_session/lint/builtin.rs +++ b/src/librustc_session/lint/builtin.rs @@ -216,10 +216,16 @@ declare_lint! { "lints that have been renamed or removed" } +declare_lint! { + pub PACKED_REFERENCES, + Allow, + "detects unaligned references to fields of packed structs", +} + declare_lint! { pub SAFE_PACKED_BORROWS, Warn, - "safe borrows of fields of packed structs were was erroneously allowed", + "safe borrows of fields of packed structs were erroneously allowed", @future_incompatible = FutureIncompatibleInfo { reference: "issue #46043 ", edition: None, @@ -545,6 +551,7 @@ declare_lint_pass! { INVALID_TYPE_PARAM_DEFAULT, CONST_ERR, RENAMED_AND_REMOVED_LINTS, + PACKED_REFERENCES, SAFE_PACKED_BORROWS, PATTERNS_IN_FNS_WITHOUT_BODY, MISSING_FRAGMENT_SPECIFIER, diff --git a/src/test/ui/lint/packed_reference.rs b/src/test/ui/lint/packed_reference.rs new file mode 100644 index 00000000000..d588ffd2120 --- /dev/null +++ b/src/test/ui/lint/packed_reference.rs @@ -0,0 +1,25 @@ +#![deny(packed_references)] + +#[repr(packed)] +pub struct Good { + data: &'static u32, + data2: [&'static u32; 2], + aligned: [u8; 32], +} + +#[repr(packed)] +pub struct JustArray { + array: [u32], +} + +fn main() { + unsafe { + let good = Good { data: &0, data2: [&0, &0], aligned: [0; 32] }; + + let _ = &good.data; //~ ERROR reference to packed field + let _ = &good.data2[0]; //~ ERROR reference to packed field + let _ = &*good.data; // ok, behind a pointer + let _ = &good.aligned; // ok, has align 1 + let _ = &good.aligned[2]; // ok, has align 1 + } +} diff --git a/src/test/ui/lint/packed_reference.stderr b/src/test/ui/lint/packed_reference.stderr new file mode 100644 index 00000000000..094fb4f34d3 --- /dev/null +++ b/src/test/ui/lint/packed_reference.stderr @@ -0,0 +1,23 @@ +error: reference to packed field is not allowed + --> $DIR/packed_reference.rs:19:17 + | +LL | let _ = &good.data; + | ^^^^^^^^^^ + | +note: the lint level is defined here + --> $DIR/packed_reference.rs:1:9 + | +LL | #![deny(packed_references)] + | ^^^^^^^^^^^^^^^^^ + = note: fields of packed structs might be misaligned, and creating a misaligned reference is undefined behavior (even if that reference is never dereferenced) + +error: reference to packed field is not allowed + --> $DIR/packed_reference.rs:20:17 + | +LL | let _ = &good.data2[0]; + | ^^^^^^^^^^^^^^ + | + = note: fields of packed structs might be misaligned, and creating a misaligned reference is undefined behavior (even if that reference is never dereferenced) + +error: aborting due to 2 previous errors + From c79535eab9500b87ab62f9e077db3953aa06b486 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 16 May 2020 16:29:27 +0200 Subject: [PATCH 2/4] remove some unused types from the tests --- src/test/ui/issues/issue-27060-rpass.rs | 11 +---------- src/test/ui/issues/issue-27060.rs | 5 ----- src/test/ui/issues/issue-27060.stderr | 6 +++--- src/test/ui/lint/packed_reference.rs | 5 ----- src/test/ui/lint/packed_reference.stderr | 4 ++-- 5 files changed, 6 insertions(+), 25 deletions(-) diff --git a/src/test/ui/issues/issue-27060-rpass.rs b/src/test/ui/issues/issue-27060-rpass.rs index b6ffc3ecb51..b20d614b303 100644 --- a/src/test/ui/issues/issue-27060-rpass.rs +++ b/src/test/ui/issues/issue-27060-rpass.rs @@ -7,19 +7,10 @@ pub struct Good { aligned: [u8; 32], } -#[repr(packed)] -pub struct JustArray { - array: [u32] -} - // kill this test when that turns to a hard error #[allow(safe_packed_borrows)] fn main() { - let good = Good { - data: &0, - data2: [&0, &0], - aligned: [0; 32] - }; + let good = Good { data: &0, data2: [&0, &0], aligned: [0; 32] }; unsafe { let _ = &good.data; // ok diff --git a/src/test/ui/issues/issue-27060.rs b/src/test/ui/issues/issue-27060.rs index 4caad03a361..78f2022ed38 100644 --- a/src/test/ui/issues/issue-27060.rs +++ b/src/test/ui/issues/issue-27060.rs @@ -5,11 +5,6 @@ pub struct Good { aligned: [u8; 32], } -#[repr(packed)] -pub struct JustArray { - array: [u32] -} - #[deny(safe_packed_borrows)] fn main() { let good = Good { diff --git a/src/test/ui/issues/issue-27060.stderr b/src/test/ui/issues/issue-27060.stderr index 6bf6348631a..d14ae4d41d5 100644 --- a/src/test/ui/issues/issue-27060.stderr +++ b/src/test/ui/issues/issue-27060.stderr @@ -1,11 +1,11 @@ error: borrow of packed field is unsafe and requires unsafe function or block (error E0133) - --> $DIR/issue-27060.rs:26:13 + --> $DIR/issue-27060.rs:21:13 | LL | let _ = &good.data; | ^^^^^^^^^^ | note: the lint level is defined here - --> $DIR/issue-27060.rs:13:8 + --> $DIR/issue-27060.rs:8:8 | LL | #[deny(safe_packed_borrows)] | ^^^^^^^^^^^^^^^^^^^ @@ -14,7 +14,7 @@ LL | #[deny(safe_packed_borrows)] = note: fields of packed structs might be misaligned: dereferencing a misaligned pointer or even just creating a misaligned reference is undefined behavior error: borrow of packed field is unsafe and requires unsafe function or block (error E0133) - --> $DIR/issue-27060.rs:28:13 + --> $DIR/issue-27060.rs:23:13 | LL | let _ = &good.data2[0]; | ^^^^^^^^^^^^^^ diff --git a/src/test/ui/lint/packed_reference.rs b/src/test/ui/lint/packed_reference.rs index d588ffd2120..c684fd62ee3 100644 --- a/src/test/ui/lint/packed_reference.rs +++ b/src/test/ui/lint/packed_reference.rs @@ -7,11 +7,6 @@ pub struct Good { aligned: [u8; 32], } -#[repr(packed)] -pub struct JustArray { - array: [u32], -} - fn main() { unsafe { let good = Good { data: &0, data2: [&0, &0], aligned: [0; 32] }; diff --git a/src/test/ui/lint/packed_reference.stderr b/src/test/ui/lint/packed_reference.stderr index 094fb4f34d3..428f4b66944 100644 --- a/src/test/ui/lint/packed_reference.stderr +++ b/src/test/ui/lint/packed_reference.stderr @@ -1,5 +1,5 @@ error: reference to packed field is not allowed - --> $DIR/packed_reference.rs:19:17 + --> $DIR/packed_reference.rs:14:17 | LL | let _ = &good.data; | ^^^^^^^^^^ @@ -12,7 +12,7 @@ LL | #![deny(packed_references)] = note: fields of packed structs might be misaligned, and creating a misaligned reference is undefined behavior (even if that reference is never dereferenced) error: reference to packed field is not allowed - --> $DIR/packed_reference.rs:20:17 + --> $DIR/packed_reference.rs:15:17 | LL | let _ = &good.data2[0]; | ^^^^^^^^^^^^^^ From 061773fd2b7fd40ba7c368de50878539d30f7bb1 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 25 May 2020 11:15:38 +0200 Subject: [PATCH 3/4] more test ref-to-packed tests --- src/test/ui/lint/packed_reference.rs | 2 ++ src/test/ui/lint/packed_reference.stderr | 18 +++++++++++++++++- 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/src/test/ui/lint/packed_reference.rs b/src/test/ui/lint/packed_reference.rs index c684fd62ee3..34942108468 100644 --- a/src/test/ui/lint/packed_reference.rs +++ b/src/test/ui/lint/packed_reference.rs @@ -12,6 +12,8 @@ fn main() { let good = Good { data: &0, data2: [&0, &0], aligned: [0; 32] }; let _ = &good.data; //~ ERROR reference to packed field + let _ = &good.data as *const _; //~ ERROR reference to packed field + let _: *const _ = &good.data; //~ ERROR reference to packed field let _ = &good.data2[0]; //~ ERROR reference to packed field let _ = &*good.data; // ok, behind a pointer let _ = &good.aligned; // ok, has align 1 diff --git a/src/test/ui/lint/packed_reference.stderr b/src/test/ui/lint/packed_reference.stderr index 428f4b66944..51158a84175 100644 --- a/src/test/ui/lint/packed_reference.stderr +++ b/src/test/ui/lint/packed_reference.stderr @@ -14,10 +14,26 @@ LL | #![deny(packed_references)] error: reference to packed field is not allowed --> $DIR/packed_reference.rs:15:17 | +LL | let _ = &good.data as *const _; + | ^^^^^^^^^^ + | + = note: fields of packed structs might be misaligned, and creating a misaligned reference is undefined behavior (even if that reference is never dereferenced) + +error: reference to packed field is not allowed + --> $DIR/packed_reference.rs:16:27 + | +LL | let _: *const _ = &good.data; + | ^^^^^^^^^^ + | + = note: fields of packed structs might be misaligned, and creating a misaligned reference is undefined behavior (even if that reference is never dereferenced) + +error: reference to packed field is not allowed + --> $DIR/packed_reference.rs:17:17 + | LL | let _ = &good.data2[0]; | ^^^^^^^^^^^^^^ | = note: fields of packed structs might be misaligned, and creating a misaligned reference is undefined behavior (even if that reference is never dereferenced) -error: aborting due to 2 previous errors +error: aborting due to 4 previous errors From d959a8f91d9945302f58a8f79613e75060c6b77d Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 25 May 2020 15:32:46 +0200 Subject: [PATCH 4/4] rename lint --- .../transform/check_packed_ref.rs | 8 ++-- src/librustc_session/lint/builtin.rs | 4 +- src/test/ui/lint/packed_reference.stderr | 39 ------------------- ...d_reference.rs => unaligned_references.rs} | 2 +- src/test/ui/lint/unaligned_references.stderr | 39 +++++++++++++++++++ 5 files changed, 46 insertions(+), 46 deletions(-) delete mode 100644 src/test/ui/lint/packed_reference.stderr rename src/test/ui/lint/{packed_reference.rs => unaligned_references.rs} (95%) create mode 100644 src/test/ui/lint/unaligned_references.stderr diff --git a/src/librustc_mir/transform/check_packed_ref.rs b/src/librustc_mir/transform/check_packed_ref.rs index 9e07a4599f6..faad1a72327 100644 --- a/src/librustc_mir/transform/check_packed_ref.rs +++ b/src/librustc_mir/transform/check_packed_ref.rs @@ -1,7 +1,7 @@ use rustc_middle::mir::visit::{PlaceContext, Visitor}; use rustc_middle::mir::*; use rustc_middle::ty::{self, TyCtxt}; -use rustc_session::lint::builtin::PACKED_REFERENCES; +use rustc_session::lint::builtin::UNALIGNED_REFERENCES; use crate::transform::{MirPass, MirSource}; use crate::util; @@ -47,13 +47,13 @@ impl<'a, 'tcx> Visitor<'tcx> for PackedRefChecker<'a, 'tcx> { .assert_crate_local() .lint_root; self.tcx.struct_span_lint_hir( - PACKED_REFERENCES, + UNALIGNED_REFERENCES, lint_root, source_info.span, |lint| { - lint.build(&format!("reference to packed field is not allowed",)) + lint.build(&format!("reference to packed field is unaligned",)) .note( - "fields of packed structs might be misaligned, and creating \ + "fields of packed structs are not properly aligned, and creating \ a misaligned reference is undefined behavior (even if that \ reference is never dereferenced)", ) diff --git a/src/librustc_session/lint/builtin.rs b/src/librustc_session/lint/builtin.rs index 0753e9e4b73..40354172048 100644 --- a/src/librustc_session/lint/builtin.rs +++ b/src/librustc_session/lint/builtin.rs @@ -217,7 +217,7 @@ declare_lint! { } declare_lint! { - pub PACKED_REFERENCES, + pub UNALIGNED_REFERENCES, Allow, "detects unaligned references to fields of packed structs", } @@ -551,7 +551,7 @@ declare_lint_pass! { INVALID_TYPE_PARAM_DEFAULT, CONST_ERR, RENAMED_AND_REMOVED_LINTS, - PACKED_REFERENCES, + UNALIGNED_REFERENCES, SAFE_PACKED_BORROWS, PATTERNS_IN_FNS_WITHOUT_BODY, MISSING_FRAGMENT_SPECIFIER, diff --git a/src/test/ui/lint/packed_reference.stderr b/src/test/ui/lint/packed_reference.stderr deleted file mode 100644 index 51158a84175..00000000000 --- a/src/test/ui/lint/packed_reference.stderr +++ /dev/null @@ -1,39 +0,0 @@ -error: reference to packed field is not allowed - --> $DIR/packed_reference.rs:14:17 - | -LL | let _ = &good.data; - | ^^^^^^^^^^ - | -note: the lint level is defined here - --> $DIR/packed_reference.rs:1:9 - | -LL | #![deny(packed_references)] - | ^^^^^^^^^^^^^^^^^ - = note: fields of packed structs might be misaligned, and creating a misaligned reference is undefined behavior (even if that reference is never dereferenced) - -error: reference to packed field is not allowed - --> $DIR/packed_reference.rs:15:17 - | -LL | let _ = &good.data as *const _; - | ^^^^^^^^^^ - | - = note: fields of packed structs might be misaligned, and creating a misaligned reference is undefined behavior (even if that reference is never dereferenced) - -error: reference to packed field is not allowed - --> $DIR/packed_reference.rs:16:27 - | -LL | let _: *const _ = &good.data; - | ^^^^^^^^^^ - | - = note: fields of packed structs might be misaligned, and creating a misaligned reference is undefined behavior (even if that reference is never dereferenced) - -error: reference to packed field is not allowed - --> $DIR/packed_reference.rs:17:17 - | -LL | let _ = &good.data2[0]; - | ^^^^^^^^^^^^^^ - | - = note: fields of packed structs might be misaligned, and creating a misaligned reference is undefined behavior (even if that reference is never dereferenced) - -error: aborting due to 4 previous errors - diff --git a/src/test/ui/lint/packed_reference.rs b/src/test/ui/lint/unaligned_references.rs similarity index 95% rename from src/test/ui/lint/packed_reference.rs rename to src/test/ui/lint/unaligned_references.rs index 34942108468..1d9f4c3db2e 100644 --- a/src/test/ui/lint/packed_reference.rs +++ b/src/test/ui/lint/unaligned_references.rs @@ -1,4 +1,4 @@ -#![deny(packed_references)] +#![deny(unaligned_references)] #[repr(packed)] pub struct Good { diff --git a/src/test/ui/lint/unaligned_references.stderr b/src/test/ui/lint/unaligned_references.stderr new file mode 100644 index 00000000000..0c594cdb30a --- /dev/null +++ b/src/test/ui/lint/unaligned_references.stderr @@ -0,0 +1,39 @@ +error: reference to packed field is unaligned + --> $DIR/unaligned_references.rs:14:17 + | +LL | let _ = &good.data; + | ^^^^^^^^^^ + | +note: the lint level is defined here + --> $DIR/unaligned_references.rs:1:9 + | +LL | #![deny(unaligned_references)] + | ^^^^^^^^^^^^^^^^^^^^ + = note: fields of packed structs are not properly aligned, and creating a misaligned reference is undefined behavior (even if that reference is never dereferenced) + +error: reference to packed field is unaligned + --> $DIR/unaligned_references.rs:15:17 + | +LL | let _ = &good.data as *const _; + | ^^^^^^^^^^ + | + = note: fields of packed structs are not properly aligned, and creating a misaligned reference is undefined behavior (even if that reference is never dereferenced) + +error: reference to packed field is unaligned + --> $DIR/unaligned_references.rs:16:27 + | +LL | let _: *const _ = &good.data; + | ^^^^^^^^^^ + | + = note: fields of packed structs are not properly aligned, and creating a misaligned reference is undefined behavior (even if that reference is never dereferenced) + +error: reference to packed field is unaligned + --> $DIR/unaligned_references.rs:17:17 + | +LL | let _ = &good.data2[0]; + | ^^^^^^^^^^^^^^ + | + = note: fields of packed structs are not properly aligned, and creating a misaligned reference is undefined behavior (even if that reference is never dereferenced) + +error: aborting due to 4 previous errors +