Skip to content

stricter alignment enforcement for ScalarPair and Vector #105006

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Nov 28, 2022
Merged

stricter alignment enforcement for ScalarPair and Vector #105006

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
58dd62a
sanity_check_layout: less rightwards drift
RalfJung Nov 28, 2022
891a4da
stricter alignment enforcement for ScalarPair and Vector
RalfJung Nov 28, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 36 additions & 26 deletions compiler/rustc_ty_utils/src/layout_sanity_check.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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>>,
Expand Down Expand Up @@ -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:#?}",
Expand All @@ -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!(
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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.
}
Expand Down Expand Up @@ -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)) => {
Expand All @@ -300,4 +311,3 @@ pub(super) fn sanity_check_layout<'tcx>(
}
}
}
}