enable number validity checking and ptr::invalid checking by default #2151
Conversation
|
The transmutes/type-punning are pretty rare to hit with Miri, but they're still used by Uninitialized numbers are a bit more complicated. |
|
So would you recommend we leave But of course if the fallout is so big that we hear about it, we can also always change the default later. And I would like uninit numbers to raise errors by default at some point... |
|
I think that it is much better to break a lot of CI in advance of adding an LLVM attribute which makes existing dubious code definitely UB. As you say, if it's really that bad we can back out the default. I suspect the CI fallout won't be that bad. I can't find Miri in Also, a PR to fix the use of |
|
The stdlib test suite is happy with this change. :) |
explain how to turn integers into fn ptrs (with an intermediate raw ptr, not a direct transmute) Direct int2ptr transmute, under the semantics I am imagining, will produce a ptr with "invalid" provenance that is invalid to deref or call. We cannot give it the same semantics as int2ptr casts since those do [something complicated](https://www.ralfj.de/blog/2022/04/11/provenance-exposed.html). To my great surprise, that is already what the example in the `transmute` docs does. :) I still added a comment to say that that part is important, and I added a section explicitly talking about this to the `fn()` type docs. With rust-lang/miri#2151, Miri will start complaining about direct int-to-fnptr transmutes (in the sense that it is UB to dereference the resulting pointer).
explain how to turn integers into fn ptrs (with an intermediate raw ptr, not a direct transmute) Direct int2ptr transmute, under the semantics I am imagining, will produce a ptr with "invalid" provenance that is invalid to deref or call. We cannot give it the same semantics as int2ptr casts since those do [something complicated](https://www.ralfj.de/blog/2022/04/11/provenance-exposed.html). To my great surprise, that is already what the example in the `transmute` docs does. :) I still added a comment to say that that part is important, and I added a section explicitly talking about this to the `fn()` type docs. With rust-lang/miri#2151, Miri will start complaining about direct int-to-fnptr transmutes (in the sense that it is UB to call the resulting pointer).
explain how to turn integers into fn ptrs (with an intermediate raw ptr, not a direct transmute) Direct int2ptr transmute, under the semantics I am imagining, will produce a ptr with "invalid" provenance that is invalid to deref or call. We cannot give it the same semantics as int2ptr casts since those do [something complicated](https://www.ralfj.de/blog/2022/04/11/provenance-exposed.html). To my great surprise, that is already what the example in the `transmute` docs does. :) I still added a comment to say that that part is important, and I added a section explicitly talking about this to the `fn()` type docs. With rust-lang/miri#2151, Miri will start complaining about direct int-to-fnptr transmutes (in the sense that it is UB to call the resulting pointer).
|
☔ The latest upstream changes (presumably #2153) made this pull request unmergeable. Please resolve the merge conflicts. |
RalfJung
changed the title
|
@bors r=oli-obk |
|
📌 Commit 8c42ef1 has been approved by |
|
@bors r=oli-obk |
enable number validity checking and ptr::invalid checking by default This removes the `-Zmiri-check-number-validity` flag, enabling its effects by default. (We don't error when the flag is passed, for backwards compatibility.) We also enable by default that transmuting an integer to a pointer now creates a pointer with `None` provenance, which is invalid to dereference (and, in the case of a function pointer, invalid to call). I did this together since it is all related to ptr2int/int2ptr transmutation. Two new flags are added to optionally take back these stricter checks: - `-Zmiri-allow-uninit-numbers` makes Miri accept uninit data in integers and floats - `-Zmiri-allow-ptr-int-transmute` makes Miri accept pointers (provenance data) in integers and floats, *and* makes Miri treat int2ptr transmutes as equivalent to a cast. The flag names make sense IMO, but they are somewhat inconsistent with our existing flags since we usually call things `-Zmiri-disable-$CHECK` rather than `-Zmiri-allow-$THING`. But `-Zmiri-disable-uninit-number-check` sounds silly? (Whenever I say "transmute" this includes union and pointer based type punning.) Cc `@saethlin` I hope this won't break everything?^^ I think the most risky part is the int2ptr transmute aspect, in particular around function pointers where no `as` casts are possible. The correct pattern is to first cast to a raw ptr and then transmute that to a fn ptr. We should probably document this better, in the `transmute` documentation and maybe in the documentation for the `fn()` type. I should run this PR against the std test suite before we land it. r? `@oli-obk` - [x] Ensure stdlib docs recommend "usize -> raw ptr -> fn ptr" for int-to-fnptr casts: rust-lang/rust#97321 - [x] Run the stdlib test suite
|
💡 This pull request was already approved, no need to approve it again.
|
|
📌 Commit 8c42ef1 has been approved by |
|
☀️ Test successful - checks-actions |
explain how to turn integers into fn ptrs (with an intermediate raw ptr, not a direct transmute) Direct int2ptr transmute, under the semantics I am imagining, will produce a ptr with "invalid" provenance that is invalid to deref or call. We cannot give it the same semantics as int2ptr casts since those do [something complicated](https://www.ralfj.de/blog/2022/04/11/provenance-exposed.html). To my great surprise, that is already what the example in the `transmute` docs does. :) I still added a comment to say that that part is important, and I added a section explicitly talking about this to the `fn()` type docs. With rust-lang/miri#2151, Miri will start complaining about direct int-to-fnptr transmutes (in the sense that it is UB to call the resulting pointer).
This removes the
-Zmiri-check-number-validityflag, enabling its effects by default. (We don't error when the flag is passed, for backwards compatibility.) We also enable by default that transmuting an integer to a pointer now creates a pointer withNoneprovenance, which is invalid to dereference (and, in the case of a function pointer, invalid to call). I did this together since it is all related to ptr2int/int2ptr transmutation.Two new flags are added to optionally take back these stricter checks:
-Zmiri-allow-uninit-numbersmakes Miri accept uninit data in integers and floats-Zmiri-allow-ptr-int-transmutemakes Miri accept pointers (provenance data) in integers and floats, and makes Miri treat int2ptr transmutes as equivalent to a cast.The flag names make sense IMO, but they are somewhat inconsistent with our existing flags since we usually call things
-Zmiri-disable-$CHECKrather than-Zmiri-allow-$THING. But-Zmiri-disable-uninit-number-checksounds silly?(Whenever I say "transmute" this includes union and pointer based type punning.)
Cc @saethlin I hope this won't break everything?^^ I think the most risky part is the int2ptr transmute aspect, in particular around function pointers where no
ascasts are possible. The correct pattern is to first cast to a raw ptr and then transmute that to a fn ptr. We should probably document this better, in thetransmutedocumentation and maybe in the documentation for thefn()type. I should run this PR against the std test suite before we land it.r? @oli-obk