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

Enable emutls by default for android #118613

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

quininer
Copy link
Contributor

@quininer quininer commented Dec 4, 2023

This is a split of #117873

This is an attempt to enable emutls by default in android.

The current unresolved problem is that if use emutls provided by compiler-builtins, then each so will have its own emutls symbol during dynamic linking, causing thread_local of each so to be independent of each other.

One solution to this problem is to link against libc++_shared.so provided by android ndk, but this cause in additional runtime dependencies and looks a bit strange.

@rustbot
Copy link
Collaborator

rustbot commented Dec 4, 2023

r? @cjgillot

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

@rustbot rustbot added O-unix Operating system: Unix-like S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Dec 4, 2023
@thomcc thomcc added the needs-fcp This change is insta-stable, so needs a completed FCP to proceed. label Dec 4, 2023
@bors
Copy link
Contributor

bors commented Dec 9, 2023

☔ The latest upstream changes (presumably #117873) made this pull request unmergeable. Please resolve the merge conflicts.

@cjgillot
Copy link
Contributor

cjgillot commented Dec 9, 2023

r? compiler

@rustbot rustbot assigned TaKO8Ki and unassigned cjgillot Dec 9, 2023
@TaKO8Ki
Copy link
Member

TaKO8Ki commented Dec 11, 2023

@bors r+ ref: #117873 (comment)

@bors
Copy link
Contributor

bors commented Dec 11, 2023

📌 Commit 05e4196 has been approved by TaKO8Ki

It is now in the queue for this repository.

@bors 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 11, 2023
@thomcc
Copy link
Member

thomcc commented Dec 11, 2023

@bors r-

This is the part of the PR that needs careful consideration (and probably FCP!)

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Dec 11, 2023
@TaKO8Ki
Copy link
Member

TaKO8Ki commented Dec 11, 2023

@thomcc I misunderstood. r? @thomcc

@rustbot rustbot assigned thomcc and unassigned TaKO8Ki Dec 11, 2023
@thomcc
Copy link
Member

thomcc commented Dec 11, 2023

(Sorry if r-ing a t-compiler PR was out of line, I figured it happened due to a misunderstanding though). It's still a draft too.

Does this version link against libc++? (And if not, do thread locals work in dylibs)

@Dylan-DPC
Copy link
Member

@quininer @TaKO8Ki what's the status of this?

@quininer
Copy link
Contributor Author

There is currently no progress and someone needs to decide how to provide emutls symbols.

I'm already satisfied with the -Ztls-model=emulated flag, so i'm not actively push for this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-fcp This change is insta-stable, so needs a completed FCP to proceed. O-unix Operating system: Unix-like S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants