3 ms·
I strongly disagree with this article. Perhaps in the days before Miri it might have made sense, but it's pretty trivial right now to discover UB in unsafe code
by mmastrac 3y ago
I strongly disagree with this article. Perhaps in the days before Miri it might have made sense, but it's pretty trivial right now to discover UB in unsafe code with a simple `cargo +nightly miri test` run.
It feels like the Rust team is a bit wary of introducing other optimizations for fear of breaking unsafe code that has lurking UB, but it's better to start working on fixing these problems _now_ rather than get stuck in the present state of limbo. It's only going to get harder to fix incorrect code (which we see an example of in this particular post).
Honestly Miri is a superpower and it needs to be the priority of the Rust team to stabilize it. There's nothing inherently wrong with unsafe code: it's unsound code that's the problem, and we have the tools to prevent this exact problem from the article.
- IshKebab 3y agoMiri can't detect all UB, so I think it's still sensible to reduce the chance of writing it in the first place.
- mmastrac 3y agoI'm not aware of any UB that can 1) be caused by unique pointer violations from Box and 2) are undetectable by Miri (assuming good code coverage), but I might be wrong about this.
- IshKebab 3y agoWell for a start, if your tests don't cover that code then it can't possibly detect it.
- mmastrac 3y agoThis is trivially solved by putting code coverage and Miri into the same project, however.
- IshKebab 3y agoIt's trivial to measure code coverage. It's definitely not trivial to achieve 100% code coverage. This is the sort of "just do things perfectly" nonsense we get from C programmers. I'm surprised to see it from Rust devs, given the whole ethos of Rust is that it acknowledges that programmers are not perfect and helping them avoid bugs as much as possible is a good thing.
- remram 3y agoI agree in general, but if you have unsafe code you should definitely make sure it is covered.
- mmastrac 3y agoIt's not nonsense. It's really not difficult to structure code for 100% coverage of unsafe code if you're thinking about it from the start. You're also perfectly fine to write code that is free of `unsafe`, freeing you from this onerous task. We're pulling out Miri _because_ we're going outside the normal guardrails. You also don't _need_ to get 100% coverage of all your unsafe code if you can be confident of the usage of unsafe. The most complex unsafe code should almost certainly be covered, but there are a lot of trivial uses of unsafe that can be shown to be correct through reasoning. Where possible I prefer to split code into safe and unsafe portions, and test the unsafe portions under Miri with as much coverage as gives me confidence in the code. I've made UB mistakes before with unsafe, but since adding Miri and code coverage, the numbers of mistakes I've made has dropped dramatically. No programmer is perfect, but one would be pretty foolish to ignore the tools at one's disposal.
- LegionMammal978 3y agoIf you have an object that's !Unpin, then Miri will not apply uniqueness rules to anything containing it [0], including boxes and &mut references. (In the example code, replacing the PhantomPinned with a () will make Miri complain again.) This is considered a temporary (if long-lived) measure to allow async executors to manipulate pinned futures without invalidating all their internal borrows. Thus, it might be seen as undetected UB, in lieu of a permanent solution. [0] https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=56500ae2945bb186030c5e51bda601c6 https://play.rust-lang.org/?version=stable&mode=debug&editio...
- mmastrac 3y agoI forgot about this. I actually had to ignore a test in Miri because of this exact issue. https://github.com/denoland/deno_core/blob/98b09fa4f77db11312d993eb8b32ca905cf58fb7/core/runtime/op_driver/erased_future.rs#L179 https://github.com/denoland/deno_core/blob/98b09fa4f77db1131... Anxiously waiting on https://github.com/rust-lang/rfcs/pull/3467 https://github.com/rust-lang/rfcs/pull/3467
- zamalek 3y agoThis is why I firmly argue (and pretend) that references are not pointers. "References are pointers" results in the belief that references will behave like C pointers and results in things like this article. At best they are a cousin of pointers. I consider the fact that they are pointers an implementation detail, just like Box is a value with 'static samantics.
- saghm 3y ago> This is why I firmly argue (and pretend) that references are not pointers. "References are pointers" results in the belief that references will behave like C pointers and results in things like this article. This is my usual mental model as well. My thinking is that if tomorrow a new Rust version came out that used some other magical implementation of references that didn't use pointers under the hood, my code should still be correct. Maybe converting between references and raw pointers would be less efficient, but the semantics of my code shouldn't change.
- bbatha 3y agoI really wish rust didn't make the guarantee that references are pointers, if I ever made a Rust 2.0 I wouldn't make this guarantee. It would make the `Copy` type less needed and help with extra `&&&` coming up in generics. It could make references to packed and unaligned bits safe. It could also support offset-references for self types.
- vlovich123 3y agoI'm using the glommio runtime & miri takes forever to just start the executor & then throws up with "can't call foreign function `sched_setaffinity` on OS `linux`". Aside from clearly a very long tail of blockers from running it on non-trivial program on tier 1, the slowness is a real usability problem. It's slower than valgrind afaict. I think dismissing issues as "but we have miri" is very short sighted and it's not clear to me that Miri will ever reach the point of catching issues in substantial codebases (the standard library is substantial in number of lines, but not substantial in terms of exercising all the OS features).
- workingjubilee 2y agowhat would "stabilize" mean when the opsem model that miri obeys continues to change?