Auto merge of #34539 - arielb1:metadata-hash, r=alexcrichton

Make the metadata lock more robust

Fixes #33778 and friends.

I also needed to add a metadata encoding version to rlibs, as they did not have it before. To keep it backwards-compatible, I added 4 zeroes to the start of the metadata, which are treated as an empty length field by older rustcs.

r? @alexcrichton

Author: bors

src/librustc/dep_graph/thread.rs

@@ -118,8 +118,6 @@ impl DepGraphThreadData {
     /// the buffer is full, this may swap.)
     #[inline]
     pub fn enqueue(&self, message: DepMessage) {
-        debug!("enqueue: {:?} tasks_pushed={}", message, self.tasks_pushed.get());
-
         // Regardless of whether dep graph construction is enabled, we
         // still want to check that we always have a valid task on the
         // stack when a read/write/etc event occurs.

src/librustc_data_structures/indexed_vec.rs

@@ -15,6 +15,7 @@ use std::marker::PhantomData;
 use std::ops::{Index, IndexMut, Range};
 use std::fmt;
 use std::vec;
+use std::u32;
 
 use rustc_serialize as serialize;
 
@@ -31,6 +32,11 @@ impl Idx for usize {
     fn index(self) -> usize { self }
 }
 
+impl Idx for u32 {
+    fn new(idx: usize) -> Self { assert!(idx <= u32::MAX as usize); idx as u32 }
+    fn index(self) -> usize { self as usize }
+}
+
 #[derive(Clone)]
 pub struct IndexVec<I: Idx, T> {
     pub raw: Vec<T>,

src/librustc_metadata/Cargo.toml

@@ -16,6 +16,7 @@ rustc = { path = "../librustc" }
 rustc_back = { path = "../librustc_back" }
 rustc_bitflags = { path = "../librustc_bitflags" }
 rustc_const_math = { path = "../librustc_const_math" }
+rustc_data_structures = { path = "../librustc_data_structures" }
 rustc_errors = { path = "../librustc_errors" }
 rustc_llvm = { path = "../librustc_llvm" }
 serialize = { path = "../libserialize" }

src/librustc_metadata/astencode.rs

@@ -60,7 +60,7 @@ use rustc_serialize::{Encodable, EncoderHelpers};
 
 struct DecodeContext<'a, 'b, 'tcx: 'a> {
     tcx: TyCtxt<'a, 'tcx, 'tcx>,
-    cdata: &'b cstore::crate_metadata,
+    cdata: &'b cstore::CrateMetadata,
     from_id_range: IdRange,
     to_id_range: IdRange,
     // Cache the last used filemap for translating spans as an optimization.
@@ -121,7 +121,7 @@ impl<'a, 'b, 'c, 'tcx> ast_map::FoldOps for &'a DecodeContext<'b, 'c, 'tcx> {
 
 /// Decodes an item from its AST in the cdata's metadata and adds it to the
 /// ast-map.
-pub fn decode_inlined_item<'a, 'tcx>(cdata: &cstore::crate_metadata,
+pub fn decode_inlined_item<'a, 'tcx>(cdata: &cstore::CrateMetadata,
                                      tcx: TyCtxt<'a, 'tcx, 'tcx>,
                                      parent_def_path: ast_map::DefPath,
                                      parent_did: DefId,
@@ -246,7 +246,7 @@ impl<S:serialize::Encoder> def_id_encoder_helpers for S
 trait def_id_decoder_helpers {
     fn read_def_id(&mut self, dcx: &DecodeContext) -> DefId;
     fn read_def_id_nodcx(&mut self,
-                         cdata: &cstore::crate_metadata) -> DefId;
+                         cdata: &cstore::CrateMetadata) -> DefId;
 }
 
 impl<D:serialize::Decoder> def_id_decoder_helpers for D
@@ -258,7 +258,7 @@ impl<D:serialize::Decoder> def_id_decoder_helpers for D
     }
 
     fn read_def_id_nodcx(&mut self,
-                         cdata: &cstore::crate_metadata)
+                         cdata: &cstore::CrateMetadata)
                          -> DefId {
         let did: DefId = Decodable::decode(self).unwrap();
         decoder::translate_def_id(cdata, did)
@@ -858,17 +858,17 @@ trait rbml_decoder_decoder_helpers<'tcx> {
     // Versions of the type reading functions that don't need the full
     // DecodeContext.
     fn read_ty_nodcx<'a>(&mut self, tcx: TyCtxt<'a, 'tcx, 'tcx>,
-                         cdata: &cstore::crate_metadata) -> Ty<'tcx>;
+                         cdata: &cstore::CrateMetadata) -> Ty<'tcx>;
     fn read_tys_nodcx<'a>(&mut self, tcx: TyCtxt<'a, 'tcx, 'tcx>,
-                          cdata: &cstore::crate_metadata) -> Vec<Ty<'tcx>>;
+                          cdata: &cstore::CrateMetadata) -> Vec<Ty<'tcx>>;
     fn read_substs_nodcx<'a>(&mut self, tcx: TyCtxt<'a, 'tcx, 'tcx>,
-                             cdata: &cstore::crate_metadata)
+                             cdata: &cstore::CrateMetadata)
                              -> subst::Substs<'tcx>;
 }
 
 impl<'a, 'tcx> rbml_decoder_decoder_helpers<'tcx> for reader::Decoder<'a> {
     fn read_ty_nodcx<'b>(&mut self, tcx: TyCtxt<'b, 'tcx, 'tcx>,
-                         cdata: &cstore::crate_metadata)
+                         cdata: &cstore::CrateMetadata)
                          -> Ty<'tcx> {
         self.read_opaque(|_, doc| {
             Ok(
@@ -879,15 +879,15 @@ impl<'a, 'tcx> rbml_decoder_decoder_helpers<'tcx> for reader::Decoder<'a> {
     }
 
     fn read_tys_nodcx<'b>(&mut self, tcx: TyCtxt<'b, 'tcx, 'tcx>,
-                          cdata: &cstore::crate_metadata) -> Vec<Ty<'tcx>> {
+                          cdata: &cstore::CrateMetadata) -> Vec<Ty<'tcx>> {
         self.read_to_vec(|this| Ok(this.read_ty_nodcx(tcx, cdata)) )
             .unwrap()
             .into_iter()
             .collect()
     }
 
     fn read_substs_nodcx<'b>(&mut self, tcx: TyCtxt<'b, 'tcx, 'tcx>,
-                             cdata: &cstore::crate_metadata)
+                             cdata: &cstore::CrateMetadata)
                              -> subst::Substs<'tcx>
     {
         self.read_opaque(|_, doc| {

src/librustc_metadata/common.rs

@@ -252,3 +252,7 @@ pub fn rustc_version() -> String {
 }
 
 pub const tag_panic_strategy: usize = 0x114;
+
+// NB: increment this if you change the format of metadata such that
+// rustc_version can't be found.
+pub const metadata_encoding_version : &'static [u8] = &[b'r', b'u', b's', b't', 0, 0, 0, 2];

src/librustc_metadata/creader.rs

@@ -12,7 +12,6 @@
 
 //! Validates all used crates and extern libraries and loads their metadata
 
-use common::rustc_version;
 use cstore::{self, CStore, CrateSource, MetadataBlob};
 use decoder;
 use loader::{self, CratePaths};
@@ -24,7 +23,7 @@ use rustc::session::{config, Session};
 use rustc::session::config::PanicStrategy;
 use rustc::session::search_paths::PathKind;
 use rustc::middle::cstore::{CrateStore, validate_crate_name, ExternCrate};
-use rustc::util::nodemap::FnvHashMap;
+use rustc::util::nodemap::{FnvHashMap, FnvHashSet};
 use rustc::hir::map as hir_map;
 
 use std::cell::{RefCell, Cell};
@@ -132,7 +131,7 @@ struct ExtensionCrate {
 }
 
 enum PMDSource {
-    Registered(Rc<cstore::crate_metadata>),
+    Registered(Rc<cstore::CrateMetadata>),
     Owned(MetadataBlob),
 }
 
@@ -236,25 +235,6 @@ impl<'a> CrateReader<'a> {
         return ret;
     }
 
-    fn verify_rustc_version(&self,
-                            name: &str,
-                            span: Span,
-                            metadata: &MetadataBlob) {
-        let crate_rustc_version = decoder::crate_rustc_version(metadata.as_slice());
-        if crate_rustc_version != Some(rustc_version()) {
-            let mut err = struct_span_fatal!(self.sess, span, E0514,
-                                             "the crate `{}` has been compiled with {}, which is \
-                                              incompatible with this version of rustc",
-                                              name,
-                                              crate_rustc_version
-                                              .as_ref().map(|s| &**s)
-                                              .unwrap_or("an old version of rustc"));
-            err.help("consider removing the compiled binaries and recompiling \
-                      with your current version of rustc");
-            err.emit();
-        }
-    }
-
     fn verify_no_symbol_conflicts(&self,
                                   span: Span,
                                   metadata: &MetadataBlob) {
@@ -294,9 +274,8 @@ impl<'a> CrateReader<'a> {
                       span: Span,
                       lib: loader::Library,
                       explicitly_linked: bool)
-                      -> (ast::CrateNum, Rc<cstore::crate_metadata>,
+                      -> (ast::CrateNum, Rc<cstore::CrateMetadata>,
                           cstore::CrateSource) {
-        self.verify_rustc_version(name, span, &lib.metadata);
         self.verify_no_symbol_conflicts(span, &lib.metadata);
 
         // Claim this crate number and cache it
@@ -318,10 +297,10 @@ impl<'a> CrateReader<'a> {
 
         let loader::Library { dylib, rlib, metadata } = lib;
 
-        let cnum_map = self.resolve_crate_deps(root, metadata.as_slice(), span);
+        let cnum_map = self.resolve_crate_deps(root, metadata.as_slice(), cnum, span);
         let staged_api = self.is_staged_api(metadata.as_slice());
 
-        let cmeta = Rc::new(cstore::crate_metadata {
+        let cmeta = Rc::new(cstore::CrateMetadata {
             name: name.to_string(),
             extern_crate: Cell::new(None),
             index: decoder::load_index(metadata.as_slice()),
@@ -364,7 +343,7 @@ impl<'a> CrateReader<'a> {
                      span: Span,
                      kind: PathKind,
                      explicitly_linked: bool)
-                     -> (ast::CrateNum, Rc<cstore::crate_metadata>, cstore::CrateSource) {
+                     -> (ast::CrateNum, Rc<cstore::CrateMetadata>, cstore::CrateSource) {
         let result = match self.existing_match(name, hash, kind) {
             Some(cnum) => LoadResult::Previous(cnum),
             None => {
@@ -381,6 +360,7 @@ impl<'a> CrateReader<'a> {
                     rejected_via_hash: vec!(),
                     rejected_via_triple: vec!(),
                     rejected_via_kind: vec!(),
+                    rejected_via_version: vec!(),
                     should_match_name: true,
                 };
                 match self.load(&mut load_ctxt) {
@@ -438,8 +418,11 @@ impl<'a> CrateReader<'a> {
 
     fn update_extern_crate(&mut self,
                            cnum: ast::CrateNum,
-                           mut extern_crate: ExternCrate)
+                           mut extern_crate: ExternCrate,
+                           visited: &mut FnvHashSet<(ast::CrateNum, bool)>)
     {
+        if !visited.insert((cnum, extern_crate.direct)) { return }
+
         let cmeta = self.cstore.get_crate_data(cnum);
         let old_extern_crate = cmeta.extern_crate.get();
 
@@ -458,24 +441,24 @@ impl<'a> CrateReader<'a> {
         }
 
         cmeta.extern_crate.set(Some(extern_crate));
-
         // Propagate the extern crate info to dependencies.
         extern_crate.direct = false;
-        for &dep_cnum in cmeta.cnum_map.borrow().values() {
-            self.update_extern_crate(dep_cnum, extern_crate);
+        for &dep_cnum in cmeta.cnum_map.borrow().iter() {
+            self.update_extern_crate(dep_cnum, extern_crate, visited);
         }
     }
 
     // Go through the crate metadata and load any crates that it references
     fn resolve_crate_deps(&mut self,
                           root: &Option<CratePaths>,
                           cdata: &[u8],
-                          span : Span)
-                          -> cstore::cnum_map {
+                          krate: ast::CrateNum,
+                          span: Span)
+                          -> cstore::CrateNumMap {
         debug!("resolving deps of external crate");
         // The map from crate numbers in the crate we're resolving to local crate
         // numbers
-        decoder::get_crate_deps(cdata).iter().map(|dep| {
+        let map: FnvHashMap<_, _> = decoder::get_crate_deps(cdata).iter().map(|dep| {
             debug!("resolving dep crate {} hash: `{}`", dep.name, dep.hash);
             let (local_cnum, _, _) = self.resolve_crate(root,
                                                         &dep.name,
@@ -485,7 +468,13 @@ impl<'a> CrateReader<'a> {
                                                         PathKind::Dependency,
                                                         dep.explicitly_linked);
             (dep.cnum, local_cnum)
-        }).collect()
+        }).collect();
+
+        let max_cnum = map.values().cloned().max().unwrap_or(0);
+
+        // we map 0 and all other holes in the map to our parent crate. The "additional"
+        // self-dependencies should be harmless.
+        (0..max_cnum+1).map(|cnum| map.get(&cnum).cloned().unwrap_or(krate)).collect()
     }
 
     fn read_extension_crate(&mut self, span: Span, info: &CrateInfo) -> ExtensionCrate {
@@ -508,6 +497,7 @@ impl<'a> CrateReader<'a> {
             rejected_via_hash: vec!(),
             rejected_via_triple: vec!(),
             rejected_via_kind: vec!(),
+            rejected_via_version: vec!(),
             should_match_name: true,
         };
         let library = self.load(&mut load_ctxt).or_else(|| {
@@ -826,7 +816,7 @@ impl<'a> CrateReader<'a> {
     fn inject_dependency_if(&self,
                             krate: ast::CrateNum,
                             what: &str,
-                            needs_dep: &Fn(&cstore::crate_metadata) -> bool) {
+                            needs_dep: &Fn(&cstore::CrateMetadata) -> bool) {
         // don't perform this validation if the session has errors, as one of
         // those errors may indicate a circular dependency which could cause
         // this to stack overflow.
@@ -837,7 +827,17 @@ impl<'a> CrateReader<'a> {
         // Before we inject any dependencies, make sure we don't inject a
         // circular dependency by validating that this crate doesn't
         // transitively depend on any crates satisfying `needs_dep`.
-        validate(self, krate, krate, what, needs_dep);
+        for dep in self.cstore.crate_dependencies_in_rpo(krate) {
+            let data = self.cstore.get_crate_data(dep);
+            if needs_dep(&data) {
+                self.sess.err(&format!("the crate `{}` cannot depend \
+                                        on a crate that needs {}, but \
+                                        it depends on `{}`",
+                                       self.cstore.get_crate_data(krate).name(),
+                                       what,
+                                       data.name()));
+            }
+        }
 
         // All crates satisfying `needs_dep` do not explicitly depend on the
         // crate provided for this compile, but in order for this compilation to
@@ -849,32 +849,8 @@ impl<'a> CrateReader<'a> {
             }
 
             info!("injecting a dep from {} to {}", cnum, krate);
-            let mut cnum_map = data.cnum_map.borrow_mut();
-            let remote_cnum = cnum_map.len() + 1;
-            let prev = cnum_map.insert(remote_cnum as ast::CrateNum, krate);
-            assert!(prev.is_none());
+            data.cnum_map.borrow_mut().push(krate);
         });
-
-        fn validate(me: &CrateReader,
-                    krate: ast::CrateNum,
-                    root: ast::CrateNum,
-                    what: &str,
-                    needs_dep: &Fn(&cstore::crate_metadata) -> bool) {
-            let data = me.cstore.get_crate_data(krate);
-            if needs_dep(&data) {
-                let krate_name = data.name();
-                let data = me.cstore.get_crate_data(root);
-                let root_name = data.name();
-                me.sess.err(&format!("the crate `{}` cannot depend \
-                                      on a crate that needs {}, but \
-                                      it depends on `{}`", root_name, what,
-                                      krate_name));
-            }
-
-            for (_, &dep) in data.cnum_map.borrow().iter() {
-                validate(me, dep, root, what, needs_dep);
-            }
-        }
     }
 }
 
@@ -948,7 +924,8 @@ impl<'a> LocalCrateReader<'a> {
                                                          span: i.span,
                                                          direct: true,
                                                          path_len: len,
-                                                     });
+                                                     },
+                                                     &mut FnvHashSet());
                     self.cstore.add_extern_mod_stmt_cnum(info.id, cnum);
                 }
             }

src/librustc_metadata/csearch.rs

@@ -9,6 +9,7 @@
 // except according to those terms.
 
 use cstore;
+use common;
 use decoder;
 use encoder;
 use loader;
@@ -588,7 +589,7 @@ impl<'tcx> CrateStore<'tcx> for cstore::CStore {
 
     fn metadata_encoding_version(&self) -> &[u8]
     {
-        encoder::metadata_encoding_version
+        common::metadata_encoding_version
     }
 
     /// Returns a map from a sufficiently visible external item (i.e. an external item that is