1
Fork 0

Rollup merge of #105006 - RalfJung:scalar-pair-alignment, r=eddyb

stricter alignment enforcement for ScalarPair

`@eddyb` [indicated](https://github.com/rust-lang/rust/pull/103926#discussion_r1033315005) that alignment violating this check might be a bug. So let's see what the test suite says.

(Only the 2nd commit actually changes behavior... but I couldn't not do that other cleanup.^^)

Does the PR CI runner even enable debug assertions though...?
This commit is contained in:
Matthias Krüger 2022-11-28 17:25:50 +01:00 committed by GitHub
commit 3dfb6ca8e2
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23

View file

@ -20,7 +20,11 @@ pub(super) fn sanity_check_layout<'tcx>(
bug!("size is not a multiple of align, in the following layout:\n{layout:#?}");
}
if cfg!(debug_assertions) {
if !cfg!(debug_assertions) {
// Stop here, the rest is kind of expensive.
return;
}
/// Yields non-ZST fields of the type
fn non_zst_fields<'tcx, 'a>(
cx: &'a LayoutCx<'tcx, TyCtxt<'tcx>>,
@ -115,10 +119,7 @@ pub(super) fn sanity_check_layout<'tcx>(
Size::ZERO,
"`Scalar` field at non-0 offset in {inner:#?}",
);
assert_eq!(
field.size, size,
"`Scalar` field with bad size in {inner:#?}",
);
assert_eq!(field.size, size, "`Scalar` field with bad size in {inner:#?}",);
assert_eq!(
field.align.abi, align,
"`Scalar` field with bad align in {inner:#?}",
@ -134,22 +135,24 @@ pub(super) fn sanity_check_layout<'tcx>(
}
}
Abi::ScalarPair(scalar1, scalar2) => {
// Sanity-check scalar pairs. These are a bit more flexible and support
// padding, but we can at least ensure both fields actually fit into the layout
// and the alignment requirement has not been weakened.
// Sanity-check scalar pairs. Computing the expected size and alignment is a bit of work.
let size1 = scalar1.size(cx);
let align1 = scalar1.align(cx).abi;
let size2 = scalar2.size(cx);
let align2 = scalar2.align(cx).abi;
assert!(
layout.layout.align().abi >= cmp::max(align1, align2),
"alignment mismatch between ABI and layout in {layout:#?}",
);
let align = cmp::max(align1, align2);
let field2_offset = size1.align_to(align2);
assert!(
layout.layout.size() >= field2_offset + size2,
let size = (field2_offset + size2).align_to(align);
assert_eq!(
layout.layout.size(),
size,
"size mismatch between ABI and layout in {layout:#?}"
);
assert_eq!(
layout.layout.align().abi,
align,
"alignment mismatch between ABI and layout in {layout:#?}",
);
// Check that the underlying pair of fields matches.
let inner = skip_newtypes(cx, layout);
assert!(
@ -177,10 +180,14 @@ pub(super) fn sanity_check_layout<'tcx>(
}
let mut fields = non_zst_fields(cx, &inner);
let (offset1, field1) = fields.next().unwrap_or_else(|| {
panic!("`ScalarPair` layout for type with not even one non-ZST field: {inner:#?}")
panic!(
"`ScalarPair` layout for type with not even one non-ZST field: {inner:#?}"
)
});
let (offset2, field2) = fields.next().unwrap_or_else(|| {
panic!("`ScalarPair` layout for type with less than two non-ZST fields: {inner:#?}")
panic!(
"`ScalarPair` layout for type with less than two non-ZST fields: {inner:#?}"
)
});
assert!(
fields.next().is_none(),
@ -228,17 +235,22 @@ pub(super) fn sanity_check_layout<'tcx>(
);
}
Abi::Vector { count, element } => {
// No padding in vectors. Alignment can be strengthened, though.
assert!(
layout.layout.align().abi >= element.align(cx).abi,
"alignment mismatch between ABI and layout in {layout:#?}"
);
// No padding in vectors, except possibly for trailing padding to make the size a multiple of align.
let size = element.size(cx) * count;
let align = cx.data_layout().vector_align(size).abi;
let size = size.align_to(align); // needed e.g. for vectors of size 3
assert!(align >= element.align(cx).abi); // just sanity-checking `vector_align`.
assert_eq!(
layout.layout.size(),
size.align_to(cx.data_layout().vector_align(size).abi),
size,
"size mismatch between ABI and layout in {layout:#?}"
);
assert_eq!(
layout.layout.align().abi,
align,
"alignment mismatch between ABI and layout in {layout:#?}"
);
// FIXME: Do some kind of check of the inner type, like for Scalar and ScalarPair.
}
Abi::Uninhabited | Abi::Aggregate { .. } => {} // Nothing to check.
}
@ -279,9 +291,8 @@ pub(super) fn sanity_check_layout<'tcx>(
continue;
}
// The top-level ABI and the ABI of the variants should be coherent.
let scalar_coherent = |s1: Scalar, s2: Scalar| {
s1.size(cx) == s2.size(cx) && s1.align(cx) == s2.align(cx)
};
let scalar_coherent =
|s1: Scalar, s2: Scalar| s1.size(cx) == s2.size(cx) && s1.align(cx) == s2.align(cx);
let abi_coherent = match (layout.abi, variant.abi) {
(Abi::Scalar(s1), Abi::Scalar(s2)) => scalar_coherent(s1, s2),
(Abi::ScalarPair(a1, b1), Abi::ScalarPair(a2, b2)) => {
@ -299,5 +310,4 @@ pub(super) fn sanity_check_layout<'tcx>(
}
}
}
}
}