Remove -Zmiri-panic-on-unsupported?

[RalfJung]

This flag turns (some) "unsupported" errors into Rust panics in the interpreted program, rather than aborting interpretation. See #1807 for context and motivation. However, the flag has never been fully implemented, and has annoying maintenance consequences -- calling the handle_unsupported_foreign_item helper actually requires some care or it will lead to some very strange behavior (and the behavior occurs only on the error path, so it is unlikely to be tested) -- in fact, the original implementation ran into that very issue, and in a recent PR a contributor repeated that mistake.

The motivation in #1807 can, I think, be covered by cargo miri nextest instead: this can be used to run all tests independent of each other, and process the results to see if any of them are about UB. So IMO we should remove -Zmiri-panic-on-unsupported; it's maintenance cost IMO outweighs its usefulness.

@landaire you suggested and implemented that flag. Do you still have a use for it, and would cargo miri nextest also cover that usecase?

[landaire]

The motivation in #1807 can, I think, be covered by cargo miri nextest instead: this can be used to run all tests independent of each other, and process the results to see if any of them are about UB. So IMO we should remove -Zmiri-panic-on-unsupported; it's maintenance cost IMO outweighs its usefulness.

I'll have to look more -- it's been a while since I thought about the internals of the initial change. This point from the original issue was important:

Tests invoking unsupported foreign functions will still be emulated up until they perform the foreign function call. Testing to this point may still yield valuable results.

If running nextest still allows for running the test to the point at which it calls a foreign function then that's probably fine? If there is no way to discern a test failure from a Miri error, that may be desired for scenarios where you want to run these sorts of tests in CI and capture UB regressions while ignoring Miri execution failures.

Do you still have a use for it?

Currently no. I left the job where I was motivated to add this flag and my current job doesn't yet have code where I'd like to use this. I use it a couple times a year but ultimately I'm not going to ask you to maintain something that has overhead and is not frequently used even by me, so removing this seems fine to me.

[RalfJung]

Thanks for the quick reply!

If running nextest still allows for running the test to the point at which it calls a foreign function then that's probably fine?

Yes, it will run each #[test] separately. And each such test runs until it hits UB, "unsupported", or finishes some other way.

I left the job where I was motivated to add this flag

If you have any idea who might be maintaining that codebase now and could ping them here (or use other channels to inform them about this issue), just in case they still need the flag, that'd be great. :)

If not I guess we'll just remove it and see if anyone complains. 🤷

[landaire]

If you have any idea who might be maintaining that codebase now and could ping them here (or use other channels to inform them about this issue), just in case they still need the flag, that'd be great. :)

I have no idea who is maintaining it now. I'd say to just move forward in removal and rejoice.

[tiif]

I never use this flag before, but I wonder how exactly can we get cargo miri nextest to ignore unsupported ffi error? I realised it is very time-consuming to manually annotate #![cfg(not(miri))] while working on tokio-rs/tokio#6812. I could help to test it out and update the documentation if this would help.

[saethlin]

cargo miri nextest run --no-fail-fast is probably what you're looking for.

[tiif]

cargo miri nextest run --no-fail-fast is probably what you're looking for.

Alright, thanks!