move CTFE engine snapshot state out of miri engine into CTFE machine instance #54380

RalfJung · 2018-09-20T08:23:45Z

It still lives in the interpret module as it needs access to all sorts of private stuff. Also rename a thing to make @eddyb happy :D

The goal was not to change any behavior.

…instance

rust-highfive · 2018-09-20T08:23:56Z

r? @michaelwoerister

(rust_highfive has picked a reviewer for you, use r? to override)

RalfJung · 2018-09-20T08:25:02Z

src/librustc_mir/transform/const_prop.rs

@@ -310,7 +310,7 @@ impl<'b, 'a, 'tcx:'b> ConstPropagator<'b, 'a, 'tcx> {
                // cannot use `const_eval` here, because that would require having the MIR
                // for the current function available, but we're producing said MIR right now
                let res = self.use_ecx(source_info, |this| {
-                    eval_promoted(&mut this.ecx, cid, this.mir, this.param_env)
+                    eval_promoted(this.tcx, cid, this.mir, this.param_env)


This is the part I am least certain about: This is the only caller of eval_promoted, and it now operates in a new EvalContext instead of doing horrible things to the existing one. Seems to work fine...

…ime order in ConstPropagator consistent with Memory

src/librustc_mir/const_eval.rs

rust-highfive · 2018-09-20T10:24:31Z

The job x86_64-gnu-llvm-5.0 of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.

[00:13:31]    Compiling rustc_metadata_utils v0.0.0 (file:///checkout/src/librustc_metadata_utils)
[00:13:31]    Compiling rustc_mir v0.0.0 (file:///checkout/src/librustc_mir)
[00:13:31]    Compiling rustc_typeck v0.0.0 (file:///checkout/src/librustc_typeck)
[00:13:31]    Compiling rustc_traits v0.0.0 (file:///checkout/src/librustc_traits)
[00:14:20] error[E0502]: cannot borrow `*ecx` as immutable because `ecx.machine.loop_detector` is also borrowed as mutable
[00:14:20]     |
[00:14:20]     |
[00:14:20] 365 |         ecx.machine.loop_detector.observe_and_analyze(
[00:14:20]     |         ------------------------- mutable borrow occurs here
[00:14:20] 366 |             &ecx.tcx,
[00:14:20] 367 |             ecx.frame().span,
[00:14:20] ...
[00:14:20] 370 |         )
[00:14:20] 370 |         )
[00:14:20]     |         - mutable borrow ends here
[00:14:20] error: aborting due to previous error
[00:14:20] 
[00:14:20] For more information about this error, try `rustc --explain E0502`.
[00:14:20] error: Could not compile `rustc_mir`.

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

RalfJung · 2018-09-20T10:25:53Z

Eh, what? I compiled this locally...

EDIT: Funny, this compiled in stage 2 but not stage 1.

RalfJung · 2018-09-21T08:20:37Z

r? @eddyb

eddyb · 2018-09-21T13:58:24Z

src/librustc_mir/interpret/snapshot.rs

 {
    fn hash_stable<W: StableHasherResult>(
        &self,
        hcx: &mut StableHashingContext<'b>,
        hasher: &mut StableHasher<W>)
    {
+        // Not hashing memory: Avoid hashing memory all the time during execution


You only need HashStable if you're putting these snapshots through queries (because incremental needs to save/reload them) - is that the case?

I have no idea. I didn't write this code and I do not understand why they are using stable hashing. See #54384 and ask @oli-obk ;)

The purpose of this PR is mostly to get the loop detector out of my way, so that my next PR does not have to be concerned with this so much.

The answer is at #54384 (comment)

eddyb · 2018-09-21T13:59:21Z

src/librustc_mir/interpret/snapshot.rs

@@ -291,6 +335,28 @@ struct FrameSnapshot<'a, 'tcx: 'a> {
    stmt: usize,
 }

+// Not using the macro because that does not support types depending on 'tcx


That's surprising, I'd expect it's because 'mir, not 'tcx - lots of types depend on 'tcx.

Ah no the problem is that it doesn't support things depending on TWO lifetimes.

Just one lifetime and any number of types.

When can we #[derive] this? :D

When can we #[derive] this? :D

#49219, which is lower priority than the edition :(

eddyb · 2018-09-22T10:21:20Z

@bors r+

bors · 2018-09-22T10:21:21Z

📌 Commit 8e74ee0 has been approved by eddyb

bors · 2018-09-23T12:34:02Z

⌛ Testing commit 8e74ee0 with merge be91c35...

@eddyb

move CTFE engine snapshot state out of miri engine into CTFE machine instance It still lives in the `interpret` module as it needs access to all sorts of private stuff. Also rename a thing to make @eddyb happy :D The goal was not to change any behavior.

bors · 2018-09-23T15:00:48Z

☀️ Test successful - status-appveyor, status-travis
Approved by: eddyb
Pushing be91c35 to master...

rust-highfive · 2018-09-23T15:02:06Z

📣 Toolstate changed by #54380!

Tested on commit be91c35.
Direct link to PR: #54380

💔 miri on windows: test-pass → build-fail (cc @oli-obk @RalfJung @eddyb, @rust-lang/infra).
💔 miri on linux: test-pass → build-fail (cc @oli-obk @RalfJung @eddyb, @rust-lang/infra).

@oli-obk

Tested on commit rust-lang/rust@be91c35. Direct link to PR: <rust-lang/rust#54380> 💔 miri on windows: test-pass → build-fail (cc @oli-obk @RalfJung @eddyb, @rust-lang/infra). 💔 miri on linux: test-pass → build-fail (cc @oli-obk @RalfJung @eddyb, @rust-lang/infra).

miri engine: basic support for pointer provenance tracking This enriches pointers with a new member, `tag`, that can be used to do provenance tracking. This is a new type parameter that propagates up through everything. It defaults to `()` (no tag), which is also the value used by CTFE -- but miri will use another type. The only actually interesting piece here, I think, is what I had to do in the memory's `get`. The problem is that `tcx` (storing the allocations for statics) uses `()` for provenance information. But the machine might need another tag. The machine has a function to do the conversion, but if a conversion actually happened, we need to store the result of this *somewhere* -- we cannot return a pointer into `tcx` as we usually would. So I introduced `MonoHashMap` which uses `RefCell` to be able to insert new entries even when we just have a shared ref. However, it is important that we can also return shared refs into the map without holding the `RefCell` opan. This is achieved by boxing the values stored in the map, so their addresses remain stable even when the map's table gets reallocated. This is all implemented in `mono_hash_map.rs`. NOTE: This PR also contains the commits from #54380 (comment). Only the [last two commits](https://github.com/rust-lang/rust/pull/54461/files/8e74ee0998a5b11f28d61600dbb881c7168a4a40..HEAD) are new.

move CTFE engine snapshot state out of miri engine into CTFE machine …

169f791

…instance

rust-highfive assigned michaelwoerister Sep 20, 2018

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 20, 2018

RalfJung commented Sep 20, 2018

View reviewed changes

RalfJung force-pushed the miri-snapshot branch from e235795 to 0958472 Compare September 20, 2018 08:27

RalfJung added 2 commits September 20, 2018 10:36

rename evaluator -> interpreter to make eddyb happy

018d128

make some things a bit more private

b1453dd

RalfJung force-pushed the miri-snapshot branch from 0958472 to b1453dd Compare September 20, 2018 08:36

RalfJung mentioned this pull request Sep 20, 2018

Fix issue #52475: Make loop detector only consider reachable memory #52626

Merged

3 tasks

RalfJung added 2 commits September 20, 2018 11:57

move loop detector constants to the module that uses them; make lifet…

c16336a

…ime order in ConstPropagator consistent with Memory

unsurprisingly, miri needs tcx

c711e15

eddyb requested changes Sep 20, 2018

View reviewed changes

src/librustc_mir/const_eval.rs Show resolved Hide resolved

RalfJung mentioned this pull request Sep 20, 2018

Rustup rust-lang/miri#459

Merged

fix stage 0 compilation

0309664

rust-highfive assigned eddyb and unassigned michaelwoerister Sep 21, 2018

eddyb reviewed Sep 21, 2018

View reviewed changes

fix comment

8e74ee0

RalfJung mentioned this pull request Sep 21, 2018

Get the eval out of miri engine #54395

Closed

eddyb approved these changes Sep 22, 2018

View reviewed changes

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 Sep 22, 2018

RalfJung mentioned this pull request Sep 22, 2018

miri engine: basic support for pointer provenance tracking #54461

Merged

bors merged commit 8e74ee0 into rust-lang:master Sep 23, 2018

bors mentioned this pull request Sep 23, 2018

Report const eval error inside the query #53821

Merged

bors mentioned this pull request Sep 23, 2018

Delayed CTFE backtraces #54487

Merged

RalfJung deleted the miri-snapshot branch November 9, 2018 15:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

move CTFE engine snapshot state out of miri engine into CTFE machine instance #54380

move CTFE engine snapshot state out of miri engine into CTFE machine instance #54380

RalfJung commented Sep 20, 2018

rust-highfive commented Sep 20, 2018

RalfJung Sep 20, 2018

rust-highfive commented Sep 20, 2018

RalfJung commented Sep 20, 2018 •

edited

Loading

RalfJung commented Sep 21, 2018

eddyb Sep 21, 2018

RalfJung Sep 21, 2018

eddyb Sep 22, 2018

eddyb Sep 21, 2018

RalfJung Sep 21, 2018

eddyb Sep 22, 2018

eddyb commented Sep 22, 2018

bors commented Sep 22, 2018

bors commented Sep 23, 2018

bors commented Sep 23, 2018

rust-highfive commented Sep 23, 2018

move CTFE engine snapshot state out of miri engine into CTFE machine instance #54380

move CTFE engine snapshot state out of miri engine into CTFE machine instance #54380

Conversation

RalfJung commented Sep 20, 2018

rust-highfive commented Sep 20, 2018

RalfJung Sep 20, 2018

Choose a reason for hiding this comment

rust-highfive commented Sep 20, 2018

RalfJung commented Sep 20, 2018 • edited Loading

RalfJung commented Sep 21, 2018

eddyb Sep 21, 2018

Choose a reason for hiding this comment

RalfJung Sep 21, 2018

Choose a reason for hiding this comment

eddyb Sep 22, 2018

Choose a reason for hiding this comment

eddyb Sep 21, 2018

Choose a reason for hiding this comment

RalfJung Sep 21, 2018

Choose a reason for hiding this comment

eddyb Sep 22, 2018

Choose a reason for hiding this comment

eddyb commented Sep 22, 2018

bors commented Sep 22, 2018

bors commented Sep 23, 2018

bors commented Sep 23, 2018

rust-highfive commented Sep 23, 2018

RalfJung commented Sep 20, 2018 •

edited

Loading