From 5f295f22a752f42de421e2d5e1545908f15df9d9 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Fri, 20 May 2016 14:03:47 -0700 Subject: [PATCH] rustc: Fix again order-dependence in extern crate 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 --- src/librustc_metadata/creader.rs | 129 +++++++++++------- .../run-make/extern-multiple-copies2/Makefile | 10 ++ .../run-make/extern-multiple-copies2/bar.rs | 18 +++ .../run-make/extern-multiple-copies2/foo1.rs | 17 +++ .../run-make/extern-multiple-copies2/foo2.rs | 18 +++ 5 files changed, 143 insertions(+), 49 deletions(-) create mode 100644 src/test/run-make/extern-multiple-copies2/Makefile create mode 100644 src/test/run-make/extern-multiple-copies2/bar.rs create mode 100644 src/test/run-make/extern-multiple-copies2/foo1.rs create mode 100644 src/test/run-make/extern-multiple-copies2/foo2.rs diff --git a/src/librustc_metadata/creader.rs b/src/librustc_metadata/creader.rs index 0eacc0907bc..af4a9222194 100644 --- a/src/librustc_metadata/creader.rs +++ b/src/librustc_metadata/creader.rs @@ -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, @@ -358,12 +363,8 @@ impl<'a> CrateReader<'a> { kind: PathKind, explicitly_linked: bool) -> (ast::CrateNum, Rc, 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, @@ -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 { + 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) @@ -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 { diff --git a/src/test/run-make/extern-multiple-copies2/Makefile b/src/test/run-make/extern-multiple-copies2/Makefile new file mode 100644 index 00000000000..567d7e78a57 --- /dev/null +++ b/src/test/run-make/extern-multiple-copies2/Makefile @@ -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 diff --git a/src/test/run-make/extern-multiple-copies2/bar.rs b/src/test/run-make/extern-multiple-copies2/bar.rs new file mode 100644 index 00000000000..b8ac34aa53e --- /dev/null +++ b/src/test/run-make/extern-multiple-copies2/bar.rs @@ -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 or the MIT license +// , 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); +} diff --git a/src/test/run-make/extern-multiple-copies2/foo1.rs b/src/test/run-make/extern-multiple-copies2/foo1.rs new file mode 100644 index 00000000000..1787772053b --- /dev/null +++ b/src/test/run-make/extern-multiple-copies2/foo1.rs @@ -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 or the MIT license +// , 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); +} diff --git a/src/test/run-make/extern-multiple-copies2/foo2.rs b/src/test/run-make/extern-multiple-copies2/foo2.rs new file mode 100644 index 00000000000..bad10304387 --- /dev/null +++ b/src/test/run-make/extern-multiple-copies2/foo2.rs @@ -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 or the MIT license +// , 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); +}