Use owned_slice instead of owning_ref to store metadata.

`owning_ref` of a `Box<T>` has soundness problems because it stores
pointers to a box that is then invalidated. Instead, use the safer
`owned_slice`, which presents a more specialized abstraction.
This doesn't change the general layout or indirection of the structure,
but only the datastructure used as a wrapper.

Author: Noratrieb

compiler/rustc_codegen_ssa/src/back/metadata.rs

@@ -13,8 +13,7 @@ use object::{
 use snap::write::FrameEncoder;
 
 use rustc_data_structures::memmap::Mmap;
-use rustc_data_structures::owning_ref::OwningRef;
-use rustc_data_structures::rustc_erase_owner;
+use rustc_data_structures::owned_slice::OwnedSlice;
 use rustc_data_structures::sync::MetadataRef;
 use rustc_metadata::EncodedMetadata;
 use rustc_session::cstore::MetadataLoader;
@@ -44,8 +43,11 @@ fn load_metadata_with(
         File::open(path).map_err(|e| format!("failed to open file '{}': {}", path.display(), e))?;
     let data = unsafe { Mmap::map(file) }
         .map_err(|e| format!("failed to mmap file '{}': {}", path.display(), e))?;
-    let metadata = OwningRef::new(data).try_map(f)?;
-    return Ok(rustc_erase_owner!(metadata.map_owner_box()));
+
+    let metadata: MetadataRef = OwnedSlice::new(Box::new(data));
+    let metadata = metadata.try_map(f)?;
+
+    Ok(metadata)
 }
 
 impl MetadataLoader for DefaultMetadataLoader {

compiler/rustc_data_structures/src/sync.rs

@@ -14,14 +14,11 @@
 //!
 //! `MTRef` is an immutable reference if cfg!(parallel_compiler), and a mutable reference otherwise.
 //!
-//! `rustc_erase_owner!` erases an OwningRef owner into Erased or Erased + Send + Sync
-//! depending on the value of cfg!(parallel_compiler).
 
-use crate::owning_ref::{Erased, OwningRef};
+use crate::owned_slice::OwnedSlice;
 use std::collections::HashMap;
 use std::hash::{BuildHasher, Hash};
 use std::ops::{Deref, DerefMut};
-
 pub use std::sync::atomic::Ordering;
 pub use std::sync::atomic::Ordering::SeqCst;
 
@@ -33,13 +30,6 @@ cfg_if! {
         impl<T: ?Sized> Send for T {}
         impl<T: ?Sized> Sync for T {}
 
-        #[macro_export]
-        macro_rules! rustc_erase_owner {
-            ($v:expr) => {
-                $v.erase_owner()
-            }
-        }
-
         use std::ops::Add;
         use std::panic::{resume_unwind, catch_unwind, AssertUnwindSafe};
 
@@ -162,7 +152,7 @@ cfg_if! {
             }
         }
 
-        pub type MetadataRef = OwningRef<Box<dyn Erased>, [u8]>;
+        pub type MetadataRef = OwnedSlice<Box<dyn Deref<Target = [u8]>>, u8>;
 
         pub use std::rc::Rc as Lrc;
         pub use std::rc::Weak as Weak;
@@ -342,20 +332,11 @@ cfg_if! {
             t.into_par_iter().for_each(for_each)
         }
 
-        pub type MetadataRef = OwningRef<Box<dyn Erased + Send + Sync>, [u8]>;
+        pub type MetadataRef = OwnedSlice<Box<dyn Deref<Target = [u8]> + Send + Sync>, u8>;
 
         /// This makes locks panic if they are already held.
         /// It is only useful when you are running in a single thread
         const ERROR_CHECKING: bool = false;
-
-        #[macro_export]
-        macro_rules! rustc_erase_owner {
-            ($v:expr) => {{
-                let v = $v;
-                ::rustc_data_structures::sync::assert_send_val(&v);
-                v.erase_send_sync_owner()
-            }}
-        }
     }
 }
 

compiler/rustc_metadata/src/lib.rs

@@ -24,8 +24,6 @@ extern crate proc_macro;
 extern crate rustc_macros;
 #[macro_use]
 extern crate rustc_middle;
-#[macro_use]
-extern crate rustc_data_structures;
 
 pub use rmeta::{provide, provide_extern};
 

compiler/rustc_metadata/src/locator.rs

@@ -217,7 +217,6 @@ use crate::rmeta::{rustc_version, MetadataBlob, METADATA_HEADER};
 
 use rustc_data_structures::fx::{FxHashMap, FxHashSet};
 use rustc_data_structures::memmap::Mmap;
-use rustc_data_structures::owning_ref::OwningRef;
 use rustc_data_structures::svh::Svh;
 use rustc_data_structures::sync::MetadataRef;
 use rustc_errors::{struct_span_err, FatalError};
@@ -231,6 +230,7 @@ use rustc_span::symbol::{sym, Symbol};
 use rustc_span::Span;
 use rustc_target::spec::{Target, TargetTriple};
 
+use rustc_data_structures::owned_slice::OwnedSlice;
 use snap::read::FrameDecoder;
 use std::fmt::Write as _;
 use std::io::{Read, Result as IoResult, Write};
@@ -774,7 +774,7 @@ fn get_metadata_section<'p>(
             // don't have to grow the buffer as much.
             let mut inflated = Vec::with_capacity(compressed_bytes.len());
             match FrameDecoder::new(compressed_bytes).read_to_end(&mut inflated) {
-                Ok(_) => rustc_erase_owner!(OwningRef::new(inflated).map_owner_box()),
+                Ok(_) => OwnedSlice::new(Box::new(inflated)),
                 Err(_) => {
                     return Err(MetadataError::LoadFailure(format!(
                         "failed to decompress metadata: {}",
@@ -799,7 +799,7 @@ fn get_metadata_section<'p>(
                 ))
             })?;
 
-            rustc_erase_owner!(OwningRef::new(mmap).map_owner_box())
+            OwnedSlice::new(Box::new(mmap))
         }
     };
     let blob = MetadataBlob::new(raw_bytes);

compiler/rustc_metadata/src/rmeta/decoder.rs

@@ -56,18 +56,13 @@ mod cstore_impl;
 #[derive(Clone)]
 pub(crate) struct MetadataBlob(Lrc<MetadataRef>);
 
-// This is needed so we can create an OwningRef into the blob.
-// The data behind a `MetadataBlob` has a stable address because it is
-// contained within an Rc/Arc.
-unsafe impl rustc_data_structures::owning_ref::StableAddress for MetadataBlob {}
-
-// This is needed so we can create an OwningRef into the blob.
+// This is needed so we can create an OwnedSlice into the blob.
 impl std::ops::Deref for MetadataBlob {
-    type Target = [u8];
+    type Target = MetadataRef;
 
     #[inline]
-    fn deref(&self) -> &[u8] {
-        &self.0[..]
+    fn deref(&self) -> &Self::Target {
+        &self.0
     }
 }
 

compiler/rustc_metadata/src/rmeta/def_path_hash_map.rs

@@ -1,14 +1,14 @@
 use crate::rmeta::DecodeContext;
 use crate::rmeta::EncodeContext;
 use crate::rmeta::MetadataBlob;
-use rustc_data_structures::owning_ref::OwningRef;
+use rustc_data_structures::owned_slice::OwnedSlice;
 use rustc_hir::def_path_hash_map::{Config as HashMapConfig, DefPathHashMap};
 use rustc_middle::parameterized_over_tcx;
 use rustc_serialize::{opaque, Decodable, Decoder, Encodable, Encoder};
 use rustc_span::def_id::{DefIndex, DefPathHash};
 
 pub(crate) enum DefPathHashMapRef<'tcx> {
-    OwnedFromMetadata(odht::HashTable<HashMapConfig, OwningRef<MetadataBlob, [u8]>>),
+    OwnedFromMetadata(odht::HashTable<HashMapConfig, OwnedSlice<MetadataBlob, u8>>),
     BorrowedFromTcx(&'tcx DefPathHashMap),
 }
 
@@ -50,9 +50,9 @@ impl<'a, 'tcx> Decodable<DecodeContext<'a, 'tcx>> for DefPathHashMapRef<'static>
 
         let len = d.read_usize();
         let pos = d.position();
-        let o = OwningRef::new(d.blob().clone()).map(|x| &x[pos..pos + len]);
+        let o = OwnedSlice::new(d.blob().clone()).map(|x| &x[pos..pos + len]);
 
-        // Although we already have the data we need via the OwningRef, we still need
+        // Although we already have the data we need via the OwnedSlice, we still need
         // to advance the DecodeContext's position so it's in a valid state after
         // the method. We use read_raw_bytes() for that.
         let _ = d.read_raw_bytes(len);