Fix #9476: Support native TLS on Mac OS X

[jacob-carlborg]

This currently only works on 64bit. I created this PR anyway to get some code review for the changes.

For 32bit, there are three options, see discussions as well [1] [2]:

• Drop it completely
• Continue using emulated TLS ✔️
• Implement a working native TLS. I'm suspecting there just some issue with how the instructions are generate for accessing a TLS variable that causes a segmentation fault. Otherwise both 32 and 64bit uses the same implementation

Runtime part: dlang/druntime#1461.

[1] http://forum.dlang.org/thread/n6u3e8$1u0b$1@digitalmars.com
[2] http://forum.dlang.org/thread/8125593F-344A-4A46-BB84-57E251EB9CB6@me.com

[jacob-carlborg]

Why are the FreeBSD and Windows tests looking for lexer.h and parser.h?

[yebblies]

Out of date make rules

[jacob-carlborg]

Ah, but they don't cause the build to fail.

[jacob-carlborg]

Added back emulated TLS for 32bit.

[jacob-carlborg]

ping @WalterBright @yebblies @klickverbot @smolt

[dnadlinger]

Seems like the AA test case still fails on the auto tester – wasn't that the one you asked me about on the LDC mailing list?

[dnadlinger]

Also be aware of ldc-developers/ldc#1252, just in case.

[jacob-carlborg]

Seems like the AA test case still fails on the auto tester – wasn't that the one you asked me about on the LDC mailing list?

Yes. I'll have a look.

Also be aware of ldc-developers/ldc#1252, just in case.

Ok, thanks.

[jacob-carlborg]

@klickverbot I'm pretty sure it's because the druntime PR is not merged. All tests pass for me locally when including the necessary changes for druntime. If I use druntime master I get the same error as the auto tester gets.

[smolt]

@jacob-carlborg I'll take a look this weekend, my week got full already.

[jacob-carlborg]

@smolt thanks, no rush 😃

[dnadlinger]

@yebblies: How can we run two PRs together on the auto-tester again? Was this something special Brad set up for you?

[smolt]

I fetched both dmd and druntime native_tls branches and all is fine here for 64-bit with OS X 10.10.5 and Xcode 7.2. Passes druntime, phobos, and dmd test suite.

The assembly looks correct for OS X thread local access, although I suppose since tests passed, all I did was verify that it is truly using OS X thread local support.

As @klickverbot brought up issue ldc-developers/ldc#1252, I checked into it with this program.

// file tls_faild.d
import core.stdc.stdio;

byte tb = 42;   // __thread_data
real zb16 = void; // __thread_bss 16-byte aligned

void main()
{
    printf("zb16 %p\n", &zb16);
    // zb16 addr is only 8-byte aligned, should be 16-byte
}

dmd tls_faild.d -c
$ otool -l tls_faild.o | sed -n '/__thread/,/align/ p'
  sectname __thread_data
   segname __DATA
      addr 0x00000000000000f0
      size 0x0000000000000001
    offset 1168
     align 2^3 (8)
(snip)
  sectname __thread_bss
   segname __DATA
      addr 0x0000000000000130
      size 0x0000000000000010
    offset 1232
     align 2^4 (16)
$ dmd tls_faild.d
$ ./tls_faild
zb16 0x7ffce9d00088

See how real zb16 in __thread_bss should be 16-byte aligned? But printf of address is only 8-byte aligned. A work around is making __thread_data always 16-byte aligned. I see here you make it 8-byte aligned: https://github.com/D-Programming-Language/dmd/pull/5346/files#diff-5a9efaa0acd4f1f9e19b7c6b3f667170R1921. Making it 16-byte aligned (change 3 to 4) works as long as there is at least one var in __thread_data section. LDC's workaround was a var with 16-byte alignment in druntime, but it doesn't work verbatim. It seems LDC honors align() for non-structs, but dmd doesn't?

[smolt]

I think comment was for 64-bit and can be removed.

[jacob-carlborg]

Yes.

[jacob-carlborg]

See how real zb16 in __thread_bss should be 16-byte aligned? But printf of address is only 8-byte aligned. A work around is making __thread_data always 16-byte aligned.

@smolt not sure I understand. Should I change the alignment of __thread_data to get the correct alignment for __thread_bss?

I looks like Clang is using the alignment of the largest variable in the section, for both __thread_data and __thread_bss. I'm not sure what's correct.

[smolt]

Without the OS X linker bug, doing what clang is doing is best (align section to maximum member alignment). I was being general, saying a workaround for the linker alignment bug is to make sure __thread_data has 16-byte alignment. You have choices of how to implement it in dmd because you are working in the backend too.

For LDC, it was easiest to declare a thread-local variable with 16-byte alignment:

https://github.com/ldc-developers/druntime/blob/ldc/src/rt/sections_ldc.d#L410

although when I tried align like that in dmd, it didn't seem to have an effect.

Whatever the workaround, I have a radar bug filed with Apple, and will update ldc-developers/ldc#1252 when I get feedback so the workaround can be lifted someday.

[jacob-carlborg]

Without the OS X linker bug, doing what clang is doing is best (align section to maximum member alignment). I was being general, saying a workaround for the linker alignment bug is to make sure __thread_data has 16-byte alignment. You have choices of how to implement it in dmd because you are working in the backend too.

Now I'm confused. Should __thread_data, __thread_bss or both have 16-byte alignment?

[smolt]

Sorry for confusion. Section __thread_data needs to be forced to 16-byte alignment (this is the workaround). Section __thread_bss alignment only needs to be its max aligned variable.

[jacob-carlborg]

Section __thread_data needs to be forced to 16-byte alignment

@smolt fixed. Also rebased.

[jacob-carlborg]

@WalterBright ping. @andralex this is a prerequisite for dynamic libraries on OS X which, IIRC, you've been asking for.

[smolt]

@jacob-carlborg looks good. BTW, I located half of the alignment bug in dyld source code. The following allocation of thread local block (buffer) in tlv_allocate_and_initialize_for_key() assumes that start is 16-byte aligned in image. When it isn't, data is shifted.

https://www.opensource.apple.com/source/dyld/dyld-360.18/src/threadLocalVariables.c

    // allocate buffer and fill with template
    void* buffer = malloc(size);
    memcpy(buffer, start, size);

I haven't heard back from Apple yet on bug I filed. I assume they will fix it in ld64

[jacob-carlborg]

ping @WalterBright @9rnsr @yebblies

[nicolasjinchereau]

can we get an ETA for review/merge on this?

[nicolasjinchereau]

@MartinNowak ping

[jacob-carlborg]

@WalterBright @yebblies @9rnsr ping

[jacob-carlborg]

@andralex ping. I would like to add that this is a perquisite for dynamic libraries that you've been asking for.

[yebblies]

Where you have an assert(0); in the c++ code you also need a return NULL or something to avoid spurious warnings.

Apart from that it looks good (from my limited understanding of this part of the compiler), but I think it really needs @WalterBright's review before merging. Let's say if Walter hasn't had a chance to review in the next week I'll merge and see what happens.

[jacob-carlborg]

Where you have an assert(0); in the c++ code you also need a return NULL or something to avoid spurious warnings.

I'll update the code.

Let's say if Walter hasn't had a chance to review in the next week I'll merge and see what happens.

Ok, fair enough. @yebblies have you looked at the runtime part as well? dlang/druntime#1461

[jacob-carlborg]

Updated.

[jacob-carlborg]

@yebblies one week later, I see no indication of any form of new review.

[yebblies]

Ok then. We can always revert if anything goes wrong.

[yebblies]

And there we go, reverted.

[jacob-carlborg]

Do you have any reference to what failed? Is it this: https://auto-tester.puremagic.com/show-run.ghtml?projectid=1&runid=102572&dataid=709075 ?

[jacob-carlborg]

@yebblies (see previous post, explicitly pinging to be sure)

[yebblies]

Yeah