miri: improve errors for type validity assertion failures #143327
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rustbot
added
S-waiting-on-review
|
The Miri subtree was changed cc @rust-lang/miri Some changes occurred to the intrinsics. Make sure the CTFE / Miri interpreter cc @rust-lang/miri, @RalfJung, @oli-obk, @lcnr Some changes occurred to the CTFE machinery Some changes occurred to the CTFE / Miri interpreter cc @rust-lang/miri |
oli-obk
reviewed
Jul 2, 2025
library/core/src/mem/maybe_uninit.rs
Outdated
Comment on lines
619
to
620
| // We do this via a raw ptr read instead of `ManuallyDrop::into_inner` so that there's | ||
| // no trace of `MaybeUninit` in Miri's error messages here. |
There was a problem hiding this comment.
how does this affect messages? It seems suboptimal to change this
There was a problem hiding this comment.
See the src/tools/miri/tests/fail/validity/uninit_integer.stderr diff.
There was a problem hiding this comment.
that error seems fine to me, I'm not sure what you're trying to prevent with this and why
There was a problem hiding this comment.
error: Undefined Behavior: constructing invalid value at .value[0]: encountered uninitialized memory, but expected an integer
The value part here is a field of ManuallyDrop, and reveals an implementation detail of assume_init. With the proposed change, the error is entirely phrased in terms of the type the user chose (the T in MaybeUninit<T>), hiding implementation details.
RalfJung
force-pushed
the
miri-type-validity-error
branch
from
e9bc78e to
f548152
Compare
|
@bors r+ |
bors
added
S-waiting-on-bors
This comment has been minimized.
This comment has been minimized.
RalfJung
force-pushed
the
miri-type-validity-error
branch
from
f548152 to
f2dc27a
Compare
|
@bors r=oli-obk rollup |
This comment has been minimized.
This comment has been minimized.
|
@bors r- |
bors
added
S-waiting-on-author
RalfJung
force-pushed
the
miri-type-validity-error
branch
from
f2dc27a to
8362508
Compare
|
@bors r=oli-obk rollup |
bors
added
S-waiting-on-bors
bors
added a commit
that referenced
this pull request
Jul 3, 2025
Rollup of 9 pull requests Successful merges: - #143192 (Improve CSS for source code block line numbers) - #143251 (bootstrap: add build.tidy-extra-checks option) - #143273 (Make the enum check work for negative discriminants) - #143292 (Explicitly handle all nodes in `generics_of` when computing parent) - #143316 (Add bootstrap check snapshot tests) - #143321 (byte-addresses memory -> byte-addressed memory) - #143324 (interpret: move the native call preparation logic into Miri) - #143325 (Use non-global interner in `test_string_interning` in bootstrap) - #143327 (miri: improve errors for type validity assertion failures) r? `@ghost` `@rustbot` modify labels: rollup
rust-timer
added a commit
that referenced
this pull request
Jul 3, 2025
Rollup merge of #143327 - RalfJung:miri-type-validity-error, r=oli-obk miri: improve errors for type validity assertion failures Miri has pretty nice errors for type validity violations, printing which field in the type the problem occurs at and so on. However, we don't see these errors when using e.g. `mem::zeroed` as that uses `assert_zero_valid` to bail out before Miri can detect the UB. Similar to what we did with `@saethlin's` UB checks, I think we should disable such language UB checks in Miri so that we can get better error messages. If we go for this we should probably say this in the intrinsic docs as well so that people don't think they can rely on these intrinsics catching anything. Furthermore, I slightly changed `MaybeUninit::assume_init` so that the `.value` field does not show up in error messages any more. `@rust-lang/miri` what do you think?
RalfJung
pushed a commit
to RalfJung/miri
that referenced
this pull request
Jul 3, 2025
Rollup of 9 pull requests Successful merges: - rust-lang/rust#143192 (Improve CSS for source code block line numbers) - rust-lang/rust#143251 (bootstrap: add build.tidy-extra-checks option) - rust-lang/rust#143273 (Make the enum check work for negative discriminants) - rust-lang/rust#143292 (Explicitly handle all nodes in `generics_of` when computing parent) - rust-lang/rust#143316 (Add bootstrap check snapshot tests) - rust-lang/rust#143321 (byte-addresses memory -> byte-addressed memory) - rust-lang/rust#143324 (interpret: move the native call preparation logic into Miri) - rust-lang/rust#143325 (Use non-global interner in `test_string_interning` in bootstrap) - rust-lang/rust#143327 (miri: improve errors for type validity assertion failures) r? `@ghost` `@rustbot` modify labels: rollup
github-actions bot
pushed a commit
to model-checking/verify-rust-std
that referenced
this pull request
Jul 11, 2025
… r=oli-obk miri: improve errors for type validity assertion failures Miri has pretty nice errors for type validity violations, printing which field in the type the problem occurs at and so on. However, we don't see these errors when using e.g. `mem::zeroed` as that uses `assert_zero_valid` to bail out before Miri can detect the UB. Similar to what we did with `@saethlin's` UB checks, I think we should disable such language UB checks in Miri so that we can get better error messages. If we go for this we should probably say this in the intrinsic docs as well so that people don't think they can rely on these intrinsics catching anything. Furthermore, I slightly changed `MaybeUninit::assume_init` so that the `.value` field does not show up in error messages any more. `@rust-lang/miri` what do you think?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Miri has pretty nice errors for type validity violations, printing which field in the type the problem occurs at and so on.
However, we don't see these errors when using e.g.
mem::zeroedas that usesassert_zero_validto bail out before Miri can detect the UB.Similar to what we did with @saethlin's UB checks, I think we should disable such language UB checks in Miri so that we can get better error messages. If we go for this we should probably say this in the intrinsic docs as well so that people don't think they can rely on these intrinsics catching anything.
Furthermore, I slightly changed
MaybeUninit::assume_initso that the.valuefield does not show up in error messages any more.@rust-lang/miri what do you think?