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

RalfJung · 2023-10-27T11:20:16Z

rustbot · 2023-10-27T11:20:25Z

(rustbot has picked a reviewer for you, use r? to override)

oli-obk · 2023-10-27T15:27:15Z

compiler/rustc_abi/src/layout.rs

@@ -1103,6 +1103,9 @@ fn univariant<
        inverse_memory_index.into_iter().map(|it| it.index() as u32).collect()
    };
    let size = min_size.align_to(align.abi);
+    if size.bytes() >= dl.obj_size_bound() {


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?

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.

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

RalfJung · 2023-10-27T16:09:09Z

@bors r=oli-obk

bors · 2023-10-27T16:09:11Z

📌 Commit 2ef5897 has been approved by oli-obk

It is now in the queue for this repository.

…li-obk fix failure to detect a too-big-type after adding padding Fixes rust-lang#117265

…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#116534 (Remove -Zdep-tasks.) - rust-lang#116609 (Bump stdarch submodule and remove special handling for LLVM intrinsics that are no longer needed) - rust-lang#116739 (Make `E0277` use short paths) - rust-lang#116945 (When encountering sealed traits, point types that implement it) - rust-lang#117025 (Cleanup and improve `--check-cfg` implementation) - rust-lang#117029 (Add FileCheck annotations to MIR-opt inlining tests ) - rust-lang#117256 (Parse rustc version at compile time) - rust-lang#117277 (fix failure to detect a too-big-type after adding padding) r? `@ghost` `@rustbot` modify labels: rollup

…kingjubilee Rollup of 8 pull requests Successful merges: - rust-lang#116534 (Remove -Zdep-tasks.) - rust-lang#116739 (Make `E0277` use short paths) - rust-lang#116816 (Create `windows/api.rs` for safer FFI) - rust-lang#116945 (When encountering sealed traits, point types that implement it) - rust-lang#117025 (Cleanup and improve `--check-cfg` implementation) - rust-lang#117256 (Parse rustc version at compile time) - rust-lang#117268 (`rustc_interface` cleanups) - rust-lang#117277 (fix failure to detect a too-big-type after adding padding) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#117277 - RalfJung:too-big-with-padding, r=oli-obk fix failure to detect a too-big-type after adding padding Fixes rust-lang#117265

rustbot assigned b-naber Oct 27, 2023

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Oct 27, 2023

RalfJung force-pushed the too-big-with-padding branch 3 times, most recently from 5306028 to 097b7ce Compare October 27, 2023 11:23

oli-obk reviewed Oct 27, 2023

View reviewed changes

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

2ef5897

RalfJung force-pushed the too-big-with-padding branch from 097b7ce to 2ef5897 Compare October 27, 2023 16:08

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 27, 2023

matthiaskrgr mentioned this pull request Oct 28, 2023

Rollup of 8 pull requests #117308

Closed

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Oct 28, 2023

Rollup merge of rust-lang#117277 - RalfJung:too-big-with-padding, r=o…

b0c996a

…li-obk fix failure to detect a too-big-type after adding padding Fixes rust-lang#117265

workingjubilee mentioned this pull request Oct 28, 2023

Rollup of 8 pull requests #117309

Merged

bors merged commit 09fd68d into rust-lang:master Oct 28, 2023
11 checks passed

rustbot added this to the 1.75.0 milestone Oct 28, 2023

RalfJung deleted the too-big-with-padding branch October 29, 2023 08:35

github-actions bot mentioned this pull request Oct 31, 2023

Automated pull from upstream master ferrocene/ferrocene#74

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

RalfJung commented Oct 27, 2023

rustbot commented Oct 27, 2023

oli-obk Oct 27, 2023

RalfJung Oct 27, 2023 •

edited

oli-obk Oct 27, 2023

RalfJung commented Oct 27, 2023

bors commented Oct 27, 2023

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

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

Conversation

RalfJung commented Oct 27, 2023

rustbot commented Oct 27, 2023

oli-obk Oct 27, 2023

Choose a reason for hiding this comment

RalfJung Oct 27, 2023 • edited

Choose a reason for hiding this comment

oli-obk Oct 27, 2023

Choose a reason for hiding this comment

RalfJung commented Oct 27, 2023

bors commented Oct 27, 2023

RalfJung Oct 27, 2023 •

edited