Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

std: Add an option to disable ELF based TLS #25858

Merged
merged 1 commit into from
Jun 1, 2015

Conversation

alexcrichton
Copy link
Member

This commit adds a ./configure option called --disable-elf-tls which disables
ELF based TLS (that which is communicated to LLVM) on platforms which already
support it. OSX 10.6 does not support this form of TLS, and some users of Rust
need to target 10.6 and are unable to do so due to the usage of TLS. The
standard library will continue to use ELF based TLS on OSX by default (as the
officially supported platform is 10.7+), but this adds an option to compile the
standard library in a way that is compatible with 10.6.

Closes #25342

This commit adds a ./configure option called `--disable-elf-tls` which disables
ELF based TLS (that which is communicated to LLVM) on platforms which already
support it. OSX 10.6 does not support this form of TLS, and some users of Rust
need to target 10.6 and are unable to do so due to the usage of TLS. The
standard library will continue to use ELF based TLS on OSX by default (as the
officially supported platform is 10.7+), but this adds an option to compile the
standard library in a way that is compatible with 10.6.
@rust-highfive
Copy link
Collaborator

r? @brson

(rust_highfive has picked a reviewer for you, use r? to override)

@alexcrichton
Copy link
Member Author

cc @rillian, this means that the custom build of Rust that Firefox uses will need to pass --disable-elf-tls to the ./configure script, but other than that everything should work out just fine!

@rillian
Copy link
Contributor

rillian commented May 28, 2015

Thanks, @alexcrichton!

I'm confused by 'ELF based TLS'. Is this the appropriate terminology for the same feature in Mach-O?

@alexcrichton
Copy link
Member Author

Ah yeah maybe it isn't the best name after all, I was going off the LLVM documentation which indicates:

The models correspond to the ELF TLS models; see ELF Handling For Thread-Local Storage for more information on under which circumstances the different models may be used. The target may choose a different TLS model if the specified model is not supported, or if a better choice of model can be made.

So maybe a more appropriate name would be --disable-elf-like-tls :)

@rillian
Copy link
Contributor

rillian commented May 28, 2015

So maybe a more appropriate name would be --disable-elf-like-tls :)

Lol. If the alternative is a pthread_get/setspecific-based implementation, would plain --disable-tls be too confusing? Otherwise --disable-linker-tls? // bikeshed

@rillian
Copy link
Contributor

rillian commented May 29, 2015

This doesn't seem to be sufficient. I built yesterday's rustc master with you pr merged, and the linkage gtest we have in the gecko tree fails with

0:06.36 ld: targeted OS version does not support use of thread local variables in __ZN10sys_common11thread_info14current_thread20h5eac201a4bb7b339c8qE for architecture x86_64
0:06.36 clang: error: linker command failed with exit code 1 (use -v to see invocation)
0:06.36 make[2]: *** [XUL] Error 1

Sounds like std::sys::current_thread(). Is the thread_local! macro change not being propagated properly? Did I mess up my build somehow?

@alexcrichton
Copy link
Member Author

Hm that's surprising, so you basically did this?

  • Checked out the current master of rust-lang/rust
  • Applied this PR
  • Configured with --disable-elf-tls
  • make
  • ./x86_64-apple-darwin/stage2/bin/rustc foo.rs <- and this failed?

@rillian
Copy link
Contributor

rillian commented May 29, 2015

No, that has always worked, I assume because rustc does the linking in that case. The gecko build calls rustc --create-type staticlib foo.rs which has also always worked. The error comes when the gecko build later invokes Apple's ld with MACOSX_DEPLOYMENT_TARGET=10.6 set in the environment to link the resulting libfoo.a into the C++ library which calls it. It recongnizes the TLS segments (or whatever it is in mach-o) and balks.

So:

  • Checked out the current rust-lang/rust master
  • Applied this PR
  • ./configure --prefix=${HOME}/.local/rust-notls --disable-elf-tls
  • make -j8 && make install
  • export PATH=$PATH:${HOME}/.local/rust-notls/bin
  • export DYLD_LIBRARY_PATH=${HOME}/.local/rust-notls
  • Checkout mozilla/gecko-dev master
  • echo --enable-rust >> mozconfig
  • Hack configure do disable the rust+10.6 interlock
  • ./mach build
  • ./mach gtest rust.*

@rillian
Copy link
Contributor

rillian commented May 29, 2015

A much simpler test is https://github.com/rillian/rust-ffi test

$ git clone https://github.com/rillian/rust-ffi
[...]
$ cd rust-ffi/
$ make MACOSX_DEPLOYMENT_TARGET=10.7
rustc --crate-type staticlib test.rs 2> libtest.a.out
c++ -g -Wall -std=c++1y -c TestRust.cpp
c++ -g -Wall -std=c++1y -o test TestRust.o libtest.a -lSystem -lpthread -lc -lm
$ make clean
[...]
$ make MACOSX_DEPLOYMENT_TARGET=10.6
rustc --crate-type staticlib test.rs 2> libtest.a.out
c++ -g -Wall -std=c++1y -c TestRust.cpp
c++ -g -Wall -std=c++1y -o test TestRust.o libtest.a -lSystem -lpthread -lc -lm
ld: targeted OS version does not support use of thread local variables in __ZN6thread7current20h6880357b0f286e7cVxbE for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)
make: *** [test] Error 1

@rillian
Copy link
Contributor

rillian commented May 29, 2015

Well, that was with the rust 1.0.0 compiler. I works with the --disable-elf-tls build, so it's probably me.

@alexcrichton
Copy link
Member Author

export PATH=$PATH:${HOME}/.local/rust-notls/bin

Perhaps you had a rustc which was listed first in your PATH? Are you sure the --disable-elf-tls build was used by Gecko?

@rillian
Copy link
Contributor

rillian commented May 30, 2015

I must have been. I did a clean build of both and it works now. Sorry for the confusion.

Works for me, please merge!

@alexcrichton
Copy link
Member Author

No worries, thanks for the confirmation @rillian!

@brson
Copy link
Contributor

brson commented Jun 1, 2015

@bors r+

@bors
Copy link
Contributor

bors commented Jun 1, 2015

📌 Commit 1b5f9cb has been approved by brson

@bors
Copy link
Contributor

bors commented Jun 1, 2015

⌛ Testing commit 1b5f9cb with merge a49ae5b...

bors added a commit that referenced this pull request Jun 1, 2015
This commit adds a ./configure option called `--disable-elf-tls` which disables
ELF based TLS (that which is communicated to LLVM) on platforms which already
support it. OSX 10.6 does not support this form of TLS, and some users of Rust
need to target 10.6 and are unable to do so due to the usage of TLS. The
standard library will continue to use ELF based TLS on OSX by default (as the
officially supported platform is 10.7+), but this adds an option to compile the
standard library in a way that is compatible with 10.6.

Closes #25342
@alexcrichton
Copy link
Member Author

I think I borked my branch here, but all tests passed so merging manually.

@alexcrichton alexcrichton merged commit 1b5f9cb into rust-lang:master Jun 1, 2015
@alexcrichton alexcrichton deleted the disable-os-tls branch June 1, 2015 22:54
@rillian
Copy link
Contributor

rillian commented Jun 15, 2015

@alexcrichton What do you think about backporting this to 1.1?

@alexcrichton
Copy link
Member Author

@rillian I figured you guys were building from source anyway, so would building from the 1.1 source be better than building from the master source? I don't think we'll be shipping a binary build of this unfortunately regardless.

@rillian
Copy link
Contributor

rillian commented Jun 15, 2015

We could build from stable release source sooner, that's all.

@alexcrichton alexcrichton added beta-nominated Nominated for backporting to the compiler in the beta channel. T-tools labels Jun 15, 2015
@alexcrichton
Copy link
Member Author

Eh, may as well nominate and see what happens!

@alexcrichton
Copy link
Member Author

Discussed with @brson and we decided to accept for backporting.

@alexcrichton alexcrichton added the beta-accepted Accepted for backporting to the compiler in the beta channel. label Jun 17, 2015
@alexcrichton alexcrichton removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Jun 17, 2015
@rillian
Copy link
Contributor

rillian commented Jun 17, 2015

Lovely, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
beta-accepted Accepted for backporting to the compiler in the beta channel.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The Rust stdlib can't target osx <=10.6 due to TLS issues
5 participants