OpenE2K
/
rust
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

rustc: Fix a leak in dependency= paths 

With the addition of separate search paths to the compiler, it was intended that
applications such as Cargo could require a `--extern` flag per `extern crate`
directive in the source. The system can currently be subverted, however, due to
the `existing_match()` logic in the crate loader.

When loading crates we first attempt to match an `extern crate` directive
against all previously loaded crates to avoid reading metadata twice. This "hit
the cache if possible" step was erroneously leaking crates across the search
path boundaries, however. For example:

    extern crate b;
    extern crate a;

If `b` depends on `a`, then it will load crate `a` when the `extern crate b`
directive is being processed. When the compiler reaches `extern crate a` it will
use the previously loaded version no matter what. If the compiler was not
invoked with `-L crate=path/to/a`, it will still succeed.

This behavior is allowing `extern crate` declarations in Cargo without a
corresponding declaration in the manifest of a dependency, which is considered
a bug.

This commit fixes this problem by keeping track of the origin search path for a
crate. Crates loaded from the dependency search path are not candidates for
crates which are loaded from the crate search path.

As a result of this fix, this is a likely a breaking change for a number of
Cargo packages. If the compiler starts informing that a crate can no longer be
found, it likely means that the dependency was forgotten in your Cargo.toml.

[breaking-change]
This commit is contained in:
Alex Crichton 2015-01-06 08:46:07 -08:00
parent 6ba9acd8ab
commit cbeb77ec7a
15 changed files with 145 additions and 78 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

64
src/librustc/metadata/creader.rs
View File

@ -58,8 +58,8 @@ fn dump_crates(cstore: &CStore) {
debug!(" hash: {}", data.hash());
opt_source.map(|cs| {
let CrateSource { dylib, rlib, cnum: _ } = cs;
dylib.map(|dl| debug!(" dylib: {}", dl.display()));
rlib.map(|rl| debug!(" rlib: {}", rl.display()));
dylib.map(|dl| debug!(" dylib: {}", dl.0.display()));
rlib.map(|rl| debug!(" rlib: {}", rl.0.display()));
});
})
}
@ -305,8 +305,8 @@ impl<'a> CrateReader<'a> {
}
}
fn existing_match(&self, name: &str,
hash: Option<&Svh>) -> Option<ast::CrateNum> {
fn existing_match(&self, name: &str, hash: Option<&Svh>, kind: PathKind)
-> Option<ast::CrateNum> {
let mut ret = None;
self.sess.cstore.iter_crate_data(|cnum, data| {
if data.name != name { return }
@ -317,27 +317,37 @@ impl<'a> CrateReader<'a> {
None => {}
}
// 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 something of
// the same name, we have to make sure it was loaded from the exact same
// location as well.
// 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
// something of the same name, we have to make sure it was loaded
// from the exact same location as well.
//
// We're also sure to compare *paths*, not actual byte slices. The
// `source` stores paths which are normalized which may be different
// from the strings on the command line.
let source = self.sess.cstore.get_used_crate_source(cnum).unwrap();
match self.sess.opts.externs.get(name) {
Some(locs) => {
let found = locs.iter().any(|l| {
let l = fs::realpath(&Path::new(&l[])).ok();
l == source.dylib || l == source.rlib
});
if found {
ret = Some(cnum);
}
if let Some(locs) = self.sess.opts.externs.get(name) {
let found = locs.iter().any(|l| {
let l = fs::realpath(&Path::new(&l[])).ok();
source.dylib.as_ref().map(|p| &p.0) == l.as_ref() ||
source.rlib.as_ref().map(|p| &p.0) == l.as_ref()
});
if found {
ret = Some(cnum);
}
None => ret = Some(cnum),
}
// 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())
.unwrap().1;
if ret.is_none() && (prev_kind == kind || prev_kind == PathKind::All) {
ret = Some(cnum);
}
});
return ret;
@ -359,8 +369,8 @@ impl<'a> CrateReader<'a> {
let crate_paths = if root.is_none() {
Some(CratePaths {
ident: ident.to_string(),
dylib: lib.dylib.clone(),
rlib: lib.rlib.clone(),
dylib: lib.dylib.clone().map(|p| p.0),
rlib: lib.rlib.clone().map(|p| p.0),
})
} else {
None
@ -400,7 +410,7 @@ impl<'a> CrateReader<'a> {
kind: PathKind)
-> (ast::CrateNum, Rc<cstore::crate_metadata>,
cstore::CrateSource) {
match self.existing_match(name, hash) {
match self.existing_match(name, hash, kind) {
None => {
let mut load_ctxt = loader::Context {
sess: self.sess,
@ -483,8 +493,8 @@ impl<'a> CrateReader<'a> {
let library = match load_ctxt.maybe_load_library_crate() {
Some(l) => l,
None if is_cross => {
// Try loading from target crates. This will abort later if we try to
// load a plugin registrar function,
// Try loading from target crates. This will abort later if we
// try to load a plugin registrar function,
target_only = true;
should_link = info.should_link;
@ -497,7 +507,9 @@ impl<'a> CrateReader<'a> {
};
let dylib = library.dylib.clone();
let register = should_link && self.existing_match(info.name.as_slice(), None).is_none();
let register = should_link && self.existing_match(info.name.as_slice(),
None,
PathKind::Crate).is_none();
let metadata = if register {
// Register crate now to avoid double-reading metadata
let (_, cmd, _) = self.register_crate(&None, &info.ident[],
@ -511,7 +523,7 @@ impl<'a> CrateReader<'a> {
PluginMetadata {
sess: self.sess,
metadata: metadata,
dylib: dylib,
dylib: dylib.map(|p| p.0),
info: info,
vi_span: span,
target_only: target_only,

11
src/librustc/metadata/cstore.rs
View File

@ -20,6 +20,7 @@ pub use self::NativeLibraryKind::*;
use back::svh::Svh;
use metadata::decoder;
use metadata::loader;
use session::search_paths::PathKind;
use util::nodemap::{FnvHashMap, NodeMap};
use std::cell::RefCell;
@ -65,8 +66,8 @@ pub enum NativeLibraryKind {
// must be non-None.
#[derive(PartialEq, Clone)]
pub struct CrateSource {
pub dylib: Option<Path>,
pub rlib: Option<Path>,
pub dylib: Option<(Path, PathKind)>,
pub rlib: Option<(Path, PathKind)>,
pub cnum: ast::CrateNum,
}
@ -178,10 +179,10 @@ impl CStore {
let mut libs = self.used_crate_sources.borrow()
.iter()
.map(|src| (src.cnum, match prefer {
RequireDynamic => src.dylib.clone(),
RequireStatic => src.rlib.clone(),
RequireDynamic => src.dylib.clone().map(|p| p.0),
RequireStatic => src.rlib.clone().map(|p| p.0),
}))
.collect::<Vec<(ast::CrateNum, Option<Path>)>>();
.collect::<Vec<_>>();
libs.sort_by(|&(a, _), &(b, _)| {
ordering.position_elem(&a).cmp(&ordering.position_elem(&b))
});

20
src/librustc/metadata/filesearch.rs
View File

@ -39,13 +39,13 @@ pub struct FileSearch<'a> {
impl<'a> FileSearch<'a> {
pub fn for_each_lib_search_path<F>(&self, mut f: F) where
F: FnMut(&Path) -> FileMatch,
F: FnMut(&Path, PathKind) -> FileMatch,
{
let mut visited_dirs = HashSet::new();
let mut found = false;
for path in self.search_paths.iter(self.kind) {
match f(path) {
for (path, kind) in self.search_paths.iter(self.kind) {
match f(path, kind) {
FileMatches => found = true,
FileDoesntMatch => ()
}
@ -56,7 +56,7 @@ impl<'a> FileSearch<'a> {
let tlib_path = make_target_lib_path(self.sysroot,
self.triple);
if !visited_dirs.contains(tlib_path.as_vec()) {
match f(&tlib_path) {
match f(&tlib_path, PathKind::All) {
FileMatches => found = true,
FileDoesntMatch => ()
}
@ -76,7 +76,7 @@ impl<'a> FileSearch<'a> {
visited_dirs.insert(tlib_path.as_vec().to_vec());
// Don't keep searching the RUST_PATH if one match turns up --
// if we did, we'd get a "multiple matching crates" error
match f(&tlib_path) {
match f(&tlib_path, PathKind::All) {
FileMatches => {
break;
}
@ -91,8 +91,10 @@ impl<'a> FileSearch<'a> {
make_target_lib_path(self.sysroot, self.triple)
}
pub fn search<F>(&self, mut pick: F) where F: FnMut(&Path) -> FileMatch {
self.for_each_lib_search_path(|lib_search_path| {
pub fn search<F>(&self, mut pick: F)
where F: FnMut(&Path, PathKind) -> FileMatch
{
self.for_each_lib_search_path(|lib_search_path, kind| {
debug!("searching {}", lib_search_path.display());
match fs::readdir(lib_search_path) {
Ok(files) => {
@ -108,7 +110,7 @@ impl<'a> FileSearch<'a> {
let files2 = files.iter().filter(|p| !is_rlib(p));
for path in files1.chain(files2) {
debug!("testing {}", path.display());
let maybe_picked = pick(path);
let maybe_picked = pick(path, kind);
match maybe_picked {
FileMatches => {
debug!("picked {}", path.display());
@ -142,7 +144,7 @@ impl<'a> FileSearch<'a> {
// Returns a list of directories where target-specific dylibs might be located.
pub fn get_dylib_search_paths(&self) -> Vec<Path> {
let mut paths = Vec::new();
self.for_each_lib_search_path(|lib_search_path| {
self.for_each_lib_search_path(|lib_search_path, _| {
paths.push(lib_search_path.clone());
FileDoesntMatch
});

47
src/librustc/metadata/loader.rs
View File

@ -215,6 +215,7 @@
use back::archive::{METADATA_FILENAME};
use back::svh::Svh;
use session::Session;
use session::search_paths::PathKind;
use llvm;
use llvm::{False, ObjectFile, mk_section_iter};
use llvm::archive_ro::ArchiveRO;
@ -229,7 +230,7 @@ use rustc_back::target::Target;
use std::ffi::CString;
use std::cmp;
use std::collections::{HashMap, HashSet};
use std::collections::HashMap;
use std::io::fs::PathExtensions;
use std::io;
use std::ptr;
@ -260,8 +261,8 @@ pub struct Context<'a> {
}
pub struct Library {
pub dylib: Option<Path>,
pub rlib: Option<Path>,
pub dylib: Option<(Path, PathKind)>,
pub rlib: Option<(Path, PathKind)>,
pub metadata: MetadataBlob,
}
@ -384,7 +385,7 @@ impl<'a> Context<'a> {
// of the crate id (path/name/id).
//
// The goal of this step is to look at as little metadata as possible.
self.filesearch.search(|path| {
self.filesearch.search(|path, kind| {
let file = match path.filename_str() {
None => return FileDoesntMatch,
Some(file) => file,
@ -404,12 +405,12 @@ impl<'a> Context<'a> {
let hash_str = hash.to_string();
let slot = candidates.entry(hash_str).get().unwrap_or_else(
|vacant_entry| vacant_entry.insert((HashSet::new(), HashSet::new())));
|vacant_entry| vacant_entry.insert((HashMap::new(), HashMap::new())));
let (ref mut rlibs, ref mut dylibs) = *slot;
if rlib {
rlibs.insert(fs::realpath(path).unwrap());
rlibs.insert(fs::realpath(path).unwrap(), kind);
} else {
dylibs.insert(fs::realpath(path).unwrap());
dylibs.insert(fs::realpath(path).unwrap(), kind);
}
FileMatches
@ -453,16 +454,16 @@ impl<'a> Context<'a> {
self.sess.note("candidates:");
for lib in libraries.iter() {
match lib.dylib {
Some(ref p) => {
Some((ref p, _)) => {
self.sess.note(&format!("path: {}",
p.display())[]);
}
None => {}
}
match lib.rlib {
Some(ref p) => {
Some((ref p, _)) => {
self.sess.note(&format!("path: {}",
p.display())[]);
p.display())[]);
}
None => {}
}
@ -483,9 +484,9 @@ impl<'a> Context<'a> {
// read the metadata from it if `*slot` is `None`. If the metadata couldn't
// be read, it is assumed that the file isn't a valid rust library (no
// errors are emitted).
fn extract_one(&mut self, m: HashSet<Path>, flavor: &str,
slot: &mut Option<MetadataBlob>) -> Option<Path> {
let mut ret = None::<Path>;
fn extract_one(&mut self, m: HashMap<Path, PathKind>, flavor: &str,
slot: &mut Option<MetadataBlob>) -> Option<(Path, PathKind)> {
let mut ret = None::<(Path, PathKind)>;
let mut error = 0u;
if slot.is_some() {
@ -500,7 +501,7 @@ impl<'a> Context<'a> {
}
}
for lib in m.into_iter() {
for (lib, kind) in m.into_iter() {
info!("{} reading metadata from: {}", flavor, lib.display());
let metadata = match get_metadata_section(self.target.options.is_like_osx,
&lib) {
@ -525,7 +526,7 @@ impl<'a> Context<'a> {
self.crate_name)[]);
self.sess.span_note(self.span,
&format!(r"candidate #1: {}",
ret.as_ref().unwrap()
ret.as_ref().unwrap().0
.display())[]);
error = 1;
ret = None;
@ -538,7 +539,7 @@ impl<'a> Context<'a> {
continue
}
*slot = Some(metadata);
ret = Some(lib);
ret = Some((lib, kind));
}
return if error > 0 {None} else {ret}
}
@ -606,8 +607,8 @@ impl<'a> Context<'a> {
// rlibs/dylibs.
let sess = self.sess;
let dylibname = self.dylibname();
let mut rlibs = HashSet::new();
let mut dylibs = HashSet::new();
let mut rlibs = HashMap::new();
let mut dylibs = HashMap::new();
{
let mut locs = locs.iter().map(|l| Path::new(&l[])).filter(|loc| {
if !loc.exists() {
@ -637,13 +638,15 @@ impl<'a> Context<'a> {
false
});
// Now that we have an iterator of good candidates, make sure there's at
// most one rlib and at most one dylib.
// Now that we have an iterator of good candidates, make sure
// there's at most one rlib and at most one dylib.
for loc in locs {
if loc.filename_str().unwrap().ends_with(".rlib") {
rlibs.insert(fs::realpath(&loc).unwrap());
rlibs.insert(fs::realpath(&loc).unwrap(),
PathKind::ExternFlag);
} else {
dylibs.insert(fs::realpath(&loc).unwrap());
dylibs.insert(fs::realpath(&loc).unwrap(),
PathKind::ExternFlag);
}
}
};

9
src/librustc/session/search_paths.rs
View File

@ -25,6 +25,7 @@ pub enum PathKind {
Native,
Crate,
Dependency,
ExternFlag,
All,
}
@ -54,14 +55,16 @@ impl SearchPaths {
}
impl<'a> Iterator for Iter<'a> {
type Item = &'a Path;
type Item = (&'a Path, PathKind);
fn next(&mut self) -> Option<&'a Path> {
fn next(&mut self) -> Option<(&'a Path, PathKind)> {
loop {
match self.iter.next() {
Some(&(kind, ref p)) if self.kind == PathKind::All ||
kind == PathKind::All ||
kind == self.kind => return Some(p),
kind == self.kind => {
return Some((p, kind))
}
Some(..) => {}
None => return None,
}

8
src/librustc_trans/back/link.rs
View File

@ -507,7 +507,7 @@ fn link_binary_output(sess: &Session,
fn archive_search_paths(sess: &Session) -> Vec<Path> {
let mut search = Vec::new();
sess.target_filesearch(PathKind::Native).for_each_lib_search_path(|path| {
sess.target_filesearch(PathKind::Native).for_each_lib_search_path(|path, _| {
search.push(path.clone());
FileDoesntMatch
});
@ -1043,7 +1043,7 @@ fn link_args(cmd: &mut Command,
// in the current crate. Upstream crates with native library dependencies
// may have their native library pulled in above.
fn add_local_native_libraries(cmd: &mut Command, sess: &Session) {
sess.target_filesearch(PathKind::All).for_each_lib_search_path(|path| {
sess.target_filesearch(PathKind::All).for_each_lib_search_path(|path, _| {
cmd.arg("-L").arg(path);
FileDoesntMatch
});
@ -1146,10 +1146,10 @@ fn add_upstream_rust_crates(cmd: &mut Command, sess: &Session,
let src = sess.cstore.get_used_crate_source(cnum).unwrap();
match kind {
cstore::RequireDynamic => {
add_dynamic_crate(cmd, sess, src.dylib.unwrap())
add_dynamic_crate(cmd, sess, src.dylib.unwrap().0)
}
cstore::RequireStatic => {
add_static_crate(cmd, sess, tmpdir, src.rlib.unwrap())
add_static_crate(cmd, sess, tmpdir, src.rlib.unwrap().0)
}
}

8
src/test/run-make/compiler-lookup-paths-2/Makefile Normal file
View File

@ -0,0 +1,8 @@
-include ../tools.mk
all:
mkdir -p $(TMPDIR)/a $(TMPDIR)/b
$(RUSTC) a.rs && mv $(TMPDIR)/liba.rlib $(TMPDIR)/a
$(RUSTC) b.rs -L $(TMPDIR)/a && mv $(TMPDIR)/libb.rlib $(TMPDIR)/b
$(RUSTC) c.rs -L crate=$(TMPDIR)/b -L dependency=$(TMPDIR)/a \
&& exit 1 || exit 0

11
src/test/run-make/compiler-lookup-paths-2/a.rs Normal file
View File

@ -0,0 +1,11 @@
// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.
#![crate_type = "lib"]

12
src/test/run-make/compiler-lookup-paths-2/b.rs Normal file
View File

@ -0,0 +1,12 @@
// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.
#![crate_type = "lib"]
extern crate a;

13
src/test/run-make/compiler-lookup-paths-2/c.rs Normal file
View File

@ -0,0 +1,13 @@
// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.
#![crate_type = "lib"]
extern crate b;
extern crate a;

2
src/test/run-make/extern-flag-disambiguates/Makefile
View File

@ -17,8 +17,10 @@ all:
$(RUSTC) -C metadata=2 -C extra-filename=-2 a.rs
$(RUSTC) b.rs --extern a=$(TMPDIR)/liba-1.rlib
$(RUSTC) c.rs --extern a=$(TMPDIR)/liba-2.rlib
@echo before
$(RUSTC) --cfg before d.rs --extern a=$(TMPDIR)/liba-1.rlib
$(call RUN,d)
@echo after
$(RUSTC) --cfg after d.rs --extern a=$(TMPDIR)/liba-1.rlib
$(call RUN,d)

4
src/test/run-make/extern-flag-disambiguates/a.rs
View File

@ -11,6 +11,6 @@
#![crate_name = "a"]
#![crate_type = "rlib"]
static FOO: uint = 3;
static FOO: usize = 3;
pub fn token() -> &'static uint { &FOO }
pub fn token() -> &'static usize { &FOO }

6
src/test/run-make/extern-flag-disambiguates/b.rs
View File

@ -13,7 +13,7 @@
extern crate a;
static FOO: uint = 3;
static FOO: usize = 3;
pub fn token() -> &'static uint { &FOO }
pub fn a_token() -> &'static uint { a::token() }
pub fn token() -> &'static usize { &FOO }
pub fn a_token() -> &'static usize { a::token() }

6
src/test/run-make/extern-flag-disambiguates/c.rs
View File

@ -13,7 +13,7 @@
extern crate a;
static FOO: uint = 3;
static FOO: usize = 3;
pub fn token() -> &'static uint { &FOO }
pub fn a_token() -> &'static uint { a::token() }
pub fn token() -> &'static usize { &FOO }
pub fn a_token() -> &'static usize { a::token() }

2
src/test/run-make/extern-flag-disambiguates/d.rs
View File

@ -13,7 +13,7 @@ extern crate b;
extern crate c;
#[cfg(after)] extern crate a;
fn t(a: &'static uint) -> uint { a as *const _ as uint }
fn t(a: &'static usize) -> usize { a as *const _ as usize }
fn main() {
assert!(t(a::token()) == t(b::a_token()));