]> git.lizzy.rs Git - rust.git/commitdiff
rip out link guards
authorNiko Matsakis <niko@alum.mit.edu>
Thu, 24 Mar 2016 14:03:22 +0000 (10:03 -0400)
committerNiko Matsakis <niko@alum.mit.edu>
Fri, 25 Mar 2016 18:07:20 +0000 (14:07 -0400)
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
src/librustc_trans/trans/base.rs
src/librustc_trans/trans/link_guard.rs [deleted file]
src/librustc_trans/trans/mod.rs

index 934e0e16a9688bb01dcd3eb4fe3265b2252f069d..c6576b7fe0d9771101634474d1c89571a3172040 100644 (file)
@@ -23,7 +23,6 @@
 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 @@ fn export_symbols(&mut self, sess: &Session, trans: &CrateTranslation,
                 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 {
index 85f3bed52544ad26afae47a433511e86ad1aec3e..7231304ec4c4f43185bda4b27c4ba139f0082115 100644 (file)
@@ -79,7 +79,6 @@
 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 @@ fn create_entry_fn(ccx: &CrateContext,
         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 (file)
index 453afa8..0000000
+++ /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 <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());
-    }
-}
index 5c38de99da355a1598e708a550ab6f985bd26cbd..930f37ce256345ca80b5728a61a2552ea6d343e3 100644 (file)
@@ -53,7 +53,6 @@
 mod glue;
 mod inline;
 mod intrinsic;
-pub mod link_guard;
 mod machine;
 mod _match;
 mod meth;