Skip to content
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

fix failure to detect a too-big-type after adding padding #117277

Merged
merged 1 commit into from
Oct 28, 2023
Merged

fix failure to detect a too-big-type after adding padding #117277

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 5 additions & 0 deletions compiler/rustc_abi/src/layout.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -539,6 +539,7 @@ pub trait LayoutCalculator {
// Align the maximum variant size to the largest alignment.
size = size.align_to(align.abi);

// FIXME(oli-obk): deduplicate and harden these checks
if size.bytes() >= dl.obj_size_bound() {
return None;
}
Expand Down Expand Up @@ -1103,6 +1104,10 @@ fn univariant<
inverse_memory_index.into_iter().map(|it| it.index() as u32).collect()
};
let size = min_size.align_to(align.abi);
// FIXME(oli-obk): deduplicate and harden these checks
if size.bytes() >= dl.obj_size_bound() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels like an arbitrary place. And now we have two of these checks. Can we instead do this in a single place before producing any LayoutS? Or maybe even making align_to and other size operations fallible?

Copy link
Member Author

@RalfJung RalfJung Oct 27, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand the structure of this code, so I'm afraid I don't know where else to put it. This is in univariant which sounds reasonably canonical? The other one is somewhere in the enum code I think. 🤷 There's a lot of mutable state and important logic hidden in functions many hundred lines long -- the code is very hard to see through and I'm not about to do a complete refactor here.

If you can tell me another place to put it I am happy yo move it, but me picking one random place after the other until you are happy does not sound like a good strategy. ;)

Or maybe even making align_to and other size operations fallible?

Currently Size has two kinds of operations -- some check the obj-size-bound and some don't. I don't know why, but maybe some part of the compiler needs to work with larger sizes? No idea.

I can make align_to take a dl so that it can do the check. But that makes little sense when Size implements * and + which can't do the check since they don't have a dl.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently Size has two kinds of operations -- some check the obj-size-bound and some don't. I don't know why, but maybe some part of the compiler needs to work with larger sizes? No idea.

most of these are likely just ad-hoc

If you can tell me another place to put it I am happy yo move it, but me picking one random place after the other until you are happy does not sound like a good strategy. ;)

😆 I'm not sure where to put it either, needs experimentation. So... just leave a FIXME(oli-obk): deduplicate and harden these checks on both checks

r=me with fixme comment

return None;
}
let mut layout_of_single_non_zst_field = None;
let mut abi = Abi::Aggregate { sized };
// Try to make this a Scalar/ScalarPair.
Expand Down
3 changes: 3 additions & 0 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 @@ -19,6 +19,9 @@ pub(super) fn sanity_check_layout<'tcx>(
if layout.size.bytes() % layout.align.abi.bytes() != 0 {
bug!("size is not a multiple of align, in the following layout:\n{layout:#?}");
}
if layout.size.bytes() >= cx.tcx.data_layout.obj_size_bound() {
bug!("size is too large, in the following layout:\n{layout:#?}");
}

if !cfg!(debug_assertions) {
// Stop here, the rest is kind of expensive.
Expand Down
18 changes: 18 additions & 0 deletions tests/ui/layout/too-big-with-padding.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// build-fail
// compile-flags: --target i686-unknown-linux-gnu --crate-type lib
// needs-llvm-components: x86
#![feature(no_core, lang_items)]
#![allow(internal_features)]
#![no_std]
#![no_core]

// 0x7fffffff is fine, but after rounding up it becomes too big
#[repr(C, align(2))]
pub struct Example([u8; 0x7fffffff]);

pub fn lib(_x: Example) {} //~ERROR: too big for the current architecture

#[lang = "sized"]
pub trait Sized {}
#[lang = "copy"]
pub trait Copy: Sized {}
8 changes: 8 additions & 0 deletions tests/ui/layout/too-big-with-padding.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
error: values of the type `Example` are too big for the current architecture
--> $DIR/too-big-with-padding.rs:13:1
|
LL | pub fn lib(_x: Example) {}
| ^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to previous error

Loading