From e8bc064c5ecf7775999beef233e2d82c9a534362 Mon Sep 17 00:00:00 2001 From: Gabriel Smith Date: Sun, 22 Jul 2018 20:33:04 -0400 Subject: [PATCH 1/5] Add regression test for issue #18804 Signed-off-by: Gabriel Smith --- .../run-pass/issue-18804/auxiliary/lib.rs | 20 +++++++++++++++++++ src/test/run-pass/issue-18804/main.rs | 20 +++++++++++++++++++ 2 files changed, 40 insertions(+) create mode 100644 src/test/run-pass/issue-18804/auxiliary/lib.rs create mode 100644 src/test/run-pass/issue-18804/main.rs diff --git a/src/test/run-pass/issue-18804/auxiliary/lib.rs b/src/test/run-pass/issue-18804/auxiliary/lib.rs new file mode 100644 index 0000000000000..06d454b2c890a --- /dev/null +++ b/src/test/run-pass/issue-18804/auxiliary/lib.rs @@ -0,0 +1,20 @@ +// Copyright 2018 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. + +#![crate_type = "rlib"] +#![feature(linkage)] + +pub fn foo() -> *const() { + extern { + #[linkage = "extern_weak"] + static FOO: *const(); + } + unsafe { FOO } +} diff --git a/src/test/run-pass/issue-18804/main.rs b/src/test/run-pass/issue-18804/main.rs new file mode 100644 index 0000000000000..de7967e8d3490 --- /dev/null +++ b/src/test/run-pass/issue-18804/main.rs @@ -0,0 +1,20 @@ +// Copyright 2018 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. + +// Test for issue #18804, #[linkage] does not propagate thorugh generic +// functions. Failure results in a linker error. + +// aux-build:lib.rs + +extern crate lib; + +fn main() { + lib::foo::(); +} From a20262c06986acb98150913e2c43cb13cead92a7 Mon Sep 17 00:00:00 2001 From: Gabriel Smith Date: Sun, 22 Jul 2018 19:30:52 -0400 Subject: [PATCH 2/5] Properly set the linkage type on non-local statics Fixes issue #18804 Signed-off-by: Gabriel Smith --- src/librustc_codegen_llvm/consts.rs | 59 ++++++++++++++++++++++++++--- 1 file changed, 53 insertions(+), 6 deletions(-) diff --git a/src/librustc_codegen_llvm/consts.rs b/src/librustc_codegen_llvm/consts.rs index 5a2d958038422..1d42c84e2befb 100644 --- a/src/librustc_codegen_llvm/consts.rs +++ b/src/librustc_codegen_llvm/consts.rs @@ -119,6 +119,8 @@ pub fn get_static(cx: &CodegenCx, def_id: DefId) -> ValueRef { let ty = instance.ty(cx.tcx); let sym = cx.tcx.symbol_name(instance).as_str(); + debug!("get_static: sym={} instance={:?}", sym, instance); + let g = if let Some(id) = cx.tcx.hir.as_local_node_id(def_id) { let llty = cx.layout_of(ty).llvm_type(cx); @@ -145,6 +147,8 @@ pub fn get_static(cx: &CodegenCx, def_id: DefId) -> ValueRef { ref attrs, span, node: hir::ForeignItemKind::Static(..), .. }) => { let g = if let Some(linkage) = cx.tcx.codegen_fn_attrs(def_id).linkage { + debug!("get_static: sym={} linkage={:?}", sym, linkage); + // If this is a static with a linkage specified, then we need to handle // it a little specially. The typesystem prevents things like &T and // extern "C" fn() from being non-null, so we can't just declare a @@ -188,6 +192,8 @@ pub fn get_static(cx: &CodegenCx, def_id: DefId) -> ValueRef { item => bug!("get_static: expected static, found {:?}", item) }; + debug!("get_static: sym={} attrs={:?}", sym, attrs); + for attr in attrs { if attr.check_name("thread_local") { llvm::set_thread_local_mode(g, cx.tls_model); @@ -197,19 +203,60 @@ pub fn get_static(cx: &CodegenCx, def_id: DefId) -> ValueRef { g } else { // FIXME(nagisa): perhaps the map of externs could be offloaded to llvm somehow? - // FIXME(nagisa): investigate whether it can be changed into define_global - let g = declare::declare_global(cx, &sym, cx.layout_of(ty).llvm_type(cx)); + debug!("get_static: sym={} item_attr={:?}", sym, cx.tcx.item_attrs(def_id)); + + let codegen_fn_attrs = cx.tcx.codegen_fn_attrs(def_id); + let llty = cx.layout_of(ty).llvm_type(cx); + let g = if let Some(linkage) = codegen_fn_attrs.linkage { + debug!("get_static: sym={} linkage={:?}", sym, linkage); + + // If this is a static with a linkage specified, then we need to handle + // it a little specially. The typesystem prevents things like &T and + // extern "C" fn() from being non-null, so we can't just declare a + // static and call it a day. Some linkages (like weak) will make it such + // that the static actually has a null value. + let llty2 = match ty.sty { + ty::TyRawPtr(ref mt) => cx.layout_of(mt.ty).llvm_type(cx), + _ => { + bug!("must have type `*const T` or `*mut T`") + } + }; + unsafe { + // Declare a symbol `foo` with the desired linkage. + let g1 = declare::declare_global(cx, &sym, llty2); + llvm::LLVMRustSetLinkage(g1, base::linkage_to_llvm(linkage)); + + // Declare an internal global `extern_with_linkage_foo` which + // is initialized with the address of `foo`. If `foo` is + // discarded during linking (for example, if `foo` has weak + // linkage and there are no definitions), then + // `extern_with_linkage_foo` will instead be initialized to + // zero. + let mut real_name = "_rust_extern_with_linkage_".to_string(); + real_name.push_str(&sym); + let g2 = declare::define_global(cx, &real_name, llty).unwrap_or_else(||{ + bug!("symbol `{}` is already defined", &sym) + }); + llvm::LLVMRustSetLinkage(g2, llvm::Linkage::InternalLinkage); + llvm::LLVMSetInitializer(g2, g1); + g2 + } + } else { + // Generate an external declaration. + // FIXME(nagisa): investigate whether it can be changed into define_global + declare::declare_global(cx, &sym, llty) + }; + // Thread-local statics in some other crate need to *always* be linked // against in a thread-local fashion, so we need to be sure to apply the // thread-local attribute locally if it was present remotely. If we // don't do this then linker errors can be generated where the linker // complains that one object files has a thread local version of the // symbol and another one doesn't. - for attr in cx.tcx.get_attrs(def_id).iter() { - if attr.check_name("thread_local") { - llvm::set_thread_local_mode(g, cx.tls_model); - } + if codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::THREAD_LOCAL) { + llvm::set_thread_local_mode(g, cx.tls_model); } + if cx.use_dll_storage_attrs && !cx.tcx.is_foreign_item(def_id) { // This item is external but not foreign, i.e. it originates from an external Rust // crate. Since we don't know whether this crate will be linked dynamically or From 0bcbe91b487ea933aba9a9e079f01133574fc98f Mon Sep 17 00:00:00 2001 From: Gabriel Smith Date: Mon, 23 Jul 2018 08:43:22 -0400 Subject: [PATCH 3/5] Deduplicate linkage checking code for statics Signed-off-by: Gabriel Smith --- src/librustc_codegen_llvm/consts.rs | 154 +++++++++++++--------------- 1 file changed, 69 insertions(+), 85 deletions(-) diff --git a/src/librustc_codegen_llvm/consts.rs b/src/librustc_codegen_llvm/consts.rs index 1d42c84e2befb..f0b5f4b887971 100644 --- a/src/librustc_codegen_llvm/consts.rs +++ b/src/librustc_codegen_llvm/consts.rs @@ -20,12 +20,14 @@ use monomorphize::MonoItem; use common::{CodegenCx, val_ty}; use declare; use monomorphize::Instance; +use syntax_pos::Span; +use syntax_pos::symbol::LocalInternedString; use type_::Type; use type_of::LayoutLlvmExt; -use rustc::ty; +use rustc::ty::{self, Ty}; use rustc::ty::layout::{Align, LayoutOf}; -use rustc::hir::{self, CodegenFnAttrFlags}; +use rustc::hir::{self, CodegenFnAttrs, CodegenFnAttrFlags}; use std::ffi::{CStr, CString}; @@ -146,47 +148,8 @@ pub fn get_static(cx: &CodegenCx, def_id: DefId) -> ValueRef { hir_map::NodeForeignItem(&hir::ForeignItem { ref attrs, span, node: hir::ForeignItemKind::Static(..), .. }) => { - let g = if let Some(linkage) = cx.tcx.codegen_fn_attrs(def_id).linkage { - debug!("get_static: sym={} linkage={:?}", sym, linkage); - - // If this is a static with a linkage specified, then we need to handle - // it a little specially. The typesystem prevents things like &T and - // extern "C" fn() from being non-null, so we can't just declare a - // static and call it a day. Some linkages (like weak) will make it such - // that the static actually has a null value. - let llty2 = match ty.sty { - ty::TyRawPtr(ref mt) => cx.layout_of(mt.ty).llvm_type(cx), - _ => { - cx.sess().span_fatal(span, "must have type `*const T` or `*mut T`"); - } - }; - unsafe { - // Declare a symbol `foo` with the desired linkage. - let g1 = declare::declare_global(cx, &sym, llty2); - llvm::LLVMRustSetLinkage(g1, base::linkage_to_llvm(linkage)); - - // Declare an internal global `extern_with_linkage_foo` which - // is initialized with the address of `foo`. If `foo` is - // discarded during linking (for example, if `foo` has weak - // linkage and there are no definitions), then - // `extern_with_linkage_foo` will instead be initialized to - // zero. - let mut real_name = "_rust_extern_with_linkage_".to_string(); - real_name.push_str(&sym); - let g2 = declare::define_global(cx, &real_name, llty).unwrap_or_else(||{ - cx.sess().span_fatal(span, - &format!("symbol `{}` is already defined", &sym)) - }); - llvm::LLVMRustSetLinkage(g2, llvm::Linkage::InternalLinkage); - llvm::LLVMSetInitializer(g2, g1); - g2 - } - } else { - // Generate an external declaration. - declare::declare_global(cx, &sym, llty) - }; - - (g, attrs) + let fn_attrs = cx.tcx.codegen_fn_attrs(def_id); + (check_and_apply_linkage(cx, &fn_attrs, ty, sym, Some(span)), attrs) } item => bug!("get_static: expected static, found {:?}", item) @@ -205,47 +168,8 @@ pub fn get_static(cx: &CodegenCx, def_id: DefId) -> ValueRef { // FIXME(nagisa): perhaps the map of externs could be offloaded to llvm somehow? debug!("get_static: sym={} item_attr={:?}", sym, cx.tcx.item_attrs(def_id)); - let codegen_fn_attrs = cx.tcx.codegen_fn_attrs(def_id); - let llty = cx.layout_of(ty).llvm_type(cx); - let g = if let Some(linkage) = codegen_fn_attrs.linkage { - debug!("get_static: sym={} linkage={:?}", sym, linkage); - - // If this is a static with a linkage specified, then we need to handle - // it a little specially. The typesystem prevents things like &T and - // extern "C" fn() from being non-null, so we can't just declare a - // static and call it a day. Some linkages (like weak) will make it such - // that the static actually has a null value. - let llty2 = match ty.sty { - ty::TyRawPtr(ref mt) => cx.layout_of(mt.ty).llvm_type(cx), - _ => { - bug!("must have type `*const T` or `*mut T`") - } - }; - unsafe { - // Declare a symbol `foo` with the desired linkage. - let g1 = declare::declare_global(cx, &sym, llty2); - llvm::LLVMRustSetLinkage(g1, base::linkage_to_llvm(linkage)); - - // Declare an internal global `extern_with_linkage_foo` which - // is initialized with the address of `foo`. If `foo` is - // discarded during linking (for example, if `foo` has weak - // linkage and there are no definitions), then - // `extern_with_linkage_foo` will instead be initialized to - // zero. - let mut real_name = "_rust_extern_with_linkage_".to_string(); - real_name.push_str(&sym); - let g2 = declare::define_global(cx, &real_name, llty).unwrap_or_else(||{ - bug!("symbol `{}` is already defined", &sym) - }); - llvm::LLVMRustSetLinkage(g2, llvm::Linkage::InternalLinkage); - llvm::LLVMSetInitializer(g2, g1); - g2 - } - } else { - // Generate an external declaration. - // FIXME(nagisa): investigate whether it can be changed into define_global - declare::declare_global(cx, &sym, llty) - }; + let attrs = cx.tcx.codegen_fn_attrs(def_id); + let g = check_and_apply_linkage(cx, &attrs, ty, sym, None); // Thread-local statics in some other crate need to *always* be linked // against in a thread-local fashion, so we need to be sure to apply the @@ -253,7 +177,7 @@ pub fn get_static(cx: &CodegenCx, def_id: DefId) -> ValueRef { // don't do this then linker errors can be generated where the linker // complains that one object files has a thread local version of the // symbol and another one doesn't. - if codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::THREAD_LOCAL) { + if attrs.flags.contains(CodegenFnAttrFlags::THREAD_LOCAL) { llvm::set_thread_local_mode(g, cx.tls_model); } @@ -289,6 +213,66 @@ pub fn get_static(cx: &CodegenCx, def_id: DefId) -> ValueRef { g } +fn check_and_apply_linkage<'tcx>( + cx: &CodegenCx<'_, 'tcx>, + attrs: &CodegenFnAttrs, + ty: Ty<'tcx>, + sym: LocalInternedString, + span: Option +) -> ValueRef { + let llty = cx.layout_of(ty).llvm_type(cx); + if let Some(linkage) = attrs.linkage { + debug!("get_static: sym={} linkage={:?}", sym, linkage); + + // If this is a static with a linkage specified, then we need to handle + // it a little specially. The typesystem prevents things like &T and + // extern "C" fn() from being non-null, so we can't just declare a + // static and call it a day. Some linkages (like weak) will make it such + // that the static actually has a null value. + let llty2 = match ty.sty { + ty::TyRawPtr(ref mt) => cx.layout_of(mt.ty).llvm_type(cx), + _ => { + if span.is_some() { + cx.sess().span_fatal(span.unwrap(), "must have type `*const T` or `*mut T`") + } else { + bug!("must have type `*const T` or `*mut T`") + } + } + }; + unsafe { + // Declare a symbol `foo` with the desired linkage. + let g1 = declare::declare_global(cx, &sym, llty2); + llvm::LLVMRustSetLinkage(g1, base::linkage_to_llvm(linkage)); + + // Declare an internal global `extern_with_linkage_foo` which + // is initialized with the address of `foo`. If `foo` is + // discarded during linking (for example, if `foo` has weak + // linkage and there are no definitions), then + // `extern_with_linkage_foo` will instead be initialized to + // zero. + let mut real_name = "_rust_extern_with_linkage_".to_string(); + real_name.push_str(&sym); + let g2 = declare::define_global(cx, &real_name, llty).unwrap_or_else(||{ + if span.is_some() { + cx.sess().span_fatal( + span.unwrap(), + &format!("symbol `{}` is already defined", &sym) + ) + } else { + bug!("symbol `{}` is already defined", &sym) + } + }); + llvm::LLVMRustSetLinkage(g2, llvm::Linkage::InternalLinkage); + llvm::LLVMSetInitializer(g2, g1); + g2 + } + } else { + // Generate an external declaration. + // FIXME(nagisa): investigate whether it can be changed into define_global + declare::declare_global(cx, &sym, llty) + } +} + pub fn codegen_static<'a, 'tcx>( cx: &CodegenCx<'a, 'tcx>, def_id: DefId, From e58e7b045c7c66a11e2b451ea48e763f6685699e Mon Sep 17 00:00:00 2001 From: Gabriel Smith Date: Tue, 24 Jul 2018 08:51:57 -0400 Subject: [PATCH 4/5] Disable regression test for issue #18804 on Emscripten and Asmjs The Emscripten compiler does not support weak symbols at the moment. Signed-off-by: Gabriel Smith --- src/test/run-pass/issue-18804/auxiliary/lib.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/test/run-pass/issue-18804/auxiliary/lib.rs b/src/test/run-pass/issue-18804/auxiliary/lib.rs index 06d454b2c890a..43434766362ff 100644 --- a/src/test/run-pass/issue-18804/auxiliary/lib.rs +++ b/src/test/run-pass/issue-18804/auxiliary/lib.rs @@ -8,6 +8,9 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +// ignore-asmjs no weak symbol support +// ignore-emscripten no weak symbol support + #![crate_type = "rlib"] #![feature(linkage)] From be5b668a2e892c4c892986809b0a7d119980037a Mon Sep 17 00:00:00 2001 From: Gabriel Smith Date: Wed, 25 Jul 2018 07:51:32 -0400 Subject: [PATCH 5/5] Place the ignore comments in the correct file for test issue-18804 Signed-off-by: Gabriel Smith --- src/test/run-pass/issue-18804/auxiliary/lib.rs | 3 --- src/test/run-pass/issue-18804/main.rs | 3 +++ 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/test/run-pass/issue-18804/auxiliary/lib.rs b/src/test/run-pass/issue-18804/auxiliary/lib.rs index 43434766362ff..06d454b2c890a 100644 --- a/src/test/run-pass/issue-18804/auxiliary/lib.rs +++ b/src/test/run-pass/issue-18804/auxiliary/lib.rs @@ -8,9 +8,6 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -// ignore-asmjs no weak symbol support -// ignore-emscripten no weak symbol support - #![crate_type = "rlib"] #![feature(linkage)] diff --git a/src/test/run-pass/issue-18804/main.rs b/src/test/run-pass/issue-18804/main.rs index de7967e8d3490..b5aa052034936 100644 --- a/src/test/run-pass/issue-18804/main.rs +++ b/src/test/run-pass/issue-18804/main.rs @@ -11,6 +11,9 @@ // Test for issue #18804, #[linkage] does not propagate thorugh generic // functions. Failure results in a linker error. +// ignore-asmjs no weak symbol support +// ignore-emscripten no weak symbol support + // aux-build:lib.rs extern crate lib;