Skip to content

Commit 2c11c3d

Browse files
RalfJungRalfJung
committed
make sure ScalarPair enums have ScalarPair variants; add some layout sanity checks
1 parent 8a2fe75 commit 2c11c3d

File tree

3 files changed

+221
-32
lines changed

3 files changed

+221
-32
lines changed

‎compiler/rustc_middle/src/ty/layout.rs

+112-11
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,91 @@ impl<'tcx> fmt::Display for LayoutError<'tcx> {
220220
}
221221
}
222222

223+
/// Enforce some basic invariants on layouts.
224+
fn sanity_check_layout<'tcx>(
225+
tcx: TyCtxt<'tcx>,
226+
param_env: ty::ParamEnv<'tcx>,
227+
layout: &TyAndLayout<'tcx>,
228+
) {
229+
// Type-level uninhabitedness should always imply ABI uninhabitedness.
230+
if tcx.conservative_is_privately_uninhabited(param_env.and(layout.ty)) {
231+
assert!(layout.abi.is_uninhabited());
232+
}
233+
234+
if cfg!(debug_assertions) {
235+
fn check_layout_abi<'tcx>(tcx: TyCtxt<'tcx>, layout: Layout<'tcx>) {
236+
match layout.abi() {
237+
Abi::Scalar(_scalar) => {
238+
// No padding in scalars.
239+
/* FIXME(#96185):
240+
assert_eq!(
241+
layout.align().abi,
242+
scalar.align(&tcx).abi,
243+
"alignment mismatch between ABI and layout in {layout:#?}"
244+
);
245+
assert_eq!(
246+
layout.size(),
247+
scalar.size(&tcx),
248+
"size mismatch between ABI and layout in {layout:#?}"
249+
);*/
250+
}
251+
Abi::ScalarPair(scalar1, scalar2) => {
252+
// Sanity-check scalar pair size.
253+
let field2_offset = scalar1.size(&tcx).align_to(scalar2.align(&tcx).abi);
254+
let total = field2_offset + scalar2.size(&tcx);
255+
assert!(
256+
layout.size() >= total,
257+
"size mismatch between ABI and layout in {layout:#?}"
258+
);
259+
}
260+
_ => {}
261+
}
262+
}
263+
264+
check_layout_abi(tcx, layout.layout);
265+
266+
if let Variants::Multiple { variants, .. } = &layout.variants {
267+
for variant in variants {
268+
check_layout_abi(tcx, *variant);
269+
// No nested "multiple".
270+
assert!(matches!(variant.variants(), Variants::Single { .. }));
271+
// Skip empty variants.
272+
if variant.size() == Size::ZERO
273+
|| variant.fields().count() == 0
274+
|| variant.abi().is_uninhabited()
275+
{
276+
// These are never actually accessed anyway, so we can skip them. (Note that
277+
// sometimes, variants with fields have size 0, and sometimes, variants without
278+
// fields have non-0 size.)
279+
continue;
280+
}
281+
// Variants should have the same or a smaller size as the full thing.
282+
if variant.size() > layout.size {
283+
bug!(
284+
"Type with size {} bytes has variant with size {} bytes: {layout:#?}",
285+
layout.size.bytes(),
286+
variant.size().bytes(),
287+
)
288+
}
289+
// The top-level ABI and the ABI of the variants should be coherent.
290+
let abi_coherent = match (layout.abi, variant.abi()) {
291+
(Abi::Scalar(..), Abi::Scalar(..)) => true,
292+
(Abi::ScalarPair(..), Abi::ScalarPair(..)) => true,
293+
(Abi::Uninhabited, _) => true,
294+
(Abi::Aggregate { .. }, _) => true,
295+
_ => false,
296+
};
297+
if !abi_coherent {
298+
bug!(
299+
"Variant ABI is incompatible with top-level ABI:\nvariant={:#?}\nTop-level: {layout:#?}",
300+
variant
301+
);
302+
}
303+
}
304+
}
305+
}
306+
}
307+
223308
#[instrument(skip(tcx, query), level = "debug")]
224309
fn layout_of<'tcx>(
225310
tcx: TyCtxt<'tcx>,
@@ -263,10 +348,7 @@ fn layout_of<'tcx>(
263348

264349
cx.record_layout_for_printing(layout);
265350

266-
// Type-level uninhabitedness should always imply ABI uninhabitedness.
267-
if tcx.conservative_is_privately_uninhabited(param_env.and(ty)) {
268-
assert!(layout.abi.is_uninhabited());
269-
}
351+
sanity_check_layout(tcx, param_env, &layout);
270352

271353
Ok(layout)
272354
})
@@ -1313,10 +1395,22 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> {
13131395
};
13141396
let mut abi = Abi::Aggregate { sized: true };
13151397

1316-
// Without latter check aligned enums with custom discriminant values
1317-
// Would result in ICE see the issue #92464 for more info
1318-
if tag.size(dl) == size || variants.iter().all(|layout| layout.is_empty()) {
1398+
if layout_variants.iter().all(|v| v.abi.is_uninhabited()) {
1399+
abi = Abi::Uninhabited;
1400+
} else if tag.size(dl) == size || variants.iter().all(|layout| layout.is_empty()) {
1401+
// Without latter check aligned enums with custom discriminant values
1402+
// Would result in ICE see the issue #92464 for more info
13191403
abi = Abi::Scalar(tag);
1404+
// Make sure the variants with fields have the same ABI as the enum itself
1405+
// (since downcasting to them is a NOP).
1406+
for variant in &mut layout_variants {
1407+
if variant.fields.count() > 0
1408+
&& matches!(variant.abi, Abi::Aggregate { .. })
1409+
{
1410+
assert_eq!(variant.size, size);
1411+
variant.abi = abi;
1412+
}
1413+
}
13201414
} else {
13211415
// Try to use a ScalarPair for all tagged enums.
13221416
let mut common_prim = None;
@@ -1385,14 +1479,21 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> {
13851479
// We can use `ScalarPair` only when it matches our
13861480
// already computed layout (including `#[repr(C)]`).
13871481
abi = pair.abi;
1482+
// Make sure the variants with fields have the same ABI as the enum itself
1483+
// (since downcasting to them is a NOP).
1484+
for variant in &mut layout_variants {
1485+
if variant.fields.count() > 0
1486+
&& matches!(variant.abi, Abi::Aggregate { .. })
1487+
{
1488+
variant.abi = abi;
1489+
// Also need to bump up the size, so that the pair fits inside.
1490+
variant.size = size;
1491+
}
1492+
}
13881493
}
13891494
}
13901495
}
13911496

1392-
if layout_variants.iter().all(|v| v.abi.is_uninhabited()) {
1393-
abi = Abi::Uninhabited;
1394-
}
1395-
13961497
let largest_niche = Niche::from_scalar(dl, Size::ZERO, tag);
13971498

13981499
let layout_variants =

‎src/test/ui/layout/debug.stderr

+32-6
Original file line numberDiff line numberDiff line change
@@ -184,9 +184,22 @@ error: layout_of(std::result::Result<i32, i32>) = Layout {
184184
variants: Single {
185185
index: 0,
186186
},
187-
abi: Aggregate {
188-
sized: true,
189-
},
187+
abi: ScalarPair(
188+
Initialized {
189+
value: Int(
190+
I32,
191+
false,
192+
),
193+
valid_range: 0..=1,
194+
},
195+
Initialized {
196+
value: Int(
197+
I32,
198+
true,
199+
),
200+
valid_range: 0..=4294967295,
201+
},
202+
),
190203
largest_niche: None,
191204
align: AbiAndPrefAlign {
192205
abi: Align(4 bytes),
@@ -206,9 +219,22 @@ error: layout_of(std::result::Result<i32, i32>) = Layout {
206219
variants: Single {
207220
index: 1,
208221
},
209-
abi: Aggregate {
210-
sized: true,
211-
},
222+
abi: ScalarPair(
223+
Initialized {
224+
value: Int(
225+
I32,
226+
false,
227+
),
228+
valid_range: 0..=1,
229+
},
230+
Initialized {
231+
value: Int(
232+
I32,
233+
true,
234+
),
235+
valid_range: 0..=4294967295,
236+
},
237+
),
212238
largest_niche: None,
213239
align: AbiAndPrefAlign {
214240
abi: Align(4 bytes),

‎src/test/ui/layout/issue-96158-scalarpair-payload-might-be-uninit.stderr

+77-15
Original file line numberDiff line numberDiff line change
@@ -30,9 +30,21 @@ error: layout_of(MissingPayloadField) = Layout {
3030
variants: Single {
3131
index: 0,
3232
},
33-
abi: Aggregate {
34-
sized: true,
35-
},
33+
abi: ScalarPair(
34+
Initialized {
35+
value: Int(
36+
I8,
37+
false,
38+
),
39+
valid_range: 0..=1,
40+
},
41+
Union {
42+
value: Int(
43+
I8,
44+
false,
45+
),
46+
},
47+
),
3648
largest_niche: None,
3749
align: AbiAndPrefAlign {
3850
abi: Align(1 bytes),
@@ -131,9 +143,22 @@ error: layout_of(CommonPayloadField) = Layout {
131143
variants: Single {
132144
index: 0,
133145
},
134-
abi: Aggregate {
135-
sized: true,
136-
},
146+
abi: ScalarPair(
147+
Initialized {
148+
value: Int(
149+
I8,
150+
false,
151+
),
152+
valid_range: 0..=1,
153+
},
154+
Initialized {
155+
value: Int(
156+
I8,
157+
false,
158+
),
159+
valid_range: 0..=255,
160+
},
161+
),
137162
largest_niche: None,
138163
align: AbiAndPrefAlign {
139164
abi: Align(1 bytes),
@@ -153,9 +178,22 @@ error: layout_of(CommonPayloadField) = Layout {
153178
variants: Single {
154179
index: 1,
155180
},
156-
abi: Aggregate {
157-
sized: true,
158-
},
181+
abi: ScalarPair(
182+
Initialized {
183+
value: Int(
184+
I8,
185+
false,
186+
),
187+
valid_range: 0..=1,
188+
},
189+
Initialized {
190+
value: Int(
191+
I8,
192+
false,
193+
),
194+
valid_range: 0..=255,
195+
},
196+
),
159197
largest_niche: None,
160198
align: AbiAndPrefAlign {
161199
abi: Align(1 bytes),
@@ -237,9 +275,21 @@ error: layout_of(CommonPayloadFieldIsMaybeUninit) = Layout {
237275
variants: Single {
238276
index: 0,
239277
},
240-
abi: Aggregate {
241-
sized: true,
242-
},
278+
abi: ScalarPair(
279+
Initialized {
280+
value: Int(
281+
I8,
282+
false,
283+
),
284+
valid_range: 0..=1,
285+
},
286+
Union {
287+
value: Int(
288+
I8,
289+
false,
290+
),
291+
},
292+
),
243293
largest_niche: None,
244294
align: AbiAndPrefAlign {
245295
abi: Align(1 bytes),
@@ -259,9 +309,21 @@ error: layout_of(CommonPayloadFieldIsMaybeUninit) = Layout {
259309
variants: Single {
260310
index: 1,
261311
},
262-
abi: Aggregate {
263-
sized: true,
264-
},
312+
abi: ScalarPair(
313+
Initialized {
314+
value: Int(
315+
I8,
316+
false,
317+
),
318+
valid_range: 0..=1,
319+
},
320+
Union {
321+
value: Int(
322+
I8,
323+
false,
324+
),
325+
},
326+
),
265327
largest_niche: None,
266328
align: AbiAndPrefAlign {
267329
abi: Align(1 bytes),

0 commit comments

Comments
 (0)