From d23239b48e2066d3f38eb03e2429427bffade822 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Mon, 6 Jul 2015 18:21:57 -0700 Subject: [PATCH 1/2] msvc: Get codegen-units working This commit alters the implementation of multiple codegen units slightly to be compatible with the MSVC linker. Currently the implementation will take the N object files created by each codegen unit and will run `ld -r` to create a new object file which is then passed along. The MSVC linker, however, is not able to do this operation. The compiler will now no longer attempt to assemble object files together but will instead just pass through all the object files as usual. This implies that rlibs may not contain more than one object file (if the library is compiled with more than one codegen unit) and the output of `-C save-temps` will have changed slightly as object files with the extension `0.o` will not be renamed to `o` unless requested otherwise. --- src/librustc_trans/back/link.rs | 86 +++++++++-------- src/librustc_trans/back/write.rs | 95 +++---------------- .../extra-filename-with-temp-outputs/Makefile | 2 +- 3 files changed, 60 insertions(+), 123 deletions(-) diff --git a/src/librustc_trans/back/link.rs b/src/librustc_trans/back/link.rs index e0495226d90..9d5dcdd855d 100644 --- a/src/librustc_trans/back/link.rs +++ b/src/librustc_trans/back/link.rs @@ -417,11 +417,10 @@ pub fn link_binary(sess: &Session, // Remove the temporary object file and metadata if we aren't saving temps if !sess.opts.cg.save_temps { - let obj_filename = outputs.temp_path(OutputTypeObject); - if !sess.opts.output_types.contains(&OutputTypeObject) { - remove(sess, &obj_filename); + for obj in object_filenames(sess, outputs) { + remove(sess, &obj); } - remove(sess, &obj_filename.with_extension("metadata.o")); + remove(sess, &outputs.with_extension("metadata.o")); } out_filenames @@ -499,7 +498,7 @@ fn link_binary_output(sess: &Session, crate_type: config::CrateType, outputs: &OutputFilenames, crate_name: &str) -> PathBuf { - let obj_filename = outputs.temp_path(OutputTypeObject); + let objects = object_filenames(sess, outputs); let out_filename = match outputs.single_output_file { Some(ref file) => file.clone(), None => { @@ -508,41 +507,41 @@ fn link_binary_output(sess: &Session, } }; - // Make sure the output and obj_filename are both writeable. - // Mac, FreeBSD, and Windows system linkers check this already -- - // however, the Linux linker will happily overwrite a read-only file. - // We should be consistent. - let obj_is_writeable = is_writeable(&obj_filename); - let out_is_writeable = is_writeable(&out_filename); - if !out_is_writeable { - sess.fatal(&format!("output file {} is not writeable -- check its \ - permissions.", - out_filename.display())); - } - else if !obj_is_writeable { - sess.fatal(&format!("object file {} is not writeable -- check its \ - permissions.", - obj_filename.display())); + // Make sure files are writeable. Mac, FreeBSD, and Windows system linkers + // check this already -- however, the Linux linker will happily overwrite a + // read-only file. We should be consistent. + for file in objects.iter().chain(Some(&out_filename)) { + if !is_writeable(file) { + sess.fatal(&format!("output file {} is not writeable -- check its \ + permissions", file.display())); + } } match crate_type { config::CrateTypeRlib => { - link_rlib(sess, Some(trans), &obj_filename, &out_filename).build(); + link_rlib(sess, Some(trans), &objects, &out_filename).build(); } config::CrateTypeStaticlib => { - link_staticlib(sess, &obj_filename, &out_filename); + link_staticlib(sess, &objects, &out_filename); } config::CrateTypeExecutable => { - link_natively(sess, trans, false, &obj_filename, &out_filename); + link_natively(sess, trans, false, &objects, &out_filename, outputs); } config::CrateTypeDylib => { - link_natively(sess, trans, true, &obj_filename, &out_filename); + link_natively(sess, trans, true, &objects, &out_filename, outputs); } } out_filename } +fn object_filenames(sess: &Session, outputs: &OutputFilenames) -> Vec { + (0..sess.opts.cg.codegen_units).map(|i| { + let ext = format!("{}.o", i); + outputs.temp_path(OutputTypeObject).with_extension(&ext) + }).collect() +} + fn archive_search_paths(sess: &Session) -> Vec { let mut search = Vec::new(); sess.target_filesearch(PathKind::Native).for_each_lib_search_path(|path, _| { @@ -560,9 +559,9 @@ fn archive_search_paths(sess: &Session) -> Vec { // native libraries and inserting all of the contents into this archive. fn link_rlib<'a>(sess: &'a Session, trans: Option<&CrateTranslation>, // None == no metadata/bytecode - obj_filename: &Path, + objects: &[PathBuf], out_filename: &Path) -> ArchiveBuilder<'a> { - info!("preparing rlib from {:?} to {:?}", obj_filename, out_filename); + info!("preparing rlib from {:?} to {:?}", objects, out_filename); let handler = &sess.diagnostic().handler; let config = ArchiveConfig { handler: handler, @@ -574,7 +573,9 @@ fn link_rlib<'a>(sess: &'a Session, command_path: command_path(sess), }; let mut ab = ArchiveBuilder::create(config); - ab.add_file(obj_filename).unwrap(); + for obj in objects { + ab.add_file(obj).unwrap(); + } for &(ref l, kind) in sess.cstore.get_used_libraries().borrow().iter() { match kind { @@ -600,7 +601,7 @@ fn link_rlib<'a>(sess: &'a Session, // this is as follows: // // * When performing LTO, this archive will be modified to remove - // obj_filename from above. The reason for this is described below. + // objects from above. The reason for this is described below. // // * When the system linker looks at an archive, it will attempt to // determine the architecture of the archive in order to see whether its @@ -639,15 +640,14 @@ fn link_rlib<'a>(sess: &'a Session, // For LTO purposes, the bytecode of this library is also inserted // into the archive. If codegen_units > 1, we insert each of the // bitcode files. - for i in 0..sess.opts.cg.codegen_units { + for obj in objects { // Note that we make sure that the bytecode filename in the // archive is never exactly 16 bytes long by adding a 16 byte // extension to it. This is to work around a bug in LLDB that // would cause it to crash if the name of a file in an archive // was exactly 16 bytes. - let bc_filename = obj_filename.with_extension(&format!("{}.bc", i)); - let bc_deflated_filename = obj_filename.with_extension( - &format!("{}.bytecode.deflate", i)); + let bc_filename = obj.with_extension("bc"); + let bc_deflated_filename = obj.with_extension("bytecode.deflate"); let mut bc_data = Vec::new(); match fs::File::open(&bc_filename).and_then(|mut f| { @@ -750,8 +750,8 @@ fn write_rlib_bytecode_object_v1(writer: &mut Write, // There's no need to include metadata in a static archive, so ensure to not // link in the metadata object file (and also don't prepare the archive with a // metadata file). -fn link_staticlib(sess: &Session, obj_filename: &Path, out_filename: &Path) { - let ab = link_rlib(sess, None, obj_filename, out_filename); +fn link_staticlib(sess: &Session, objects: &[PathBuf], out_filename: &Path) { + let ab = link_rlib(sess, None, objects, out_filename); let mut ab = match sess.target.target.options.is_like_osx { true => ab.build().extend(), false => ab, @@ -806,8 +806,9 @@ fn link_staticlib(sess: &Session, obj_filename: &Path, out_filename: &Path) { // This will invoke the system linker/cc to create the resulting file. This // links to all upstream files as well. fn link_natively(sess: &Session, trans: &CrateTranslation, dylib: bool, - obj_filename: &Path, out_filename: &Path) { - info!("preparing dylib? ({}) from {:?} to {:?}", dylib, obj_filename, + objects: &[PathBuf], out_filename: &Path, + outputs: &OutputFilenames) { + info!("preparing dylib? ({}) from {:?} to {:?}", dylib, objects, out_filename); let tmpdir = TempDir::new("rustc").ok().expect("needs a temp dir"); @@ -828,7 +829,7 @@ fn link_natively(sess: &Session, trans: &CrateTranslation, dylib: bool, Box::new(GnuLinker { cmd: &mut cmd, sess: &sess }) as Box }; link_args(&mut *linker, sess, dylib, tmpdir.path(), - trans, obj_filename, out_filename); + trans, objects, out_filename, outputs); if !sess.target.target.options.no_compiler_rt { linker.link_staticlib("compiler-rt"); } @@ -884,8 +885,9 @@ fn link_args(cmd: &mut Linker, dylib: bool, tmpdir: &Path, trans: &CrateTranslation, - obj_filename: &Path, - out_filename: &Path) { + objects: &[PathBuf], + out_filename: &Path, + outputs: &OutputFilenames) { // The default library location, we need this to find the runtime. // The location of crates will be determined as needed. @@ -895,7 +897,9 @@ fn link_args(cmd: &mut Linker, let t = &sess.target.target; cmd.include_path(&fix_windows_verbatim_for_gcc(&lib_path)); - cmd.add_object(obj_filename); + for obj in objects { + cmd.add_object(obj); + } cmd.output_filename(out_filename); // Stack growth requires statically linking a __morestack function. Note @@ -922,7 +926,7 @@ fn link_args(cmd: &mut Linker, // executable. This metadata is in a separate object file from the main // object file, so we link that in here. if dylib { - cmd.add_object(&obj_filename.with_extension("metadata.o")); + cmd.add_object(&outputs.with_extension("metadata.o")); } // Try to strip as much out of the generated object by removing unused diff --git a/src/librustc_trans/back/write.rs b/src/librustc_trans/back/write.rs index 90ddba4e09c..0a9db8a651e 100644 --- a/src/librustc_trans/back/write.rs +++ b/src/librustc_trans/back/write.rs @@ -27,7 +27,6 @@ use std::ffi::{CStr, CString}; use std::fs; use std::mem; use std::path::Path; -use std::process::Stdio; use std::ptr; use std::str; use std::sync::{Arc, Mutex}; @@ -619,6 +618,8 @@ pub fn run_passes(sess: &Session, let needs_crate_bitcode = sess.crate_types.borrow().contains(&config::CrateTypeRlib) && sess.opts.output_types.contains(&config::OutputTypeExe); + let needs_crate_object = + sess.opts.output_types.contains(&config::OutputTypeExe); if needs_crate_bitcode { modules_config.emit_bc = true; } @@ -696,7 +697,8 @@ pub fn run_passes(sess: &Session, if sess.opts.cg.codegen_units == 1 { // 1) Only one codegen unit. In this case it's no difficulty // to copy `foo.0.x` to `foo.x`. - copy_gracefully(&crate_output.with_extension(ext), &crate_output.path(output_type)); + copy_gracefully(&crate_output.with_extension(ext), + &crate_output.path(output_type)); if !sess.opts.cg.save_temps && !keep_numbered { // The user just wants `foo.x`, not `foo.0.x`. remove(sess, &crate_output.with_extension(ext)); @@ -715,76 +717,11 @@ pub fn run_passes(sess: &Session, } }; - let link_obj = |output_path: &Path| { - // Running `ld -r` on a single input is kind of pointless. - if sess.opts.cg.codegen_units == 1 { - copy_gracefully(&crate_output.with_extension("0.o"), output_path); - // Leave the .0.o file around, to mimic the behavior of the normal - // code path. - return; - } - - // Some builds of MinGW GCC will pass --force-exe-suffix to ld, which - // will automatically add a .exe extension if the extension is not - // already .exe or .dll. To ensure consistent behavior on Windows, we - // add the .exe suffix explicitly and then rename the output file to - // the desired path. This will give the correct behavior whether or - // not GCC adds --force-exe-suffix. - let windows_output_path = - if sess.target.target.options.is_like_windows { - Some(output_path.with_extension("o.exe")) - } else { - None - }; - - let (pname, mut cmd) = get_linker(sess); - - cmd.args(&sess.target.target.options.pre_link_args); - cmd.arg("-nostdlib"); - - for index in 0..trans.modules.len() { - cmd.arg(&crate_output.with_extension(&format!("{}.o", index))); - } - - cmd.arg("-r").arg("-o") - .arg(windows_output_path.as_ref().map(|s| &**s).unwrap_or(output_path)); - - cmd.args(&sess.target.target.options.post_link_args); - - if sess.opts.debugging_opts.print_link_args { - println!("{:?}", &cmd); - } - - cmd.stdin(Stdio::null()); - match cmd.status() { - Ok(status) => { - if !status.success() { - sess.err(&format!("linking of {} with `{:?}` failed", - output_path.display(), cmd)); - sess.abort_if_errors(); - } - }, - Err(e) => { - sess.err(&format!("could not exec the linker `{}`: {}", - pname, e)); - sess.abort_if_errors(); - }, - } - - match windows_output_path { - Some(ref windows_path) => { - fs::rename(windows_path, output_path).unwrap(); - }, - None => { - // The file is already named according to `output_path`. - } - } - }; - // Flag to indicate whether the user explicitly requested bitcode. // Otherwise, we produced it only as a temporary output, and will need // to get rid of it. let mut user_wants_bitcode = false; + let mut user_wants_objects = false; for output_type in output_types { match *output_type { config::OutputTypeBitcode => { @@ -801,17 +738,10 @@ pub fn run_passes(sess: &Session, copy_if_one_unit("0.s", config::OutputTypeAssembly, false); } config::OutputTypeObject => { - link_obj(&crate_output.path(config::OutputTypeObject)); - } - config::OutputTypeExe => { - // If config::OutputTypeObject is already in the list, then - // `crate.o` will be handled by the config::OutputTypeObject case. - // Otherwise, we need to create the temporary object so we - // can run the linker. - if !sess.opts.output_types.contains(&config::OutputTypeObject) { - link_obj(&crate_output.temp_path(config::OutputTypeObject)); - } + user_wants_objects = true; + copy_if_one_unit("0.o", config::OutputTypeObject, true); } + config::OutputTypeExe | config::OutputTypeDepInfo => {} } } @@ -848,15 +778,18 @@ pub fn run_passes(sess: &Session, let keep_numbered_bitcode = needs_crate_bitcode || (user_wants_bitcode && sess.opts.cg.codegen_units > 1); + let keep_numbered_objects = needs_crate_object || + (user_wants_objects && sess.opts.cg.codegen_units > 1); + for i in 0..trans.modules.len() { - if modules_config.emit_obj { + if modules_config.emit_obj && !keep_numbered_objects { let ext = format!("{}.o", i); - remove(sess, &crate_output.with_extension(&ext[..])); + remove(sess, &crate_output.with_extension(&ext)); } if modules_config.emit_bc && !keep_numbered_bitcode { let ext = format!("{}.bc", i); - remove(sess, &crate_output.with_extension(&ext[..])); + remove(sess, &crate_output.with_extension(&ext)); } } diff --git a/src/test/run-make/extra-filename-with-temp-outputs/Makefile b/src/test/run-make/extra-filename-with-temp-outputs/Makefile index 28c22a173cc..d33c18a6f3c 100644 --- a/src/test/run-make/extra-filename-with-temp-outputs/Makefile +++ b/src/test/run-make/extra-filename-with-temp-outputs/Makefile @@ -2,5 +2,5 @@ all: $(RUSTC) -C extra-filename=bar foo.rs -C save-temps - rm $(TMPDIR)/foobar.o + rm $(TMPDIR)/foobar.0.o rm $(TMPDIR)/$(call BIN,foobar) From 9bc8e6d1472a57441afe3592078838d2bc767996 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Tue, 7 Jul 2015 21:33:44 -0700 Subject: [PATCH 2/2] trans: Link rlibs to dylibs with --whole-archive This commit starts passing the `--whole-archive` flag (`-force_load` on OSX) to the linker when linking rlibs into dylibs. The primary purpose of this commit is to ensure that the linker doesn't strip out objects from an archive when creating a dynamic library. Information on how this can go wrong can be found in issues #14344 and #25185. The unfortunate part about passing this flag to the linker is that we have to preprocess the rlib to remove the metadata and compressed bytecode found within. This means that creating a dylib will now take longer to link as we've got to copy around the input rlibs to a temporary location, modify them, and then invoke the linker. This isn't done for executables, however, so the "hello world" compile time is not affected. This fix was instigated because of the previous commit where rlibs may not contain multiple object files instead of one due to codegen units being greater than one. That change prevented the main distribution from being compiled with more than one codegen-unit and this commit fixes that. Closes #14344 Closes #25185 --- src/liballoc/lib.rs | 1 + src/libcollections/lib.rs | 1 + src/liblibc/lib.rs | 1 + src/librustc/metadata/encoder.rs | 6 +- src/librustc_driver/driver.rs | 4 +- src/librustc_driver/lib.rs | 6 +- src/librustc_trans/back/link.rs | 182 +++++++++--------- src/librustc_trans/back/linker.rs | 16 ++ src/librustc_trans/back/lto.rs | 39 +--- src/test/auxiliary/issue-14344-1.rs | 15 ++ src/test/auxiliary/issue-14344-2.rs | 13 ++ src/test/auxiliary/issue-25185-1.rs | 18 ++ src/test/auxiliary/issue-25185-2.rs | 13 ++ .../run-make/extern-fn-reachable/Makefile | 4 +- src/test/run-pass/issue-14344.rs | 20 ++ src/test/run-pass/issue-25185.rs | 21 ++ 16 files changed, 230 insertions(+), 130 deletions(-) create mode 100644 src/test/auxiliary/issue-14344-1.rs create mode 100644 src/test/auxiliary/issue-14344-2.rs create mode 100644 src/test/auxiliary/issue-25185-1.rs create mode 100644 src/test/auxiliary/issue-25185-2.rs create mode 100644 src/test/run-pass/issue-14344.rs create mode 100644 src/test/run-pass/issue-25185.rs diff --git a/src/liballoc/lib.rs b/src/liballoc/lib.rs index 7dcf7a76da0..905012bbb64 100644 --- a/src/liballoc/lib.rs +++ b/src/liballoc/lib.rs @@ -148,4 +148,5 @@ pub fn oom() -> ! { // optimize it out). #[doc(hidden)] #[unstable(feature = "issue_14344_fixme")] +#[cfg(stage0)] pub fn fixme_14344_be_sure_to_link_to_collections() {} diff --git a/src/libcollections/lib.rs b/src/libcollections/lib.rs index 42adbe10e50..3c90a2c54e1 100644 --- a/src/libcollections/lib.rs +++ b/src/libcollections/lib.rs @@ -138,6 +138,7 @@ pub mod btree_set { // FIXME(#14344) this shouldn't be necessary #[doc(hidden)] #[unstable(feature = "issue_14344_fixme")] +#[cfg(stage0)] pub fn fixme_14344_be_sure_to_link_to_collections() {} #[cfg(not(test))] diff --git a/src/liblibc/lib.rs b/src/liblibc/lib.rs index 2c5ebc25f6b..102894bec13 100644 --- a/src/liblibc/lib.rs +++ b/src/liblibc/lib.rs @@ -6431,6 +6431,7 @@ pub mod funcs { } #[doc(hidden)] +#[cfg(stage0)] pub fn issue_14344_workaround() {} // FIXME #14344 force linkage to happen correctly #[test] fn work_on_windows() { } // FIXME #10872 needed for a happy windows diff --git a/src/librustc/metadata/encoder.rs b/src/librustc/metadata/encoder.rs index b677e7b8570..a9e9f17bdce 100644 --- a/src/librustc/metadata/encoder.rs +++ b/src/librustc/metadata/encoder.rs @@ -2136,11 +2136,7 @@ fn encode_metadata_inner(wr: &mut Cursor>, let mut rbml_w = Encoder::new(wr); encode_crate_name(&mut rbml_w, &ecx.link_meta.crate_name); - encode_crate_triple(&mut rbml_w, - &tcx.sess - .opts - .target_triple - ); + encode_crate_triple(&mut rbml_w, &tcx.sess.opts.target_triple); encode_hash(&mut rbml_w, &ecx.link_meta.crate_hash); encode_dylib_dependency_formats(&mut rbml_w, &ecx); diff --git a/src/librustc_driver/driver.rs b/src/librustc_driver/driver.rs index ae6136a049a..9541076df82 100644 --- a/src/librustc_driver/driver.rs +++ b/src/librustc_driver/driver.rs @@ -804,8 +804,8 @@ fn write_out_deps(sess: &Session, match *output_type { config::OutputTypeExe => { for output in sess.crate_types.borrow().iter() { - let p = link::filename_for_input(sess, *output, - id, &file); + let p = link::filename_for_input(sess, *output, id, + outputs); out_filenames.push(p); } } diff --git a/src/librustc_driver/lib.rs b/src/librustc_driver/lib.rs index 36438ccc784..282971daa28 100644 --- a/src/librustc_driver/lib.rs +++ b/src/librustc_driver/lib.rs @@ -452,10 +452,8 @@ impl RustcDefaultCalls { let metadata = driver::collect_crate_metadata(sess, attrs); *sess.crate_metadata.borrow_mut() = metadata; for &style in &crate_types { - let fname = link::filename_for_input(sess, - style, - &id, - &t_outputs.with_extension("")); + let fname = link::filename_for_input(sess, style, &id, + &t_outputs); println!("{}", fname.file_name().unwrap() .to_string_lossy()); } diff --git a/src/librustc_trans/back/link.rs b/src/librustc_trans/back/link.rs index 9d5dcdd855d..21bc61593c9 100644 --- a/src/librustc_trans/back/link.rs +++ b/src/librustc_trans/back/link.rs @@ -464,26 +464,25 @@ fn is_writeable(p: &Path) -> bool { pub fn filename_for_input(sess: &Session, crate_type: config::CrateType, - name: &str, - out_filename: &Path) -> PathBuf { - let libname = format!("{}{}", name, sess.opts.cg.extra_filename); + crate_name: &str, + outputs: &OutputFilenames) -> PathBuf { + let libname = format!("{}{}", crate_name, sess.opts.cg.extra_filename); match crate_type { config::CrateTypeRlib => { - out_filename.with_file_name(&format!("lib{}.rlib", libname)) + outputs.out_directory.join(&format!("lib{}.rlib", libname)) } config::CrateTypeDylib => { let (prefix, suffix) = (&sess.target.target.options.dll_prefix, &sess.target.target.options.dll_suffix); - out_filename.with_file_name(&format!("{}{}{}", - prefix, - libname, - suffix)) + outputs.out_directory.join(&format!("{}{}{}", prefix, libname, + suffix)) } config::CrateTypeStaticlib => { - out_filename.with_file_name(&format!("lib{}.a", libname)) + outputs.out_directory.join(&format!("lib{}.a", libname)) } config::CrateTypeExecutable => { let suffix = &sess.target.target.options.exe_suffix; + let out_filename = outputs.path(OutputTypeExe); if suffix.is_empty() { out_filename.to_path_buf() } else { @@ -501,10 +500,7 @@ fn link_binary_output(sess: &Session, let objects = object_filenames(sess, outputs); let out_filename = match outputs.single_output_file { Some(ref file) => file.clone(), - None => { - let out_filename = outputs.path(OutputTypeExe); - filename_for_input(sess, crate_type, crate_name, &out_filename) - } + None => filename_for_input(sess, crate_type, crate_name, outputs), }; // Make sure files are writeable. Mac, FreeBSD, and Windows system linkers @@ -551,6 +547,19 @@ fn archive_search_paths(sess: &Session) -> Vec { return search; } +fn archive_config<'a>(sess: &'a Session, + output: &Path) -> ArchiveConfig<'a> { + ArchiveConfig { + handler: &sess.diagnostic().handler, + dst: output.to_path_buf(), + lib_search_paths: archive_search_paths(sess), + slib_prefix: sess.target.target.options.staticlib_prefix.clone(), + slib_suffix: sess.target.target.options.staticlib_suffix.clone(), + ar_prog: get_ar_prog(sess), + command_path: command_path(sess), + } +} + // Create an 'rlib' // // An rlib in its current incarnation is essentially a renamed .a file. The @@ -562,17 +571,7 @@ fn link_rlib<'a>(sess: &'a Session, objects: &[PathBuf], out_filename: &Path) -> ArchiveBuilder<'a> { info!("preparing rlib from {:?} to {:?}", objects, out_filename); - let handler = &sess.diagnostic().handler; - let config = ArchiveConfig { - handler: handler, - dst: out_filename.to_path_buf(), - lib_search_paths: archive_search_paths(sess), - slib_prefix: sess.target.target.options.staticlib_prefix.clone(), - slib_suffix: sess.target.target.options.staticlib_suffix.clone(), - ar_prog: get_ar_prog(sess), - command_path: command_path(sess), - }; - let mut ab = ArchiveBuilder::create(config); + let mut ab = ArchiveBuilder::create(archive_config(sess, out_filename)); for obj in objects { ab.add_file(obj).unwrap(); } @@ -1131,7 +1130,7 @@ fn add_upstream_rust_crates(cmd: &mut Linker, sess: &Session, add_dynamic_crate(cmd, sess, &src.dylib.unwrap().0) } cstore::RequireStatic => { - add_static_crate(cmd, sess, tmpdir, &src.rlib.unwrap().0) + add_static_crate(cmd, sess, tmpdir, dylib, &src.rlib.unwrap().0) } } @@ -1147,71 +1146,80 @@ fn add_upstream_rust_crates(cmd: &mut Linker, sess: &Session, } // Adds the static "rlib" versions of all crates to the command line. + // There's a bit of magic which happens here specifically related to LTO and + // dynamic libraries. Specifically: + // + // * For LTO, we remove upstream object files. + // * For dylibs we remove metadata and bytecode from upstream rlibs + // + // When performing LTO, all of the bytecode from the upstream libraries has + // already been included in our object file output. As a result we need to + // remove the object files in the upstream libraries so the linker doesn't + // try to include them twice (or whine about duplicate symbols). We must + // continue to include the rest of the rlib, however, as it may contain + // static native libraries which must be linked in. + // + // When making a dynamic library, linkers by default don't include any + // object files in an archive if they're not necessary to resolve the link. + // We basically want to convert the archive (rlib) to a dylib, though, so we + // *do* want everything included in the output, regardless of whether the + // linker thinks it's needed or not. As a result we must use the + // --whole-archive option (or the platform equivalent). When using this + // option the linker will fail if there are non-objects in the archive (such + // as our own metadata and/or bytecode). All in all, for rlibs to be + // entirely included in dylibs, we need to remove all non-object files. + // + // Note, however, that if we're not doing LTO or we're not producing a dylib + // (aka we're making an executable), we can just pass the rlib blindly to + // the linker (fast) because it's fine if it's not actually included as + // we're at the end of the dependency chain. fn add_static_crate(cmd: &mut Linker, sess: &Session, tmpdir: &Path, - cratepath: &Path) { - // When performing LTO on an executable output, all of the - // bytecode from the upstream libraries has already been - // included in our object file output. We need to modify all of - // the upstream archives to remove their corresponding object - // file to make sure we don't pull the same code in twice. - // - // We must continue to link to the upstream archives to be sure - // to pull in native static dependencies. As the final caveat, - // on Linux it is apparently illegal to link to a blank archive, - // so if an archive no longer has any object files in it after - // we remove `lib.o`, then don't link against it at all. - // - // If we're not doing LTO, then our job is simply to just link - // against the archive. - if sess.lto() { - let name = cratepath.file_name().unwrap().to_str().unwrap(); - let name = &name[3..name.len() - 5]; // chop off lib/.rlib - time(sess.time_passes(), - &format!("altering {}.rlib", name), - (), |()| { - let dst = tmpdir.join(cratepath.file_name().unwrap()); - match fs::copy(&cratepath, &dst) { - Ok(..) => {} - Err(e) => { - sess.fatal(&format!("failed to copy {} to {}: {}", - cratepath.display(), - dst.display(), e)); - } - } - // Fix up permissions of the copy, as fs::copy() preserves - // permissions, but the original file may have been installed - // by a package manager and may be read-only. - match fs::metadata(&dst).and_then(|m| { - let mut perms = m.permissions(); - perms.set_readonly(false); - fs::set_permissions(&dst, perms) - }) { - Ok(..) => {} - Err(e) => { - sess.fatal(&format!("failed to chmod {} when preparing \ - for LTO: {}", dst.display(), e)); - } - } - let handler = &sess.diagnostic().handler; - let config = ArchiveConfig { - handler: handler, - dst: dst.clone(), - lib_search_paths: archive_search_paths(sess), - slib_prefix: sess.target.target.options.staticlib_prefix.clone(), - slib_suffix: sess.target.target.options.staticlib_suffix.clone(), - ar_prog: get_ar_prog(sess), - command_path: command_path(sess), - }; - let mut archive = Archive::open(config); - archive.remove_file(&format!("{}.o", name)); - let files = archive.files(); - if files.iter().any(|s| s.ends_with(".o")) { - cmd.link_rlib(&dst); - } - }); - } else { + dylib: bool, cratepath: &Path) { + if !sess.lto() && !dylib { cmd.link_rlib(&fix_windows_verbatim_for_gcc(cratepath)); + return } + + let dst = tmpdir.join(cratepath.file_name().unwrap()); + let name = cratepath.file_name().unwrap().to_str().unwrap(); + let name = &name[3..name.len() - 5]; // chop off lib/.rlib + + time(sess.time_passes(), &format!("altering {}.rlib", name), (), |()| { + let err = (|| { + io::copy(&mut try!(fs::File::open(&cratepath)), + &mut try!(fs::File::create(&dst))) + })(); + if let Err(e) = err { + sess.fatal(&format!("failed to copy {} to {}: {}", + cratepath.display(), dst.display(), e)); + } + + let mut archive = Archive::open(archive_config(sess, &dst)); + archive.remove_file(METADATA_FILENAME); + + let mut any_objects = false; + for f in archive.files() { + if f.ends_with("bytecode.deflate") { + archive.remove_file(&f); + continue + } + let canonical = f.replace("-", "_"); + let canonical_name = name.replace("-", "_"); + if sess.lto() && canonical.starts_with(&canonical_name) && + canonical.ends_with(".o") { + let num = &f[name.len()..f.len() - 2]; + if num.len() > 0 && num[1..].parse::().is_ok() { + archive.remove_file(&f); + continue + } + } + any_objects = true; + } + + if any_objects { + cmd.link_whole_rlib(&fix_windows_verbatim_for_gcc(&dst)); + } + }); } // Same thing as above, but for dynamic crates instead of static crates. diff --git a/src/librustc_trans/back/linker.rs b/src/librustc_trans/back/linker.rs index 7253334d699..518a6c24840 100644 --- a/src/librustc_trans/back/linker.rs +++ b/src/librustc_trans/back/linker.rs @@ -30,6 +30,7 @@ pub trait Linker { fn link_framework(&mut self, framework: &str); fn link_staticlib(&mut self, lib: &str); fn link_rlib(&mut self, lib: &Path); + fn link_whole_rlib(&mut self, lib: &Path); fn link_whole_staticlib(&mut self, lib: &str, search_path: &[PathBuf]); fn include_path(&mut self, path: &Path); fn framework_path(&mut self, path: &Path); @@ -96,6 +97,17 @@ impl<'a> Linker for GnuLinker<'a> { } } + fn link_whole_rlib(&mut self, lib: &Path) { + if self.sess.target.target.options.is_like_osx { + let mut v = OsString::from("-Wl,-force_load,"); + v.push(lib); + self.cmd.arg(&v); + } else { + self.cmd.arg("-Wl,--whole-archive").arg(lib) + .arg("-Wl,--no-whole-archive"); + } + } + fn gc_sections(&mut self, is_dylib: bool) { // The dead_strip option to the linker specifies that functions and data // unreachable by the entry point will be removed. This is quite useful @@ -250,6 +262,10 @@ impl<'a> Linker for MsvcLinker<'a> { // not supported? self.link_staticlib(lib); } + fn link_whole_rlib(&mut self, path: &Path) { + // not supported? + self.link_rlib(path); + } fn optimize(&mut self) { // Needs more investigation of `/OPT` arguments } diff --git a/src/librustc_trans/back/lto.rs b/src/librustc_trans/back/lto.rs index e13a5e97f75..dfeb866c5b3 100644 --- a/src/librustc_trans/back/lto.rs +++ b/src/librustc_trans/back/lto.rs @@ -56,33 +56,14 @@ pub fn run(sess: &session::Session, llmod: ModuleRef, }; let archive = ArchiveRO::open(&path).expect("wanted an rlib"); - let file = path.file_name().unwrap().to_str().unwrap(); - let file = &file[3..file.len() - 5]; // chop off lib/.rlib - debug!("reading {}", file); - for i in 0.. { - let filename = format!("{}.{}.bytecode.deflate", file, i); - let msg = format!("check for {}", filename); - let bc_encoded = time(sess.time_passes(), &msg, (), |_| { - archive.iter().find(|section| { - section.name() == Some(&filename[..]) - }) - }); - let bc_encoded = match bc_encoded { - Some(data) => data, - None => { - if i == 0 { - // No bitcode was found at all. - sess.fatal(&format!("missing compressed bytecode in {}", - path.display())); - } - // No more bitcode files to read. - break - } - }; - let bc_encoded = bc_encoded.data(); + let bytecodes = archive.iter().filter_map(|child| { + child.name().map(|name| (name, child)) + }).filter(|&(name, _)| name.ends_with("bytecode.deflate")); + for (name, data) in bytecodes { + let bc_encoded = data.data(); let bc_decoded = if is_versioned_bytecode_format(bc_encoded) { - time(sess.time_passes(), &format!("decode {}.{}.bc", file, i), (), |_| { + time(sess.time_passes(), &format!("decode {}", name), (), |_| { // Read the version let version = extract_bytecode_format_version(bc_encoded); @@ -106,7 +87,7 @@ pub fn run(sess: &session::Session, llmod: ModuleRef, } }) } else { - time(sess.time_passes(), &format!("decode {}.{}.bc", file, i), (), |_| { + time(sess.time_passes(), &format!("decode {}", name), (), |_| { // the object must be in the old, pre-versioning format, so simply // inflate everything and let LLVM decide if it can make sense of it match flate::inflate_bytes(bc_encoded) { @@ -120,10 +101,8 @@ pub fn run(sess: &session::Session, llmod: ModuleRef, }; let ptr = bc_decoded.as_ptr(); - debug!("linking {}, part {}", name, i); - time(sess.time_passes(), - &format!("ll link {}.{}", name, i), - (), + debug!("linking {}", name); + time(sess.time_passes(), &format!("ll link {}", name), (), |()| unsafe { if !llvm::LLVMRustLinkInExternalBitcode(llmod, ptr as *const libc::c_char, diff --git a/src/test/auxiliary/issue-14344-1.rs b/src/test/auxiliary/issue-14344-1.rs new file mode 100644 index 00000000000..78c03bac33f --- /dev/null +++ b/src/test/auxiliary/issue-14344-1.rs @@ -0,0 +1,15 @@ +// 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. + +// no-prefer-dynamic + +#![crate_type = "rlib"] + +pub fn foo() {} diff --git a/src/test/auxiliary/issue-14344-2.rs b/src/test/auxiliary/issue-14344-2.rs new file mode 100644 index 00000000000..9df35e50adb --- /dev/null +++ b/src/test/auxiliary/issue-14344-2.rs @@ -0,0 +1,13 @@ +// 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. + +extern crate issue_14344_1; + +pub fn bar() {} diff --git a/src/test/auxiliary/issue-25185-1.rs b/src/test/auxiliary/issue-25185-1.rs new file mode 100644 index 00000000000..b9da39cbbcb --- /dev/null +++ b/src/test/auxiliary/issue-25185-1.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. + +// no-prefer-dynamic + +#![crate_type = "rlib"] + +#[link(name = "rust_test_helpers", kind = "static")] +extern { + pub fn rust_dbg_extern_identity_u32(u: u32) -> u32; +} diff --git a/src/test/auxiliary/issue-25185-2.rs b/src/test/auxiliary/issue-25185-2.rs new file mode 100644 index 00000000000..00b5277d6c0 --- /dev/null +++ b/src/test/auxiliary/issue-25185-2.rs @@ -0,0 +1,13 @@ +// 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. + +extern crate issue_25185_1; + +pub use issue_25185_1::rust_dbg_extern_identity_u32; diff --git a/src/test/run-make/extern-fn-reachable/Makefile b/src/test/run-make/extern-fn-reachable/Makefile index 56748b1eb9b..79a9a3c640f 100644 --- a/src/test/run-make/extern-fn-reachable/Makefile +++ b/src/test/run-make/extern-fn-reachable/Makefile @@ -4,6 +4,6 @@ TARGET_RPATH_DIR:=$(TARGET_RPATH_DIR):$(TMPDIR) all: - $(RUSTC) dylib.rs -o $(TMPDIR)/libdylib.so - $(RUSTC) main.rs + $(RUSTC) dylib.rs -o $(TMPDIR)/libdylib.so -C prefer-dynamic + $(RUSTC) main.rs -C prefer-dynamic $(call RUN,main) diff --git a/src/test/run-pass/issue-14344.rs b/src/test/run-pass/issue-14344.rs new file mode 100644 index 00000000000..06b8f44ed26 --- /dev/null +++ b/src/test/run-pass/issue-14344.rs @@ -0,0 +1,20 @@ +// 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. + +// aux-build:issue-14344-1.rs +// aux-build:issue-14344-2.rs + +extern crate issue_14344_1; +extern crate issue_14344_2; + +fn main() { + issue_14344_1::foo(); + issue_14344_2::bar(); +} diff --git a/src/test/run-pass/issue-25185.rs b/src/test/run-pass/issue-25185.rs new file mode 100644 index 00000000000..d8d2d5078c5 --- /dev/null +++ b/src/test/run-pass/issue-25185.rs @@ -0,0 +1,21 @@ +// 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. + +// aux-build:issue-25185-1.rs +// aux-build:issue-25185-2.rs + +extern crate issue_25185_2; + +fn main() { + let x = unsafe { + issue_25185_2::rust_dbg_extern_identity_u32(1) + }; + assert_eq!(x, 1); +}