Remove unsound offset_of macro

src/sys/windows/named_pipe.rs

@@ -1,41 +1,22 @@
-use crate::event::Source;
-use crate::sys::windows::{Event, Overlapped};
-use crate::{poll, Registry};
-use winapi::um::minwinbase::OVERLAPPED_ENTRY;
-
 use std::ffi::OsStr;
-use std::fmt;
 use std::io::{self, Read, Write};
-use std::mem;
+use std::mem::{self, size_of};
 use std::os::windows::io::{AsRawHandle, FromRawHandle, IntoRawHandle, RawHandle};
-use std::slice;
 use std::sync::atomic::Ordering::{Relaxed, SeqCst};
 use std::sync::atomic::{AtomicBool, AtomicUsize};
 use std::sync::{Arc, Mutex};
+use std::{fmt, slice};
 
-use crate::{Interest, Token};
 use miow::iocp::{CompletionPort, CompletionStatus};
 use miow::pipe;
 use winapi::shared::winerror::{ERROR_BROKEN_PIPE, ERROR_PIPE_LISTENING};
 use winapi::um::ioapiset::CancelIoEx;
+use winapi::um::minwinbase::{OVERLAPPED, OVERLAPPED_ENTRY};
 
-/// # Safety
-///
-/// Only valid if the strict is annotated with `#[repr(C)]`. This is only used
-/// with `Overlapped` and `Inner`, which are correctly annotated.
-macro_rules! offset_of {
-    ($t:ty, $($field:ident).+) => (
-        &(*(0 as *const $t)).$($field).+ as *const _ as usize
-    )
-}
-
-macro_rules! overlapped2arc {
-    ($e:expr, $t:ty, $($field:ident).+) => ({
-        let offset = offset_of!($t, $($field).+);
-        debug_assert!(offset < mem::size_of::<$t>());
-        Arc::from_raw(($e as usize - offset) as *mut $t)
-    })
-}
+use crate::event::Source;
+use crate::sys::windows::{Event, Overlapped};
+use crate::{poll, Registry};
+use crate::{Interest, Token};
 
 /// Non-blocking windows named pipe.
 ///
@@ -83,21 +64,72 @@ pub struct NamedPipe {
     inner: Arc<Inner>,
 }
 
+/// # Notes
+///
+/// The memory layout of this structure must be fixed as the `*_OFFSET`
+/// constants depend on it, see the `offset_constants` test.
 #[repr(C)]
 struct Inner {
-    handle: pipe::NamedPipe,
-
+    // NOTE: careful modifying the order of these three fields, the `ptr_from_*`
+    // methods depend on the layout!
     connect: Overlapped,
-    connecting: AtomicBool,
-
     read: Overlapped,
     write: Overlapped,
-
+    // END NOTE.
+    handle: pipe::NamedPipe,
+    connecting: AtomicBool,
     io: Mutex<Io>,
-
     pool: Mutex<BufferPool>,
 }
 
+impl Inner {
+    /// Converts a pointer to `Inner.connect` to a pointer to `Inner`.
+    ///
+    /// # Unsafety
+    ///
+    /// Caller must ensure `ptr` is pointing to `Inner.connect`.
+    unsafe fn ptr_from_conn_overlapped(ptr: *mut OVERLAPPED) -> *const Inner {
+        // `connect` is the first field, so the pointer are the same.
+        ptr.cast()
+    }
+
+    /// Same as [`ptr_from_conn_overlapped`] but for `Inner.read`.
+    unsafe fn ptr_from_read_overlapped(ptr: *mut OVERLAPPED) -> *const Inner {
+        // `read` is after `connect: Overlapped`.
+        (ptr as usize + size_of::<Overlapped>()) as *const Inner
+    }
+
+    /// Same as [`ptr_from_conn_overlapped`] but for `Inner.write`.
+    unsafe fn ptr_from_write_overlapped(ptr: *mut OVERLAPPED) -> *const Inner {
+        // `read` is after `connect: Overlapped` and `read: Overlapped`.
+        (ptr as usize + (2 * size_of::<Overlapped>())) as *const Inner
+    }
+}
+
+#[test]
+fn ptr_from() {
+    use std::mem::ManuallyDrop;
+    use std::ptr;
+
+    let pipe = unsafe { ManuallyDrop::new(NamedPipe::from_raw_handle(ptr::null_mut())) };
+    let inner: &Inner = &pipe.inner;
+    assert_eq!(
+        inner as *const Inner,
+        unsafe { Inner::ptr_from_conn_overlapped(&inner.connect as *const _ as *mut OVERLAPPED) },
+        "`ptr_from_conn_overlapped` incorrect"
+    );
+    assert_eq!(
+        inner as *const Inner,
+        unsafe { Inner::ptr_from_read_overlapped(&inner.read as *const _ as *mut OVERLAPPED) },
+        "`ptr_from_read_overlapped` incorrect"
+    );
+    assert_eq!(
+        inner as *const Inner,
+        unsafe { Inner::ptr_from_write_overlapped(&inner.write as *const _ as *mut OVERLAPPED) },
+        "`ptr_from_write_overlapped` incorrect"
+    );
+}
+
 struct Io {
     // Uniquely identifies the selector associated with this named pipe
     cp: Option<Arc<CompletionPort>>,
@@ -587,7 +619,7 @@ fn connect_done(status: &OVERLAPPED_ENTRY, events: Option<&mut Vec<Event>>) {
     // Acquire the `Arc<Inner>`. Note that we should be guaranteed that
     // the refcount is available to us due to the `mem::forget` in
     // `connect` above.
-    let me = unsafe { overlapped2arc!(status.overlapped(), Inner, connect) };
+    let me = unsafe { Arc::from_raw(Inner::ptr_from_conn_overlapped(status.overlapped())) };
 
     // Flag ourselves as no longer using the `connect` overlapped instances.
     let prev = me.connecting.swap(false, SeqCst);
@@ -613,7 +645,7 @@ fn read_done(status: &OVERLAPPED_ENTRY, events: Option<&mut Vec<Event>>) {
     // Acquire the `FromRawArc<Inner>`. Note that we should be guaranteed that
     // the refcount is available to us due to the `mem::forget` in
     // `schedule_read` above.
-    let me = unsafe { overlapped2arc!(status.overlapped(), Inner, read) };
+    let me = unsafe { Arc::from_raw(Inner::ptr_from_read_overlapped(status.overlapped())) };
 
     // Move from the `Pending` to `Ok` state.
     let mut io = me.io.lock().unwrap();
@@ -645,7 +677,7 @@ fn write_done(status: &OVERLAPPED_ENTRY, events: Option<&mut Vec<Event>>) {
     // Acquire the `Arc<Inner>`. Note that we should be guaranteed that
     // the refcount is available to us due to the `mem::forget` in
     // `schedule_write` above.
-    let me = unsafe { overlapped2arc!(status.overlapped(), Inner, write) };
+    let me = unsafe { Arc::from_raw(Inner::ptr_from_write_overlapped(status.overlapped())) };
 
     // Make the state change out of `Pending`. If we wrote the entire buffer
     // then we're writable again and otherwise we schedule another write.

tests/regressions.rs

@@ -9,7 +9,7 @@ use mio::net::{TcpListener, TcpStream};
 use mio::{Events, Interest, Poll, Token, Waker};
 
 mod util;
-use util::{any_local_address, init, init_with_poll, temp_file};
+use util::{any_local_address, init, init_with_poll};
 
 const ID1: Token = Token(1);
 const WAKE_TOKEN: Token = Token(10);
@@ -109,6 +109,7 @@ fn issue_1205() {
 #[cfg(unix)]
 fn issue_1403() {
     use mio::net::UnixDatagram;
+    use util::temp_file;
 
     init();