From ee1caae33c9082c44387b2dc9b939db9f764a8f3 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 28 Mar 2021 12:50:20 +0200 Subject: [PATCH 1/2] unaligned_references: align(N) fields in packed(N) structs are fine --- compiler/rustc_mir/src/util/alignment.rs | 35 ++++++++++++++------ src/test/ui/lint/unaligned_references.rs | 15 +++++++++ src/test/ui/lint/unaligned_references.stderr | 24 ++++++++++---- 3 files changed, 56 insertions(+), 18 deletions(-) diff --git a/compiler/rustc_mir/src/util/alignment.rs b/compiler/rustc_mir/src/util/alignment.rs index f567c9cfaab86..5d4ca871faa2e 100644 --- a/compiler/rustc_mir/src/util/alignment.rs +++ b/compiler/rustc_mir/src/util/alignment.rs @@ -1,5 +1,6 @@ use rustc_middle::mir::*; use rustc_middle::ty::{self, TyCtxt}; +use rustc_target::abi::Align; /// Returns `true` if this place is allowed to be less aligned /// than its containing struct (because it is within a packed @@ -14,17 +15,25 @@ where L: HasLocalDecls<'tcx>, { debug!("is_disaligned({:?})", place); - if !is_within_packed(tcx, local_decls, place) { - debug!("is_disaligned({:?}) - not within packed", place); - return false; - } + let pack = match is_within_packed(tcx, local_decls, place) { + None => { + debug!("is_disaligned({:?}) - not within packed", place); + return false; + } + Some(pack) => pack, + }; let ty = place.ty(local_decls, tcx).ty; match tcx.layout_raw(param_env.and(ty)) { - Ok(layout) if layout.align.abi.bytes() == 1 => { - // if the alignment is 1, the type can't be further - // disaligned. - debug!("is_disaligned({:?}) - align = 1", place); + Ok(layout) if layout.align.abi <= pack => { + // If the packed alignment is greater or equal to the field alignment, the type won't be + // further disaligned. + debug!( + "is_disaligned({:?}) - align = {}, packed = {}; not disaligned", + place, + layout.align.abi.bytes(), + pack.bytes() + ); false } _ => { @@ -34,7 +43,11 @@ where } } -fn is_within_packed<'tcx, L>(tcx: TyCtxt<'tcx>, local_decls: &L, place: Place<'tcx>) -> bool +fn is_within_packed<'tcx, L>( + tcx: TyCtxt<'tcx>, + local_decls: &L, + place: Place<'tcx>, +) -> Option where L: HasLocalDecls<'tcx>, { @@ -45,7 +58,7 @@ where ProjectionElem::Field(..) => { let ty = place_base.ty(local_decls, tcx).ty; match ty.kind() { - ty::Adt(def, _) if def.repr.packed() => return true, + ty::Adt(def, _) => return def.repr.pack, _ => {} } } @@ -53,5 +66,5 @@ where } } - false + None } diff --git a/src/test/ui/lint/unaligned_references.rs b/src/test/ui/lint/unaligned_references.rs index ad38c21d96cf9..d06b06b504f06 100644 --- a/src/test/ui/lint/unaligned_references.rs +++ b/src/test/ui/lint/unaligned_references.rs @@ -8,6 +8,13 @@ pub struct Good { aligned: [u8; 32], } +#[repr(packed(2))] +pub struct Packed2 { + x: u32, + y: u16, + z: u8, +} + fn main() { unsafe { let good = Good { data: 0, ptr: &0, data2: [0, 0], aligned: [0; 32] }; @@ -32,4 +39,12 @@ fn main() { let _ = &good.aligned; // ok, has align 1 let _ = &good.aligned[2]; // ok, has align 1 } + + unsafe { + let packed2 = Packed2 { x: 0, y: 0, z: 0 }; + let _ = &packed2.x; //~ ERROR reference to packed field + //~^ previously accepted + let _ = &packed2.y; // ok, has align 2 in packed(2) struct + let _ = &packed2.z; // ok, has align 1 + } } diff --git a/src/test/ui/lint/unaligned_references.stderr b/src/test/ui/lint/unaligned_references.stderr index 9ae25f5b59ed1..b4cce3cfea217 100644 --- a/src/test/ui/lint/unaligned_references.stderr +++ b/src/test/ui/lint/unaligned_references.stderr @@ -1,5 +1,5 @@ error: reference to packed field is unaligned - --> $DIR/unaligned_references.rs:15:17 + --> $DIR/unaligned_references.rs:22:17 | LL | let _ = &good.ptr; | ^^^^^^^^^ @@ -14,7 +14,7 @@ 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:17:17 + --> $DIR/unaligned_references.rs:24:17 | LL | let _ = &good.data; | ^^^^^^^^^^ @@ -24,7 +24,7 @@ LL | let _ = &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:20:17 + --> $DIR/unaligned_references.rs:27:17 | LL | let _ = &good.data as *const _; | ^^^^^^^^^^ @@ -34,7 +34,7 @@ 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:22:27 + --> $DIR/unaligned_references.rs:29:27 | LL | let _: *const _ = &good.data; | ^^^^^^^^^^ @@ -44,7 +44,7 @@ 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:25:17 + --> $DIR/unaligned_references.rs:32:17 | LL | let _ = good.data.clone(); | ^^^^^^^^^ @@ -54,7 +54,7 @@ LL | let _ = good.data.clone(); = 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:28:17 + --> $DIR/unaligned_references.rs:35:17 | LL | let _ = &good.data2[0]; | ^^^^^^^^^^^^^^ @@ -63,5 +63,15 @@ LL | let _ = &good.data2[0]; = note: for more information, see issue #82523 = 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 6 previous errors +error: reference to packed field is unaligned + --> $DIR/unaligned_references.rs:45:17 + | +LL | let _ = &packed2.x; + | ^^^^^^^^^^ + | + = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! + = note: for more information, see issue #82523 + = 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 7 previous errors From 1ab05c13dde13d08a2e1990960582cb9970e90a2 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 28 Mar 2021 13:54:27 +0200 Subject: [PATCH 2/2] adjust old test --- .../packed-struct-borrow-element-64bit.rs | 17 +++++++++++++++++ .../packed-struct-borrow-element-64bit.stderr | 13 +++++++++++++ .../ui/packed/packed-struct-borrow-element.rs | 5 ----- .../packed/packed-struct-borrow-element.stderr | 12 +----------- 4 files changed, 31 insertions(+), 16 deletions(-) create mode 100644 src/test/ui/packed/packed-struct-borrow-element-64bit.rs create mode 100644 src/test/ui/packed/packed-struct-borrow-element-64bit.stderr diff --git a/src/test/ui/packed/packed-struct-borrow-element-64bit.rs b/src/test/ui/packed/packed-struct-borrow-element-64bit.rs new file mode 100644 index 0000000000000..ad932fdcd01c4 --- /dev/null +++ b/src/test/ui/packed/packed-struct-borrow-element-64bit.rs @@ -0,0 +1,17 @@ +// run-pass (note: this is spec-UB, but it works for now) +// ignore-32bit (needs `usize` to be 8-aligned to reproduce all the errors below) +#![allow(dead_code)] +// ignore-emscripten weird assertion? + +#[repr(C, packed(4))] +struct Foo4C { + bar: u8, + baz: usize +} + +pub fn main() { + let foo = Foo4C { bar: 1, baz: 2 }; + let brw = &foo.baz; //~WARN reference to packed field is unaligned + //~^ previously accepted + assert_eq!(*brw, 2); +} diff --git a/src/test/ui/packed/packed-struct-borrow-element-64bit.stderr b/src/test/ui/packed/packed-struct-borrow-element-64bit.stderr new file mode 100644 index 0000000000000..766d8a72c349e --- /dev/null +++ b/src/test/ui/packed/packed-struct-borrow-element-64bit.stderr @@ -0,0 +1,13 @@ +warning: reference to packed field is unaligned + --> $DIR/packed-struct-borrow-element-64bit.rs:14:15 + | +LL | let brw = &foo.baz; + | ^^^^^^^^ + | + = note: `#[warn(unaligned_references)]` on by default + = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! + = note: for more information, see issue #82523 + = note: fields of packed structs are not properly aligned, and creating a misaligned reference is undefined behavior (even if that reference is never dereferenced) + +warning: 1 warning emitted + diff --git a/src/test/ui/packed/packed-struct-borrow-element.rs b/src/test/ui/packed/packed-struct-borrow-element.rs index 5dad084eecf6d..fb6c9c11d5694 100644 --- a/src/test/ui/packed/packed-struct-borrow-element.rs +++ b/src/test/ui/packed/packed-struct-borrow-element.rs @@ -30,9 +30,4 @@ pub fn main() { let brw = &foo.baz; //~WARN reference to packed field is unaligned //~^ previously accepted assert_eq!(*brw, 2); - - let foo = Foo4C { bar: 1, baz: 2 }; - let brw = &foo.baz; //~WARN reference to packed field is unaligned - //~^ previously accepted - assert_eq!(*brw, 2); } diff --git a/src/test/ui/packed/packed-struct-borrow-element.stderr b/src/test/ui/packed/packed-struct-borrow-element.stderr index d9d9a71ff58e9..5764e951a4600 100644 --- a/src/test/ui/packed/packed-struct-borrow-element.stderr +++ b/src/test/ui/packed/packed-struct-borrow-element.stderr @@ -19,15 +19,5 @@ LL | let brw = &foo.baz; = note: for more information, see issue #82523 = note: fields of packed structs are not properly aligned, and creating a misaligned reference is undefined behavior (even if that reference is never dereferenced) -warning: reference to packed field is unaligned - --> $DIR/packed-struct-borrow-element.rs:35:15 - | -LL | let brw = &foo.baz; - | ^^^^^^^^ - | - = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! - = note: for more information, see issue #82523 - = note: fields of packed structs are not properly aligned, and creating a misaligned reference is undefined behavior (even if that reference is never dereferenced) - -warning: 3 warnings emitted +warning: 2 warnings emitted