Skip to content

Avoid const-eval ICEs from invalid CoerceShared impls - #159497

Open
chenyukang wants to merge 2 commits into
rust-lang:mainfrom
chenyukang:yukang-fix-158149-reborrow-invalid-coercion
Open

Avoid const-eval ICEs from invalid CoerceShared impls#159497
chenyukang wants to merge 2 commits into
rust-lang:mainfrom
chenyukang:yukang-fix-158149-reborrow-invalid-coercion

Conversation

@chenyukang

@chenyukang chenyukang commented Jul 18, 2026

Copy link
Copy Markdown
Member

Fixes #158149

Do not allow malformed coercion MIR to reach CTFE, which may trigger a size-mismatch ICE.

@rustbot 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 Jul 18, 2026
@rustbot

rustbot commented Jul 18, 2026

Copy link
Copy Markdown
Collaborator

r? @oli-obk

rustbot has assigned @oli-obk.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: compiler
  • compiler expanded to 74 candidates
  • Random selection from 16 candidates

@aapoalas aapoalas left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't know about the error taint stuff and all that, but from a Reborrow-traits point of view this seems fine.

View changes since this review

Comment on lines +2521 to +2524
let coerce_shared = tcx.require_lang_item(LangItem::CoerceShared, body_span);
tcx.for_each_relevant_impl(coerce_shared, borrowed_ty, |impl_def_id| {
if let Some(impl_def_id) = impl_def_id.as_local()
&& let Err(guar) = tcx.ensure_result().coerce_shared_info(impl_def_id)

@oli-obk oli-obk Jul 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

maybe we can get this automatically everywhere by loading coerce_shared_info when the trait solver looks at an impl of CoerceShared and tainting the infcx there. It is fragile to only do it here.

View changes since the review

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I moved the check to match_impl, should be a better location.

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 21, 2026
@chenyukang
chenyukang force-pushed the yukang-fix-158149-reborrow-invalid-coercion branch from f6918ff to 9996b9d Compare July 21, 2026 11:26
@rustbot

rustbot commented Jul 21, 2026

Copy link
Copy Markdown
Collaborator

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

@oli-obk oli-obk left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The new solver needs the same treatment, I think. Make the ice test have a revision for both solvers, too

View changes since this review

return Err(());
}

if self.tcx().is_lang_item(obligation.predicate.def_id(), LangItem::CoerceShared)

@oli-obk oli-obk Jul 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Doesn't selection already check this language item somewhere? Add the check there

View changes since the review

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@chenyukang chenyukang Jul 21, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I moved the check into coerce_shared_reborrow

now we can make these tests no ICE:
912df9a

but I find another testcase will be ICE which we can not cover for this fix(also upstream main):

#![feature(reborrow)]
#![allow(dead_code)]
use std::marker::CoerceShared;

struct InvalidMut<'a>(&'a u8);

#[derive(Copy, Clone)]
struct InvalidRef<'a>(&'a ());

fn invalid<'a>(value: InvalidMut<'a>)
where
    InvalidMut<'a>: CoerceShared<InvalidRef<'a>>,
{
    let _: InvalidRef<'a> = value;
    //~^ ERROR mismatched types
}

fn main() {}

the reason is we haven't do enough validation for ImplSource::Param.
cc @aapoalas

@rust-log-analyzer

This comment has been minimized.

@chenyukang
chenyukang force-pushed the yukang-fix-158149-reborrow-invalid-coercion branch from 9996b9d to 912df9a Compare July 21, 2026 16:30
@rustbot

rustbot commented Jul 21, 2026

Copy link
Copy Markdown
Collaborator

This PR changes a file inside tests/crashes. If a crash was fixed, please move into the corresponding ui subdir and add 'Fixes #' to the PR description to autoclose the issue upon merge.

@rust-bors

rust-bors Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

☔ The latest upstream changes (presumably #160586) made this pull request unmergeable. Please resolve the merge conflicts by rebasing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[ICE]: reborrow: size mismatch: compiler/rustc_const_eval/src/interpret/place.rs

5 participants