Skip to content

Commit

Permalink
rustc: Fix mixing crates with different share_generics
Browse files Browse the repository at this point in the history
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
alexcrichton committed Sep 23, 2019
1 parent 5d531ae commit 50c57d8
Show file tree
Hide file tree
Showing 15 changed files with 93 additions and 14 deletions.
4 changes: 3 additions & 1 deletion src/librustc/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -631,7 +631,9 @@ rustc_queries! {
desc { "dylib dependency formats of crate" }
}

query dependency_formats(_: CrateNum) -> Lrc<crate::middle::dependency_format::Dependencies> {
query dependency_formats(_: CrateNum)
-> Lrc<crate::middle::dependency_format::Dependencies>
{
desc { "get the linkage format of all dependencies" }
}
}
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
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
27 changes: 26 additions & 1 deletion src/librustc_metadata/cstore_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ 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::session::config::CrateType;
use rustc::hir::def;
use rustc::hir;
use rustc::session::{CrateDisambiguator, Session};
Expand Down Expand Up @@ -239,7 +241,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. As a result, if we're creating a dylib or this crate
// is being included from a different dynamic library, then we filter
// out all `Generic` symbols here.
let formats = tcx.dependency_formats(LOCAL_CRATE);
let remove_generics = formats.iter().any(|(ty, list)| {
*ty == CrateType::Dylib ||
list.get(def_id.krate.as_usize() - 1) == Some(&Linkage::IncludedFromDylib)
});
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
1 change: 0 additions & 1 deletion src/librustc_metadata/dependency_format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -368,4 +368,3 @@ fn verify_ok(tcx: TyCtxt<'_>, list: &[Linkage]) {
}
}
}

Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
// compile-flags:-Zshare-generics=yes
// no-prefer-dynamic

#![crate_type="rlib"]

Expand Down
1 change: 1 addition & 0 deletions src/test/codegen-units/partitioning/shared-generics.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
// ignore-tidy-linelength
// no-prefer-dynamic
// compile-flags:-Zprint-mono-items=eager -Zshare-generics=yes -Zincremental=tmp/partitioning-tests/shared-generics-exe

#![crate_type="rlib"]
Expand Down
1 change: 1 addition & 0 deletions src/test/compile-fail/two-panic-runtimes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
// aux-build:panic-runtime-lang-items.rs

#![no_std]
#![no_main]

extern crate panic_runtime_unwind;
extern crate panic_runtime_unwind2;
Expand Down
39 changes: 39 additions & 0 deletions src/test/run-make-fulldeps/issue-64319/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
-include ../tools.mk

# Different optimization levels imply different values for `-Zshare-generics`,
# so try out a whole bunch of combinations to make sure everything is compatible
all:
# First up, try some defaults
$(RUSTC) --crate-type rlib foo.rs
$(RUSTC) --crate-type dylib bar.rs -C opt-level=3

# Next try mixing up some things explicitly
$(RUSTC) --crate-type rlib foo.rs -Z share-generics=no
$(RUSTC) --crate-type dylib bar.rs -Z share-generics=no
$(RUSTC) --crate-type rlib foo.rs -Z share-generics=no
$(RUSTC) --crate-type dylib bar.rs -Z share-generics=yes
$(RUSTC) --crate-type rlib foo.rs -Z share-generics=yes
$(RUSTC) --crate-type dylib bar.rs -Z share-generics=no
$(RUSTC) --crate-type rlib foo.rs -Z share-generics=yes
$(RUSTC) --crate-type dylib bar.rs -Z share-generics=yes

# Now combine a whole bunch of options together
$(RUSTC) --crate-type rlib foo.rs
$(RUSTC) --crate-type dylib bar.rs
$(RUSTC) --crate-type dylib bar.rs -Z share-generics=no
$(RUSTC) --crate-type dylib bar.rs -Z share-generics=yes
$(RUSTC) --crate-type dylib bar.rs -C opt-level=1
$(RUSTC) --crate-type dylib bar.rs -C opt-level=1 -Z share-generics=no
$(RUSTC) --crate-type dylib bar.rs -C opt-level=1 -Z share-generics=yes
$(RUSTC) --crate-type dylib bar.rs -C opt-level=2
$(RUSTC) --crate-type dylib bar.rs -C opt-level=2 -Z share-generics=no
$(RUSTC) --crate-type dylib bar.rs -C opt-level=2 -Z share-generics=yes
$(RUSTC) --crate-type dylib bar.rs -C opt-level=3
$(RUSTC) --crate-type dylib bar.rs -C opt-level=3 -Z share-generics=no
$(RUSTC) --crate-type dylib bar.rs -C opt-level=3 -Z share-generics=yes
$(RUSTC) --crate-type dylib bar.rs -C opt-level=s
$(RUSTC) --crate-type dylib bar.rs -C opt-level=s -Z share-generics=no
$(RUSTC) --crate-type dylib bar.rs -C opt-level=s -Z share-generics=yes
$(RUSTC) --crate-type dylib bar.rs -C opt-level=z
$(RUSTC) --crate-type dylib bar.rs -C opt-level=z -Z share-generics=no
$(RUSTC) --crate-type dylib bar.rs -C opt-level=z -Z share-generics=yes
5 changes: 5 additions & 0 deletions src/test/run-make-fulldeps/issue-64319/bar.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
extern crate foo;

pub fn bar() {
foo::foo();
}
9 changes: 9 additions & 0 deletions src/test/run-make-fulldeps/issue-64319/foo.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
pub fn foo() {
bar::<usize>();
}

pub fn bar<T>() {
baz();
}

fn baz() {}
4 changes: 2 additions & 2 deletions src/test/run-make-fulldeps/symbol-visibility/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -79,12 +79,12 @@ all:
# Check that a Rust dylib exports its monomorphic functions, including generics this time
[ "$$($(NM) $(TMPDIR)/$(RDYLIB_NAME) | grep -c public_c_function_from_rust_dylib)" -eq "1" ]
[ "$$($(NM) $(TMPDIR)/$(RDYLIB_NAME) | grep -c public_rust_function_from_rust_dylib)" -eq "1" ]
[ "$$($(NM) $(TMPDIR)/$(RDYLIB_NAME) | grep -c public_generic_function_from_rust_dylib)" -eq "1" ]
[ "$$($(NM) $(TMPDIR)/$(RDYLIB_NAME) | grep -c public_generic_function_from_rust_dylib)" -eq "0" ]

# Check that a Rust dylib exports the monomorphic functions from its dependencies
[ "$$($(NM) $(TMPDIR)/$(RDYLIB_NAME) | grep -c public_c_function_from_rlib)" -eq "1" ]
[ "$$($(NM) $(TMPDIR)/$(RDYLIB_NAME) | grep -c public_rust_function_from_rlib)" -eq "1" ]
[ "$$($(NM) $(TMPDIR)/$(RDYLIB_NAME) | grep -c public_generic_function_from_rlib)" -eq "1" ]
[ "$$($(NM) $(TMPDIR)/$(RDYLIB_NAME) | grep -c public_generic_function_from_rlib)" -eq "0" ]

# Check that an executable does not export any dynamic symbols
[ "$$($(NM) $(TMPDIR)/$(EXE_NAME) | grep -c public_c_function_from_rlib)" -eq "0" ]
Expand Down
3 changes: 1 addition & 2 deletions src/test/ui/panic-runtime/transitive-link-a-bunch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,8 @@
// ignore-wasm32-bare compiled with panic=abort by default

#![no_std]
#![no_main]

extern crate wants_panic_runtime_unwind;
extern crate wants_panic_runtime_abort;
extern crate panic_runtime_lang_items;

fn main() {}
3 changes: 1 addition & 2 deletions src/test/ui/panic-runtime/want-unwind-got-abort.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,7 @@
// ignore-wasm32-bare compiled with panic=abort by default

#![no_std]
#![no_main]

extern crate panic_runtime_abort;
extern crate panic_runtime_lang_items;

fn main() {}
3 changes: 1 addition & 2 deletions src/test/ui/panic-runtime/want-unwind-got-abort2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,7 @@
// ignore-wasm32-bare compiled with panic=abort by default

#![no_std]
#![no_main]

extern crate wants_panic_runtime_abort;
extern crate panic_runtime_lang_items;

fn main() {}

0 comments on commit 50c57d8

Please sign in to comment.