Add emulated TLS support #117873

quininer · 2023-11-13T13:05:43Z

This is a reopen of #96317 . many android devices still only use 128 pthread keys, so using emutls can be helpful.

Currently LLVM uses emutls by default for some targets (such as android, openbsd), but rust does not use it, because has_thread_local is false.

This commit has some changes to allow users to enable emutls:

add -Zhas-thread-local flag to specify that std uses #[thread_local] instead of pthread key.
when using emutls, decorate symbol names to find thread local symbol correctly.
change -Zforce-emulated-tls to -Ztls-model=emulated to explicitly specify whether to generate emutls.

r? @Amanieu

rustbot · 2023-11-13T13:05:54Z

These commits modify compiler targets.
(See the Target Tier Policy.)

quininer · 2023-11-13T14:22:30Z

I noticed that gcc.a with __emutls_get_address is no longer provided after ndk r23 (like libunwind issue #85806 (comment) ), which means that the android target may be forced to link libc++_shared.so.

~~If this is a problem, then we might provide an flag to allow users to enable emutls, or reimplement emutls in Rust.~~

~~Zig has a similar problem, and they implement their emutls. ziglang/zig#7577~~

I realize compiler-builtins has opened emutls.c at rust-lang/compiler-builtins#458, I'll try again soon.

quininer · 2023-11-16T09:16:41Z

I still can't get it to work.

LLVM add __emutls_v. prefix for thread local symbols of using emutls , but rustc's exported_symbols still outputs the original symbol name. a linking error occurs when I try to build std.

  = note: ld.lld: error: version script assignment of 'global' to symbol '_ZN3std4hash6random11RandomState3new4KEYS7__getit5__KEY17h67dba5b589f8f44fE' failed: symbol not defined
          ld.lld: error: version script assignment of 'global' to symbol '_ZN3std4sync4mpmc5waker17current_thread_id5DUMMY7__getit5__KEY17h25d816e212fdf2deE' failed: symbol not defined
          ld.lld: error: version script assignment of 'global' to symbol '_ZN3std4sync4mpmc7context7Context4with7CONTEXT7__getit5__KEY17h471bc5da0b12d731E' failed: symbol not defined

Amanieu · 2023-11-19T10:10:36Z

You'll need to make changes to the symbol handling code in compiler/rustc_codegen_ssa/src/back/symbol_export.rs to add the emutls prefix. I think you can handle this in linking_symbol_name_for_instance_in_crate, but I'm not very familiar with that code.

You'll want to add a field to the target spec to explicitly indicate that a target uses emulated TLS. At the moment I believe this only applies to Android and OpenHarmony (ohos) targets.

quininer · 2023-11-20T06:09:34Z

@Amanieu

I see that force_emulated_tls already exists, so i use that, and if that doesn't make sense i can add another emulated_tls.

I tried modifying linking_symbol_name_for_instance_in_crate but this doesn't work because the inconsistent symbol name in exported_symbols instead of linked_symbols. so i added an exporting_symbol_name_for_instance_in_crate referring to linking_symbol_name_for_instance_in_crate.

Amanieu · 2023-11-20T06:34:51Z

It might be worth renaming that target spec option to use_emulated_tls. It makes more sense than "force".

rustbot · 2023-11-20T08:00:01Z

These commits modify compiler targets.
(See the Target Tier Policy.)

Amanieu · 2023-11-23T06:55:04Z

LGTM. Let me quickly double-check with the Android target maintainers: @chriswailes @maurer @mgeisler

maurer · 2023-11-23T18:09:21Z

Android hasn't required emutls since android Q (released in Sept 2019). Using emulated TLS has a number of negative effects around dlclose, speed, sanitizer breakage, etc.

While the upstream clang isn't yet using it by default, we do have several places it is turned on in a regular Android build. We haven't turned it on globally mostly due to legacy vendor toolchains and builds which do not use lld.

I agree that enabling this for targeting SDK revisions before Q could be nice, but I would like an option equivalent to -fno-emulated-lts to allow us to use the ELF TLS provided by bionic. Since Rust doesn't have toolchain/linker diversity like C does, it might be good to just use it when targeting Q+.

I'm also pinging people internally who know more about the status of migrating clang to use -fno-emulated-lts for more information.

tl;dr:

Can we control this with a flag? Even a Z flag would be fine for now. Once we learn to change targets by OS versions we could hook it to SDK version instead, but that hasn't happened yet.
Can we have that flag force native TLS rather than emulated?
If you can wait until next week (we're on vacation for Thanksgiving), I can get clarification from the people behind the document I linked above and make sure everything I'm saying is right.

quininer · 2023-11-24T02:30:47Z

@maurer

I agree with add a Z flag, but I need some guidance on how to implement it. i'm not sure how the Z flag is supposed to affect target.

Enjoy your holiday!

quininer · 2023-11-24T03:34:14Z

Another option is to turn off use_emulated_tls and has_thread_local in android target and merge other parts as is. then users who wish to enable emutls can compile it using their own target.

Amanieu · 2023-11-28T13:20:16Z

For a command-line option, you could extend the existing -Z tls-model option to support emulated TLS as an option. I don't think the standard TLS models apply when using emulated TLS.

rustbot · 2023-11-29T03:23:40Z

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

Some changes occurred in compiler/rustc_codegen_gcc

cc @antoyo

Amanieu · 2023-11-29T03:24:53Z

Looks like a failed rebase?

quininer · 2023-11-29T03:29:03Z

Looks like a failed rebase?

~~I'll solve this after lunch. :D~~ Fixed

quininer · 2023-11-29T03:33:59Z

compiler/rustc_codegen_llvm/src/context.rs

@@ -120,6 +120,7 @@ fn to_llvm_tls_model(tls_model: TlsModel) -> llvm::ThreadLocalMode {
        TlsModel::LocalDynamic => llvm::ThreadLocalMode::LocalDynamic,
        TlsModel::InitialExec => llvm::ThreadLocalMode::InitialExec,
        TlsModel::LocalExec => llvm::ThreadLocalMode::LocalExec,
+        TlsModel::EmulatedTls => llvm::ThreadLocalMode::GeneralDynamic,


I tried mapping to NotThreadLocal, but this causes LLVM to not generate thread local symbol.

quininer · 2023-11-29T04:49:38Z

src/doc/unstable-book/src/compiler-flags/tls-model.md

@@ -20,6 +20,8 @@ loaded at program startup.
 The TLS data must not be in a library loaded after startup (via `dlopen`).
 - `local-exec` - model usable only if the TLS data is defined directly in the executable,
 but not in a shared library, and is accessed only from that executable.
+- `emulated-tls` - Uses thread-specific data keys to implement emulated TLS.


I took this quote from @MaskRay 's post. ❤️

rfcbot · 2023-12-04T19:13:12Z

@thomcc proposal cancelled.

quininer · 2023-12-06T03:19:59Z

Can we go ahead and merge this? :) @Amanieu

MaskRay · 2023-12-06T03:47:10Z

The first comment (description) can use a better description. I was confused even if I have done a lot of clang driver work and know TLS pretty well...

quininer · 2023-12-06T16:20:36Z

I've added a commit message to explain what this commit does.

Currently LLVM uses emutls by default for some targets (such as android, openbsd), but rust does not use it, because `has_thread_local` is false. This commit has some changes to allow users to enable emutls: 1. add `-Zhas-thread-local` flag to specify that std uses `#[thread_local]` instead of pthread key. 2. when using emutls, decorate symbol names to find thread local symbol correctly. 3. change `-Zforce-emulated-tls` to `-Ztls-model=emulated` to explicitly specify whether to generate emutls.

Amanieu · 2023-12-08T23:28:11Z

Can you copy the commit message to the PR description? Github will include the PR description in the merge commit.

r=me once the description is updated.

quininer · 2023-12-09T02:08:46Z

@Amanieu done

Amanieu · 2023-12-09T02:37:35Z

@bors r+

bors · 2023-12-09T02:37:38Z

📌 Commit e5b7689 has been approved by Amanieu

It is now in the queue for this repository.

bors · 2023-12-09T05:32:39Z

⌛ Testing commit e5b7689 with merge 608f324...

bors · 2023-12-09T07:47:11Z

☀️ Test successful - checks-actions
Approved by: Amanieu
Pushing 608f324 to master...

rust-timer · 2023-12-09T09:34:20Z

Finished benchmarking commit (608f324): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	1.4%	[0.4%, 2.9%]	4
Regressions ❌ (secondary)	1.9%	[1.9%, 1.9%]	1
Improvements ✅ (primary)	-1.5%	[-1.5%, -1.5%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.8%	[-1.5%, 2.9%]	5

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 668.465s -> 670.776s (0.35%)
Artifact size: 314.06 MiB -> 314.08 MiB (0.00%)

Add emulated TLS support This is a reopen of rust-lang#96317 . many android devices still only use 128 pthread keys, so using emutls can be helpful. Currently LLVM uses emutls by default for some targets (such as android, openbsd), but rust does not use it, because `has_thread_local` is false. This commit has some changes to allow users to enable emutls: 1. add `-Zhas-thread-local` flag to specify that std uses `#[thread_local]` instead of pthread key. 2. when using emutls, decorate symbol names to find thread local symbol correctly. 3. change `-Zforce-emulated-tls` to `-Ztls-model=emulated` to explicitly specify whether to generate emutls. r? `@Amanieu`

rustbot assigned Amanieu Nov 13, 2023

quininer marked this pull request as draft November 13, 2023 14:29

This comment has been minimized.

Sign in to view

quininer marked this pull request as ready for review November 20, 2023 07:59

quininer changed the title ~~Set has_thread_local=true for android~~ Use emulated TLS for android Nov 23, 2023

quininer force-pushed the android-emutls branch from a9d9cd1 to 5079b3b Compare November 29, 2023 03:23

quininer force-pushed the android-emutls branch from 5079b3b to 9baad2a Compare November 29, 2023 03:31

quininer commented Nov 29, 2023

View reviewed changes

rfcbot removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Dec 4, 2023

quininer force-pushed the android-emutls branch from b906b7d to a754920 Compare December 5, 2023 16:25

This comment has been minimized.

Sign in to view

quininer force-pushed the android-emutls branch from a754920 to 10dfab7 Compare December 5, 2023 16:34

quininer force-pushed the android-emutls branch from 10dfab7 to cc16abe Compare December 6, 2023 16:17

quininer force-pushed the android-emutls branch from cc16abe to e5b7689 Compare December 6, 2023 16:21

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 9, 2023

bors added the merged-by-bors This PR was explicitly merged by bors. label Dec 9, 2023

bors merged commit 608f324 into rust-lang:master Dec 9, 2023
12 checks passed

rustbot added this to the 1.76.0 milestone Dec 9, 2023

bors mentioned this pull request Dec 9, 2023

Unify TLS destructor list implementations #116850

Closed

quininer deleted the android-emutls branch December 9, 2023 08:25

adamreichold mentioned this pull request Dec 15, 2023

How to avoid smuggling of data via TLS and scoped-tls and other means rust-lang/unsafe-code-guidelines#484

Open

mati865 mentioned this pull request Apr 15, 2024

Miri sometimes reports memory leak in thread-local storage on *-pc-windows-gnu #123583

Closed

workingjubilee mentioned this pull request Jun 17, 2024

TLS destructors on the main thread are not always run #28129

Open

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add emulated TLS support #117873

Add emulated TLS support #117873

quininer commented Nov 13, 2023 •

edited

Loading

rustbot commented Nov 13, 2023

quininer commented Nov 13, 2023 •

edited

Loading

quininer commented Nov 16, 2023

Amanieu commented Nov 19, 2023

This comment has been minimized.

quininer commented Nov 20, 2023

This comment has been minimized.

Amanieu commented Nov 20, 2023

This comment has been minimized.

rustbot commented Nov 20, 2023

Amanieu commented Nov 23, 2023

maurer commented Nov 23, 2023 •

edited

Loading

quininer commented Nov 24, 2023

quininer commented Nov 24, 2023

Amanieu commented Nov 28, 2023

rustbot commented Nov 29, 2023

Amanieu commented Nov 29, 2023

quininer commented Nov 29, 2023 •

edited

Loading

quininer Nov 29, 2023

quininer Nov 29, 2023

rfcbot commented Dec 4, 2023

This comment has been minimized.

quininer commented Dec 6, 2023

MaskRay commented Dec 6, 2023

quininer commented Dec 6, 2023

Amanieu commented Dec 8, 2023

quininer commented Dec 9, 2023

Amanieu commented Dec 9, 2023

bors commented Dec 9, 2023

bors commented Dec 9, 2023

bors commented Dec 9, 2023

rust-timer commented Dec 9, 2023

Add emulated TLS support #117873

Add emulated TLS support #117873

Conversation

quininer commented Nov 13, 2023 • edited Loading

rustbot commented Nov 13, 2023

quininer commented Nov 13, 2023 • edited Loading

quininer commented Nov 16, 2023

Amanieu commented Nov 19, 2023

This comment has been minimized.

quininer commented Nov 20, 2023

This comment has been minimized.

Amanieu commented Nov 20, 2023

This comment has been minimized.

rustbot commented Nov 20, 2023

Amanieu commented Nov 23, 2023

maurer commented Nov 23, 2023 • edited Loading

quininer commented Nov 24, 2023

quininer commented Nov 24, 2023

Amanieu commented Nov 28, 2023

rustbot commented Nov 29, 2023

Amanieu commented Nov 29, 2023

quininer commented Nov 29, 2023 • edited Loading

quininer Nov 29, 2023

Choose a reason for hiding this comment

quininer Nov 29, 2023

Choose a reason for hiding this comment

rfcbot commented Dec 4, 2023

This comment has been minimized.

quininer commented Dec 6, 2023

MaskRay commented Dec 6, 2023

quininer commented Dec 6, 2023

Amanieu commented Dec 8, 2023

quininer commented Dec 9, 2023

Amanieu commented Dec 9, 2023

bors commented Dec 9, 2023

bors commented Dec 9, 2023

bors commented Dec 9, 2023

rust-timer commented Dec 9, 2023

Overall result: no relevant changes - no action needed

Instruction count

Max RSS (memory usage)

Cycles

Binary size

quininer commented Nov 13, 2023 •

edited

Loading

quininer commented Nov 13, 2023 •

edited

Loading

maurer commented Nov 23, 2023 •

edited

Loading

quininer commented Nov 29, 2023 •

edited

Loading