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.
This commit is contained in:
Niko Matsakis 2016-03-24 10:03:22 -04:00
parent b385ce1223
commit 4c527457f1
4 changed files with 0 additions and 162 deletions

View File

@ -23,7 +23,6 @@ use session::config::CrateTypeDylib;
use session::config; use session::config;
use syntax::ast; use syntax::ast;
use trans::CrateTranslation; use trans::CrateTranslation;
use trans::link_guard;
/// Linker abstraction used by back::link to build up the command to invoke a /// Linker abstraction used by back::link to build up the command to invoke a
/// linker. /// linker.
@ -361,25 +360,6 @@ impl<'a> Linker for MsvcLinker<'a> {
writeln!(f, " {}", symbol)?; 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(()) Ok(())
})(); })();
if let Err(e) = res { if let Err(e) = res {

View File

@ -79,7 +79,6 @@ use trans::expr;
use trans::glue; use trans::glue;
use trans::inline; use trans::inline;
use trans::intrinsic; use trans::intrinsic;
use trans::link_guard;
use trans::machine; use trans::machine;
use trans::machine::{llalign_of_min, llsize_of, llsize_of_real}; use trans::machine::{llalign_of_min, llsize_of, llsize_of_real};
use trans::meth; use trans::meth;
@ -2384,7 +2383,6 @@ pub fn create_entry_wrapper(ccx: &CrateContext, sp: Span, main_llfn: ValueRef) {
unsafe { unsafe {
llvm::LLVMPositionBuilderAtEnd(bld, llbb); llvm::LLVMPositionBuilderAtEnd(bld, llbb);
link_guard::insert_reference_to_link_guard(ccx, llbb);
debuginfo::gdb::insert_reference_to_gdb_debug_scripts_section_global(ccx); debuginfo::gdb::insert_reference_to_gdb_debug_scripts_section_global(ccx);
let (start_fn, args) = if use_start_lang_item { 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); symbol_names_test::report_symbol_names(&ccx);
} }
emit_link_guard_if_necessary(&shared_ccx);
for ccx in shared_ccx.iter() { for ccx in shared_ccx.iter() {
if ccx.sess().opts.debuginfo != NoDebugInfo { if ccx.sess().opts.debuginfo != NoDebugInfo {
debuginfo::finalize(&ccx); debuginfo::finalize(&ccx);
@ -2825,8 +2821,6 @@ pub fn trans_crate<'tcx>(tcx: &TyCtxt<'tcx>,
if sess.entry_fn.borrow().is_some() { if sess.entry_fn.borrow().is_some() {
reachable_symbols.push("main".to_string()); 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 // 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 // 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 /// 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 /// this in two walks. The first walk just finds module items. It then
/// walks the full contents of those module items and translates all /// walks the full contents of those module items and translates all

View File

@ -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 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, 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());
}
}

View File

@ -53,7 +53,6 @@ mod expr;
mod glue; mod glue;
mod inline; mod inline;
mod intrinsic; mod intrinsic;
pub mod link_guard;
mod machine; mod machine;
mod _match; mod _match;
mod meth; mod meth;