From 9adcbac30dd229490b0eb3f794fa0cd89e5f457b Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Tue, 17 Sep 2013 11:24:05 -0700 Subject: [PATCH] Prevent a rare linkage issue with an xcrate static If a static is flagged as address_insignificant, then for LLVM to actually perform the relevant optimization it must have an internal linkage type. What this means, though, is that the static will not be available to other crates. Hence, if you have a generic function with an inner static, it will fail to link when built as a library because other crates will attempt to use the inner static externally. This gets around the issue by inlining the static into the metadata. The same relevant optimization is then applied separately in the external crate. What this ends up meaning is that all statics tagged with #[address_insignificant] will appear at most once per crate (by value), but they could appear in multiple crates. This should be the last blocker for using format! ... --- src/librustc/middle/trans/base.rs | 27 ++++++++++++++++--- src/librustc/middle/trans/inline.rs | 8 +++++- .../auxiliary/xcrate_address_insignificant.rs | 19 +++++++++++++ .../run-pass/xcrate-address-insignificant.rs | 18 +++++++++++++ 4 files changed, 67 insertions(+), 5 deletions(-) create mode 100644 src/test/auxiliary/xcrate_address_insignificant.rs create mode 100644 src/test/run-pass/xcrate-address-insignificant.rs diff --git a/src/librustc/middle/trans/base.rs b/src/librustc/middle/trans/base.rs index b4979c335b5..6f70d2e601d 100644 --- a/src/librustc/middle/trans/base.rs +++ b/src/librustc/middle/trans/base.rs @@ -2559,10 +2559,7 @@ pub fn get_item_val(ccx: @mut CrateContext, id: ast::NodeId) -> ValueRef { // LLVM type is not fully determined by the Rust type. let (v, inlineable) = consts::const_expr(ccx, expr); ccx.const_values.insert(id, v); - if !inlineable { - debug!("%s not inlined", sym); - ccx.non_inlineable_statics.insert(id); - } + let mut inlineable = inlineable; exprt = true; unsafe { @@ -2578,8 +2575,30 @@ pub fn get_item_val(ccx: @mut CrateContext, id: ast::NodeId) -> ValueRef { lib::llvm::SetUnnamedAddr(g, true); lib::llvm::SetLinkage(g, lib::llvm::InternalLinkage); + + // This is a curious case where we must make + // all of these statics inlineable. If a + // global is tagged as + // address_insignificant, then LLVM won't + // coalesce globals unless they have an + // internal linkage type. This means that + // external crates cannot use this global. + // This is a problem for things like inner + // statics in generic functions, because the + // function will be inlined into another + // crate and then attempt to link to the + // static in the original crate, only to + // find that it's not there. On the other + // side of inlininig, the crates knows to + // not declare this static as + // available_externally (because it isn't) + inlineable = true; } + if !inlineable { + debug!("%s not inlined", sym); + ccx.non_inlineable_statics.insert(id); + } ccx.item_symbols.insert(i.id, sym); g } diff --git a/src/librustc/middle/trans/inline.rs b/src/librustc/middle/trans/inline.rs index a571e56a48e..8900b50b49d 100644 --- a/src/librustc/middle/trans/inline.rs +++ b/src/librustc/middle/trans/inline.rs @@ -21,6 +21,7 @@ use std::vec; use syntax::ast; use syntax::ast_map::path_name; use syntax::ast_util::local_def; +use syntax::attr; pub fn maybe_instantiate_inline(ccx: @mut CrateContext, fn_id: ast::DefId) -> ast::DefId { @@ -68,7 +69,12 @@ pub fn maybe_instantiate_inline(ccx: @mut CrateContext, fn_id: ast::DefId) match item.node { ast::item_static(*) => { let g = get_item_val(ccx, item.id); - SetLinkage(g, AvailableExternallyLinkage); + // see the comment in get_item_val() as to why this check is + // performed here. + if !attr::contains_name(item.attrs, + "address_insignificant") { + SetLinkage(g, AvailableExternallyLinkage); + } } _ => {} } diff --git a/src/test/auxiliary/xcrate_address_insignificant.rs b/src/test/auxiliary/xcrate_address_insignificant.rs new file mode 100644 index 00000000000..08e3eff0c8c --- /dev/null +++ b/src/test/auxiliary/xcrate_address_insignificant.rs @@ -0,0 +1,19 @@ +// Copyright 2013 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. + +pub fn foo() -> int { + #[address_insignificant] + static a: int = 3; + a +} + +pub fn bar() -> int { + foo::() +} diff --git a/src/test/run-pass/xcrate-address-insignificant.rs b/src/test/run-pass/xcrate-address-insignificant.rs new file mode 100644 index 00000000000..1bf3763834a --- /dev/null +++ b/src/test/run-pass/xcrate-address-insignificant.rs @@ -0,0 +1,18 @@ +// Copyright 2013 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. + +// xfail-fast windows doesn't like aux-build +// aux-build:xcrate_address_insignificant.rs + +extern mod foo(name = "xcrate_address_insignificant"); + +fn main() { + assert_eq!(foo::foo::(), foo::bar()); +}