From d0bc87f11ab47ded14469c1b18fc5fe4ae0a3a9c Mon Sep 17 00:00:00 2001 From: Gilad Naaman Date: Tue, 2 Jul 2019 20:56:30 +0300 Subject: [PATCH 1/2] Fixed soundness issues and removed nested member access. --- build.rs | 4 ++-- src/constant_impl.rs | 13 ------------- src/lib.rs | 6 +----- src/offset_of.rs | 46 ++++++++++++++++---------------------------- src/span_of.rs | 33 +------------------------------ 5 files changed, 21 insertions(+), 81 deletions(-) delete mode 100644 src/constant_impl.rs diff --git a/build.rs b/build.rs index 57e9afd..1d885ba 100644 --- a/build.rs +++ b/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.37.0").unwrap() { + println!("cargo:rustc-cfg=memoffset_maybe_uninit"); } } diff --git a/src/constant_impl.rs b/src/constant_impl.rs deleted file mode 100644 index ecd22b6..0000000 --- a/src/constant_impl.rs +++ /dev/null @@ -1,13 +0,0 @@ -// A helper to get a const fn version of size_of_val -#[doc(hidden)] -pub const fn size_of(_: &T) -> usize { - ::mem::size_of::() -} - -// 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 { - pub ptr: &'static T, - pub int: usize, -} diff --git a/src/lib.rs b/src/lib.rs index 72620a8..6bfc622 100644 --- a/src/lib.rs +++ b/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; diff --git a/src/offset_of.rs b/src/offset_of.rs index cedd798..9a943c8 100644 --- a/src/offset_of.rs +++ b/src/offset_of.rs @@ -66,30 +66,30 @@ /// } /// ``` #[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:ty, $field:tt) => (unsafe { + // Create an instance of the container and calculate the offset to its + // field. Although we are creating references to uninitialized data this + // is fine since we are not dereferencing them. + let val = $crate::mem::MaybeUninit::<$parent>::uninitialized(); + let &$container { $field: ref f, .. } = &*val.as_ptr(); + #[allow(unused_unsafe)] + let result = unsafe { (f as *const _ as *const u8).offset_from(val.as_ptr() as *const u8) }; + result as isize }); } #[macro_export] -#[cfg(not(memoffset_constant_expression))] +#[cfg(not(memoffset_maybe_uninit))] macro_rules! offset_of { - ($father:ty, $($field:tt)+) => ({ + ($parent:ty, $field:tt) => ({ + 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 }); } @@ -109,18 +109,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)] diff --git a/src/span_of.rs b/src/span_of.rs index 85758b5..f19f7cc 100644 --- a/src/span_of.rs +++ b/src/span_of.rs @@ -267,30 +267,15 @@ mod tests { span_of!(Test, y..), offset_of!(Test, y)..mem::size_of::() ); - assert_eq!( - span_of!(Test, y[0]..), - offset_of!(Test, y[0])..mem::size_of::() - ); + assert_eq!( span_of!(Test, z..), offset_of!(Test, z)..mem::size_of::() ); - assert_eq!( - span_of!(Test, z.foo..), - offset_of!(Test, z.foo)..mem::size_of::() - ); assert_eq!( span_of!(Test, egg..), offset_of!(Test, egg)..mem::size_of::() ); - assert_eq!( - span_of!(Test, egg[0]..), - offset_of!(Test, egg[0])..mem::size_of::() - ); - assert_eq!( - span_of!(Test, egg[0][0]..), - offset_of!(Test, egg[0][0])..mem::size_of::() - ); 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::() - ); - assert_eq!( - span_of!(Test, egg[0][0]..egg[1][0]), - offset_of!(Test, egg[0][0])..offset_of!(Test, egg[1][0]) - ); } } From 9e67d272b3c537301820746e16ffa3b13c05e07a Mon Sep 17 00:00:00 2001 From: Gilad Naaman Date: Thu, 4 Jul 2019 19:05:36 +0300 Subject: [PATCH 2/2] Fixed MaybeUninit for 1.36 --- .travis.yml | 8 +++---- Cargo.toml | 2 +- build.rs | 2 +- src/offset_of.rs | 54 +++++++++++++++--------------------------------- src/span_of.rs | 2 +- 5 files changed, 24 insertions(+), 44 deletions(-) diff --git a/.travis.yml b/.travis.yml index a8b6fcc..0cda5fb 100644 --- a/.travis.yml +++ b/.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: diff --git a/Cargo.toml b/Cargo.toml index 9d8b68c..a1a5bf0 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "memoffset" -version = "0.3.0" +version = "0.4.0" authors = ["Gilad Naaman "] description = "offset_of functionality for Rust structs." license = "MIT" diff --git a/build.rs b/build.rs index 1d885ba..89638e9 100644 --- a/build.rs +++ b/build.rs @@ -6,7 +6,7 @@ fn main() { assert!(version().unwrap().major >= 1); // Check for a minimum version - if version().unwrap() >= Version::parse("1.37.0").unwrap() { + if version().unwrap() >= Version::parse("1.36.0").unwrap() { println!("cargo:rustc-cfg=memoffset_maybe_uninit"); } } diff --git a/src/offset_of.rs b/src/offset_of.rs index 9a943c8..82d7397 100644 --- a/src/offset_of.rs +++ b/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_maybe_uninit)] macro_rules! offset_of { - ($parent:ty, $field:tt) => (unsafe { - // Create an instance of the container and calculate the offset to its - // field. Although we are creating references to uninitialized data this - // is fine since we are not dereferencing them. - let val = $crate::mem::MaybeUninit::<$parent>::uninitialized(); - let &$container { $field: ref f, .. } = &*val.as_ptr(); - #[allow(unused_unsafe)] - let result = unsafe { (f as *const _ as *const u8).offset_from(val.as_ptr() as *const u8) }; - result as isize + ($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_maybe_uninit))] macro_rules! offset_of { - ($parent: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 field_ptr = unsafe { &base_ptr.$field }; let offset = (field_ptr as *const _ as usize) - (base_ptr as *const _ as usize); offset - }); + }}; } #[cfg(test)] diff --git a/src/span_of.rs b/src/span_of.rs index f19f7cc..de1a3e2 100644 --- a/src/span_of.rs +++ b/src/span_of.rs @@ -267,7 +267,7 @@ mod tests { span_of!(Test, y..), offset_of!(Test, y)..mem::size_of::() ); - + assert_eq!( span_of!(Test, z..), offset_of!(Test, z)..mem::size_of::()