Merge pull request #11 from Gilnaa/bugfix/soundness

Fixed soundness issues and removed nested member access.

.travis.yml

@@ -1,16 +1,16 @@
 sudo: false
 language: rust
 rust:
-  - 1.19.0  # Oldest supported
-  - 1.33.0  # Oldest supported with constant expressions
+  - 1.25.0  # Oldest supported
+  - 1.36.0  # Oldest supported with MaybeUninit
   - stable
   - beta
   - nightly
 matrix:
   include:
   - env: RUSTFMT
-    rust: 1.19.0
-    rust: 1.33.0
+    rust: 1.25.0
+    rust: 1.36.0
     install:
     - rustup component add rustfmt-preview
     script:

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "memoffset"
-version = "0.3.0"
+version = "0.4.0"
 authors = ["Gilad Naaman <gilad.naaman@gmail.com>"]
 description = "offset_of functionality for Rust structs."
 license = "MIT"

build.rs

@@ -6,7 +6,7 @@ fn main() {
     assert!(version().unwrap().major >= 1);
 
     // Check for a minimum version
-    if version().unwrap() >= Version::parse("1.33.0").unwrap() {
-        println!("cargo:rustc-cfg=memoffset_constant_expression");
+    if version().unwrap() >= Version::parse("1.36.0").unwrap() {
+        println!("cargo:rustc-cfg=memoffset_maybe_uninit");
     }
 }

src/constant_impl.rs

@@ -1,13 +0,0 @@
-// A helper to get a const fn version of size_of_val
-#[doc(hidden)]
-pub const fn size_of<T>(_: &T) -> usize {
-    ::mem::size_of::<T>()
-}
-
-// While constant pointer transmutation isn't stable, union transmutation is
-// This hack should go away after rust-lang/rust#51910
-#[doc(hidden)]
-pub union Transmuter<T: 'static> {
-    pub ptr: &'static T,
-    pub int: usize,
-}

src/lib.rs

@@ -67,12 +67,8 @@
 #[doc(hidden)]
 pub use core::mem;
 
-#[cfg(memoffset_constant_expression)]
 #[doc(hidden)]
-mod constant_impl;
-
-#[cfg(memoffset_constant_expression)]
-pub use constant_impl::{size_of, Transmuter};
+pub use core::ptr;
 
 #[macro_use]
 mod offset_of;

src/offset_of.rs

@@ -23,7 +23,7 @@
 ///
 /// *Note*: This macro may not make much sense when used on structs that are not `#[repr(C, packed)]`
 ///
-/// ## Examples - Simple
+/// ## Examples
 /// ```
 /// #[macro_use]
 /// extern crate memoffset;
@@ -38,59 +38,39 @@
 /// fn main() {
 ///     assert_eq!(offset_of!(Foo, a), 0);
 ///     assert_eq!(offset_of!(Foo, b), 4);
-///     assert_eq!(offset_of!(Foo, c[2]), 14);
-/// }
-/// ```
-///
-/// ## Examples - Advanced
-/// ```
-/// #[macro_use]
-/// extern crate memoffset;
-///
-/// #[repr(C, packed)]
-/// struct UnnecessarilyComplicatedStruct {
-///     member: [UnnecessarilyComplexStruct; 12]
-/// }
-///
-/// #[repr(C, packed)]
-/// struct UnnecessarilyComplexStruct {
-///     a: u32,
-///     b: u64,
-///     c: [u8; 5]
-/// }
-///
-///
-/// fn main() {
-///     const OFFSET: usize = offset_of!(UnnecessarilyComplicatedStruct, member[3].c[3]);
-///     assert_eq!(OFFSET, 66);
 /// }
 /// ```
 #[macro_export]
-#[cfg(memoffset_constant_expression)]
+#[cfg(memoffset_maybe_uninit)]
 macro_rules! offset_of {
-    ($parent:ty, $($field:tt)+) => (unsafe {
-        let x: &'static $parent = $crate::Transmuter::<$parent> { int: 0 }.ptr;
-        $crate::Transmuter { ptr: &x.$($field)+ }.int
+    ($parent:tt, $field:tt) => (unsafe {
+        // Create an instance of the container and calculate the offset to its field.
+        // Here we're using an uninitialized instance of $parent.
+        // Since we're not using its field, there's no UB caused by reading uninitialized memoty.
+        // There *IS*, though, UB caused by creating references to uninitialized data,
+        // which is illegal since the compiler is allowed to assume that a reference
+        // points to valid data.
+        // AFAIK we cannot avoid UB completely.
+        let val = $crate::mem::MaybeUninit::<$parent>::uninit();
+        let &$parent { $field: ref f, .. } = &*val.as_ptr();
+        (f as *const _ as *const u8 as usize) - (val.as_ptr() as *const u8 as usize)
     });
 }
 
 #[macro_export]
-#[cfg(not(memoffset_constant_expression))]
+#[cfg(not(memoffset_maybe_uninit))]
 macro_rules! offset_of {
-    ($father:ty, $($field:tt)+) => ({
+    ($parent:ty, $field:tt) => {{
+        // This is UB since we're dealing with dangling references.
+        // We're never dereferencing it, but it's UB nonetheless.
+        // AFAIK we cannot avoid UB completely.
+        let non_null = $crate::ptr::NonNull::<$parent>::dangling();
+        let base_ptr = unsafe { non_null.as_ref() };
         #[allow(unused_unsafe)]
-        let root: $father = unsafe { $crate::mem::uninitialized() };
-
-        let base = &root as *const _ as usize;
-
-        // Future error: borrow of packed field requires unsafe function or block (error E0133)
-        #[allow(unused_unsafe)]
-        let member =  unsafe { &root.$($field)* as *const _ as usize };
-
-        $crate::mem::forget(root);
-
-        member - base
-    });
+        let field_ptr = unsafe { &base_ptr.$field };
+        let offset = (field_ptr as *const _ as usize) - (base_ptr as *const _ as usize);
+        offset
+    }};
 }
 
 #[cfg(test)]
@@ -109,18 +89,6 @@ mod tests {
         assert_eq!(offset_of!(Foo, c), 8);
     }
 
-    #[test]
-    fn offset_index() {
-        assert_eq!(offset_of!(Foo, b[2]), 6);
-    }
-
-    #[test]
-    #[should_panic]
-    #[allow(const_err)]
-    fn offset_index_out_of_bounds() {
-        offset_of!(Foo, b[4]);
-    }
-
     #[test]
     fn tuple_struct() {
         #[repr(C, packed)]

src/span_of.rs

@@ -267,30 +267,15 @@ mod tests {
             span_of!(Test, y..),
             offset_of!(Test, y)..mem::size_of::<Test>()
         );
-        assert_eq!(
-            span_of!(Test, y[0]..),
-            offset_of!(Test, y[0])..mem::size_of::<Test>()
-        );
+
         assert_eq!(
             span_of!(Test, z..),
             offset_of!(Test, z)..mem::size_of::<Test>()
         );
-        assert_eq!(
-            span_of!(Test, z.foo..),
-            offset_of!(Test, z.foo)..mem::size_of::<Test>()
-        );
         assert_eq!(
             span_of!(Test, egg..),
             offset_of!(Test, egg)..mem::size_of::<Test>()
         );
-        assert_eq!(
-            span_of!(Test, egg[0]..),
-            offset_of!(Test, egg[0])..mem::size_of::<Test>()
-        );
-        assert_eq!(
-            span_of!(Test, egg[0][0]..),
-            offset_of!(Test, egg[0][0])..mem::size_of::<Test>()
-        );
         assert_eq!(
             span_of!(Test, x..y),
             offset_of!(Test, x)..offset_of!(Test, y)
@@ -299,25 +284,9 @@ mod tests {
             span_of!(Test, x..=y),
             offset_of!(Test, x)..offset_of!(Test, y) + mem::size_of::<[u8; 56]>()
         );
-        assert_eq!(
-            span_of!(Test, x..y[4]),
-            offset_of!(Test, x)..offset_of!(Test, y[4])
-        );
         assert_eq!(
             span_of!(Test, x..=y[4]),
             offset_of!(Test, x)..offset_of!(Test, y) + mem::size_of::<[u8; 5]>()
         );
-        assert_eq!(
-            span_of!(Test, x..z.foo),
-            offset_of!(Test, x)..offset_of!(Test, z.foo)
-        );
-        assert_eq!(
-            span_of!(Test, x..=z.foo),
-            offset_of!(Test, x)..offset_of!(Test, z.foo) + mem::size_of::<u32>()
-        );
-        assert_eq!(
-            span_of!(Test, egg[0][0]..egg[1][0]),
-            offset_of!(Test, egg[0][0])..offset_of!(Test, egg[1][0])
-        );
     }
 }