From 47498dea0934f481dcca00c9b19a9bd9b099a408 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Wed, 15 Nov 2017 05:16:23 -0800 Subject: [PATCH 1/2] rustbuild: Update LLVM and enable ThinLTO This commit updates LLVM to fix #45511 (https://reviews.llvm.org/D39981) and also reenables ThinLTO for libtest now that we shouldn't hit #45768. This also opportunistically enables ThinLTO for libstd which was previously blocked (#45661) on test failures related to debuginfo with a presumed cause of #45511. Closes #45511 --- src/bootstrap/builder.rs | 4 +--- src/libcompiler_builtins | 2 +- src/llvm | 2 +- src/test/run-make/sanitizer-leak/Makefile | 11 ++++++++++- 4 files changed, 13 insertions(+), 6 deletions(-) diff --git a/src/bootstrap/builder.rs b/src/bootstrap/builder.rs index b202df76f7c85..888aa4449f8ee 100644 --- a/src/bootstrap/builder.rs +++ b/src/bootstrap/builder.rs @@ -624,9 +624,7 @@ impl<'a> Builder<'a> { cargo.arg("--release"); } - if mode != Mode::Libstd && // FIXME(#45320) - mode != Mode::Libtest && // FIXME(#45511) - self.config.rust_codegen_units.is_none() && + if self.config.rust_codegen_units.is_none() && self.build.is_rust_llvm(compiler.host) { cargo.env("RUSTC_THINLTO", "1"); diff --git a/src/libcompiler_builtins b/src/libcompiler_builtins index f5532b22b5d74..02b3734a5ba6d 160000 --- a/src/libcompiler_builtins +++ b/src/libcompiler_builtins @@ -1 +1 @@ -Subproject commit f5532b22b5d741f3ea207b5b07e3e1ca63476f9b +Subproject commit 02b3734a5ba6de984eb5a02c50860cc014e58d56 diff --git a/src/llvm b/src/llvm index 51f104bf1cc6c..e45c75de11484 160000 --- a/src/llvm +++ b/src/llvm @@ -1 +1 @@ -Subproject commit 51f104bf1cc6c3a588a11c90a3b4a4a18ee080ac +Subproject commit e45c75de1148456a9eb1a67c14a66df4dfb50c94 diff --git a/src/test/run-make/sanitizer-leak/Makefile b/src/test/run-make/sanitizer-leak/Makefile index b18dd1d45eda4..fa6f7626fccf7 100644 --- a/src/test/run-make/sanitizer-leak/Makefile +++ b/src/test/run-make/sanitizer-leak/Makefile @@ -1,10 +1,19 @@ -include ../tools.mk +LOG := $(TMPDIR)/log.txt + +# FIXME(#46126) ThinLTO for libstd broke this test +ifeq (1,0) all: ifeq ($(TARGET),x86_64-unknown-linux-gnu) ifdef SANITIZER_SUPPORT $(RUSTC) -C opt-level=1 -g -Z sanitizer=leak -Z print-link-args leak.rs | grep -q librustc_lsan - $(TMPDIR)/leak 2>&1 | grep -q 'detected memory leaks' + $(TMPDIR)/leak 2>&1 | tee $(LOG) + grep -q 'detected memory leaks' $(LOG) +endif endif + +else +all: endif From 95e9609b9dade04590b7f3b9f6c3f7b02d116b3f Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Fri, 24 Nov 2017 09:18:22 -0800 Subject: [PATCH 2/2] std: Flag Windows TLS dtor symbol as #[used] Turns out ThinLTO was internalizing this symbol and eliminating it. Worse yet if you compiled with LTO turns out no TLS destructors would run on Windows! The `#[used]` annotation should be a more bulletproof implementation (in the face of LTO) of preserving this symbol all the way through in LLVM and ensuring it makes it all the way to the linker which will take care of it. --- src/libstd/lib.rs | 1 + src/libstd/sys/windows/thread_local.rs | 3 +- .../run-pass/lto-still-runs-thread-dtors.rs | 41 +++++++++++++++++++ 3 files changed, 44 insertions(+), 1 deletion(-) create mode 100644 src/test/run-pass/lto-still-runs-thread-dtors.rs diff --git a/src/libstd/lib.rs b/src/libstd/lib.rs index ccc89ccdcf4c3..9587bb424bd4e 100644 --- a/src/libstd/lib.rs +++ b/src/libstd/lib.rs @@ -332,6 +332,7 @@ #![feature(doc_spotlight)] #![cfg_attr(test, feature(update_panic_count))] #![cfg_attr(windows, feature(const_atomic_ptr_new))] +#![cfg_attr(windows, feature(used))] #![default_lib_allocator] diff --git a/src/libstd/sys/windows/thread_local.rs b/src/libstd/sys/windows/thread_local.rs index 7ae9ed917bdba..cdad320e12241 100644 --- a/src/libstd/sys/windows/thread_local.rs +++ b/src/libstd/sys/windows/thread_local.rs @@ -200,8 +200,9 @@ unsafe fn register_dtor(key: Key, dtor: Dtor) { // the address of the symbol to ensure it sticks around. #[link_section = ".CRT$XLB"] -#[linkage = "external"] #[allow(dead_code, unused_variables)] +#[used] // we don't want LLVM eliminating this symbol for any reason, and + // when the symbol makes it to the linker the linker will take over pub static p_thread_callback: unsafe extern "system" fn(c::LPVOID, c::DWORD, c::LPVOID) = on_tls_callback; diff --git a/src/test/run-pass/lto-still-runs-thread-dtors.rs b/src/test/run-pass/lto-still-runs-thread-dtors.rs new file mode 100644 index 0000000000000..91fb7aa51d4b0 --- /dev/null +++ b/src/test/run-pass/lto-still-runs-thread-dtors.rs @@ -0,0 +1,41 @@ +// Copyright 2017 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. + +// compile-flags: -C lto +// no-prefer-dynamic +// ignore-emscripten no threads support + +use std::thread; + +static mut HIT: usize = 0; + +thread_local!(static A: Foo = Foo); + +struct Foo; + +impl Drop for Foo { + fn drop(&mut self) { + unsafe { + HIT += 1; + } + } +} + +fn main() { + unsafe { + assert_eq!(HIT, 0); + thread::spawn(|| { + assert_eq!(HIT, 0); + A.with(|_| ()); + assert_eq!(HIT, 0); + }).join().unwrap(); + assert_eq!(HIT, 1); + } +}