From 02190f397ecb32bca42e5b631dc235381d01b377 Mon Sep 17 00:00:00 2001 From: Colin Pronovost Date: Wed, 23 May 2018 15:19:07 -0400 Subject: [PATCH 1/2] Make globals with private linkage unnamed. Fixes #50862. --- src/librustc_codegen_llvm/consts.rs | 20 +++++++++++------- src/librustc_codegen_llvm/declare.rs | 10 ++++++++- src/librustc_codegen_llvm/llvm/ffi.rs | 1 + src/librustc_codegen_llvm/meth.rs | 2 +- src/librustc_codegen_llvm/mir/block.rs | 4 ++-- src/librustc_codegen_llvm/mir/constant.rs | 4 ++-- src/librustc_codegen_llvm/mir/place.rs | 2 +- src/rustllvm/RustWrapper.cpp | 10 +++++++++ src/test/codegen/consts.rs | 4 ++-- src/test/codegen/remap_path_prefix/main.rs | 2 +- .../symbols-are-reasonable/Makefile | 13 ------------ .../symbols-are-reasonable/lib.rs | 21 ------------------- 12 files changed, 42 insertions(+), 51 deletions(-) delete mode 100644 src/test/run-make-fulldeps/symbols-are-reasonable/Makefile delete mode 100644 src/test/run-make-fulldeps/symbols-are-reasonable/lib.rs diff --git a/src/librustc_codegen_llvm/consts.rs b/src/librustc_codegen_llvm/consts.rs index 72ff65361cada..21bf490beb0fb 100644 --- a/src/librustc_codegen_llvm/consts.rs +++ b/src/librustc_codegen_llvm/consts.rs @@ -66,16 +66,22 @@ pub fn addr_of_mut( cx: &CodegenCx<'ll, '_>, cv: &'ll Value, align: Align, - kind: &str, + kind: Option<&str>, ) -> &'ll Value { unsafe { - let name = cx.generate_local_symbol_name(kind); - let gv = declare::define_global(cx, &name[..], val_ty(cv)).unwrap_or_else(||{ - bug!("symbol `{}` is already defined", name); - }); + let gv = match kind { + Some(kind) if !cx.tcx.sess.fewer_names() => { + let name = cx.generate_local_symbol_name(kind); + let gv = declare::define_global(cx, &name[..], val_ty(cv)).unwrap_or_else(||{ + bug!("symbol `{}` is already defined", name); + }); + llvm::LLVMRustSetLinkage(gv, llvm::Linkage::PrivateLinkage); + gv + }, + _ => declare::define_private_global(cx, val_ty(cv)), + }; llvm::LLVMSetInitializer(gv, cv); set_global_alignment(cx, gv, align); - llvm::LLVMRustSetLinkage(gv, llvm::Linkage::PrivateLinkage); SetUnnamedAddr(gv, true); gv } @@ -85,7 +91,7 @@ pub fn addr_of( cx: &CodegenCx<'ll, '_>, cv: &'ll Value, align: Align, - kind: &str, + kind: Option<&str>, ) -> &'ll Value { if let Some(&gv) = cx.const_globals.borrow().get(&cv) { unsafe { diff --git a/src/librustc_codegen_llvm/declare.rs b/src/librustc_codegen_llvm/declare.rs index 9812d7f9a41a2..a0310eecd591d 100644 --- a/src/librustc_codegen_llvm/declare.rs +++ b/src/librustc_codegen_llvm/declare.rs @@ -35,7 +35,6 @@ use value::Value; use std::ffi::CString; - /// Declare a global value. /// /// If there’s a value with the same name already declared, the function will @@ -170,6 +169,15 @@ pub fn define_global(cx: &CodegenCx<'ll, '_>, name: &str, ty: &'ll Type) -> Opti } } +/// Declare a private global +/// +/// Use this function when you intend to define a global without a name. +pub fn define_private_global(cx: &CodegenCx<'ll, '_>, ty: &'ll Type) -> &'ll Value { + unsafe { + llvm::LLVMRustInsertPrivateGlobal(cx.llmod, ty) + } +} + /// Declare a Rust function with an intention to define it. /// /// Use this function when you intend to define a function. This function will diff --git a/src/librustc_codegen_llvm/llvm/ffi.rs b/src/librustc_codegen_llvm/llvm/ffi.rs index 898d3d6735363..7130445313485 100644 --- a/src/librustc_codegen_llvm/llvm/ffi.rs +++ b/src/librustc_codegen_llvm/llvm/ffi.rs @@ -622,6 +622,7 @@ extern "C" { pub fn LLVMAddGlobal(M: &'a Module, Ty: &'a Type, Name: *const c_char) -> &'a Value; pub fn LLVMGetNamedGlobal(M: &Module, Name: *const c_char) -> Option<&Value>; pub fn LLVMRustGetOrInsertGlobal(M: &'a Module, Name: *const c_char, T: &'a Type) -> &'a Value; + pub fn LLVMRustInsertPrivateGlobal(M: &'a Module, T: &'a Type) -> &'a Value; pub fn LLVMGetFirstGlobal(M: &Module) -> Option<&Value>; pub fn LLVMGetNextGlobal(GlobalVar: &Value) -> Option<&Value>; pub fn LLVMDeleteGlobal(GlobalVar: &Value); diff --git a/src/librustc_codegen_llvm/meth.rs b/src/librustc_codegen_llvm/meth.rs index 9c0dd0dc3d8b5..8a1159bc4773c 100644 --- a/src/librustc_codegen_llvm/meth.rs +++ b/src/librustc_codegen_llvm/meth.rs @@ -106,7 +106,7 @@ pub fn get_vtable( let vtable_const = C_struct(cx, &components, false); let align = cx.data_layout().pointer_align; - let vtable = consts::addr_of(cx, vtable_const, align, "vtable"); + let vtable = consts::addr_of(cx, vtable_const, align, Some("vtable")); debuginfo::create_vtable_metadata(cx, ty, vtable); diff --git a/src/librustc_codegen_llvm/mir/block.rs b/src/librustc_codegen_llvm/mir/block.rs index 684ecfaeec8f1..4e389c3b915f0 100644 --- a/src/librustc_codegen_llvm/mir/block.rs +++ b/src/librustc_codegen_llvm/mir/block.rs @@ -377,7 +377,7 @@ impl FunctionCx<'a, 'll, 'tcx> { let file_line_col = consts::addr_of(bx.cx, file_line_col, align, - "panic_bounds_check_loc"); + Some("panic_bounds_check_loc")); (lang_items::PanicBoundsCheckFnLangItem, vec![file_line_col, index, len]) } @@ -391,7 +391,7 @@ impl FunctionCx<'a, 'll, 'tcx> { let msg_file_line_col = consts::addr_of(bx.cx, msg_file_line_col, align, - "panic_loc"); + Some("panic_loc")); (lang_items::PanicFnLangItem, vec![msg_file_line_col]) } diff --git a/src/librustc_codegen_llvm/mir/constant.rs b/src/librustc_codegen_llvm/mir/constant.rs index 341ed9df64b59..e414af07c9ca6 100644 --- a/src/librustc_codegen_llvm/mir/constant.rs +++ b/src/librustc_codegen_llvm/mir/constant.rs @@ -56,9 +56,9 @@ pub fn scalar_to_llvm( Some(AllocType::Memory(alloc)) => { let init = const_alloc_to_llvm(cx, alloc); if alloc.runtime_mutability == Mutability::Mutable { - consts::addr_of_mut(cx, init, alloc.align, "byte_str") + consts::addr_of_mut(cx, init, alloc.align, None) } else { - consts::addr_of(cx, init, alloc.align, "byte_str") + consts::addr_of(cx, init, alloc.align, None) } } Some(AllocType::Function(fn_instance)) => { diff --git a/src/librustc_codegen_llvm/mir/place.rs b/src/librustc_codegen_llvm/mir/place.rs index abc3dbdab2f5b..6fa0845dd0ceb 100644 --- a/src/librustc_codegen_llvm/mir/place.rs +++ b/src/librustc_codegen_llvm/mir/place.rs @@ -63,7 +63,7 @@ impl PlaceRef<'ll, 'tcx> { offset: Size, ) -> PlaceRef<'ll, 'tcx> { let init = const_alloc_to_llvm(bx.cx, alloc); - let base_addr = consts::addr_of(bx.cx, init, layout.align, "byte_str"); + let base_addr = consts::addr_of(bx.cx, init, layout.align, None); let llval = unsafe { LLVMConstInBoundsGEP( consts::bitcast(base_addr, Type::i8p(bx.cx)), diff --git a/src/rustllvm/RustWrapper.cpp b/src/rustllvm/RustWrapper.cpp index f2b5297285ca7..ce865dfbec271 100644 --- a/src/rustllvm/RustWrapper.cpp +++ b/src/rustllvm/RustWrapper.cpp @@ -12,6 +12,7 @@ #include "llvm/IR/DebugInfoMetadata.h" #include "llvm/IR/DiagnosticInfo.h" #include "llvm/IR/DiagnosticPrinter.h" +#include "llvm/IR/GlobalVariable.h" #include "llvm/IR/Instructions.h" #include "llvm/Object/Archive.h" #include "llvm/Object/ObjectFile.h" @@ -116,6 +117,15 @@ LLVMRustGetOrInsertGlobal(LLVMModuleRef M, const char *Name, LLVMTypeRef Ty) { return wrap(unwrap(M)->getOrInsertGlobal(Name, unwrap(Ty))); } +extern "C" LLVMValueRef +LLVMRustInsertPrivateGlobal(LLVMModuleRef M, LLVMTypeRef Ty) { + return wrap(new GlobalVariable(*unwrap(M), + unwrap(Ty), + false, + GlobalValue::PrivateLinkage, + nullptr)); +} + extern "C" LLVMTypeRef LLVMRustMetadataTypeInContext(LLVMContextRef C) { return wrap(Type::getMetadataTy(*unwrap(C))); } diff --git a/src/test/codegen/consts.rs b/src/test/codegen/consts.rs index 30fffbb769b30..301f554448627 100644 --- a/src/test/codegen/consts.rs +++ b/src/test/codegen/consts.rs @@ -21,11 +21,11 @@ // CHECK: @STATIC = {{.*}}, align 4 // This checks the constants from inline_enum_const -// CHECK: @byte_str.{{[0-9]+}} = {{.*}}, align 2 +// CHECK: @{{[0-9]+}} = {{.*}}, align 2 // This checks the constants from {low,high}_align_const, they share the same // constant, but the alignment differs, so the higher one should be used -// CHECK: [[LOW_HIGH:@byte_str.[0-9]+]] = {{.*}}, align 4 +// CHECK: [[LOW_HIGH:@[0-9]+]] = {{.*}}, align 4 #[derive(Copy, Clone)] diff --git a/src/test/codegen/remap_path_prefix/main.rs b/src/test/codegen/remap_path_prefix/main.rs index 4fb8c37558d48..dd0f89c931d8a 100644 --- a/src/test/codegen/remap_path_prefix/main.rs +++ b/src/test/codegen/remap_path_prefix/main.rs @@ -22,7 +22,7 @@ mod aux_mod; include!("aux_mod.rs"); // Here we check that the expansion of the file!() macro is mapped. -// CHECK: @byte_str.1 = private unnamed_addr constant <{ [34 x i8] }> <{ [34 x i8] c"/the/src/remap_path_prefix/main.rs" }>, align 1 +// CHECK: @0 = private unnamed_addr constant <{ [34 x i8] }> <{ [34 x i8] c"/the/src/remap_path_prefix/main.rs" }>, align 1 pub static FILE_PATH: &'static str = file!(); fn main() { diff --git a/src/test/run-make-fulldeps/symbols-are-reasonable/Makefile b/src/test/run-make-fulldeps/symbols-are-reasonable/Makefile deleted file mode 100644 index a6d294d2a1c0a..0000000000000 --- a/src/test/run-make-fulldeps/symbols-are-reasonable/Makefile +++ /dev/null @@ -1,13 +0,0 @@ --include ../tools.mk - -# check that the compile generated symbols for strings, binaries, -# vtables, etc. have semisane names (e.g. `str.1234`); it's relatively -# easy to accidentally modify the compiler internals to make them -# become things like `str"str"(1234)`. - -OUT=$(TMPDIR)/lib.s - -all: - $(RUSTC) lib.rs --emit=asm --crate-type=staticlib - # just check for symbol declarations with the names we're expecting. - $(CGREP) -e 'str\.[0-9]+:' 'byte_str\.[0-9]+:' 'vtable\.[0-9]+' < $(OUT) diff --git a/src/test/run-make-fulldeps/symbols-are-reasonable/lib.rs b/src/test/run-make-fulldeps/symbols-are-reasonable/lib.rs deleted file mode 100644 index b9285b24cd63f..0000000000000 --- a/src/test/run-make-fulldeps/symbols-are-reasonable/lib.rs +++ /dev/null @@ -1,21 +0,0 @@ -// Copyright 2014 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 static X: &'static str = "foobarbaz"; -pub static Y: &'static [u8] = include_bytes!("lib.rs"); - -trait Foo { fn dummy(&self) { } } -impl Foo for usize {} - -#[no_mangle] -pub extern "C" fn dummy() { - // force the vtable to be created - let _x = &1usize as &Foo; -} From 3da7c65e92e212143bafdb4adf9cfb8054209a93 Mon Sep 17 00:00:00 2001 From: Colin Pronovost Date: Wed, 30 May 2018 16:46:56 -0400 Subject: [PATCH 2/2] Make sure we prepare for thin LTO whenever we are emitting bitcode Emitting LLVM bitcode uses ThinLTOBuffers, so we need to prepare for thin LTO or we will likely cause errors in LLVM. --- src/librustc_codegen_llvm/back/write.rs | 31 +++++++++++++++++++ src/test/run-pass-fulldeps/myriad-closures.rs | 2 +- src/test/run-pass/issue-38226.rs | 2 +- src/test/ui/feature-gate-unwind-attributes.rs | 2 +- 4 files changed, 34 insertions(+), 3 deletions(-) diff --git a/src/librustc_codegen_llvm/back/write.rs b/src/librustc_codegen_llvm/back/write.rs index db044878fe745..5cb7215a1981d 100644 --- a/src/librustc_codegen_llvm/back/write.rs +++ b/src/librustc_codegen_llvm/back/write.rs @@ -541,11 +541,23 @@ unsafe fn optimize(cgcx: &CodegenContext, }; if config.verify_llvm_ir { assert!(addpass("verify")); } + + // Some options cause LLVM bitcode to be emitted, which uses ThinLTOBuffers, so we need + // to make sure we run LLVM's NameAnonGlobals pass when emitting bitcode; otherwise + // we'll get errors in LLVM. + let using_thin_buffers = llvm::LLVMRustThinLTOAvailable() && (config.emit_bc + || config.obj_is_bitcode || config.emit_bc_compressed || config.embed_bitcode); + let mut have_name_anon_globals_pass = false; if !config.no_prepopulate_passes { llvm::LLVMRustAddAnalysisPasses(tm, fpm, llmod); llvm::LLVMRustAddAnalysisPasses(tm, mpm, llmod); let opt_level = config.opt_level.unwrap_or(llvm::CodeGenOptLevel::None); let prepare_for_thin_lto = cgcx.lto == Lto::Thin || cgcx.lto == Lto::ThinLocal; + have_name_anon_globals_pass = have_name_anon_globals_pass || prepare_for_thin_lto; + if using_thin_buffers && !prepare_for_thin_lto { + assert!(addpass("name-anon-globals")); + have_name_anon_globals_pass = true; + } with_llvm_pmb(llmod, &config, opt_level, prepare_for_thin_lto, &mut |b| { llvm::LLVMPassManagerBuilderPopulateFunctionPassManager(b, fpm); llvm::LLVMPassManagerBuilderPopulateModulePassManager(b, mpm); @@ -557,6 +569,9 @@ unsafe fn optimize(cgcx: &CodegenContext, diag_handler.warn(&format!("unknown pass `{}`, ignoring", pass)); } + if pass == "name-anon-globals" { + have_name_anon_globals_pass = true; + } } for pass in &cgcx.plugin_passes { @@ -565,6 +580,22 @@ unsafe fn optimize(cgcx: &CodegenContext, `{}` but LLVM does not \ recognize it", pass)); } + if pass == "name-anon-globals" { + have_name_anon_globals_pass = true; + } + } + + if using_thin_buffers && !have_name_anon_globals_pass { + // As described above, this will probably cause an error in LLVM + if config.no_prepopulate_passes { + diag_handler.err("The current compilation is going to use thin LTO buffers \ + without running LLVM's NameAnonGlobals pass. \ + This will likely cause errors in LLVM. Consider adding \ + -C passes=name-anon-globals to the compiler command line."); + } else { + bug!("We are using thin LTO buffers without running the NameAnonGlobals pass. \ + This will likely cause errors in LLVM and shoud never happen."); + } } } diff --git a/src/test/run-pass-fulldeps/myriad-closures.rs b/src/test/run-pass-fulldeps/myriad-closures.rs index a946ec635b29f..baf27d6f57c3c 100644 --- a/src/test/run-pass-fulldeps/myriad-closures.rs +++ b/src/test/run-pass-fulldeps/myriad-closures.rs @@ -14,7 +14,7 @@ // See https://github.com/rust-lang/rust/issues/34793 for more information. // Make sure we don't optimize anything away: -// compile-flags: -C no-prepopulate-passes +// compile-flags: -C no-prepopulate-passes -Cpasses=name-anon-globals // Expand something exponentially macro_rules! go_bacterial { diff --git a/src/test/run-pass/issue-38226.rs b/src/test/run-pass/issue-38226.rs index 33604212af951..dd9f9be7da713 100644 --- a/src/test/run-pass/issue-38226.rs +++ b/src/test/run-pass/issue-38226.rs @@ -15,7 +15,7 @@ // Need -Cno-prepopulate-passes to really disable inlining, otherwise the faulty // code gets optimized out: -// compile-flags: -Cno-prepopulate-passes +// compile-flags: -Cno-prepopulate-passes -Cpasses=name-anon-globals extern crate issue_38226_aux; diff --git a/src/test/ui/feature-gate-unwind-attributes.rs b/src/test/ui/feature-gate-unwind-attributes.rs index c8f9cd943cdaa..681842e30e006 100644 --- a/src/test/ui/feature-gate-unwind-attributes.rs +++ b/src/test/ui/feature-gate-unwind-attributes.rs @@ -8,7 +8,7 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -// compile-flags: -C no-prepopulate-passes +// compile-flags: -C no-prepopulate-passes -Cpasses=name-anon-globals #![crate_type = "lib"]