Do not use unsafe in generated code

Fixes issue #340
stepancheg · Sep 30, 2018 · e8fc5d2 · e8fc5d2
1 parent 838c873
commit e8fc5d2
Show file tree

Hide file tree

Showing 18 changed files with 2,003 additions and 2,282 deletions.
diff --git a/protobuf-codegen/src/code_writer.rs b/protobuf-codegen/src/code_writer.rs
@@ -63,7 +63,6 @@ impl<'a> CodeWriter<'a> {
         self.write_line("#![allow(non_snake_case)]");
         self.write_line("#![allow(non_upper_case_globals)]");
         self.write_line("#![allow(trivial_casts)]");
-        self.write_line("#![allow(unsafe_code)]");
         self.write_line("#![allow(unused_imports)]");
         self.write_line("#![allow(unused_results)]");
     }
@@ -104,13 +103,13 @@ impl<'a> CodeWriter<'a> {
     pub fn lazy_static(&mut self, name: &str, ty: &str) {
         self.stmt_block(
             &format!(
-                "static mut {}: ::protobuf::lazy::Lazy<{}> = ::protobuf::lazy::Lazy",
+                "static {}: ::protobuf::rt::Lazy<{}> = ::protobuf::rt::Lazy",
                 name,
                 ty
             ),
             |w| {
-                w.field_entry("lock", "::protobuf::lazy::ONCE_INIT");
-                w.field_entry("ptr", &format!("0 as *const {}", ty));
+                w.field_entry("lock", "::protobuf::rt::LAZY_ONCE_INIT");
+                w.field_entry("ptr", &format!("::std::cell::UnsafeCell::new(0 as *const {})", ty));
             },
         );
     }
@@ -120,16 +119,14 @@ impl<'a> CodeWriter<'a> {
         F : Fn(&mut CodeWriter),
     {
         self.lazy_static(name, ty);
-        self.unsafe_expr(|w| {
-            w.write_line(&format!("{}.get(|| {{", name));
-            w.indented(|w| init(w));
-            w.write_line(&format!("}})"));
-        });
+        self.write_line(&format!("{}.get(|| {{", name));
+        self.indented(|w| init(w));
+        self.write_line(&format!("}})"));
     }
 
     pub fn lazy_static_decl_get_simple(&mut self, name: &str, ty: &str, init: &str) {
         self.lazy_static(name, ty);
-        self.unsafe_expr(|w| { w.write_line(&format!("{}.get({})", name, init)); });
+        self.write_line(&format!("{}.get({})", name, init));
     }
 
     pub fn block<F>(&mut self, first_line: &str, last_line: &str, cb: F)

diff --git a/protobuf-codegen/src/lib.rs b/protobuf-codegen/src/lib.rs
@@ -99,10 +99,8 @@ fn write_file_descriptor_data(file: &FileDescriptorProto, w: &mut CodeWriter) {
     });
     w.write_line("");
     w.pub_fn("file_descriptor_proto() -> &'static ::protobuf::descriptor::FileDescriptorProto", |w| {
-        w.unsafe_expr(|w| {
-            w.block("file_descriptor_proto_lazy.get(|| {", "})", |w| {
-                w.write_line("parse_descriptor_proto()");
-            });
+        w.block("file_descriptor_proto_lazy.get(|| {", "})", |w| {
+            w.write_line("parse_descriptor_proto()");
         });
     });
 }

diff --git a/protobuf/src/descriptor.rs b/protobuf/src/descriptor.rs
diff --git a/protobuf/src/lazy.rs b/protobuf/src/lazy.rs
@@ -1,34 +1,30 @@
 //! Lazily initialized data.
 //! Used in generated code.
 
-use std::mem;
 use std::sync;
+use std::cell::UnsafeCell;
 
 /// Lasily initialized data.
 // Fields are public until `const` functions available in stable.
 pub struct Lazy<T> {
     #[doc(hidden)]
     pub lock: sync::Once,
     #[doc(hidden)]
-    pub ptr: *const T,
+    pub ptr: UnsafeCell<*const T>,
 }
 
+unsafe impl<T> Sync for Lazy<T> {}
+
 impl<T> Lazy<T> {
     /// Get lazy field value, initialize it with given function if not yet.
-    pub fn get<F>(&'static mut self, init: F) -> &'static T
+    pub fn get<F>(&'static self, init: F) -> &'static T
     where
         F : FnOnce() -> T,
     {
-        // ~ decouple the lifetimes of 'self' and 'self.lock' such we
-        // can initialize self.ptr in the call_once closure (note: we
-        // do have to initialize self.ptr in the closure to guarantee
-        // the ptr is valid for all calling threads at any point in
-        // time)
-        let lock: &sync::Once = unsafe { mem::transmute(&self.lock) };
-        lock.call_once(|| unsafe {
-            self.ptr = mem::transmute(Box::new(init()));
+        self.lock.call_once(|| unsafe {
+            *self.ptr.get() = Box::into_raw(Box::new(init()));
         });
-        unsafe { &*self.ptr }
+        unsafe { &**self.ptr.get() }
     }
 }
 
@@ -42,6 +38,7 @@ mod test {
     use std::thread;
     use std::sync::{Arc, Barrier};
     use std::sync::atomic::{ATOMIC_ISIZE_INIT, AtomicIsize, Ordering};
+    use std::cell::UnsafeCell;
 
     #[test]
     fn many_threads_calling_get() {
@@ -51,7 +48,7 @@ mod test {
 
         static mut LAZY: Lazy<String> = Lazy {
             lock: ONCE_INIT,
-            ptr: 0 as *const String,
+            ptr: UnsafeCell::new(0 as *const String),
         };
         static CALL_COUNT: AtomicIsize = ATOMIC_ISIZE_INIT;
 
@@ -62,7 +59,7 @@ mod test {
             unsafe {
                 LAZY = Lazy {
                     lock: ONCE_INIT,
-                    ptr: 0 as *const String,
+                    ptr: UnsafeCell::new(0 as *const String),
                 }
             }
             CALL_COUNT.store(0, Ordering::SeqCst);

diff --git a/protobuf/src/lib.rs b/protobuf/src/lib.rs
@@ -45,7 +45,7 @@ mod core;
 mod enums;
 mod oneof;
 pub mod rt;
-pub mod lazy;
+mod lazy;
 pub mod compiler_plugin;
 mod repeated;
 mod singular;
@@ -95,7 +95,6 @@ mod protobuf {
     pub use rt;
     pub use text_format;
     pub use reflect::types;
-    pub use lazy;
     pub use well_known_types;
     pub use ext;
     pub use unknown::UnknownFields;