Skip to content

rustc: Fix a leak in dependency= paths #21113

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 16, 2015
Merged

rustc: Fix a leak in dependency= paths #21113

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 38 additions & 26 deletions src/librustc/metadata/creader.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()));
});
})
}
Expand Down Expand Up @@ -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 }
Expand All @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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;

Expand All @@ -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[],
Expand All @@ -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,
Expand Down
11 changes: 6 additions & 5 deletions src/librustc/metadata/cstore.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
}

Expand Down Expand Up @@ -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))
});
Expand Down
20 changes: 11 additions & 9 deletions src/librustc/metadata/filesearch.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 => ()
}
Expand All @@ -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 => ()
}
Expand All @@ -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;
}
Expand All @@ -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) => {
Expand All @@ -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());
Expand Down Expand Up @@ -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
});
Expand Down
47 changes: 25 additions & 22 deletions src/librustc/metadata/loader.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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,
}

Expand Down Expand Up @@ -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,
Expand All @@ -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
Expand Down Expand Up @@ -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 => {}
}
Expand All @@ -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() {
Expand All @@ -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) {
Expand All @@ -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;
Expand All @@ -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}
}
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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);
}
}
};
Expand Down
9 changes: 6 additions & 3 deletions src/librustc/session/search_paths.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ pub enum PathKind {
Native,
Crate,
Dependency,
ExternFlag,
All,
}

Expand Down Expand Up @@ -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,
}
Expand Down
Loading