Skip to content

Commit

Permalink
rustc: Fix again order-dependence in extern crate
Browse files Browse the repository at this point in the history
Originally fixed in #29961 the bug was unfortunately still present in the face
of crates using `#[macro_use]`. This commit refactors for the two code paths to
share common logic to ensure that they both pick up the same bug fix.

Closes #33762
  • Loading branch information
alexcrichton committed May 20, 2016
1 parent f4e4ec7 commit 5f295f2
Show file tree
Hide file tree
Showing 5 changed files with 143 additions and 49 deletions.
129 changes: 80 additions & 49 deletions src/librustc_metadata/creader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,11 @@ impl PMDSource {
}
}

enum LoadResult {
Previous(ast::CrateNum),
Loaded(loader::Library),
}

impl<'a> CrateReader<'a> {
pub fn new(sess: &'a Session,
cstore: &'a CStore,
Expand Down Expand Up @@ -358,12 +363,8 @@ impl<'a> CrateReader<'a> {
kind: PathKind,
explicitly_linked: bool)
-> (ast::CrateNum, Rc<cstore::crate_metadata>, cstore::CrateSource) {
enum LookupResult {
Previous(ast::CrateNum),
Loaded(loader::Library),
}
let result = match self.existing_match(name, hash, kind) {
Some(cnum) => LookupResult::Previous(cnum),
Some(cnum) => LoadResult::Previous(cnum),
None => {
let mut load_ctxt = loader::Context {
sess: self.sess,
Expand All @@ -380,40 +381,59 @@ impl<'a> CrateReader<'a> {
rejected_via_kind: vec!(),
should_match_name: true,
};
let library = load_ctxt.load_library_crate();

// In the case that we're loading a crate, but not matching
// against a hash, we could load a crate which has the same hash
// as an already loaded crate. If this is the case prevent
// duplicates by just using the first crate.
let meta_hash = decoder::get_crate_hash(library.metadata
.as_slice());
let mut result = LookupResult::Loaded(library);
self.cstore.iter_crate_data(|cnum, data| {
if data.name() == name && meta_hash == data.hash() {
assert!(hash.is_none());
result = LookupResult::Previous(cnum);
}
});
result
match self.load(&mut load_ctxt) {
Some(result) => result,
None => load_ctxt.report_load_errs(),
}
}
};

match result {
LookupResult::Previous(cnum) => {
LoadResult::Previous(cnum) => {
let data = self.cstore.get_crate_data(cnum);
if explicitly_linked && !data.explicitly_linked.get() {
data.explicitly_linked.set(explicitly_linked);
}
(cnum, data, self.cstore.used_crate_source(cnum))
}
LookupResult::Loaded(library) => {
LoadResult::Loaded(library) => {
self.register_crate(root, ident, name, span, library,
explicitly_linked)
}
}
}

fn load(&mut self, loader: &mut loader::Context) -> Option<LoadResult> {
let library = match loader.maybe_load_library_crate() {
Some(lib) => lib,
None => return None,
};

// In the case that we're loading a crate, but not matching
// against a hash, we could load a crate which has the same hash
// as an already loaded crate. If this is the case prevent
// duplicates by just using the first crate.
//
// Note that we only do this for target triple crates, though, as we
// don't want to match a host crate against an equivalent target one
// already loaded.
if loader.triple == self.sess.opts.target_triple {
let meta_hash = decoder::get_crate_hash(library.metadata.as_slice());
let meta_name = decoder::get_crate_name(library.metadata.as_slice())
.to_string();
let mut result = LoadResult::Loaded(library);
self.cstore.iter_crate_data(|cnum, data| {
if data.name() == meta_name && meta_hash == data.hash() {
assert!(loader.hash.is_none());
result = LoadResult::Previous(cnum);
}
});
Some(result)
} else {
Some(LoadResult::Loaded(library))
}
}

fn update_extern_crate(&mut self,
cnum: ast::CrateNum,
mut extern_crate: ExternCrate)
Expand Down Expand Up @@ -488,35 +508,46 @@ impl<'a> CrateReader<'a> {
rejected_via_kind: vec!(),
should_match_name: true,
};
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,
target_only = true;
should_link = info.should_link;

load_ctxt.target = &self.sess.target.target;
load_ctxt.triple = target_triple;
load_ctxt.filesearch = self.sess.target_filesearch(PathKind::Crate);
load_ctxt.load_library_crate()
let library = self.load(&mut load_ctxt).or_else(|| {
if !is_cross {
return None
}
None => { load_ctxt.report_load_errs(); },
// 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;

load_ctxt.target = &self.sess.target.target;
load_ctxt.triple = target_triple;
load_ctxt.filesearch = self.sess.target_filesearch(PathKind::Crate);

self.load(&mut load_ctxt)
});
let library = match library {
Some(l) => l,
None => load_ctxt.report_load_errs(),
};

let dylib = library.dylib.clone();
let register = should_link && self.existing_match(&info.name,
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,
&info.name, span, library,
true);
PMDSource::Registered(cmd)
} else {
// Not registering the crate; just hold on to the metadata
PMDSource::Owned(library.metadata)
let (dylib, metadata) = match library {
LoadResult::Previous(cnum) => {
let dylib = self.cstore.opt_used_crate_source(cnum).unwrap().dylib;
let data = self.cstore.get_crate_data(cnum);
(dylib, PMDSource::Registered(data))
}
LoadResult::Loaded(library) => {
let dylib = library.dylib.clone();
let metadata = if should_link {
// Register crate now to avoid double-reading metadata
let (_, cmd, _) = self.register_crate(&None, &info.ident,
&info.name, span,
library, true);
PMDSource::Registered(cmd)
} else {
// Not registering the crate; just hold on to the metadata
PMDSource::Owned(library.metadata)
};
(dylib, metadata)
}
};

ExtensionCrate {
Expand Down
10 changes: 10 additions & 0 deletions src/test/run-make/extern-multiple-copies2/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
-include ../tools.mk

all:
$(RUSTC) foo1.rs
$(RUSTC) foo2.rs
mkdir $(TMPDIR)/foo
cp $(TMPDIR)/libfoo1.rlib $(TMPDIR)/foo/libfoo1.rlib
$(RUSTC) bar.rs \
--extern foo1=$(TMPDIR)/foo/libfoo1.rlib \
--extern foo2=$(TMPDIR)/libfoo2.rlib
18 changes: 18 additions & 0 deletions src/test/run-make/extern-multiple-copies2/bar.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// 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.

#[macro_use]
extern crate foo2; // foo2 first to exhibit the bug
#[macro_use]
extern crate foo1;

fn main() {
foo2::foo2(foo1::A);
}
17 changes: 17 additions & 0 deletions src/test/run-make/extern-multiple-copies2/foo1.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// 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 = "rlib"]

pub struct A;

pub fn foo1(a: A) {
drop(a);
}
18 changes: 18 additions & 0 deletions src/test/run-make/extern-multiple-copies2/foo2.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// 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 = "rlib"]

#[macro_use]
extern crate foo1;

pub fn foo2(a: foo1::A) {
foo1::foo1(a);
}

0 comments on commit 5f295f2

Please sign in to comment.