Skip to content

Commit f2e1a3a

Browse files
committed
Auto merge of rust-lang#125360 - RalfJung:packed-field-reorder, r=fmease
don't inhibit random field reordering on repr(packed(1)) `inhibit_struct_field_reordering_opt` being false means we exclude this type from random field shuffling. However, `packed(1)` types can still be shuffled! The logic was added in rust-lang#48528 since it's pointless to reorder fields in packed(1) types (there's no padding that could be saved) -- but that shouldn't inhibit `-Zrandomize-layout` (which did not exist at the time). We could add an optimization elsewhere to not bother sorting the fields for `repr(packed)` types, but I don't think that's worth the effort. This *does* change the behavior in that we may now reorder fields of `packed(1)` structs (e.g. if there are niches, we'll try to move them to the start/end, according to `NicheBias`). We were always allowed to do that but so far we didn't. Quoting the [reference](https://doc.rust-lang.org/reference/type-layout.html): > On their own, align and packed do not provide guarantees about the order of fields in the layout of a struct or the layout of an enum variant, although they may be combined with representations (such as C) which do provide such guarantees.
2 parents 4cf5723 + 37aeb75 commit f2e1a3a

File tree

6 files changed

+14
-19
lines changed

6 files changed

+14
-19
lines changed

compiler/rustc_abi/src/layout.rs

+6-4
Original file line numberDiff line numberDiff line change
@@ -970,7 +970,7 @@ fn univariant<
970970
let mut align = if pack.is_some() { dl.i8_align } else { dl.aggregate_align };
971971
let mut max_repr_align = repr.align;
972972
let mut inverse_memory_index: IndexVec<u32, FieldIdx> = fields.indices().collect();
973-
let optimize = !repr.inhibit_struct_field_reordering_opt();
973+
let optimize = !repr.inhibit_struct_field_reordering();
974974
if optimize && fields.len() > 1 {
975975
let end = if let StructKind::MaybeUnsized = kind { fields.len() - 1 } else { fields.len() };
976976
let optimizing = &mut inverse_memory_index.raw[..end];
@@ -1007,13 +1007,15 @@ fn univariant<
10071007
// Calculates a sort key to group fields by their alignment or possibly some
10081008
// size-derived pseudo-alignment.
10091009
let alignment_group_key = |layout: &F| {
1010+
// The two branches here return values that cannot be meaningfully compared with
1011+
// each other. However, we know that consistently for all executions of
1012+
// `alignment_group_key`, one or the other branch will be taken, so this is okay.
10101013
if let Some(pack) = pack {
10111014
// Return the packed alignment in bytes.
10121015
layout.align.abi.min(pack).bytes()
10131016
} else {
1014-
// Returns `log2(effective-align)`. This is ok since `pack` applies to all
1015-
// fields equally. The calculation assumes that size is an integer multiple of
1016-
// align, except for ZSTs.
1017+
// Returns `log2(effective-align)`. The calculation assumes that size is an
1018+
// integer multiple of align, except for ZSTs.
10171019
let align = layout.align.abi.bytes();
10181020
let size = layout.size.bytes();
10191021
let niche_size = layout.largest_niche.map(|n| n.available(dl)).unwrap_or(0);

compiler/rustc_abi/src/lib.rs

+4-11
Original file line numberDiff line numberDiff line change
@@ -137,23 +137,16 @@ impl ReprOptions {
137137
self.c() || self.int.is_some()
138138
}
139139

140-
/// Returns `true` if this `#[repr()]` should inhibit struct field reordering
141-
/// optimizations, such as with `repr(C)`, `repr(packed(1))`, or `repr(<int>)`.
142-
pub fn inhibit_struct_field_reordering_opt(&self) -> bool {
143-
if let Some(pack) = self.pack {
144-
if pack.bytes() == 1 {
145-
return true;
146-
}
147-
}
148-
140+
/// Returns `true` if this `#[repr()]` guarantees a fixed field order,
141+
/// e.g. `repr(C)` or `repr(<int>)`.
142+
pub fn inhibit_struct_field_reordering(&self) -> bool {
149143
self.flags.intersects(ReprFlags::IS_UNOPTIMISABLE) || self.int.is_some()
150144
}
151145

152146
/// Returns `true` if this type is valid for reordering and `-Z randomize-layout`
153147
/// was enabled for its declaration crate.
154148
pub fn can_randomize_type_layout(&self) -> bool {
155-
!self.inhibit_struct_field_reordering_opt()
156-
&& self.flags.contains(ReprFlags::RANDOMIZE_LAYOUT)
149+
!self.inhibit_struct_field_reordering() && self.flags.contains(ReprFlags::RANDOMIZE_LAYOUT)
157150
}
158151

159152
/// Returns `true` if this `#[repr()]` should inhibit union ABI optimisations.

src/tools/clippy/clippy_lints/src/transmute/transmute_undefined_repr.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -278,7 +278,7 @@ fn reduce_ty<'tcx>(cx: &LateContext<'tcx>, mut ty: Ty<'tcx>) -> ReducedTy<'tcx>
278278
ty = sized_ty;
279279
continue;
280280
}
281-
if def.repr().inhibit_struct_field_reordering_opt() {
281+
if def.repr().inhibit_struct_field_reordering() {
282282
ReducedTy::OrderedFields(Some(sized_ty))
283283
} else {
284284
ReducedTy::UnorderedFields(ty)

src/tools/miri/tests/fail/reading_half_a_pointer.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
#![allow(dead_code)]
22

33
// We use packed structs to get around alignment restrictions
4-
#[repr(packed)]
4+
#[repr(C, packed)]
55
struct Data {
66
pad: u8,
77
ptr: &'static i32,

src/tools/miri/tests/fail/unaligned_pointers/field_requires_parent_struct_alignment2.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ pub struct Aligned {
77
_pad: [u8; 11],
88
packed: Packed,
99
}
10-
#[repr(packed)]
10+
#[repr(C, packed)]
1111
#[derive(Default, Copy, Clone)]
1212
pub struct Packed {
1313
_pad: [u8; 5],

tests/mir-opt/const_allocation3.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ fn main() {
77
FOO;
88
}
99

10-
#[repr(packed)]
10+
#[repr(C, packed)]
1111
struct Packed {
1212
a: [u8; 28],
1313
b: &'static i32,

0 commit comments

Comments
 (0)