Fix a soundness hole allowing multiple mutable borrows #124
Conversation
|
Ah that's a bit of a nuisance... Looks like |
This means manually setting the value in an unsafe way, because we can't use `Cell` with the `const INIT` value.
|
Ok, I've tweaked the |
|
@KodrAus Somewhere else in the codebase a |
src/heap_lazy.rs
Outdated
| // have exclusive access to the value field | ||
| // The value hasn't been previously initialized | ||
| unsafe { | ||
| ptr::write(&self.0 as *const _ as *mut _, r); |
There was a problem hiding this comment.
You can't turn the &'static self into a mutable pointer without UnsafeCell, this is UB.
There was a problem hiding this comment.
I thought that might be the case. I'll revert this last commit and bump our minimum version to 1.22 so that we can use Cell properly.
We might have to be a bit patient with rolling this one out, the last version bump we did for 1.1.0 was a bit unpopular (to be fair that bumped about 10 versions, instead of 1).
EDIT: Actually it's not 1.24 that we need.
Not at the moment, sorry, the queue is pretty full |
@eddyb Ah well spotted! I totally missed the @pietroalbini No problem :) I think we'll hold on to this one for a little while anyways so we don't poison the well with lots of version bumps over a short period. |
src/inline_lazy.rs
Outdated
| unsafe { | ||
| match self.0 { | ||
| match *self.0.as_ptr() { | ||
| Some(ref x) => x, | ||
| None => unreachable_unchecked(), |
There was a problem hiding this comment.
Might be worth a debug_assert!(false) or so?
There was a problem hiding this comment.
Good idea 👍 It seems like a fair thing to be more defensive about
KodrAus
force-pushed
the
fix/static-mut
branch
from
1c726d4 to
9b7bfcf
Compare
|
Ok, looking around it seems like @eddyb does this look good to you? |
|
Just noticed that we can close #125 now that we don't use |
|
There is so much context around these macros, doesn't make it easier to analyze.^^ For
(I think this could be further optimized with |
Ah good idea. I've added a FIXME for replacing It sounds like this is ready to merge so I'll go ahead and do that now, but will leave it for a day or two before pushing to crates.io so we can opportunistically bundle #130 too. |
Fixes #117
cc @eddyb @RalfJung
See #117 for more details, we don't need
&'static mut selfreceivers onLazyto initialise and get its value. This also has the nice side benefit of making our variousLazyimpls have a consistent API.This is a potentially breaking change, @pietroalbini do you think it would be possible to schedule a crater run? Thanks!