[crater experiment] Enable validate-mir by default

[saethlin]

#106612 made our MIR validation checks pickier. So this branch enables validation after every pass, to exercise those checks (and MIR validation in general) as much as possible.

Additionally, we recently merged #106294 which comes with this remark:

This [the UB this PR weaponizes] is hopefully rare enough to not break anything.

It would be good to know if these changes are causing issues in the ecosystem sooner rather than later.

Just a crater build run would exercise the MIR checks, but I would also like to see if crater can tell us anything about the noundef change. I think if we can do a crater test run with +rustflags=-Copt-level=3 we have a chance of breaking tests if someone is accidentally relying on UB.

---

sigh I forgot to r? @ghost, sorry about that. If someone wants to cruise through and fixup the labels/etc please do.

[saethlin]

@bors try

[rustbot]

r? @eholk

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

[bors]

⌛ Trying commit 9943b56 with merge 613d1972a772987ed4ee06faccbab0e2fc839871...

[rustbot]

Failed to set assignee to ghost: invalid assignee

Note: Only org members, users with write permissions, or people who have commented on the PR may be assigned.

[rust-log-analyzer]

The job x86_64-gnu-llvm-13 failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

running 16 tests
...............F
failures:

---- tests::test_unstable_options_tracking_hash stdout ----
thread 'tests::test_unstable_options_tracking_hash' panicked at 'assertion failed: `(left != right)`
  left: `true`,
 right: `true`', compiler/rustc_interface/src/tests.rs:698:5


failures:
    tests::test_unstable_options_tracking_hash
    tests::test_unstable_options_tracking_hash

test result: FAILED. 15 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.03s

error: test failed, to rerun pass `-p rustc_interface --lib`

[bors]

☀️ Try build successful - checks-actions
Build commit: 613d1972a772987ed4ee06faccbab0e2fc839871 (613d1972a772987ed4ee06faccbab0e2fc839871)

[jyn514]

@craterbot run start=master#159ba8a92c9e2fa4121f106176309521f4af87e9+rustflags=-Copt-level=3 end=try#613d1972a772987ed4ee06faccbab0e2fc839871+rustflags=-Copt-level=3 mode=build-and-test

[craterbot]

👌 Experiment pr-107051 created and queued.
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[craterbot]

🚧 Experiment pr-107051 is now running

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[craterbot]

🎉 Experiment pr-107051 is completed!
📊 269 regressed and 116 fixed (253207 total)
📰 Open the full report.

⚠️ If you notice any spurious failure please add them to the blacklist!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[saethlin]

Well, I'm very glad we did this. Looks like we have a lot of legitimate regressions. I'll start triaging them in an hour or two and update this PR with my assessment.

[matthiaskrgr]

for a quick peek: rg -o "broken MIR.*$" | awk 'NF=5' | sort -n | uniq

[saethlin]

I haven't reviewed all of the test-failed crates, but of those I have reviewed they are all flaky tests not UB. So I'm comfortable saying the noundef change isn't causing obvious widespread issues. That's nice.

At least two of the ICEs bisect to this:

searched nightlies: from nightly-2022-11-01 to nightly-2023-01-21
regressed nightly: nightly-2022-11-20
searched commit range: b833ad5...c5d82ed
regressed commit: 2f8d804

bisected with cargo-bisect-rustc v0.6.5

Host triple: x86_64-unknown-linux-gnu
Reproduce with:

cargo bisect-rustc --start 2022-11-01 --end 2023-01-21 --script script 

If I had to guess, this PR is causing these validation failures: #104411 but that might just be because it is improving the validator.

I do however notice in the pi_ecs example, we get this error (formatted by hand):

Field projection `(*_1).field[0]` specified type
`pi_ecs::storage::SecondaryMap<pi_ecs::storage::LocalVersion, Node2Index>`,
but actual type is
`<Node2Index as pi_ecs::component::Component>::Storage`

And in pi_ecs: https://github.com/GaiaWorld/pi_ecs/blob/e46bb571032376d13b2cedf770291654f7e2d13f/src/component.rs#L23-L29

pub trait Component: ThreadSync + 'static {
	type Storage: Map<Key = LocalVersion, Val = Self> + Index<LocalVersion, Output=Self> + IndexMut<LocalVersion, Output=Self> + ThreadSync + 'static;
}

impl<C> Component for C where C: ThreadSync + 'static {
	default type Storage = SecondaryMap<LocalVersion, Self>;
}

So basically I'm well over my head figuring out what's actually happening here, and we found something but it wasn't what we were looking for.

[saethlin]

The ICE in safer_ffi bisects to:

searched nightlies: from nightly-2023-01-01 to nightly-2023-01-22
regressed nightly: nightly-2023-01-20
searched commit range: 333ee6c...4c83bd0
regressed commit: 8b11574

bisected with cargo-bisect-rustc v0.6.5

Host triple: x86_64-unknown-linux-gnu
Reproduce with:

cargo bisect-rustc --start 2023-01-01 --end 2023-01-22 --script script --regress=ice 

cc @Nilstrieb

[saethlin]

I do not know enough about the above 2 sources of MIR validation failures to write up competent issues for them. I'll leave this open for a few more days and try to recruit some help.

[Nilstrieb]

The ICE in safer_ffi is expected and #106191, which is currently blocked on me and the lang team

[Nilstrieb]

Actually, looking at the safer-ffi logs it's also full of broken MIR? This should really not be caused by my change

[Nilstrieb]

I minimized one of the crates (pi_ecs)

[saethlin]

Going to close this for cleanliness but try to keep it in my work queue to come back to if the above issue is fixed.