Skip to content

Commit

Permalink
Rollup merge of rust-lang#64324 - alexcrichton:share-generics-again, …
Browse files Browse the repository at this point in the history
…r=michaelwoerister

rustc: Fix mixing crates with different `share_generics`

This commit addresses rust-lang#64319 by removing the `dylib` crate type from the
list of crate type that exports generic symbols. The bug in rust-lang#64319
arises because a `dylib` crate type was trying to export a symbol in an
uptream crate but it miscalculated the symbol name of the uptream
symbol. This isn't really necessary, though, since `dylib` crates aren't
that heavily used, so we can just conservatively say that the `dylib`
crate type never exports generic symbols, forcibly removing them from
the exported symbol lists if were to otherwise find them.

The fix here happens in two places:

* First is in the `local_crate_exports_generics` method, indicating that
  it's now `false` for the `Dylib` crate type. Only rlibs actually
  export generics at this point.

* Next is when we load exported symbols from upstream crate. If, for our
  compilation session, the crate may be included from a dynamic library,
  then its generic symbols are removed. When the crate was linked into a
  dynamic library its symbols weren't exported, so we can't consider
  them a candidate to link against.

Overally this should avoid situations where we incorrectly calculate the
upstream symbol names in the face of differnet `share_generics` options,
ultimately...

Closes rust-lang#64319
  • Loading branch information
Centril committed Sep 25, 2019
2 parents dcd473d + f00c634 commit 40fae88
Show file tree
Hide file tree
Showing 26 changed files with 565 additions and 433 deletions.
378 changes: 6 additions & 372 deletions src/librustc/middle/dependency_format.rs

Large diffs are not rendered by default.

6 changes: 6 additions & 0 deletions src/librustc/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -630,6 +630,12 @@ rustc_queries! {
-> &'tcx [(CrateNum, LinkagePreference)] {
desc { "dylib dependency formats of crate" }
}

query dependency_formats(_: CrateNum)
-> Lrc<crate::middle::dependency_format::Dependencies>
{
desc { "get the linkage format of all dependencies" }
}
}

Codegen {
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/session/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -687,7 +687,7 @@ pub enum EntryFnType {

impl_stable_hash_via_hash!(EntryFnType);

#[derive(Copy, PartialEq, PartialOrd, Clone, Ord, Eq, Hash, Debug)]
#[derive(Copy, PartialEq, PartialOrd, Clone, Ord, Eq, Hash, Debug, HashStable)]
pub enum CrateType {
Executable,
Dylib,
Expand Down
3 changes: 0 additions & 3 deletions src/librustc/session/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ use rustc_data_structures::fingerprint::Fingerprint;

use crate::lint;
use crate::lint::builtin::BuiltinLintDiagnostics;
use crate::middle::dependency_format;
use crate::session::config::{OutputType, PrintRequest, SwitchWithOptPath};
use crate::session::search_paths::{PathKind, SearchPath};
use crate::util::nodemap::{FxHashMap, FxHashSet};
Expand Down Expand Up @@ -91,7 +90,6 @@ pub struct Session {
pub plugin_llvm_passes: OneThread<RefCell<Vec<String>>>,
pub plugin_attributes: Lock<Vec<(Symbol, AttributeType)>>,
pub crate_types: Once<Vec<config::CrateType>>,
pub dependency_formats: Once<dependency_format::Dependencies>,
/// The `crate_disambiguator` is constructed out of all the `-C metadata`
/// arguments passed to the compiler. Its value together with the crate-name
/// forms a unique global identifier for the crate. It is used to allow
Expand Down Expand Up @@ -1248,7 +1246,6 @@ fn build_session_(
plugin_llvm_passes: OneThread::new(RefCell::new(Vec::new())),
plugin_attributes: Lock::new(Vec::new()),
crate_types: Once::new(),
dependency_formats: Once::new(),
crate_disambiguator: Once::new(),
features: Once::new(),
recursion_limit: Once::new(),
Expand Down
4 changes: 2 additions & 2 deletions src/librustc/ty/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1510,9 +1510,9 @@ impl<'tcx> TyCtxt<'tcx> {
CrateType::Executable |
CrateType::Staticlib |
CrateType::ProcMacro |
CrateType::Dylib |
CrateType::Cdylib => false,
CrateType::Rlib |
CrateType::Dylib => true,
CrateType::Rlib => true,
}
})
}
Expand Down
61 changes: 39 additions & 22 deletions src/librustc_codegen_ssa/back/link.rs
Original file line number Diff line number Diff line change
Expand Up @@ -219,15 +219,24 @@ pub fn get_linker(sess: &Session, linker: &Path, flavor: LinkerFlavor) -> (PathB
(linker.to_path_buf(), cmd)
}

pub fn each_linked_rlib(sess: &Session,
info: &CrateInfo,
f: &mut dyn FnMut(CrateNum, &Path)) -> Result<(), String> {
pub fn each_linked_rlib(
info: &CrateInfo,
f: &mut dyn FnMut(CrateNum, &Path),
) -> Result<(), String> {
let crates = info.used_crates_static.iter();
let fmts = sess.dependency_formats.borrow();
let fmts = fmts.get(&config::CrateType::Executable)
.or_else(|| fmts.get(&config::CrateType::Staticlib))
.or_else(|| fmts.get(&config::CrateType::Cdylib))
.or_else(|| fmts.get(&config::CrateType::ProcMacro));
let mut fmts = None;
for (ty, list) in info.dependency_formats.iter() {
match ty {
config::CrateType::Executable |
config::CrateType::Staticlib |
config::CrateType::Cdylib |
config::CrateType::ProcMacro => {
fmts = Some(list);
break;
}
_ => {}
}
}
let fmts = match fmts {
Some(f) => f,
None => return Err("could not find formats for rlibs".to_string())
Expand Down Expand Up @@ -406,7 +415,7 @@ fn link_staticlib<'a, B: ArchiveBuilder<'a>>(sess: &'a Session,
tempdir);
let mut all_native_libs = vec![];

let res = each_linked_rlib(sess, &codegen_results.crate_info, &mut |cnum, path| {
let res = each_linked_rlib(&codegen_results.crate_info, &mut |cnum, path| {
let name = &codegen_results.crate_info.crate_name[&cnum];
let native_libs = &codegen_results.crate_info.native_libraries[&cnum];

Expand Down Expand Up @@ -1294,11 +1303,13 @@ pub fn add_local_native_libraries(cmd: &mut dyn Linker,
// Rust crates are not considered at all when creating an rlib output. All
// dependencies will be linked when producing the final output (instead of
// the intermediate rlib version)
fn add_upstream_rust_crates<'a, B: ArchiveBuilder<'a>>(cmd: &mut dyn Linker,
sess: &'a Session,
codegen_results: &CodegenResults,
crate_type: config::CrateType,
tmpdir: &Path) {
fn add_upstream_rust_crates<'a, B: ArchiveBuilder<'a>>(
cmd: &mut dyn Linker,
sess: &'a Session,
codegen_results: &CodegenResults,
crate_type: config::CrateType,
tmpdir: &Path,
) {
// All of the heavy lifting has previously been accomplished by the
// dependency_format module of the compiler. This is just crawling the
// output of that module, adding crates as necessary.
Expand All @@ -1307,8 +1318,10 @@ fn add_upstream_rust_crates<'a, B: ArchiveBuilder<'a>>(cmd: &mut dyn Linker,
// will slurp up the object files inside), and linking to a dynamic library
// involves just passing the right -l flag.

let formats = sess.dependency_formats.borrow();
let data = formats.get(&crate_type).unwrap();
let (_, data) = codegen_results.crate_info.dependency_formats
.iter()
.find(|(ty, _)| *ty == crate_type)
.expect("failed to find crate type in dependency format list");

// Invoke get_used_crates to ensure that we get a topological sorting of
// crates.
Expand Down Expand Up @@ -1620,10 +1633,12 @@ fn add_upstream_rust_crates<'a, B: ArchiveBuilder<'a>>(cmd: &mut dyn Linker,
// generic function calls a native function, then the generic function must
// be instantiated in the target crate, meaning that the native symbol must
// also be resolved in the target crate.
pub fn add_upstream_native_libraries(cmd: &mut dyn Linker,
sess: &Session,
codegen_results: &CodegenResults,
crate_type: config::CrateType) {
pub fn add_upstream_native_libraries(
cmd: &mut dyn Linker,
sess: &Session,
codegen_results: &CodegenResults,
crate_type: config::CrateType,
) {
// Be sure to use a topological sorting of crates because there may be
// interdependencies between native libraries. When passing -nodefaultlibs,
// for example, almost all native libraries depend on libc, so we have to
Expand All @@ -1633,8 +1648,10 @@ pub fn add_upstream_native_libraries(cmd: &mut dyn Linker,
// This passes RequireStatic, but the actual requirement doesn't matter,
// we're just getting an ordering of crate numbers, we're not worried about
// the paths.
let formats = sess.dependency_formats.borrow();
let data = formats.get(&crate_type).unwrap();
let (_, data) = codegen_results.crate_info.dependency_formats
.iter()
.find(|(ty, _)| *ty == crate_type)
.expect("failed to find crate type in dependency format list");

let crates = &codegen_results.crate_info.used_crates_static;
for &(cnum, _) in crates {
Expand Down
34 changes: 29 additions & 5 deletions src/librustc_codegen_ssa/back/linker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ use rustc::middle::dependency_format::Linkage;
use rustc::session::Session;
use rustc::session::config::{self, CrateType, OptLevel, DebugInfo,
LinkerPluginLto, Lto};
use rustc::middle::exported_symbols::ExportedSymbol;
use rustc::ty::TyCtxt;
use rustc_target::spec::{LinkerFlavor, LldFlavor};
use rustc_serialize::{json, Encoder};
Expand Down Expand Up @@ -1092,18 +1093,41 @@ fn exported_symbols(tcx: TyCtxt<'_>, crate_type: CrateType) -> Vec<String> {
}
}

let formats = tcx.sess.dependency_formats.borrow();
let deps = formats[&crate_type].iter();
let formats = tcx.dependency_formats(LOCAL_CRATE);
let deps = formats.iter().filter_map(|(t, list)| {
if *t == crate_type {
Some(list)
} else {
None
}
}).next().unwrap();

for (index, dep_format) in deps.enumerate() {
for (index, dep_format) in deps.iter().enumerate() {
let cnum = CrateNum::new(index + 1);
// For each dependency that we are linking to statically ...
if *dep_format == Linkage::Static {
// ... we add its symbol list to our export list.
for &(symbol, level) in tcx.exported_symbols(cnum).iter() {
if level.is_below_threshold(export_threshold) {
symbols.push(symbol.symbol_name(tcx).to_string());
if !level.is_below_threshold(export_threshold) {
continue;
}

// Do not export generic symbols from upstream crates in linked
// artifact (notably the `dylib` crate type). The main reason
// for this is that `symbol_name` is actually wrong for generic
// symbols because it guesses the name we'd give them locally
// rather than the name it has upstream (depending on
// `share_generics` settings and such).
//
// To fix that issue we just say that linked artifacts, aka
// `dylib`s, never export generic symbols and they aren't
// available to downstream crates. (the not available part is
// handled elsewhere).
if let ExportedSymbol::Generic(..) = symbol {
continue;
}

symbols.push(symbol.symbol_name(tcx).to_string());
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_codegen_ssa/back/symbol_export.rs
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,7 @@ fn upstream_monomorphizations_provider(
};

for &cnum in cnums.iter() {
for &(ref exported_symbol, _) in tcx.exported_symbols(cnum).iter() {
for (exported_symbol, _) in tcx.exported_symbols(cnum).iter() {
if let &ExportedSymbol::Generic(def_id, substs) = exported_symbol {
let substs_map = instances.entry(def_id).or_default();

Expand Down
2 changes: 1 addition & 1 deletion src/librustc_codegen_ssa/back/write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1048,7 +1048,7 @@ fn start_executing_work<B: ExtraBackendMethods>(
}).expect("failed to spawn helper thread");

let mut each_linked_rlib_for_lto = Vec::new();
drop(link::each_linked_rlib(sess, crate_info, &mut |cnum, path| {
drop(link::each_linked_rlib(crate_info, &mut |cnum, path| {
if link::ignored_for_lto(sess, crate_info, cnum) {
return
}
Expand Down
3 changes: 2 additions & 1 deletion src/librustc_codegen_ssa/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -539,7 +539,7 @@ pub fn codegen_crate<B: ExtraBackendMethods>(
// linkage, then it's already got an allocator shim and we'll be using that
// one instead. If nothing exists then it's our job to generate the
// allocator!
let any_dynamic_crate = tcx.sess.dependency_formats.borrow()
let any_dynamic_crate = tcx.dependency_formats(LOCAL_CRATE)
.iter()
.any(|(_, list)| {
use rustc::middle::dependency_format::Linkage;
Expand Down Expand Up @@ -731,6 +731,7 @@ impl CrateInfo {
used_crate_source: Default::default(),
lang_item_to_crate: Default::default(),
missing_lang_items: Default::default(),
dependency_formats: tcx.dependency_formats(LOCAL_CRATE),
};
let lang_items = tcx.lang_items();

Expand Down
2 changes: 2 additions & 0 deletions src/librustc_codegen_ssa/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ use rustc_data_structures::fx::{FxHashMap, FxHashSet};
use rustc_data_structures::sync::Lrc;
use rustc_data_structures::svh::Svh;
use rustc::middle::cstore::{LibSource, CrateSource, NativeLibrary};
use rustc::middle::dependency_format::Dependencies;
use syntax_pos::symbol::Symbol;

mod error_codes;
Expand Down Expand Up @@ -142,6 +143,7 @@ pub struct CrateInfo {
pub used_crates_dynamic: Vec<(CrateNum, LibSource)>,
pub lang_item_to_crate: FxHashMap<LangItem, CrateNum>,
pub missing_lang_items: FxHashMap<CrateNum, Vec<LangItem>>,
pub dependency_formats: Lrc<Dependencies>,
}


Expand Down
4 changes: 0 additions & 4 deletions src/librustc_interface/passes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1075,10 +1075,6 @@ pub fn start_codegen<'tcx>(
tcx.print_debug_stats();
}

time(tcx.sess, "resolving dependency formats", || {
middle::dependency_format::calculate(tcx)
});

let (metadata, need_metadata_module) = time(tcx.sess, "metadata encoding and writing", || {
encode_and_write_metadata(tcx, outputs)
});
Expand Down
31 changes: 30 additions & 1 deletion src/librustc_metadata/cstore_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ use rustc::middle::cstore::{CrateStore, DepKind,
EncodedMetadata, NativeLibraryKind};
use rustc::middle::exported_symbols::ExportedSymbol;
use rustc::middle::stability::DeprecationEntry;
use rustc::middle::dependency_format::Linkage;
use rustc::hir::def;
use rustc::hir;
use rustc::session::{CrateDisambiguator, Session};
Expand Down Expand Up @@ -239,7 +240,30 @@ provide! { <'tcx> tcx, def_id, other, cdata,

used_crate_source => { Lrc::new(cdata.source.clone()) }

exported_symbols => { Arc::new(cdata.exported_symbols(tcx)) }
exported_symbols => {
let mut syms = cdata.exported_symbols(tcx);

// When linked into a dylib crates don't export their generic symbols,
// so if that's happening then we can't load upstream monomorphizations
// from this crate.
let formats = tcx.dependency_formats(LOCAL_CRATE);
let remove_generics = formats.iter().any(|(_ty, list)| {
match list.get(def_id.krate.as_usize() - 1) {
Some(Linkage::IncludedFromDylib) | Some(Linkage::Dynamic) => true,
_ => false,
}
});
if remove_generics {
syms.retain(|(sym, _threshold)| {
match sym {
ExportedSymbol::Generic(..) => false,
_ => return true,
}
});
}

Arc::new(syms)
}
}

pub fn provide(providers: &mut Providers<'_>) {
Expand Down Expand Up @@ -370,6 +394,11 @@ pub fn provide(providers: &mut Providers<'_>) {
tcx.arena.alloc(visible_parent_map)
},

dependency_formats: |tcx, cnum| {
assert_eq!(cnum, LOCAL_CRATE);
Lrc::new(crate::dependency_format::calculate(tcx))
},

..*providers
};
}
Expand Down
Loading

0 comments on commit 40fae88

Please sign in to comment.