Skip to content
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.

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

Revert PR 64324: dylibs export generics again (for now) #66018

Merged
merged 2 commits into from
Nov 1, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 7 additions & 1 deletion src/librustc/ty/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1514,8 +1514,14 @@ impl<'tcx> TyCtxt<'tcx> {
CrateType::Executable |
CrateType::Staticlib |
CrateType::ProcMacro |
CrateType::Dylib |
CrateType::Cdylib => false,

// FIXME rust-lang/rust#64319, rust-lang/rust#64872:
// We want to block export of generics from dylibs,
// but we must fix rust-lang/rust#65890 before we can
// do that robustly.
CrateType::Dylib => true,

CrateType::Rlib => true,
}
})
Expand Down
19 changes: 4 additions & 15 deletions src/librustc_codegen_ssa/back/linker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ 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 @@ -1112,20 +1111,10 @@ fn exported_symbols(tcx: TyCtxt<'_>, crate_type: CrateType) -> Vec<String> {
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;
}
// FIXME rust-lang/rust#64319, rust-lang/rust#64872:
// We want to block export of generics from dylibs,
// but we must fix rust-lang/rust#65890 before we can
// do that robustly.

symbols.push(symbol.symbol_name(tcx).to_string());
}
Expand Down
26 changes: 5 additions & 21 deletions src/librustc_metadata/cstore_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ use rustc::ty::query::QueryConfig;
use rustc::middle::cstore::{CrateSource, 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 @@ -235,26 +234,11 @@ provide! { <'tcx> tcx, def_id, other, cdata,
used_crate_source => { Lrc::new(cdata.source.clone()) }

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,
}
});
}
let syms = cdata.exported_symbols(tcx);

// FIXME rust-lang/rust#64319, rust-lang/rust#64872: We want
// to block export of generics from dylibs, but we must fix
// rust-lang/rust#65890 before we can do that robustly.

Arc::new(syms)
}
Expand Down
39 changes: 0 additions & 39 deletions src/test/run-make-fulldeps/issue-64319/Makefile

This file was deleted.

5 changes: 0 additions & 5 deletions src/test/run-make-fulldeps/issue-64319/bar.rs

This file was deleted.

9 changes: 0 additions & 9 deletions src/test/run-make-fulldeps/issue-64319/foo.rs

This file was deleted.

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 "0" ]
[ "$$($(NM) $(TMPDIR)/$(RDYLIB_NAME) | grep -c public_generic_function_from_rust_dylib)" -eq "1" ]

# 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 "0" ]
[ "$$($(NM) $(TMPDIR)/$(RDYLIB_NAME) | grep -c public_generic_function_from_rlib)" -eq "1" ]

# 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
16 changes: 16 additions & 0 deletions src/test/ui/cross-crate/issue-64872/auxiliary/a_def_obj.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// compile-flags: -C debuginfo=2

// no-prefer-dynamic
#![crate_type = "rlib"]

pub trait Object { fn method(&self) { } }

impl Object for u32 { }
impl Object for () { }
impl<T> Object for &T { }

pub fn unused() {
let ref u = 0_u32;
let _d = &u as &dyn crate::Object;
_d.method()
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
// compile-flags: -C debuginfo=2 -C prefer-dynamic

#![crate_type="dylib"]

extern crate a_def_obj;

pub use a_def_obj::Object;
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// no-prefer-dynamic
// compile-flags: -C debuginfo=2
#![crate_type="rlib"]

extern crate b_reexport_obj;
use b_reexport_obj::Object;

pub fn another_dyn_debug() {
let ref u = 1_u32;
let _d = &u as &dyn crate::Object;
_d.method()
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
// compile-flags: -C debuginfo=2 -C prefer-dynamic

#![crate_type="rlib"]

extern crate c_another_vtable_for_obj;

pub fn chain() {
c_another_vtable_for_obj::another_dyn_debug();
}
17 changes: 17 additions & 0 deletions src/test/ui/cross-crate/issue-64872/issue-64872.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// run-pass

// note that these aux-build directives must be in this order: the
// later crates depend on the earlier ones. (The particular bug that
// is being exercised here used to exhibit itself during the build of
// `chain_of_rlibs_and_dylibs.dylib`)

// aux-build:a_def_obj.rs
// aux-build:b_reexport_obj.rs
// aux-build:c_another_vtable_for_obj.rs
// aux-build:d_chain_of_rlibs_and_dylibs.rs

extern crate d_chain_of_rlibs_and_dylibs;

pub fn main() {
d_chain_of_rlibs_and_dylibs::chain();
}