fix ICE in CMSE type validation#130064
Conversation
rustbot
added
S-waiting-on-review
|
HIR ty lowering was modified cc @fmease |
| fn is_valid_cmse_inputs<'tcx>( | ||
| tcx: TyCtxt<'tcx>, | ||
| fn_sig: ty::PolyFnSig<'tcx>, | ||
| fn_sig: ty::FnSig<'tcx>, |
There was a problem hiding this comment.
One tweak -- can you change this back to PolyFnSig and do the binder instantiate call inside this fn?
| for (index, arg_def) in fn_sig.inputs().iter().enumerate() { | ||
| let layout = tcx.layout_of(ParamEnv::reveal_all().and(*arg_def.skip_binder()))?; | ||
| for (index, ty) in fn_sig.inputs().iter().enumerate() { | ||
| let layout = tcx.layout_of(ParamEnv::reveal_all().and(*ty))?; |
There was a problem hiding this comment.
Side-note: this param-env is not right, so it'll probably ICE in other cases...
There was a problem hiding this comment.
yeah, #130104 was just reported and is not fixed by this branch (currently). How do we get a correct ParamEnv here?
There was a problem hiding this comment.
actually that second ICE is due to encountering Infer being considered a bug in layout computation
rust/compiler/rustc_ty_utils/src/layout.rs
Lines 677 to 683 in 263a3ae
Moving ty::Infer(_) to the ty::Placeholder(..) branch fixes that problem, but might hide bugs in other cases?
There was a problem hiding this comment.
yeah, that is 100% not the right fix. the problem here is that we need to delay the calculation of these layouts until after type-checking is done for types that show up.
There was a problem hiding this comment.
here #127814 (comment) is where some discussion around the current design happened. Is there an existing way to perform that traversal after type checking to perform this layout check?
|
pls in the future squash changes like this into one commit, a one-liner is not really worth the additional commit lol |
|
@bors r+ rollup |
bors
added
S-waiting-on-bors
…piler-errors fix ICE in CMSE type validation fixes rust-lang#129983 tracking issue: rust-lang#81391 r? `@compiler-errors`
…iaskrgr Rollup of 9 pull requests Successful merges: - rust-lang#129929 (`rustc_mir_transform` cleanups, round 2) - rust-lang#130022 (Dataflow/borrowck lifetime cleanups) - rust-lang#130064 (fix ICE in CMSE type validation) - rust-lang#130067 (Remove redundant check in `symlink_hard_link` test) - rust-lang#130131 (Print a helpful message if any tests were skipped for being up-to-date) - rust-lang#130137 (Fix ICE caused by missing span in a region error) - rust-lang#130153 (use verbose flag as a default value for `rust.verbose-tests`) - rust-lang#130154 (Stabilize `char::MIN`) - rust-lang#130158 (Update books) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#130064 - folkertdev:fix-issue-129983, r=compiler-errors fix ICE in CMSE type validation fixes rust-lang#129983 tracking issue: rust-lang#81391 r? ``@compiler-errors``
folkertdev
added
the
F-abi_cmse_nonsecure_call
4 participants
fixes #129983
tracking issue: #81391
r? @compiler-errors