Let miri and const eval execute intrinsics' fallback bodies by oli-obk · Pull Request #124293 · rust-lang/rust

oli-obk · 2024-04-23T13:33:09Z

fixes rust-lang/miri#3397

r? @RalfJung

rustbot · 2024-04-23T13:33:19Z

Some changes occurred to the CTFE / Miri engine

cc @rust-lang/miri

The Miri subtree was changed

cc @rust-lang/miri

compiler/rustc_const_eval/src/const_eval/dummy_machine.rs

oli-obk · 2024-04-23T13:35:02Z

compiler/rustc_const_eval/src/interpret/terminator.rs

                    unwind,
-                )
+                )? {
+                    assert!(!self.tcx.intrinsic(fallback.def_id()).unwrap().must_be_overridden);


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.

oli-obk · 2024-04-23T13:35:22Z

src/tools/miri/src/shims/intrinsics/mod.rs

+            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");
+            }


We could skip this check for safe intrinsics

src/tools/miri/src/shims/intrinsics/mod.rs

RalfJung

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.

compiler/rustc_const_eval/src/const_eval/machine.rs

compiler/rustc_const_eval/src/interpret/machine.rs

compiler/rustc_const_eval/src/interpret/terminator.rs

src/tools/miri/src/shims/intrinsics/atomic.rs

src/tools/miri/src/shims/intrinsics/mod.rs

RalfJung · 2024-04-29T15:27:54Z

@rustbot author

oli-obk · 2024-04-29T16:29:59Z

@rustbot ready

src/tools/miri/src/shims/intrinsics/mod.rs

compiler/rustc_const_eval/src/const_eval/machine.rs

src/tools/miri/src/shims/intrinsics/mod.rs

RalfJung · 2024-04-30T15:00:31Z

src/tools/miri/src/shims/intrinsics/mod.rs

+            return Ok(Some(ty::Instance {
+                def: ty::InstanceDef::Item(instance.def_id()),
+                args: instance.args,
+            }))


A few lines above we have "Handle intrinsics without return place" for diverging intrinsics -- shouldn't that also check for a fallback body?

probably. Let's add that when we have such an intrinsic with a fallback body

Let's at least add a FIXME then.

src/tools/miri/src/shims/intrinsics/mod.rs

RalfJung · 2024-05-02T10:22:29Z

r=me with tests blessed and FIXME added.

…to preserve all UB that the native intrinsic would have

RalfJung · 2024-05-03T17:46:32Z

@bors r+

bors · 2024-05-03T17:46:34Z

📌 Commit 821d23b has been approved by RalfJung

It is now in the queue for this repository.

…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

compiler-errors · 2024-05-04T03:33:25Z

I believe this failed in a rollup: #124688 (comment)

compiler-errors · 2024-05-04T03:33:27Z

@bors r-

RalfJung · 2024-05-04T06:38:02Z

Yeah, const_(de)allocate need the new attribute.

I'll just quickly add them since Oli is on vacation.

RalfJung · 2024-05-04T06:41:28Z

@bors r+

bors · 2024-05-04T06:41:30Z

📌 Commit 4e97c6c has been approved by RalfJung

It is now in the queue for this repository.

…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``

rustbot assigned RalfJung Apr 23, 2024

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 Apr 23, 2024

oli-obk commented Apr 23, 2024

View reviewed changes

compiler/rustc_const_eval/src/const_eval/dummy_machine.rs Show resolved Hide resolved

oli-obk commented Apr 23, 2024

View reviewed changes

Noratrieb reviewed Apr 23, 2024

View reviewed changes

src/tools/miri/src/shims/intrinsics/mod.rs Outdated Show resolved Hide resolved

oli-obk commented Apr 23, 2024

View reviewed changes

src/tools/miri/src/shims/intrinsics/mod.rs Show resolved Hide resolved

RalfJung requested changes Apr 27, 2024

View reviewed changes

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 Apr 29, 2024

oli-obk force-pushed the miri_intrinsic_fallback_body branch 2 times, most recently from f971b1e to e0ee8f7 Compare April 29, 2024 15:53

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 29, 2024

oli-obk commented Apr 29, 2024

View reviewed changes

src/tools/miri/src/shims/intrinsics/mod.rs Outdated Show resolved Hide resolved

RalfJung reviewed Apr 30, 2024

View reviewed changes

This comment has been minimized.

Sign in to view

oli-obk added 2 commits May 3, 2024 09:01

Let miri and const eval execute intrinsics' fallback bodies

351658a

Ensure miri only uses fallback bodies that have manually been vetted …

821d23b

…to preserve all UB that the native intrinsic would have

oli-obk force-pushed the miri_intrinsic_fallback_body branch from 465a755 to 821d23b Compare May 3, 2024 09:37

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-review Status: Awaiting review from the assignee but also interested parties. labels May 3, 2024

compiler-errors mentioned this pull request May 4, 2024

Rollup of 8 pull requests #124688

Closed

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 May 4, 2024

mark const_(de)allocate intrinsics as suitable for Miri

4e97c6c

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 May 4, 2024

matthiaskrgr mentioned this pull request May 4, 2024

Rollup of 6 pull requests #124697

Closed

matthiaskrgr mentioned this pull request May 4, 2024

Rollup of 8 pull requests #124703

Merged

bors merged commit ceb7b5e into rust-lang:master May 4, 2024

rustbot added this to the 1.80.0 milestone May 4, 2024

oli-obk deleted the miri_intrinsic_fallback_body branch May 20, 2024 07:22

davidtwco mentioned this pull request May 28, 2024

Add rust_analyzer as a predefined tool #125241

Merged

Uh oh!

Conversation

oli-obk commented Apr 23, 2024

Uh oh!

rustbot commented Apr 23, 2024

Uh oh!

Uh oh!

oli-obk Apr 23, 2024

Choose a reason for hiding this comment

Uh oh!

oli-obk Apr 23, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

RalfJung left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

RalfJung commented Apr 29, 2024

Uh oh!

oli-obk commented Apr 29, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

RalfJung Apr 30, 2024

Choose a reason for hiding this comment

Uh oh!

oli-obk May 2, 2024

Choose a reason for hiding this comment

Uh oh!

RalfJung May 2, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

This comment has been minimized.

RalfJung commented May 2, 2024

Uh oh!

RalfJung commented May 3, 2024

Uh oh!

bors commented May 3, 2024

Uh oh!

compiler-errors commented May 4, 2024

Uh oh!

compiler-errors commented May 4, 2024

Uh oh!

RalfJung commented May 4, 2024

Uh oh!

RalfJung commented May 4, 2024

Uh oh!

bors commented May 4, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants