offset_of is unsound

[RalfJung]

The macro at

mio/src/sys/windows/named_pipe.rs

Line 26 in 1667a70

macro_rules! offset_of {

is unsound and causes UB due to using * on a NULL pointer. See the reference for details.

The memoffset crate provides a version of this macro that avoids UB (on older versions of rustc, this is not always possible, so it falls back to the 'least incorrect' version when needed).

[Thomasdezeeuw]

I think we can use std::ptr::addr_of for this. It's however only stable since 1.51. I don't think it's worth it pulling in a dependency for a single line.

[RalfJung]

Note that addr_of!(place) does not change how place is evaluated. In other words, *NULL is always UB no matter in which context, including addr_of!.

[Thomasdezeeuw]

Note that addr_of!(place) does not change how place is evaluated. In other words, *NULL is always UB no matter in which context, including addr_of!.

overlapped2arc! (which calls offset_of!) is only used on references so it should ever be called with a null pointer, but a comment saying it's UB to do so would be good.

[RalfJung]

I must have misunderstood; I thought you meant to use addr_of! instead of & in this code:

mio/src/sys/windows/named_pipe.rs

Line 28 in 1667a70

&(*(0 as *const $t)).$($field).+ as *const _ as usize

That would still be wrong.

[Thomasdezeeuw]

As far as I know the overlapped2arc! macro is used to go from a pointer to one of the Inner fields, e.g. Inner.read to the Inner struct itself. So you're saying we can't use std::ptr::addr_of to determine the difference between a pointer to Inner and e.g. Inner.read, to do the reverse in overlapped2arc!?

I was thinking something along the following lines:

    let inner: Inner = // ...
    let offset = std::ptr::addr_of!(inner.read) as usize - (&inner as *const _ as usize);
    Arc::from_raw(($e as usize - offset) as *mut $t)

[RalfJung]

So you're saying we can't use std::ptr::addr_of to determine the difference between a pointer to Inner and e.g. Inner.read, to do the reverse in overlapped2arc!?

I'm saying you cannot do addr_of!((*(0 as *const $t)).$($field).+) as usize, which is what I thought you meant when you said you wanted to use addr_of!.

If you have an existing instance of Inner, then that is much better indeed -- if inner is properly initialized here, you can then even use & and do not have to raise the MSRV. Given that the macro uses a NULL pointer, I assumed it would not have access to an existing instance of Inner.