Skip to content

Fix ICE in read_discriminant for enums with non-contiguous discriminants#154776

Merged
rust-bors[bot] merged 1 commit intorust-lang:mainfrom
enthropy7:fix-ice-discriminant-none
Apr 4, 2026
Merged

Fix ICE in read_discriminant for enums with non-contiguous discriminants#154776
rust-bors[bot] merged 1 commit intorust-lang:mainfrom
enthropy7:fix-ice-discriminant-none

Conversation

@enthropy7
Copy link
Copy Markdown
Contributor

@enthropy7 enthropy7 commented Apr 3, 2026 •
edited by rustbot
Loading

so, investigation of #153758 took a while. it seems, that reverting back to older approach is the best we can do there.

read_discriminant ICEs (unwrap on None) when an enum's valid_range is wider than its set of actual discriminants. this happens for enums with gaps in their discriminant values — e.g. {0, 2, 3, 4, 5} has valid_range 0..=5 `` which includes 1, or `{i64::MIN, i64::MAX}` has a wrapping range covering everything. b840338 commit added a `valid_range` check and replaced the prior `.ok_or_else(|| err_ub!(InvalidTag(...)))` with `.unwrap()`, assuming the range check would guarantee a matching variant. that assumption is wrong for non-contiguous discriminants. my fix restores the fallible lookup, returning `InvalidTag` UB instead of panicking. also it affected crates - sec1, ntex-mqtt, when compiled with optimizations, where the `jump_threading` MIR pass constructs constants with tag values inside the `valid_range` but not matching any actual variant.

@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented Apr 3, 2026

Some changes occurred to the CTFE machinery

cc @RalfJung, @oli-obk, @lcnr

Some changes occurred to the CTFE / Miri interpreter

cc @rust-lang/miri

@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 Apr 3, 2026
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented Apr 3, 2026

r? @mati865

rustbot has assigned @mati865.
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 69 candidates
  • Random selection from 11 candidates

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@enthropy7 enthropy7 force-pushed the fix-ice-discriminant-none branch from 72cf8ac to a082578 Compare April 3, 2026 19:36
RalfJung
RalfJung reviewed Apr 4, 2026
View reviewed changes
tests/ui/consts/const-eval/ub-enum.rs
Copy link
Copy Markdown
Member

@RalfJung RalfJung Apr 4, 2026

Choose a reason for hiding this comment

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

What does this have to do with #153758 ? #153758 is about a program that doesn't even typecheck, which then erroneously reaches the interpreter. But here you have an example that apparently does typecheck?

RalfJung
RalfJung reviewed Apr 4, 2026
View reviewed changes
compiler/rustc_const_eval/src/interpret/discriminant.rs
// layout range above, this cannot fail.
// Convert discriminant to variant index. The tag may pass the layout range
// check above but still not match any actual variant discriminant (e.g.,
// non-contiguous discriminants with a wrapping valid_range).
Copy link
Copy Markdown
Member

@RalfJung RalfJung Apr 4, 2026

Choose a reason for hiding this comment

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

Good catch! I got that wrong in b840338.

@RalfJung
Copy link
Copy Markdown
Member

RalfJung commented Apr 4, 2026

@bors r+ rollup

The change and test look good, thanks. I'm not sure of the relation to #153758 but the PR also isn't marked as fixing that issue so that's not a blocker. The ICE site is indeed exactly what this PR works on so -- I guess it could really fix that issue as well. Did you manually check that?

@rust-bors
Copy link
Copy Markdown
Contributor

rust-bors bot commented Apr 4, 2026

📌 Commit a082578 has been approved by RalfJung

It is now in the queue for this repository.

@rust-bors rust-bors bot 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 Apr 4, 2026
@matthiaskrgr matthiaskrgr mentioned this pull request Apr 4, 2026
Merged
@enthropy7
Copy link
Copy Markdown
Contributor Author

enthropy7 commented Apr 4, 2026

yes, I've tested it - all three reproducers from #153758 (original, refined, and with generic_assert) ICE at the same line 128 without the fix. the path is assert_eq! → promoted constant → KnownPanicsLint/ConstPropagator → read_discriminant. the code has type errors, but the compiler still attempts MIR optimization and hits the unwrap.

@RalfJung
Copy link
Copy Markdown
Member

RalfJung commented Apr 4, 2026

the code has type errors, but the compiler still attempts MIR optimization and hits the unwrap.

FWIW that is also a bug that should be fixed IMO.

@enthropy7
Copy link
Copy Markdown
Contributor Author

enthropy7 commented Apr 4, 2026

the code has type errors, but the compiler still attempts MIR optimization and hits the unwrap.

FWIW that is also a bug that should be fixed IMO.

agreed, that's a separate issue though, i will work on this

rust-bors bot pushed a commit that referenced this pull request Apr 4, 2026
@bors
Auto merge of #154802 - matthiaskrgr:rollup-HWNmEyC, r=matthiaskrgr
0312a55 
Rollup of 5 pull requests

Successful merges:

 - #154376 (Remove more BuiltinLintDiag variants - part 4)
 - #154731 (llvm: Fix array ABI test to not check equality implementation)
 - #127534 (feat(core): impl Step for NonZero<u*>)
 - #154703 (Fix trailing comma in lifetime suggestion for empty angle brackets)
 - #154776 (Fix ICE in read_discriminant for enums with non-contiguous discriminants)
@rust-bors rust-bors bot merged commit 7f18b25 into rust-lang:main Apr 4, 2026
11 checks passed
rust-timer added a commit that referenced this pull request Apr 4, 2026
@rust-timer
Unrolled build for #154776
60015ea 
Rollup merge of #154776 - enthropy7:fix-ice-discriminant-none, r=RalfJung

Fix ICE in read_discriminant for enums with non-contiguous discriminants

so, investigation of #153758 took a while. it seems, that reverting back to older approach is the best we can do there.

> `read_discriminant `ICEs (unwrap on `None`) when an enum's `valid_range` is wider than its set of actual discriminants. this happens for enums with gaps in their discriminant values — e.g. `{0, 2, 3, 4, 5} `has `` valid_range`` 0..=5 `` which includes `1`, or `{i64::MIN, i64::MAX}` has a wrapping range covering everything. [b840338](b840338) commit added a `valid_range` check and replaced the prior `.ok_or_else(|| err_ub!(InvalidTag(...)))` with `.unwrap()`, assuming the range check would guarantee a matching variant. that assumption is wrong for non-contiguous discriminants. my fix restores the fallible lookup, returning `InvalidTag` UB instead of panicking. also it affected crates - **sec1, ntex-mqtt**, when compiled with optimizations, where the `jump_threading` MIR pass constructs constants with tag values inside the `valid_range` but not matching any actual variant.
@rustbot rustbot added this to the 1.96.0 milestone Apr 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@RalfJung RalfJung RalfJung left review comments

Assignees

@mati865 mati865

Labels

S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Milestone

1.96.0

Development

Successfully merging this pull request may close these issues.

5 participants

@enthropy7 @rustbot @rust-log-analyzer @RalfJung @mati865