From 9097ce905427c30bd262f62a403f1e987ebb10c6 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 2 Aug 2022 12:21:38 -0400 Subject: [PATCH] fix is_disaligned logic for nested packed structs --- .../rustc_const_eval/src/util/alignment.rs | 28 ++++----- src/test/ui/issues/issue-99838.rs | 40 +++++++++++++ src/test/ui/lint/unaligned_references.rs | 53 +++++++++++++++++ src/test/ui/lint/unaligned_references.stderr | 58 ++++++++++++++++++- 4 files changed, 162 insertions(+), 17 deletions(-) create mode 100644 src/test/ui/issues/issue-99838.rs diff --git a/compiler/rustc_const_eval/src/util/alignment.rs b/compiler/rustc_const_eval/src/util/alignment.rs index fe4a726fe12a0..4f39dad205ae3 100644 --- a/compiler/rustc_const_eval/src/util/alignment.rs +++ b/compiler/rustc_const_eval/src/util/alignment.rs @@ -48,20 +48,16 @@ fn is_within_packed<'tcx, L>( where L: HasLocalDecls<'tcx>, { - for (place_base, elem) in place.iter_projections().rev() { - match elem { - // encountered a Deref, which is ABI-aligned - ProjectionElem::Deref => break, - ProjectionElem::Field(..) => { - let ty = place_base.ty(local_decls, tcx).ty; - match ty.kind() { - ty::Adt(def, _) => return def.repr().pack, - _ => {} - } - } - _ => {} - } - } - - None + place + .iter_projections() + .rev() + // Stop at `Deref`; standard ABI alignment applies there. + .take_while(|(_base, elem)| !matches!(elem, ProjectionElem::Deref)) + // Consider the packed alignments at play here... + .filter_map(|(base, _elem)| { + base.ty(local_decls, tcx).ty.ty_adt_def().and_then(|adt| adt.repr().pack) + }) + // ... and compute their minimum. + // The overall smallest alignment is what matters. + .min() } diff --git a/src/test/ui/issues/issue-99838.rs b/src/test/ui/issues/issue-99838.rs new file mode 100644 index 0000000000000..eaeeac72b25e1 --- /dev/null +++ b/src/test/ui/issues/issue-99838.rs @@ -0,0 +1,40 @@ +// run-pass +#![feature(bench_black_box)] +use std::hint; + +struct U16(u16); + +impl Drop for U16 { + fn drop(&mut self) { + // Prevent LLVM from optimizing away our alignment check. + assert!(hint::black_box(self as *mut U16 as usize) % 2 == 0); + } +} + +struct HasDrop; + +impl Drop for HasDrop { + fn drop(&mut self) {} +} + +struct Wrapper { + _a: U16, + b: HasDrop, +} + +#[repr(packed)] +struct Misalign(u8, Wrapper); + +fn main() { + let m = Misalign( + 0, + Wrapper { + _a: U16(10), + b: HasDrop, + }, + ); + // Put it somewhere definitely even (so the `a` field is definitely at an odd address). + let m: ([u16; 0], Misalign) = ([], m); + // Move out one field, so we run custom per-field drop logic below. + let _x = m.1.1.b; +} diff --git a/src/test/ui/lint/unaligned_references.rs b/src/test/ui/lint/unaligned_references.rs index d06b06b504f06..e547f031a9cfd 100644 --- a/src/test/ui/lint/unaligned_references.rs +++ b/src/test/ui/lint/unaligned_references.rs @@ -47,4 +47,57 @@ fn main() { let _ = &packed2.y; // ok, has align 2 in packed(2) struct let _ = &packed2.z; // ok, has align 1 } + + unsafe { + struct U16(u16); + + impl Drop for U16 { + fn drop(&mut self) { + println!("{:p}", self); + } + } + + struct HasDrop; + + impl Drop for HasDrop { + fn drop(&mut self) {} + } + + #[allow(unused)] + struct Wrapper { + a: U16, + b: HasDrop, + } + #[allow(unused)] + #[repr(packed(2))] + struct Wrapper2 { + a: U16, + b: HasDrop, + } + + // An outer struct with more restrictive packing than the inner struct -- make sure we + // notice that! + #[repr(packed)] + struct Misalign(u8, T); + + let m1 = Misalign( + 0, + Wrapper { + a: U16(10), + b: HasDrop, + }, + ); + let _ref = &m1.1.a; //~ ERROR reference to packed field + //~^ previously accepted + + let m2 = Misalign( + 0, + Wrapper2 { + a: U16(10), + b: HasDrop, + }, + ); + let _ref = &m2.1.a; //~ ERROR reference to packed field + //~^ previously accepted + } } diff --git a/src/test/ui/lint/unaligned_references.stderr b/src/test/ui/lint/unaligned_references.stderr index ed5dd2ec01181..97dbec2861cd2 100644 --- a/src/test/ui/lint/unaligned_references.stderr +++ b/src/test/ui/lint/unaligned_references.stderr @@ -80,7 +80,29 @@ LL | let _ = &packed2.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: aborting due to 7 previous errors +error: reference to packed field is unaligned + --> $DIR/unaligned_references.rs:90:20 + | +LL | let _ref = &m1.1.a; + | ^^^^^^^ + | + = 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) + = 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: reference to packed field is unaligned + --> $DIR/unaligned_references.rs:100:20 + | +LL | let _ref = &m2.1.a; + | ^^^^^^^ + | + = 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) + = 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 Future incompatibility report: Future breakage diagnostic: error: reference to packed field is unaligned @@ -201,3 +223,37 @@ 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) = 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) +Future breakage diagnostic: +error: reference to packed field is unaligned + --> $DIR/unaligned_references.rs:90:20 + | +LL | let _ref = &m1.1.a; + | ^^^^^^^ + | +note: the lint level is defined here + --> $DIR/unaligned_references.rs:1:9 + | +LL | #![deny(unaligned_references)] + | ^^^^^^^^^^^^^^^^^^^^ + = 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) + = 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) + +Future breakage diagnostic: +error: reference to packed field is unaligned + --> $DIR/unaligned_references.rs:100:20 + | +LL | let _ref = &m2.1.a; + | ^^^^^^^ + | +note: the lint level is defined here + --> $DIR/unaligned_references.rs:1:9 + | +LL | #![deny(unaligned_references)] + | ^^^^^^^^^^^^^^^^^^^^ + = 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) + = 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) +