Skip to content

Fix #9476: Support native TLS on Mac OS X#5570

Merged
yebblies merged 3 commits intodlang:masterfrom
jacob-carlborg:native_tls2
May 5, 2016
Merged

Fix #9476: Support native TLS on Mac OS X#5570
yebblies merged 3 commits intodlang:masterfrom
jacob-carlborg:native_tls2

Conversation

@jacob-carlborg
Copy link
Contributor

Second try at implementing native TLS on OS X. See #5346 for reference.

Runtime part dlang/druntime#1523

@yebblies
Copy link
Contributor

Looks like Martin's saying the druntime pull will break some existing shared library functionality. Can that be fixed?

src/root/port.d Outdated
*
* Returns: a new string which is the concatenation of the two strings
*/
static char* concat(/*const*/ char* str1, uint str1Len, /*const*/ char* str2, uint str2Len)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, just get rid of concat and use OutBuffer.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated to use OutBuffer.

@jacob-carlborg
Copy link
Contributor Author

Looks like Martin's saying the druntime pull will break some existing shared library functionality. Can that be fixed?

Yeah, I don't see how my change will break that. He'll have to give a better explanation.

@jacob-carlborg
Copy link
Contributor Author

Is the FreeBSD test expected to fail sometimes? The test output contains test fails depending on environment.

@jacob-carlborg
Copy link
Contributor Author

I think we cleared up the misunderstanding with the shared library support that Martin was concerned about dlang/druntime#1461 (comment).

@jacob-carlborg
Copy link
Contributor Author

@yebblies @MartinNowak could we try this again? This time all test are passing except for 64bit OS X, which is expected since the druntime part is required as well.

@jacob-carlborg
Copy link
Contributor Author

@yebblies @MartinNowak @WalterBright @9rnsr please review.

@smolt
Copy link
Contributor

smolt commented Apr 18, 2016

@jacob-carlborg - something to be aware of. I ran into a possible bug related to OS X TLS after updating to latest Xcode 7.3. See ldc-developers/ldc#1444. Error is ld: section __DATA/__thread_bss extends beyond end of file. It might be worth waiting a bit until the problem is understood because dmd is immune right now with its own TLS implementation.

@deadalnix - you filed an llvm bug on this I think too. Do you know more on it? My hunch is a linker bug, but I am not sure.

@jacob-carlborg
Copy link
Contributor Author

Hmm, that's unfortunate.

@deadalnix
Copy link
Contributor

@smolt Yes I'm not sure what the problem is. It may well be a binutil problem rather than an llvm one. I'm just not sure.

@smolt
Copy link
Contributor

smolt commented Apr 22, 2016

I was informed my Apple radar bug 25769807 is duplicate of 24667656 which is Closed. I assume that means there will be a fix in an upcoming Xcode release.

@deadalnix
Copy link
Contributor

So this is an XCode bug ?

@smolt
Copy link
Contributor

smolt commented May 1, 2016

Yes, in that Xcode includes entire clang/llvm compile chain. I don't know what specific component has bug, they don't tell me.

@deadalnix
Copy link
Contributor

I bet on the linker.
On May 1, 2016 08:24, "Dan Olson" notifications@github.com wrote:

Yes, in that Xcode includes entire clang/llvm compile chain. I don't know
what specific component has bug, they don't tell me.


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#5570 (comment)

@yebblies yebblies merged commit 58b204f into dlang:master May 5, 2016
@deadalnix
Copy link
Contributor

@yebblies Were you able to solve the linker problem ? Was something needed or just updating the linker did the trick ?

@yebblies
Copy link
Contributor

yebblies commented May 6, 2016

Nope.

@smolt
Copy link
Contributor

smolt commented May 6, 2016

I got an update to Xcode 7.3.1. I'll have to see if the error goes away when I rerun test suite against LDC.

@deadalnix
Copy link
Contributor

OK software update somehow screwed up the update on my mac. I was able to get it back in shape and the bug is gone as well. Thanks everybody.

@nicolasjinchereau
Copy link

Has anyone benchmarked to check relative performance of accessing TLS vars?

@smolt
Copy link
Contributor

smolt commented May 7, 2016

Also confirmed bug is gone Xcode 7.3.1.

@MartinNowak
Copy link
Member

WAT? Please prepare a changelog entry mentioning the minimum required OSX and XCode versions, and add some micro-benchmark numbers along with it.

@deadalnix
Copy link
Contributor

Don't freak out. OSX broke its linker and the fixed it. It doesn't require
a cutting edge version of osx/xcode.
On May 7, 2016 19:22, "Martin Nowak" notifications@github.com wrote:

WAT? Please prepare a changelog entry mentioning the minimum required OSX
and XCode versions, and add some micro-benchmark numbers along with it.


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#5570 (comment)

@MartinNowak
Copy link
Member

Whatever version it is, you have to tell this to people not involved in this PR.
How would they ever find out other than running through the same trouble as you?

@smolt
Copy link
Contributor

smolt commented May 7, 2016

I believe the bug appeared with linker delivered in Xcode 7.3 and then fixed with Xcode 7.3.1. It affected C code too. @jacob-carlborg I think just a warning to avoid from Xcode 7.3 in change log. We should do that in LDC release notes too.

@deadalnix
Copy link
Contributor

By linking any program that has tls greater than some non trivial size, in
D or otherwize.

OSX even has an auto update for the thing (it just somehow failed on my
machine).
On May 7, 2016 19:36, "Martin Nowak" notifications@github.com wrote:

Whatever version it is, you have to tell this to people not involved in
this PR.
How would they ever find out other than running through the same trouble
as you?


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#5570 (comment)

@MartinNowak
Copy link
Member

MartinNowak commented May 8, 2016

Well just add the changelog entry, you should inform people about using native TLS anyhow.
https://github.com/dlang/dmd/blob/master/changelog.dd

@jacob-carlborg jacob-carlborg deleted the native_tls2 branch May 8, 2016 14:43
@jacob-carlborg
Copy link
Contributor Author

@MartinNowak #5740

@jacob-carlborg
Copy link
Contributor Author

@MartinNowak this updates the requirements for OS X as well: dlang/dlang.org#1292.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants