Do not allow LLVM to increase a TLS's alignment on macOS.

[kennytm]

This addresses the various TLS segfault on macOS 10.10.

Fix #51794.
Fix #51758.
Fix #50867.
Fix #48866.
Fix #46355.
Fix #44056.

[kennytm]

@bors try

[bors]

⌛ Trying commit f2422bd with merge 77a8ed9...

[bors]

☀️ Test successful - status-travis
State: approved= try=True

[kennytm]

@bors try

[bors]

⌛ Trying commit 8c6328c278eb5ccb63a5606856d3cf547866be2f with merge 21c06fce0627d729df71d090dc65c60826f04b65...

[bors]

💔 Test failed - status-travis

[kennytm]

@bors try

[bors]

⌛ Trying commit a075ac2631d37e74d62eea73c7480d836c5c765b with merge 7eed9fcb944a0205e2c38dd96f19918edf90747c...

[bors]

☀️ Test successful - status-travis
State: approved= try=True

[alexcrichton]

Would it be possible to localize this fix to TLS? Something like swap_nonoverlapping_bytes can come up in a lot of performance-sensitive situations so usage of asm! may cause a large-ish performance hit.

If possible it'd be great here to only fix the TLS side of things and perhaps have extra code on OSX that allocates more space (maybe like via a union?) and manually aligns things instead of relying on dyld to align things

[hanna-kruppe]

alternatively, if there's an llvm option to disable the "bump alignment of globals if it would be beneficial" transformation, that would be simpler and more reliable. I haven't checked, if there's no option for it already then writing that patch and getting it accepted and backporting it probably isn't worthwhile.

[kennytm]

I'd love to have a TLS-only solution, but I don't know how 😄. But I've verified that this does solve the various crashes, so consider this a backup plan.

---

For self reference: minimal repro for the forced 32-byte alignment problem:

#![crate_type = "lib"]
#![feature(stdsimd)]

use std::{ptr, simd};

static mut STATIC_VAR: [u8; 32] = [0; 32];

pub unsafe fn f(x: &mut [u8; 32]) {
    let t = simd::u8x32::load_unaligned_unchecked(x);
    ptr::copy_nonoverlapping(STATIC_VAR.as_ptr(), x.as_mut_ptr(), x.len());
    t.store_unaligned_unchecked(&mut STATIC_VAR);
}

Changing STATIC_VAR to pub would make the alignment 1-byte, so there must be a trigger somewhere to control alignment increase.

[kennytm]

@eddyb @alexcrichton @rkruppe Found the place where the alignment increase can be inhibited (well it's pretty obvious afterthought)

https://github.com/rust-lang/llvm/blob/7a5f6ca55e7a72dd483b39289b9a88eaaca4b512/lib/IR/Globals.cpp#L215-L254

The function is already specialized for ELF, so it should be trivial to specialize it for Mach-O TLS.

diff --git a/lib/IR/Globals.cpp b/lib/IR/Globals.cpp
index da1b6c5e0c9..a742c4af4fd 100644
--- a/lib/IR/Globals.cpp
+++ b/lib/IR/Globals.cpp
@@ -250,6 +250,19 @@ bool GlobalValue::canIncreaseAlignment() const {
   if (isELF && hasDefaultVisibility() && !hasLocalLinkage())
     return false;
 
+  // The dynamic linker on macOS 10.10 has a bug (radar 24221680):
+  // it does not respect the alignment given on the TLS section.
+  // It is thus unsafe to increase the alignment since the runtime
+  // would violate the assumption and may cause segmentation fault.
+  //
+  // See also:
+  //  - <https://github.com/ldc-developers/ldc/issues/1252>
+  //  - <https://github.com/rust-lang/rust/issues/44056>
+  bool isMachO =
+      (!Parent || Triple(Parent->getTargetTriple()).isOSBinFormatMachO());
+  if (isMachO && isThreadLocal())
+    return false;
+
   return true;
 }

How should we proceed?

(An alternate solution without modifying LLVM would be (a) always set a #[link_section] for TLS, or (b) make the TLS not "isStrongDefinitionForLinker")

[alexcrichton]

@kennytm nice find! Perhaps we could try shoving everything into something like a __rustc,__tls section on OSX-only and see if that fixes the issue?

[kennytm]

@alexcrichton The section name must be __DATA,__thread_data or __DATA,__thread_bss (for zero initializer), otherwise we'll get a linker error.

The linker error

error: linking with `cc` failed: exit code: 1
  |
  = note: "cc" "-m64" "-L" <snip>
  = note: 0  0x10580a2c0  __assert_rtn + 129
          1  0x1058d1eb3  ld::passes::tlvp::doPass(Options const&, ld::Internal&) + 1987
          2  0x10580b16b  main + 913
          A linker snapshot was created at:
              /tmp/1-2018-05-28-205411.ld-snapshot
          ld: Assertion failed: (0 && "wrong content type for target in tlv defs"), function doPass, file /Library/Caches/com.apple.xbs/Sources/ld64/ld64-351.8/src/ld/passes/tlvp.cpp, line 293.
          clang: error: linker command failed with exit code 1 (use -v to see invocation)

Explicitly setting the section name to "__DATA,__thread_data" in rustc_codegen_llvm::consts::codegen_static is a valid solution, but this would bloat code like

#[thread_local]
static mut ZERO_ARRAY: [u32; 250_000] = [0; 250_000];
// adds 1 MB to the compiled output unless placed into __DATA,__thread_bss

These TLS should be placed in __DATA,__thread_bss, but we need to know in codegen-llvm whether the RHS is zero or not (can we?).

[oli-obk]

we need to know in codegen-llvm whether the RHS is zero or not (can we?).

That can be detected easily (the miri allocations are accessible and you can access individual bytes), and we should probably be doing this in general

[bors]

⌛ Testing commit e3d113e with merge 96b4733...

[bors]

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing 96b4733 to master...

[pietroalbini]

We might want to backport this into beta.

[Mark-Simulacrum]

I've nominated for @rust-lang/compiler to discuss this for beta and stable backport acceptance (note that we now have a companion tag, stable-nominated).

[Mark-Simulacrum]

This is not a clean stable backport and the changes are not trivial AFAICT.

[nikomatsakis]

We decided that we would backport to beta but not stable -- the stable backport is rather delicate.

[RalfJung]

FWIW, this introduced an extremely subtle bug: even if all the "bytes" are 0, there might still be pointers in this allocation and then the 0 just means the offset is 0, but a relocation needs to be emitted.

See #62655 (comment).

[bluss]

I was wondering if this change means that repr(align(32)) is not respected in TLS? https://users.rust-lang.org/t/elusive-alignment-in-thread-local-issue-macos-only/67307 (Ok, investigating LLVM IR says that alignment is respected wherever I read, so it's still an elusive issue..)

[khvzak]

Just want to mention that (this PR)? apparently causes issues with mlua on macOS using LuaJIT backend.
See LuaJIT/LuaJIT#1099

What is weird, everything was working fine on Rust < 1.72. Probably at this stage I'll just exclude test that causes problems on macos.