Skip to content

Insert checks for enum discriminants when debug assertions are enabled #141759

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

Jump to bottom
Merged
merged 1 commit into from
Jun 28, 2025
Merged

Insert checks for enum discriminants when debug assertions are enabled #141759

merged 1 commit into from
Jun 28, 2025

Conversation

@1c3t3a
Copy link
Member

@1c3t3a 1c3t3a commented May 30, 2025 •
edited
Loading

Similar to the existing null-pointer and alignment checks, this checks for valid enum discriminants on creation of enums through unsafe transmutes. Essentially this sanitizes patterns like the following:

let val: MyEnum = unsafe { std::mem::transmute<u32, MyEnum>(42) };

An extension of this check will be done in a follow-up that explicitly sanitizes for extern enum values that come into Rust from e.g. C/C++.

This check is similar to Miri's capabilities of checking for valid construction of enum values.

This PR is inspired by saethlin@'s PR
#104862. Thank you so much for keeping this code up and the detailed comments!

I also pair-programmed large parts of this together with vabr-g@.

r? @saethlin

@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. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels May 30, 2025
@rustbot
Copy link
Collaborator

rustbot commented May 30, 2025

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

This PR changes MIR

cc @oli-obk, @RalfJung, @JakobDegen, @davidtwco, @vakaras

Some changes occurred in compiler/rustc_codegen_ssa

cc @WaffleLapkin

Some changes occurred in compiler/rustc_codegen_cranelift

cc @bjorn3

Some changes occurred to the CTFE machinery

cc @RalfJung, @oli-obk, @lcnr

rust-analyzer is developed in its own repository. If possible, consider making this change to rust-lang/rust-analyzer instead.

cc @rust-lang/rust-analyzer

@rustbot

This comment has been minimized.

Sign in to view
@1c3t3a 1c3t3a force-pushed the discriminants-query branch from 6d3fe75 to a7dd718 Compare May 30, 2025 09:46
@rustbot

This comment has been minimized.

Sign in to view
Veykril
Veykril reviewed May 30, 2025
View reviewed changes
@1c3t3a 1c3t3a force-pushed the discriminants-query branch from a7dd718 to 4f3342e Compare May 30, 2025 09:59
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@1c3t3a 1c3t3a force-pushed the discriminants-query branch from 54b6e74 to b03960e Compare May 30, 2025 13:33
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@1c3t3a 1c3t3a force-pushed the discriminants-query branch from b03960e to 228b656 Compare May 30, 2025 13:59
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@1c3t3a 1c3t3a force-pushed the discriminants-query branch from 228b656 to d1d8f88 Compare June 2, 2025 14:34
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@1c3t3a 1c3t3a force-pushed the discriminants-query branch from d1d8f88 to 93b24d7 Compare June 2, 2025 20:23
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@1c3t3a 1c3t3a force-pushed the discriminants-query branch from 93b24d7 to c2a8415 Compare June 3, 2025 12:31
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@1c3t3a 1c3t3a force-pushed the discriminants-query branch from c2a8415 to d769d6b Compare June 4, 2025 01:51
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@1c3t3a 1c3t3a force-pushed the discriminants-query branch from d769d6b to 68665ad Compare June 4, 2025 02:32
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@1c3t3a 1c3t3a force-pushed the discriminants-query branch from 68665ad to 1225079 Compare June 6, 2025 15:35
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@1c3t3a 1c3t3a force-pushed the discriminants-query branch from 1225079 to c52f534 Compare June 6, 2025 19:54
@bors
Copy link
Collaborator

bors commented Jun 28, 2025

⌛ Testing commit 1087042 with merge eac4ca2...

@matthiaskrgr
Copy link
Member

looks like its stuck
@bors retry r-

@bors bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jun 28, 2025
@matthiaskrgr
Copy link
Member

@bors r=@saethlin

@bors
Copy link
Collaborator

bors commented Jun 28, 2025

📌 Commit 1087042 has been approved by saethlin

It is now in the queue for this repository.

@bors 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 28, 2025
bors added a commit that referenced this pull request Jun 28, 2025
@bors
Auto merge of #141759 - 1c3t3a:discriminants-query, r=saethlin
1f9b61c 
Insert checks for enum discriminants when debug assertions are enabled

Similar to the existing null-pointer and alignment checks, this checks for valid enum discriminants on creation of enums through unsafe transmutes. Essentially this sanitizes patterns like the following:
```rust
let val: MyEnum = unsafe { std::mem::transmute<u32, MyEnum>(42) };
```

An extension of this check will be done in a follow-up that explicitly sanitizes for extern enum values that come into Rust from e.g. C/C++.

This check is similar to Miri's capabilities of checking for valid construction of enum values.

This PR is inspired by saethlin@'s PR
#104862. Thank you so much for keeping this code up and the detailed comments!

I also pair-programmed large parts of this together with vabr-g@.

r? `@saethlin`
@bors
Copy link
Collaborator

bors commented Jun 28, 2025

⌛ Testing commit 1087042 with merge 1f9b61c...

@bors
Copy link
Collaborator

bors commented Jun 28, 2025

⌛ Testing commit 1087042 with merge b63223c...

@bors
Copy link
Collaborator

bors commented Jun 28, 2025

☀️ Test successful - checks-actions
Approved by: saethlin
Pushing b63223c to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jun 28, 2025
@bors bors merged commit b63223c into rust-lang:master Jun 28, 2025
11 checks passed
@rustbot rustbot added this to the 1.90.0 milestone Jun 28, 2025
@github-actions
Copy link
Contributor

What is this? This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.

Comparing d41e12f (parent) -> b63223c (this PR)

Test differences

Show 56 test diffs

Stage 1

  • [ui] tests/ui/mir/enum/convert_non_enum_break.rs: [missing] -> pass (J0)
  • [ui] tests/ui/mir/enum/convert_non_enum_niche_break.rs: [missing] -> pass (J0)
  • [ui] tests/ui/mir/enum/convert_non_enum_niche_ok.rs: [missing] -> pass (J0)
  • [ui] tests/ui/mir/enum/convert_non_enum_ok.rs: [missing] -> pass (J0)
  • [ui] tests/ui/mir/enum/niche_option_tuple_break.rs: [missing] -> pass (J0)
  • [ui] tests/ui/mir/enum/niche_option_tuple_ok.rs: [missing] -> pass (J0)
  • [ui] tests/ui/mir/enum/numbered_variants_break.rs: [missing] -> pass (J0)
  • [ui] tests/ui/mir/enum/numbered_variants_ok.rs: [missing] -> pass (J0)
  • [ui] tests/ui/mir/enum/option_with_bigger_niche_break.rs: [missing] -> pass (J0)
  • [ui] tests/ui/mir/enum/option_with_bigger_niche_ok.rs: [missing] -> pass (J0)
  • [ui] tests/ui/mir/enum/plain_no_data_break.rs: [missing] -> pass (J0)
  • [ui] tests/ui/mir/enum/plain_no_data_ok.rs: [missing] -> pass (J0)
  • [ui] tests/ui/mir/enum/single_ok.rs: [missing] -> pass (J0)
  • [ui] tests/ui/mir/enum/single_with_repr_break.rs: [missing] -> pass (J0)
  • [ui] tests/ui/mir/enum/single_with_repr_ok.rs: [missing] -> pass (J0)
  • [ui] tests/ui/mir/enum/with_niche_int_break.rs: [missing] -> pass (J0)
  • [ui] tests/ui/mir/enum/with_niche_int_ok.rs: [missing] -> pass (J0)
  • [ui] tests/ui/mir/enum/with_niche_ptr_ok.rs: [missing] -> pass (J0)
  • [ui] tests/ui/mir/enum/wrap_break.rs: [missing] -> pass (J0)
  • [ui] tests/ui/mir/enum/wrap_ok.rs: [missing] -> pass (J0)

Stage 2

  • [ui] tests/ui/mir/enum/convert_non_enum_break.rs: [missing] -> pass (J1)
  • [ui] tests/ui/mir/enum/convert_non_enum_niche_break.rs: [missing] -> pass (J1)
  • [ui] tests/ui/mir/enum/convert_non_enum_niche_ok.rs: [missing] -> pass (J1)
  • [ui] tests/ui/mir/enum/convert_non_enum_ok.rs: [missing] -> pass (J1)
  • [ui] tests/ui/mir/enum/niche_option_tuple_break.rs: [missing] -> pass (J1)
  • [ui] tests/ui/mir/enum/niche_option_tuple_ok.rs: [missing] -> pass (J1)
  • [ui] tests/ui/mir/enum/numbered_variants_break.rs: [missing] -> pass (J1)
  • [ui] tests/ui/mir/enum/numbered_variants_ok.rs: [missing] -> pass (J1)
  • [ui] tests/ui/mir/enum/option_with_bigger_niche_break.rs: [missing] -> pass (J1)
  • [ui] tests/ui/mir/enum/option_with_bigger_niche_ok.rs: [missing] -> pass (J1)
  • [ui] tests/ui/mir/enum/plain_no_data_break.rs: [missing] -> pass (J1)
  • [ui] tests/ui/mir/enum/plain_no_data_ok.rs: [missing] -> pass (J1)
  • [ui] tests/ui/mir/enum/single_ok.rs: [missing] -> pass (J1)
  • [ui] tests/ui/mir/enum/single_with_repr_break.rs: [missing] -> pass (J1)
  • [ui] tests/ui/mir/enum/single_with_repr_ok.rs: [missing] -> pass (J1)
  • [ui] tests/ui/mir/enum/with_niche_int_break.rs: [missing] -> pass (J1)
  • [ui] tests/ui/mir/enum/with_niche_int_ok.rs: [missing] -> pass (J1)
  • [ui] tests/ui/mir/enum/with_niche_ptr_ok.rs: [missing] -> pass (J1)
  • [ui] tests/ui/mir/enum/wrap_break.rs: [missing] -> pass (J1)
  • [ui] tests/ui/mir/enum/wrap_ok.rs: [missing] -> pass (J1)

Additionally, 16 doctest diffs were found. These are ignored, as they are noisy.

Job group index

Test dashboard

Run

cargo run --manifest-path src/ci/citool/Cargo.toml -- \
    test-dashboard b63223c152212832ce37a109e26cc5f84c577532 --output-dir test-dashboard

And then open test-dashboard/index.html in your browser to see an overview of all executed tests.

Job duration changes

  1. x86_64-apple-2: 4991.5s -> 3249.6s (-34.9%)
  2. x86_64-apple-1: 6732.2s -> 8791.0s (30.6%)
  3. mingw-check-1: 1784.0s -> 1525.7s (-14.5%)
  4. i686-gnu-2: 6282.9s -> 5375.5s (-14.4%)
  5. dist-aarch64-apple: 6203.2s -> 5314.1s (-14.3%)
  6. x86_64-rust-for-linux: 2876.5s -> 2529.2s (-12.1%)
  7. aarch64-gnu-debug: 4037.6s -> 3563.2s (-11.7%)
  8. i686-gnu-nopt-1: 8017.5s -> 7098.3s (-11.5%)
  9. x86_64-gnu-llvm-20-1: 3628.4s -> 3222.4s (-11.2%)
  10. x86_64-gnu-aux: 6464.3s -> 5776.4s (-10.6%)
How to interpret the job duration changes?

Job durations can vary a lot, based on the actual runner instance
that executed the job, system noise, invalidated caches, etc. The table above is provided
mostly for t-infra members, for simpler debugging of potential CI slow-downs.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (b63223c): comparison URL.

Overall result: ❌ regressions - no action needed

@rustbot label: -perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

mean range count
Regressions ❌
(primary)		 0.1% [0.1%, 0.1%] 1
Regressions ❌
(secondary)		 - - 0
Improvements ✅
(primary)		 - - 0
Improvements ✅
(secondary)		 - - 0
All ❌✅ (primary) 0.1% [0.1%, 0.1%] 1

Max RSS (memory usage)

Results (secondary -0.9%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)		 - - 0
Regressions ❌
(secondary)		 2.3% [2.3%, 2.3%] 1
Improvements ✅
(primary)		 - - 0
Improvements ✅
(secondary)		 -2.4% [-2.7%, -2.2%] 2
All ❌✅ (primary) - - 0

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 690.233s -> 690.587s (0.05%)
Artifact size: 372.05 MiB -> 372.13 MiB (0.02%)

This was referenced Jun 29, 2025
Closed
Closed
@carolynzech carolynzech mentioned this pull request Jun 30, 2025
Merged
github-merge-queue bot pushed a commit to model-checking/kani that referenced this pull request Jun 30, 2025
Upgrade toolchain to 06/30 (#4188)
6bd6afa 
Culprit PRs:

- rust-lang/rust#142927
- rust-lang/rust#141759

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 and MIT licenses.
@fneddy fneddy mentioned this pull request Jul 2, 2025
Closed
github-actions bot pushed a commit to model-checking/verify-rust-std that referenced this pull request Jul 4, 2025
@bors
Auto merge of rust-lang#141759 - 1c3t3a:discriminants-query, r=saethlin
bee72cf 
Insert checks for enum discriminants when debug assertions are enabled

Similar to the existing null-pointer and alignment checks, this checks for valid enum discriminants on creation of enums through unsafe transmutes. Essentially this sanitizes patterns like the following:
```rust
let val: MyEnum = unsafe { std::mem::transmute<u32, MyEnum>(42) };
```

An extension of this check will be done in a follow-up that explicitly sanitizes for extern enum values that come into Rust from e.g. C/C++.

This check is similar to Miri's capabilities of checking for valid construction of enum values.

This PR is inspired by saethlin@'s PR
rust-lang#104862. Thank you so much for keeping this code up and the detailed comments!

I also pair-programmed large parts of this together with vabr-g@.

r? `@saethlin`
@1c3t3a 1c3t3a mentioned this pull request Jul 25, 2025
Open
7 tasks
This was referenced Sep 10, 2025
Closed
Open
@kaleidawave
Copy link

I think this has landed in 1.90.0. A library I use that does this in a proc-macro now panics. Can fix easily. Maybe needed to be included in breaking changes here: https://github.com/rust-lang/rust/releases/tag/1.90.0

@saethlin
Copy link
Member

saethlin commented Sep 19, 2025 •
edited
Loading

Turning UB into a deterministic crash is not a breaking change. If you think the check is wrong you should file an issue. If you think that this would be useful to include in the release notes to raise awareness, you should file an issue or create a PR.

(Though it's unclear to me how adding a bullet to the release notes would have changed things for you 🤷)

@iddey iddey mentioned this pull request Oct 10, 2025
Merged
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Veykril Veykril Veykril left review comments

@saethlin saethlin saethlin left review comments

@scottmcm scottmcm scottmcm left review comments

Assignees

@saethlin saethlin

Labels

merged-by-bors This PR was explicitly merged by bors. 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. T-libs Relevant to the library team, which will review and decide on the PR/issue.

Projects

None yet

Milestone

1.90.0

Development

Successfully merging this pull request may close these issues.