From 4c527457f147881dc864b8b737c4288540b32208 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Thu, 24 Mar 2016 10:03:22 -0400 Subject: [PATCH] rip out link guards As discussed in https://github.com/rust-lang/rust/pull/32293#issuecomment-200597130, adding link guards are a heuristic that is causing undue complications: - the link guards inject extra public symbols, which is not always OK. - link guards as implemented could be a non-trivial performance hit, because no attempt is made to "de-duplicate" the dependency graph, so at worst you have O(N!) calls to the link guard functions. Nonetheless, link guards are very helpful in detecting errors, so it may be worth adding them back in some modified form in the future. --- src/librustc_trans/back/linker.rs | 20 ----- src/librustc_trans/trans/base.rs | 24 ----- src/librustc_trans/trans/link_guard.rs | 117 ------------------------- src/librustc_trans/trans/mod.rs | 1 - 4 files changed, 162 deletions(-) delete mode 100644 src/librustc_trans/trans/link_guard.rs diff --git a/src/librustc_trans/back/linker.rs b/src/librustc_trans/back/linker.rs index 934e0e16a96..c6576b7fe0d 100644 --- a/src/librustc_trans/back/linker.rs +++ b/src/librustc_trans/back/linker.rs @@ -23,7 +23,6 @@ use session::config::CrateTypeDylib; use session::config; use syntax::ast; use trans::CrateTranslation; -use trans::link_guard; /// Linker abstraction used by back::link to build up the command to invoke a /// linker. @@ -361,25 +360,6 @@ impl<'a> Linker for MsvcLinker<'a> { writeln!(f, " {}", symbol)?; } - // Add link-guard symbols - { - // local crate - let symbol = link_guard::link_guard_name(&trans.link.crate_name[..], - &trans.link.crate_hash); - try!(writeln!(f, " {}", symbol)); - } - // statically linked dependencies - for (i, format) in formats[&CrateTypeDylib].iter().enumerate() { - if *format == Linkage::Static { - let cnum = (i + 1) as ast::CrateNum; - let crate_name = cstore.original_crate_name(cnum); - let svh = cstore.crate_hash(cnum); - - let symbol = link_guard::link_guard_name(&crate_name[..], &svh); - try!(writeln!(f, " {}", symbol)); - } - } - Ok(()) })(); if let Err(e) = res { diff --git a/src/librustc_trans/trans/base.rs b/src/librustc_trans/trans/base.rs index 85f3bed5254..7231304ec4c 100644 --- a/src/librustc_trans/trans/base.rs +++ b/src/librustc_trans/trans/base.rs @@ -79,7 +79,6 @@ use trans::expr; use trans::glue; use trans::inline; use trans::intrinsic; -use trans::link_guard; use trans::machine; use trans::machine::{llalign_of_min, llsize_of, llsize_of_real}; use trans::meth; @@ -2384,7 +2383,6 @@ pub fn create_entry_wrapper(ccx: &CrateContext, sp: Span, main_llfn: ValueRef) { unsafe { llvm::LLVMPositionBuilderAtEnd(bld, llbb); - link_guard::insert_reference_to_link_guard(ccx, llbb); debuginfo::gdb::insert_reference_to_gdb_debug_scripts_section_global(ccx); let (start_fn, args) = if use_start_lang_item { @@ -2763,8 +2761,6 @@ pub fn trans_crate<'tcx>(tcx: &TyCtxt<'tcx>, symbol_names_test::report_symbol_names(&ccx); } - emit_link_guard_if_necessary(&shared_ccx); - for ccx in shared_ccx.iter() { if ccx.sess().opts.debuginfo != NoDebugInfo { debuginfo::finalize(&ccx); @@ -2825,8 +2821,6 @@ pub fn trans_crate<'tcx>(tcx: &TyCtxt<'tcx>, if sess.entry_fn.borrow().is_some() { reachable_symbols.push("main".to_string()); } - reachable_symbols.push(link_guard::link_guard_name(&link_meta.crate_name, - &link_meta.crate_hash)); // For the purposes of LTO, we add to the reachable set all of the upstream // reachable extern fns. These functions are all part of the public ABI of @@ -2870,24 +2864,6 @@ pub fn trans_crate<'tcx>(tcx: &TyCtxt<'tcx>, } } -fn emit_link_guard_if_necessary(shared_ccx: &SharedCrateContext) { - let link_meta = shared_ccx.link_meta(); - let link_guard_name = link_guard::link_guard_name(&link_meta.crate_name, - &link_meta.crate_hash); - let link_guard_name = CString::new(link_guard_name).unwrap(); - - // Check if the link-guard has already been emitted in a codegen unit - let link_guard_already_emitted = shared_ccx.iter().any(|ccx| { - let link_guard = unsafe { llvm::LLVMGetNamedValue(ccx.llmod(), - link_guard_name.as_ptr()) }; - !link_guard.is_null() - }); - - if !link_guard_already_emitted { - link_guard::get_or_insert_link_guard(&shared_ccx.get_ccx(0)); - } -} - /// We visit all the items in the krate and translate them. We do /// this in two walks. The first walk just finds module items. It then /// walks the full contents of those module items and translates all diff --git a/src/librustc_trans/trans/link_guard.rs b/src/librustc_trans/trans/link_guard.rs deleted file mode 100644 index 453afa827e8..00000000000 --- a/src/librustc_trans/trans/link_guard.rs +++ /dev/null @@ -1,117 +0,0 @@ -// Copyright 2012-2016 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. - -use back::svh::Svh; -use libc::c_uint; -use llvm; -use std::ffi::CString; -use std::ptr; -use trans::attributes; -use trans::builder; -use trans::CrateContext; -use trans::declare; -use trans::type_::Type; - -const GUARD_PREFIX: &'static str = "__rustc_link_guard_"; - -pub fn link_guard_name(crate_name: &str, crate_svh: &Svh) -> String { - - let mut guard_name = String::new(); - - guard_name.push_str(GUARD_PREFIX); - guard_name.push_str(crate_name); - guard_name.push_str("_"); - guard_name.push_str(crate_svh.as_str()); - - guard_name -} - -pub fn get_or_insert_link_guard<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>) - -> llvm::ValueRef { - - let guard_name = link_guard_name(&ccx.tcx().crate_name[..], - &ccx.link_meta().crate_hash); - - let guard_function = unsafe { - let guard_name_c_string = CString::new(&guard_name[..]).unwrap(); - llvm::LLVMGetNamedValue(ccx.llmod(), guard_name_c_string.as_ptr()) - }; - - if guard_function != ptr::null_mut() { - return guard_function; - } - - let llfty = Type::func(&[], &Type::void(ccx)); - if declare::get_defined_value(ccx, &guard_name[..]).is_some() { - ccx.sess().bug( - &format!("Link guard already defined")); - } - let guard_function = declare::declare_cfn(ccx, &guard_name[..], llfty); - - attributes::emit_uwtable(guard_function, true); - attributes::unwind(guard_function, false); - - let bld = ccx.raw_builder(); - unsafe { - let llbb = llvm::LLVMAppendBasicBlockInContext(ccx.llcx(), - guard_function, - "link_guard_top\0".as_ptr() as *const _); - llvm::LLVMPositionBuilderAtEnd(bld, llbb); - - for crate_num in ccx.sess().cstore.crates() { - if !ccx.sess().cstore.is_explicitly_linked(crate_num) { - continue; - } - - let crate_name = ccx.sess().cstore.original_crate_name(crate_num); - let svh = ccx.sess().cstore.crate_hash(crate_num); - - let dependency_guard_name = link_guard_name(&crate_name[..], &svh); - - if declare::get_defined_value(ccx, &dependency_guard_name[..]).is_some() { - ccx.sess().bug( - &format!("Link guard already defined for dependency `{}`", - crate_name)); - } - let decl = declare::declare_cfn(ccx, &dependency_guard_name[..], llfty); - attributes::unwind(decl, false); - - llvm::LLVMPositionBuilderAtEnd(bld, llbb); - - let args: &[llvm::ValueRef] = &[]; - llvm::LLVMRustBuildCall(bld, - decl, - args.as_ptr(), - args.len() as c_uint, - 0 as *mut _, - builder::noname()); - } - - llvm::LLVMBuildRetVoid(bld); - } - - guard_function -} - -pub fn insert_reference_to_link_guard<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, - llbb: llvm::BasicBlockRef) { - let guard_function = get_or_insert_link_guard(ccx); - - unsafe { - llvm::LLVMPositionBuilderAtEnd(ccx.raw_builder(), llbb); - let args: &[llvm::ValueRef] = &[]; - llvm::LLVMRustBuildCall(ccx.raw_builder(), - guard_function, - args.as_ptr(), - args.len() as c_uint, - 0 as *mut _, - builder::noname()); - } -} diff --git a/src/librustc_trans/trans/mod.rs b/src/librustc_trans/trans/mod.rs index 5c38de99da3..930f37ce256 100644 --- a/src/librustc_trans/trans/mod.rs +++ b/src/librustc_trans/trans/mod.rs @@ -53,7 +53,6 @@ mod expr; mod glue; mod inline; mod intrinsic; -pub mod link_guard; mod machine; mod _match; mod meth;