-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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.
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
Let miri and const eval execute intrinsics' fallback bodies #124293
Conversation
Some changes occurred to the CTFE / Miri engine cc @rust-lang/miri The Miri subtree was changed cc @rust-lang/miri |
self, | ||
instance, | ||
&self.copy_fn_args(args), | ||
destination, | ||
target, | ||
unwind, | ||
) | ||
)? { | ||
assert!(!self.tcx.intrinsic(fallback.def_id()).unwrap().must_be_overridden); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We only assert here, so backends are forced to choose a way to report the lack of intrinsic implementation, as miri and ctfe differ here in the message we want to send.
let intrinsic_fallback_checks_ub = Symbol::intern("intrinsic_fallback_checks_ub"); | ||
if !this.tcx.item_attrs(instance.def_id()).iter().any(|attr| attr.path_matches(&[sym::miri, intrinsic_fallback_checks_ub])) { | ||
throw_unsup_format!("miri can only use intrinsics that preserve UB. After verifying that `{intrinsic_name}` does so, add the `#[miri::intrinsic_fallback_checks_ub]` attribute to it"); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could skip this check for safe intrinsics
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The approach LGTM, thanks! I left some notes, mostly minor.
Please also add a test ensuring that we notice when an intrinsic does not have the intrinsic_fallback_checks_ub
attribute.
@rustbot author |
f971b1e
to
e0ee8f7
Compare
@rustbot ready |
return Ok(Some(ty::Instance { | ||
def: ty::InstanceDef::Item(instance.def_id()), | ||
args: instance.args, | ||
})) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few lines above we have "Handle intrinsics without return place" for diverging intrinsics -- shouldn't that also check for a fallback body?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
probably. Let's add that when we have such an intrinsic with a fallback body
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's at least add a FIXME then.
This comment has been minimized.
This comment has been minimized.
r=me with tests blessed and FIXME added. |
…to preserve all UB that the native intrinsic would have
465a755
to
821d23b
Compare
@bors r+ |
…dy, r=RalfJung Let miri and const eval execute intrinsics' fallback bodies fixes rust-lang/miri#3397 r? `@RalfJung`
…mpiler-errors Rollup of 8 pull requests Successful merges: - rust-lang#124293 (Let miri and const eval execute intrinsics' fallback bodies) - rust-lang#124418 (Use a proof tree visitor to refine the `Obligation` for error reporting in new solver) - rust-lang#124480 (Change `SIGPIPE` ui from `#[unix_sigpipe = "..."]` to `-Zon-broken-pipe=...`) - rust-lang#124648 (Trim crate graph) - rust-lang#124656 (release notes 1.78: add link to interior-mut breaking change) - rust-lang#124658 (Migrate `run-make/doctests-keep-binaries` to new rmake.rs format) - rust-lang#124681 (zkvm: fix run_tests) - rust-lang#124687 (Make `Bounds.clauses` private) r? `@ghost` `@rustbot` modify labels: rollup
I believe this failed in a rollup: #124688 (comment) |
@bors r- |
Yeah, const_(de)allocate need the new attribute. I'll just quickly add them since Oli is on vacation. |
@bors r+ |
…dy, r=RalfJung Let miri and const eval execute intrinsics' fallback bodies fixes rust-lang/miri#3397 r? `@RalfJung`
…iaskrgr Rollup of 6 pull requests Successful merges: - rust-lang#123356 (Reduce code size of `thread::set_current`) - rust-lang#124159 (Move thread parking to `sys::sync`) - rust-lang#124293 (Let miri and const eval execute intrinsics' fallback bodies) - rust-lang#124500 (lldb-formatters: Use StdSliceSyntheticProvider for &str) - rust-lang#124677 (Set non-leaf frame pointers on Fuchsia targets) - rust-lang#124692 (We do not coerce `&mut &mut T -> *mut mut T`) r? `@ghost` `@rustbot` modify labels: rollup
…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#123356 (Reduce code size of `thread::set_current`) - rust-lang#124159 (Move thread parking to `sys::sync`) - rust-lang#124293 (Let miri and const eval execute intrinsics' fallback bodies) - rust-lang#124677 (Set non-leaf frame pointers on Fuchsia targets) - rust-lang#124692 (We do not coerce `&mut &mut T -> *mut mut T`) - rust-lang#124698 (Rewrite `rustdoc-determinism` test in Rust) - rust-lang#124700 (Remove an unnecessary cast) - rust-lang#124701 (Docs: suggest `uN::checked_sub` instead of check-then-unchecked) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#124293 - oli-obk:miri_intrinsic_fallback_body, r=RalfJung Let miri and const eval execute intrinsics' fallback bodies fixes rust-lang/miri#3397 r? ``@RalfJung``
fixes rust-lang/miri#3397
r? @RalfJung