-
Notifications
You must be signed in to change notification settings - Fork 13.9k
Allow borrowing array elements from packed structs with ABI align <= packed align #145419
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
base: master
Are you sure you want to change the base?
Changes from 13 commits
301137d
19f51f2
a2a94b9
2e84ab8
d8785b9
dd1537c
99dedb0
4620527
2fe4b7f
f0c8e9e
4a65fdc
585af05
b37f571
ef2538b
8c07e0a
dd6a119
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,12 +1,12 @@ | ||
| use rustc_abi::Align; | ||
| use rustc_middle::mir::*; | ||
| use rustc_middle::ty::{self, TyCtxt}; | ||
| use rustc_middle::ty::{self, Ty, TyCtxt}; | ||
| use tracing::debug; | ||
|
|
||
| /// Returns `true` if this place is allowed to be less aligned | ||
| /// than its containing struct (because it is within a packed | ||
| /// struct). | ||
| pub fn is_disaligned<'tcx, L>( | ||
| pub fn is_potentially_misaligned<'tcx, L>( | ||
| tcx: TyCtxt<'tcx>, | ||
| local_decls: &L, | ||
| typing_env: ty::TypingEnv<'tcx>, | ||
|
|
@@ -15,38 +15,74 @@ pub fn is_disaligned<'tcx, L>( | |
| where | ||
| L: HasLocalDecls<'tcx>, | ||
| { | ||
| debug!("is_disaligned({:?})", place); | ||
| debug!("is_potentially_misaligned({:?})", place); | ||
| let Some(pack) = is_within_packed(tcx, local_decls, place) else { | ||
| debug!("is_disaligned({:?}) - not within packed", place); | ||
| debug!("is_potentially_misaligned({:?}) - not within packed", place); | ||
| return false; | ||
| }; | ||
|
|
||
| let ty = place.ty(local_decls, tcx).ty; | ||
| let unsized_tail = || tcx.struct_tail_for_codegen(ty, typing_env); | ||
|
|
||
| match tcx.layout_of(typing_env.as_query_input(ty)) { | ||
| Ok(layout) | ||
| Ok(layout) => { | ||
| if layout.align.abi <= pack | ||
| && (layout.is_sized() | ||
| || matches!(unsized_tail().kind(), ty::Slice(..) | ty::Str)) => | ||
| { | ||
| // If the packed alignment is greater or equal to the field alignment, the type won't be | ||
| // further disaligned. | ||
| // However we need to ensure the field is sized; for unsized fields, `layout.align` is | ||
| // just an approximation -- except when the unsized tail is a slice, where the alignment | ||
| // is fully determined by the type. | ||
| debug!( | ||
| "is_disaligned({:?}) - align = {}, packed = {}; not disaligned", | ||
| place, | ||
| layout.align.abi.bytes(), | ||
| pack.bytes() | ||
| ); | ||
| false | ||
| && (layout.is_sized() || matches!(unsized_tail().kind(), ty::Slice(..) | ty::Str)) | ||
| { | ||
| // If the packed alignment is greater or equal to the field alignment, the type won't be | ||
| // further disaligned. | ||
| // However we need to ensure the field is sized; for unsized fields, `layout.align` is | ||
| // just an approximation -- except when the unsized tail is a slice, where the alignment | ||
| // is fully determined by the type. | ||
| debug!( | ||
| "is_potentially_misaligned({:?}) - align = {}, packed = {}; not disaligned", | ||
| place, | ||
| layout.align.abi.bytes(), | ||
| pack.bytes() | ||
| ); | ||
| false | ||
| } else { | ||
| true | ||
| } | ||
| } | ||
| Err(_) => { | ||
| // Soundness: For any `T`, the ABI alignment requirement of `[T]` equals that of `T`. | ||
| // Proof sketch: | ||
| // (1) From `&[T]` we can obtain `&T`, hence align([T]) >= align(T). | ||
| // (2) Using `std::array::from_ref(&T)` we can obtain `&[T; 1]` (and thus `&[T]`), | ||
| // hence align(T) >= align([T]). | ||
| // Therefore align([T]) == align(T). Length does not affect alignment. | ||
|
|
||
| // Try to determine alignment from the type structure | ||
| if let Some(element_align) = get_element_alignment(tcx, ty) { | ||
| element_align > pack | ||
| } else { | ||
| // If we still can't determine alignment, conservatively assume disaligned | ||
| true | ||
| } | ||
| } | ||
| _ => { | ||
| // We cannot figure out the layout. Conservatively assume that this is disaligned. | ||
| debug!("is_disaligned({:?}) - true", place); | ||
| true | ||
| } | ||
| } | ||
|
|
||
| /// Try to determine the alignment of an array element type | ||
| fn get_element_alignment<'tcx>(_tcx: TyCtxt<'tcx>, ty: Ty<'tcx>) -> Option<Align> { | ||
| match ty.kind() { | ||
| ty::Array(element_ty, _) | ty::Slice(element_ty) => { | ||
| // Only allow u8 and i8 arrays when layout computation fails | ||
| // Other types are conservatively assumed to be misaligned | ||
| match element_ty.kind() { | ||
|
||
| ty::Uint(ty::UintTy::U8) | ty::Int(ty::IntTy::I8) => { | ||
| // For u8 and i8, we know their alignment is 1 | ||
| Some(Align::from_bytes(1).unwrap()) | ||
| } | ||
| _ => { | ||
| // For other types, we cannot safely determine alignment | ||
| // Conservatively return None to indicate potential misalignment | ||
| None | ||
| } | ||
| } | ||
| } | ||
| _ => None, | ||
| } | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| //@ check-pass | ||
| #![allow(dead_code)] | ||
|
|
||
| #[repr(C, packed)] | ||
| struct PascalString<const CAP: usize> { | ||
| len: u8, | ||
| buf: [u8; CAP], | ||
| } | ||
|
|
||
| fn bar<const CAP: usize>(s: &PascalString<CAP>) -> &str { | ||
| // Goal: this line should not trigger E0793 | ||
| std::str::from_utf8(&s.buf[0..s.len as usize]).unwrap() | ||
| } | ||
|
|
||
| fn main() { | ||
| let p = PascalString::<10> { len: 3, buf: *b"abc\0\0\0\0\0\0\0" }; | ||
| let s = bar(&p); | ||
| assert_eq!(s, "abc"); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| //@ check-pass | ||
| #![allow(dead_code)] | ||
|
|
||
| #[repr(C, packed)] | ||
| struct PascalStringI8<const CAP: usize> { | ||
|
||
| len: u8, | ||
| buf: [i8; CAP], | ||
| } | ||
|
|
||
| fn bar<const CAP: usize>(s: &PascalStringI8<CAP>) -> &[i8] { | ||
| // Goal: this line should not trigger E0793 for i8 arrays | ||
| &s.buf[0..s.len as usize] | ||
| } | ||
|
|
||
| fn main() { | ||
| let p = PascalStringI8::<10> { len: 3, buf: [1, 2, 3, 0, 0, 0, 0, 0, 0, 0] }; | ||
| let s = bar(&p); | ||
| assert_eq!(s, &[1, 2, 3]); | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think referencing standard library operations here makes much sense. Maybe we can reference the Reference wherever it defines the alignment of arrays, but TBH that doesn't seem necessary. What's relevant is to explicitly invoke the fact o that arrays are aligned like their element type, that's the one key reasoning step needed here. Let's not drown that in noise.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
still relevant