Auto merge of #149273 - bjorn3:crate_locator_improvements, r=<try>

Don't leak sysroot crates through dependencies

Author: rust-bors[bot]

compiler/rustc_codegen_ssa/src/back/link.rs

@@ -278,7 +278,7 @@ pub fn each_linked_rlib(
         }
         let crate_name = info.crate_name[&cnum];
         let used_crate_source = &info.used_crate_source[&cnum];
-        if let Some((path, _)) = &used_crate_source.rlib {
+        if let Some(path) = &used_crate_source.rlib {
             f(cnum, path);
         } else if used_crate_source.rmeta.is_some() {
             return Err(errors::LinkRlibError::OnlyRmetaFound { crate_name });
@@ -542,7 +542,7 @@ fn link_staticlib(
         };
         let crate_name = codegen_results.crate_info.crate_name[&cnum];
         let used_crate_source = &codegen_results.crate_info.used_crate_source[&cnum];
-        if let Some((path, _)) = &used_crate_source.dylib {
+        if let Some(path) = &used_crate_source.dylib {
             all_rust_dylibs.push(&**path);
         } else if used_crate_source.rmeta.is_some() {
             sess.dcx().emit_fatal(errors::LinkRlibError::OnlyRmetaFound { crate_name });
@@ -620,7 +620,6 @@ fn link_dwarf_object(sess: &Session, cg_results: &CodegenResults, executable_out
             .used_crate_source
             .items()
             .filter_map(|(_, csource)| csource.rlib.as_ref())
-            .map(|(path, _)| path)
             .into_sorted_stable_ord();
 
         for input_rlib in input_rlibs {
@@ -2178,12 +2177,7 @@ fn add_rpath_args(
             .crate_info
             .used_crates
             .iter()
-            .filter_map(|cnum| {
-                codegen_results.crate_info.used_crate_source[cnum]
-                    .dylib
-                    .as_ref()
-                    .map(|(path, _)| &**path)
-            })
+            .filter_map(|cnum| codegen_results.crate_info.used_crate_source[cnum].dylib.as_deref())
             .collect::<Vec<_>>();
         let rpath_config = RPathConfig {
             libs: &*libs,
@@ -2661,7 +2655,7 @@ fn add_native_libs_from_crate(
 
     if link_static && cnum != LOCAL_CRATE && !bundled_libs.is_empty() {
         // If rlib contains native libs as archives, unpack them to tmpdir.
-        let rlib = &codegen_results.crate_info.used_crate_source[&cnum].rlib.as_ref().unwrap().0;
+        let rlib = codegen_results.crate_info.used_crate_source[&cnum].rlib.as_ref().unwrap();
         archive_builder_builder
             .extract_bundled_libs(rlib, tmpdir, bundled_libs)
             .unwrap_or_else(|e| sess.dcx().emit_fatal(e));
@@ -2832,7 +2826,7 @@ fn add_upstream_rust_crates(
             }
             Linkage::Dynamic => {
                 let src = &codegen_results.crate_info.used_crate_source[&cnum];
-                add_dynamic_crate(cmd, sess, &src.dylib.as_ref().unwrap().0);
+                add_dynamic_crate(cmd, sess, src.dylib.as_ref().unwrap());
             }
         }
 
@@ -2960,7 +2954,7 @@ fn add_static_crate(
     bundled_lib_file_names: &FxIndexSet<Symbol>,
 ) {
     let src = &codegen_results.crate_info.used_crate_source[&cnum];
-    let cratepath = &src.rlib.as_ref().unwrap().0;
+    let cratepath = src.rlib.as_ref().unwrap();
 
     let mut link_upstream =
         |path: &Path| cmd.link_staticlib_by_path(&rehome_lib_path(sess, path), false);

compiler/rustc_data_structures/src/fx.rs

@@ -1,6 +1,6 @@
 use std::hash::BuildHasherDefault;
 
-pub use rustc_hash::{FxHashMap, FxHashSet, FxHasher};
+pub use rustc_hash::{FxBuildHasher, FxHashMap, FxHashSet, FxHasher};
 
 pub type StdEntry<'a, K, V> = std::collections::hash_map::Entry<'a, K, V>;
 

compiler/rustc_interface/src/passes.rs

@@ -684,19 +684,19 @@ fn write_out_deps(tcx: TyCtxt<'_>, outputs: &OutputFilenames, out_filenames: &[P
 
             for &cnum in tcx.crates(()) {
                 let source = tcx.used_crate_source(cnum);
-                if let Some((path, _)) = &source.dylib {
+                if let Some(path) = &source.dylib {
                     files.extend(hash_iter_files(
                         iter::once(escape_dep_filename(&path.display().to_string())),
                         checksum_hash_algo,
                     ));
                 }
-                if let Some((path, _)) = &source.rlib {
+                if let Some(path) = &source.rlib {
                     files.extend(hash_iter_files(
                         iter::once(escape_dep_filename(&path.display().to_string())),
                         checksum_hash_algo,
                     ));
                 }
-                if let Some((path, _)) = &source.rmeta {
+                if let Some(path) = &source.rmeta {
                     files.extend(hash_iter_files(
                         iter::once(escape_dep_filename(&path.display().to_string())),
                         checksum_hash_algo,

compiler/rustc_metadata/src/creader.rs

@@ -135,16 +135,16 @@ impl<'a> std::fmt::Debug for CrateDump<'a> {
             writeln!(fmt, "  priv: {:?}", data.is_private_dep())?;
             let CrateSource { dylib, rlib, rmeta, sdylib_interface } = data.source();
             if let Some(dylib) = dylib {
-                writeln!(fmt, "  dylib: {}", dylib.0.display())?;
+                writeln!(fmt, "  dylib: {}", dylib.display())?;
             }
             if let Some(rlib) = rlib {
-                writeln!(fmt, "   rlib: {}", rlib.0.display())?;
+                writeln!(fmt, "   rlib: {}", rlib.display())?;
             }
             if let Some(rmeta) = rmeta {
-                writeln!(fmt, "   rmeta: {}", rmeta.0.display())?;
+                writeln!(fmt, "   rmeta: {}", rmeta.display())?;
             }
             if let Some(sdylib_interface) = sdylib_interface {
-                writeln!(fmt, "   sdylib interface: {}", sdylib_interface.0.display())?;
+                writeln!(fmt, "   sdylib interface: {}", sdylib_interface.display())?;
             }
         }
         Ok(())
@@ -516,10 +516,9 @@ impl CStore {
 
     fn existing_match(
         &self,
-        externs: &Externs,
+        _externs: &Externs,
         name: Symbol,
         hash: Option<Svh>,
-        kind: PathKind,
     ) -> Option<CrateNum> {
         for (cnum, data) in self.iter_crate_data() {
             if data.name() != name {
@@ -536,6 +535,13 @@ impl CStore {
                 None => {}
             }
 
+            /*
+            if !data.used() {
+                // Ignore speculatively loaded crates. Using them can hide ambiguity errors:
+                // https://github.com/rust-lang/rust/issues/147966#issuecomment-3444198198
+                continue;
+            }
+
             // When the hash is None we're dealing with a top-level dependency
             // in which case we may have a specification on the command line for
             // this library. Even though an upstream library may have loaded
@@ -551,37 +557,20 @@ impl CStore {
                 if let Some(mut files) = entry.files() {
                     if files.any(|l| {
                         let l = l.canonicalized();
-                        source.dylib.as_ref().map(|(p, _)| p) == Some(l)
-                            || source.rlib.as_ref().map(|(p, _)| p) == Some(l)
-                            || source.rmeta.as_ref().map(|(p, _)| p) == Some(l)
+                        source.dylib.as_ref() == Some(l)
+                            || source.rlib.as_ref() == Some(l)
+                            || source.rmeta.as_ref() == Some(l)
                     }) {
                         return Some(cnum);
                     }
                 }
                 continue;
             }
 
-            // Alright, so we've gotten this far which means that `data` has the
-            // right name, we don't have a hash, and we don't have a --extern
-            // pointing for ourselves. We're still not quite yet done because we
-            // have to make sure that this crate was found in the crate lookup
-            // path (this is a top-level dependency) as we don't want to
-            // implicitly load anything inside the dependency lookup path.
-            let prev_kind = source
-                .dylib
-                .as_ref()
-                .or(source.rlib.as_ref())
-                .or(source.rmeta.as_ref())
-                .expect("No sources for crate")
-                .1;
-            if kind.matches(prev_kind) {
-                return Some(cnum);
-            } else {
-                debug!(
-                    "failed to load existing crate {}; kind {:?} did not match prev_kind {:?}",
-                    name, kind, prev_kind
-                );
-            }
+            // While the crate name matched, no --extern crate_name=path matched. It is possible
+            // that we have already loaded the target crate, but if that happens CStore::load will
+            // indicate so and we gracefully handle this, just potentially wasting a bit of time.
+            */
         }
 
         None
@@ -677,7 +666,7 @@ impl CStore {
                 None => (&source, &crate_root),
             };
             let dlsym_dylib = dlsym_source.dylib.as_ref().expect("no dylib for a proc-macro crate");
-            Some(self.dlsym_proc_macros(tcx.sess, &dlsym_dylib.0, dlsym_root.stable_crate_id())?)
+            Some(self.dlsym_proc_macros(tcx.sess, dlsym_dylib, dlsym_root.stable_crate_id())?)
         } else {
             None
         };
@@ -818,9 +807,7 @@ impl CStore {
         let path_kind = if dep.is_some() { PathKind::Dependency } else { PathKind::Crate };
         let private_dep = origin.private_dep();
 
-        let result = if let Some(cnum) =
-            self.existing_match(&tcx.sess.opts.externs, name, hash, path_kind)
-        {
+        let result = if let Some(cnum) = self.existing_match(&tcx.sess.opts.externs, name, hash) {
             (LoadResult::Previous(cnum), None)
         } else {
             info!("falling back to a load");

compiler/rustc_metadata/src/locator.rs

@@ -218,7 +218,7 @@ use std::ops::Deref;
 use std::path::{Path, PathBuf};
 use std::{cmp, fmt};
 
-use rustc_data_structures::fx::{FxHashSet, FxIndexMap};
+use rustc_data_structures::fx::{FxHashSet, FxIndexMap, FxIndexSet};
 use rustc_data_structures::memmap::Mmap;
 use rustc_data_structures::owned_slice::{OwnedSlice, slice_owned};
 use rustc_data_structures::svh::Svh;
@@ -401,7 +401,7 @@ impl<'a> CrateLocator<'a> {
 
         let mut candidates: FxIndexMap<
             _,
-            (FxIndexMap<_, _>, FxIndexMap<_, _>, FxIndexMap<_, _>, FxIndexMap<_, _>),
+            (FxIndexSet<_>, FxIndexSet<_>, FxIndexSet<_>, FxIndexSet<_>),
         > = Default::default();
 
         // First, find all possible candidate rlibs and dylibs purely based on
@@ -460,10 +460,10 @@ impl<'a> CrateLocator<'a> {
                         // filesystem code should not care, but this is nicer for diagnostics.
                         let path = spf.path.to_path_buf();
                         match kind {
-                            CrateFlavor::Rlib => rlibs.insert(path, search_path.kind),
-                            CrateFlavor::Rmeta => rmetas.insert(path, search_path.kind),
-                            CrateFlavor::Dylib => dylibs.insert(path, search_path.kind),
-                            CrateFlavor::SDylib => interfaces.insert(path, search_path.kind),
+                            CrateFlavor::Rlib => rlibs.insert(path),
+                            CrateFlavor::Rmeta => rmetas.insert(path),
+                            CrateFlavor::Dylib => dylibs.insert(path),
+                            CrateFlavor::SDylib => interfaces.insert(path),
                         };
                     }
                 }
@@ -524,10 +524,10 @@ impl<'a> CrateLocator<'a> {
     fn extract_lib(
         &self,
         crate_rejections: &mut CrateRejections,
-        rlibs: FxIndexMap<PathBuf, PathKind>,
-        rmetas: FxIndexMap<PathBuf, PathKind>,
-        dylibs: FxIndexMap<PathBuf, PathKind>,
-        interfaces: FxIndexMap<PathBuf, PathKind>,
+        rlibs: FxIndexSet<PathBuf>,
+        rmetas: FxIndexSet<PathBuf>,
+        dylibs: FxIndexSet<PathBuf>,
+        interfaces: FxIndexSet<PathBuf>,
     ) -> Result<Option<(Svh, Library)>, CrateError> {
         let mut slot = None;
         // Order here matters, rmeta should come first.
@@ -575,10 +575,10 @@ impl<'a> CrateLocator<'a> {
     fn extract_one(
         &self,
         crate_rejections: &mut CrateRejections,
-        m: FxIndexMap<PathBuf, PathKind>,
+        m: FxIndexSet<PathBuf>,
         flavor: CrateFlavor,
         slot: &mut Option<(Svh, MetadataBlob, PathBuf, CrateFlavor)>,
-    ) -> Result<Option<(PathBuf, PathKind)>, CrateError> {
+    ) -> Result<Option<PathBuf>, CrateError> {
         // If we are producing an rlib, and we've already loaded metadata, then
         // we should not attempt to discover further crate sources (unless we're
         // locating a proc macro; exact logic is in needs_crate_flavor). This means
@@ -594,9 +594,9 @@ impl<'a> CrateLocator<'a> {
             }
         }
 
-        let mut ret: Option<(PathBuf, PathKind)> = None;
+        let mut ret: Option<PathBuf> = None;
         let mut err_data: Option<Vec<PathBuf>> = None;
-        for (lib, kind) in m {
+        for lib in m {
             info!("{} reading metadata from: {}", flavor, lib.display());
             if flavor == CrateFlavor::Rmeta && lib.metadata().is_ok_and(|m| m.len() == 0) {
                 // Empty files will cause get_metadata_section to fail. Rmeta
@@ -640,7 +640,7 @@ impl<'a> CrateLocator<'a> {
                     info!("no metadata found: {}", err);
                     // Metadata was loaded from interface file earlier.
                     if let Some((.., CrateFlavor::SDylib)) = slot {
-                        ret = Some((lib, kind));
+                        ret = Some(lib);
                         continue;
                     }
                     // The file was present and created by the same compiler version, but we
@@ -689,7 +689,7 @@ impl<'a> CrateLocator<'a> {
             // As a result, we favor the sysroot crate here. Note that the
             // candidates are all canonicalized, so we canonicalize the sysroot
             // as well.
-            if let Some((prev, _)) = &ret {
+            if let Some(prev) = &ret {
                 let sysroot = self.sysroot;
                 let sysroot = try_canonicalize(sysroot).unwrap_or_else(|_| sysroot.to_path_buf());
                 if prev.starts_with(&sysroot) {
@@ -714,7 +714,7 @@ impl<'a> CrateLocator<'a> {
             } else {
                 *slot = Some((hash, metadata, lib.clone(), flavor));
             }
-            ret = Some((lib, kind));
+            ret = Some(lib);
         }
 
         if let Some(candidates) = err_data {
@@ -774,10 +774,10 @@ impl<'a> CrateLocator<'a> {
         // First, filter out all libraries that look suspicious. We only accept
         // files which actually exist that have the correct naming scheme for
         // rlibs/dylibs.
-        let mut rlibs = FxIndexMap::default();
-        let mut rmetas = FxIndexMap::default();
-        let mut dylibs = FxIndexMap::default();
-        let mut sdylib_interfaces = FxIndexMap::default();
+        let mut rlibs = FxIndexSet::default();
+        let mut rmetas = FxIndexSet::default();
+        let mut dylibs = FxIndexSet::default();
+        let mut sdylib_interfaces = FxIndexSet::default();
         for loc in &self.exact_paths {
             let loc_canon = loc.canonicalized();
             let loc_orig = loc.original();
@@ -798,21 +798,21 @@ impl<'a> CrateLocator<'a> {
             };
             if file.starts_with("lib") {
                 if file.ends_with(".rlib") {
-                    rlibs.insert(loc_canon.clone(), PathKind::ExternFlag);
+                    rlibs.insert(loc_canon.clone());
                     continue;
                 }
                 if file.ends_with(".rmeta") {
-                    rmetas.insert(loc_canon.clone(), PathKind::ExternFlag);
+                    rmetas.insert(loc_canon.clone());
                     continue;
                 }
                 if file.ends_with(".rs") {
-                    sdylib_interfaces.insert(loc_canon.clone(), PathKind::ExternFlag);
+                    sdylib_interfaces.insert(loc_canon.clone());
                 }
             }
             let dll_prefix = self.target.dll_prefix.as_ref();
             let dll_suffix = self.target.dll_suffix.as_ref();
             if file.starts_with(dll_prefix) && file.ends_with(dll_suffix) {
-                dylibs.insert(loc_canon.clone(), PathKind::ExternFlag);
+                dylibs.insert(loc_canon.clone());
                 continue;
             }
             crate_rejections

compiler/rustc_session/src/cstore.rs

@@ -15,24 +15,22 @@ use rustc_hir::definitions::{DefKey, DefPath, DefPathHash, Definitions};
 use rustc_macros::{Decodable, Encodable, HashStable_Generic};
 use rustc_span::{Span, Symbol};
 
-use crate::search_paths::PathKind;
-
 // lonely orphan structs and enums looking for a better home
 
 /// Where a crate came from on the local filesystem. One of these three options
 /// must be non-None.
 #[derive(PartialEq, Clone, Debug, HashStable_Generic, Encodable, Decodable)]
 pub struct CrateSource {
-    pub dylib: Option<(PathBuf, PathKind)>,
-    pub rlib: Option<(PathBuf, PathKind)>,
-    pub rmeta: Option<(PathBuf, PathKind)>,
-    pub sdylib_interface: Option<(PathBuf, PathKind)>,
+    pub dylib: Option<PathBuf>,
+    pub rlib: Option<PathBuf>,
+    pub rmeta: Option<PathBuf>,
+    pub sdylib_interface: Option<PathBuf>,
 }
 
 impl CrateSource {
     #[inline]
     pub fn paths(&self) -> impl Iterator<Item = &PathBuf> {
-        self.dylib.iter().chain(self.rlib.iter()).chain(self.rmeta.iter()).map(|p| &p.0)
+        self.dylib.iter().chain(self.rlib.iter()).chain(self.rmeta.iter())
     }
 }
 

compiler/rustc_session/src/search_paths.rs

@@ -71,7 +71,6 @@ pub enum PathKind {
     Crate,
     Dependency,
     Framework,
-    ExternFlag,
     All,
 }