Fix a soundness hole allowing multiple mutable borrows (#124)

* fix a soundness hole allowing multiple mutable borrows

* bump min version to 1.24.1

* remove static mut from macros

* leave a debug_assert in the supposedly unreachable block

* remove redundant unsafe block

* add a FIXME to replace Option with MaybeInitialized

Author: KodrAus

.travis.yml

@@ -1,7 +1,7 @@
 language: rust
 matrix:
   include:
-    - rust: 1.21.0
+    - rust: 1.24.1
     - rust: stable
     - os: osx
     - rust: beta

src/heap_lazy.rs

@@ -8,27 +8,27 @@
 extern crate std;
 
 use self::std::prelude::v1::*;
+use self::std::cell::Cell;
 use self::std::sync::Once;
 pub use self::std::sync::ONCE_INIT;
 
-pub struct Lazy<T: Sync>(*const T, Once);
+pub struct Lazy<T: Sync>(Cell<*const T>, Once);
 
 impl<T: Sync> Lazy<T> {
-    pub const INIT: Self = Lazy(0 as *const T, ONCE_INIT);
+    pub const INIT: Self = Lazy(Cell::new(0 as *const T), ONCE_INIT);
 
     #[inline(always)]
-    pub fn get<F>(&'static mut self, f: F) -> &T
+    pub fn get<F>(&'static self, f: F) -> &T
     where
         F: FnOnce() -> T,
     {
-        unsafe {
-            let r = &mut self.0;
-            self.1.call_once(|| {
-                *r = Box::into_raw(Box::new(f()));
-            });
-
-            &*self.0
-        }
+        self.1.call_once(|| {
+            self.0.set(Box::into_raw(Box::new(f())));
+        });
+
+        // `self.0` is guaranteed to have a value by this point
+        // The `Once` will catch and propegate panics
+        unsafe { &*self.0.get() }
     }
 }
 
@@ -38,6 +38,6 @@ unsafe impl<T: Sync> Sync for Lazy<T> {}
 #[doc(hidden)]
 macro_rules! __lazy_static_create {
     ($NAME:ident, $T:ty) => {
-        static mut $NAME: $crate::lazy::Lazy<$T> = $crate::lazy::Lazy::INIT;
+        static $NAME: $crate::lazy::Lazy<$T> = $crate::lazy::Lazy::INIT;
     };
 }

src/inline_lazy.rs

@@ -9,29 +9,35 @@ extern crate core;
 extern crate std;
 
 use self::std::prelude::v1::*;
+use self::std::cell::Cell;
 use self::std::sync::Once;
 pub use self::std::sync::ONCE_INIT;
 
-pub struct Lazy<T: Sync>(Option<T>, Once);
+// FIXME: Replace Option<T> with MaybeInitialized<T>
+pub struct Lazy<T: Sync>(Cell<Option<T>>, Once);
 
 impl<T: Sync> Lazy<T> {
-    pub const INIT: Self = Lazy(None, ONCE_INIT);
+    pub const INIT: Self = Lazy(Cell::new(None), ONCE_INIT);
 
     #[inline(always)]
-    pub fn get<F>(&'static mut self, f: F) -> &T
+    pub fn get<F>(&'static self, f: F) -> &T
     where
         F: FnOnce() -> T,
     {
-        {
-            let r = &mut self.0;
-            self.1.call_once(|| {
-                *r = Some(f());
-            });
-        }
+        self.1.call_once(|| {
+            self.0.set(Some(f()));
+        });
+
+        // `self.0` is guaranteed to be `Some` by this point
+        // The `Once` will catch and propegate panics
         unsafe {
-            match self.0 {
+            match *self.0.as_ptr() {
                 Some(ref x) => x,
-                None => unreachable_unchecked(),
+                None => {
+                    debug_assert!(false, "attempted to derefence an uninitialized lazy static. This is a bug");
+
+                    unreachable_unchecked()
+                },
             }
         }
     }
@@ -43,7 +49,7 @@ unsafe impl<T: Sync> Sync for Lazy<T> {}
 #[doc(hidden)]
 macro_rules! __lazy_static_create {
     ($NAME:ident, $T:ty) => {
-        static mut $NAME: $crate::lazy::Lazy<$T> = $crate::lazy::Lazy::INIT;
+        static $NAME: $crate::lazy::Lazy<$T> = $crate::lazy::Lazy::INIT;
     };
 }
 

src/lib.rs

@@ -137,19 +137,16 @@ macro_rules! __lazy_static_internal {
     (@TAIL, $N:ident : $T:ty = $e:expr) => {
         impl $crate::__Deref for $N {
             type Target = $T;
-            #[allow(unsafe_code)]
             fn deref(&self) -> &$T {
-                unsafe {
-                    #[inline(always)]
-                    fn __static_ref_initialize() -> $T { $e }
-
-                    #[inline(always)]
-                    unsafe fn __stability() -> &'static $T {
-                        __lazy_static_create!(LAZY, $T);
-                        LAZY.get(__static_ref_initialize)
-                    }
-                    __stability()
+                #[inline(always)]
+                fn __static_ref_initialize() -> $T { $e }
+
+                #[inline(always)]
+                fn __stability() -> &'static $T {
+                    __lazy_static_create!(LAZY, $T);
+                    LAZY.get(__static_ref_initialize)
                 }
+                __stability()
             }
         }
         impl $crate::LazyStatic for $N {