From d21f9b7fd6d4e41257bb4a6db64e9873767374dc Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Fri, 1 Nov 2019 13:46:05 +0100 Subject: [PATCH 1/2] targeted revert of PR rust-lang/rust#64324 (just undo change to dylib generics export). Includes the anticipated fallout to run-make-fulldeps test suite from this change. (We need to reopen issue 64319 as part of landing this.) --- src/librustc/ty/context.rs | 8 +++- src/librustc_codegen_ssa/back/linker.rs | 19 ++------- src/librustc_metadata/cstore_impl.rs | 26 +++---------- .../run-make-fulldeps/issue-64319/Makefile | 39 ------------------- src/test/run-make-fulldeps/issue-64319/bar.rs | 5 --- src/test/run-make-fulldeps/issue-64319/foo.rs | 9 ----- .../symbol-visibility/Makefile | 4 +- 7 files changed, 18 insertions(+), 92 deletions(-) delete mode 100644 src/test/run-make-fulldeps/issue-64319/Makefile delete mode 100644 src/test/run-make-fulldeps/issue-64319/bar.rs delete mode 100644 src/test/run-make-fulldeps/issue-64319/foo.rs diff --git a/src/librustc/ty/context.rs b/src/librustc/ty/context.rs index bdf9b2d7f3f27..8d8974c6cbbe8 100644 --- a/src/librustc/ty/context.rs +++ b/src/librustc/ty/context.rs @@ -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, } }) diff --git a/src/librustc_codegen_ssa/back/linker.rs b/src/librustc_codegen_ssa/back/linker.rs index ff87f0b1547ce..999cc40658503 100644 --- a/src/librustc_codegen_ssa/back/linker.rs +++ b/src/librustc_codegen_ssa/back/linker.rs @@ -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}; @@ -1112,20 +1111,10 @@ fn exported_symbols(tcx: TyCtxt<'_>, crate_type: CrateType) -> Vec { 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()); } diff --git a/src/librustc_metadata/cstore_impl.rs b/src/librustc_metadata/cstore_impl.rs index d942a19194a14..5e5c94b03140e 100644 --- a/src/librustc_metadata/cstore_impl.rs +++ b/src/librustc_metadata/cstore_impl.rs @@ -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}; @@ -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) } diff --git a/src/test/run-make-fulldeps/issue-64319/Makefile b/src/test/run-make-fulldeps/issue-64319/Makefile deleted file mode 100644 index b2c6b8b3cbbf2..0000000000000 --- a/src/test/run-make-fulldeps/issue-64319/Makefile +++ /dev/null @@ -1,39 +0,0 @@ --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 diff --git a/src/test/run-make-fulldeps/issue-64319/bar.rs b/src/test/run-make-fulldeps/issue-64319/bar.rs deleted file mode 100644 index 3895c0b6cdbb3..0000000000000 --- a/src/test/run-make-fulldeps/issue-64319/bar.rs +++ /dev/null @@ -1,5 +0,0 @@ -extern crate foo; - -pub fn bar() { - foo::foo(); -} diff --git a/src/test/run-make-fulldeps/issue-64319/foo.rs b/src/test/run-make-fulldeps/issue-64319/foo.rs deleted file mode 100644 index c54a238e9add7..0000000000000 --- a/src/test/run-make-fulldeps/issue-64319/foo.rs +++ /dev/null @@ -1,9 +0,0 @@ -pub fn foo() { - bar::(); -} - -pub fn bar() { - baz(); -} - -fn baz() {} diff --git a/src/test/run-make-fulldeps/symbol-visibility/Makefile b/src/test/run-make-fulldeps/symbol-visibility/Makefile index 840fe801a953c..7901866015bf2 100644 --- a/src/test/run-make-fulldeps/symbol-visibility/Makefile +++ b/src/test/run-make-fulldeps/symbol-visibility/Makefile @@ -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" ] From 6457914ff663973c86d9c5a5ff3ff60cb460f2a1 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Wed, 30 Oct 2019 16:32:40 +0100 Subject: [PATCH 2/2] ui test formulation of regression test for issue 64872. (Many thanks to alex for 1. making this even smaller than what I had originally minimized, and 2. pointing out that there is precedent for having ui tests with crate dependency chains of length > 2, thus allowing me to avoid encoding this as a run-make test.) --- .../issue-64872/auxiliary/a_def_obj.rs | 16 ++++++++++++++++ .../issue-64872/auxiliary/b_reexport_obj.rs | 7 +++++++ .../auxiliary/c_another_vtable_for_obj.rs | 12 ++++++++++++ .../auxiliary/d_chain_of_rlibs_and_dylibs.rs | 9 +++++++++ .../ui/cross-crate/issue-64872/issue-64872.rs | 17 +++++++++++++++++ 5 files changed, 61 insertions(+) create mode 100644 src/test/ui/cross-crate/issue-64872/auxiliary/a_def_obj.rs create mode 100644 src/test/ui/cross-crate/issue-64872/auxiliary/b_reexport_obj.rs create mode 100644 src/test/ui/cross-crate/issue-64872/auxiliary/c_another_vtable_for_obj.rs create mode 100644 src/test/ui/cross-crate/issue-64872/auxiliary/d_chain_of_rlibs_and_dylibs.rs create mode 100644 src/test/ui/cross-crate/issue-64872/issue-64872.rs diff --git a/src/test/ui/cross-crate/issue-64872/auxiliary/a_def_obj.rs b/src/test/ui/cross-crate/issue-64872/auxiliary/a_def_obj.rs new file mode 100644 index 0000000000000..82bb95f1ef2b3 --- /dev/null +++ b/src/test/ui/cross-crate/issue-64872/auxiliary/a_def_obj.rs @@ -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 Object for &T { } + +pub fn unused() { + let ref u = 0_u32; + let _d = &u as &dyn crate::Object; + _d.method() +} diff --git a/src/test/ui/cross-crate/issue-64872/auxiliary/b_reexport_obj.rs b/src/test/ui/cross-crate/issue-64872/auxiliary/b_reexport_obj.rs new file mode 100644 index 0000000000000..21c0274b991fc --- /dev/null +++ b/src/test/ui/cross-crate/issue-64872/auxiliary/b_reexport_obj.rs @@ -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; diff --git a/src/test/ui/cross-crate/issue-64872/auxiliary/c_another_vtable_for_obj.rs b/src/test/ui/cross-crate/issue-64872/auxiliary/c_another_vtable_for_obj.rs new file mode 100644 index 0000000000000..611238f56173a --- /dev/null +++ b/src/test/ui/cross-crate/issue-64872/auxiliary/c_another_vtable_for_obj.rs @@ -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() +} diff --git a/src/test/ui/cross-crate/issue-64872/auxiliary/d_chain_of_rlibs_and_dylibs.rs b/src/test/ui/cross-crate/issue-64872/auxiliary/d_chain_of_rlibs_and_dylibs.rs new file mode 100644 index 0000000000000..8d73f9b666f1e --- /dev/null +++ b/src/test/ui/cross-crate/issue-64872/auxiliary/d_chain_of_rlibs_and_dylibs.rs @@ -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(); +} diff --git a/src/test/ui/cross-crate/issue-64872/issue-64872.rs b/src/test/ui/cross-crate/issue-64872/issue-64872.rs new file mode 100644 index 0000000000000..20fe2053cc7c3 --- /dev/null +++ b/src/test/ui/cross-crate/issue-64872/issue-64872.rs @@ -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(); +}