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

Fix crash on Android due to wrong TLS model #925

Merged
merged 1 commit into from
Nov 16, 2024

Conversation

TroutZhang
Copy link
Contributor

For android ndk, it is always a dynamic .so to let java load.
So "initial-exec" would either not work, or crash for ndk 26+ with minSdk 29+.

check here: https://github.com/android/ndk/wiki/Changelog-r26#changes

Issue 1679: Clang will now automatically enable ELF TLS for minSdkVersion 29 or higher.

@TroutZhang
Copy link
Contributor Author

Here is the PR for upstream: mjansson/rpmalloc#347

@TroutZhang
Copy link
Contributor Author

Fixes #779

@wolfpld
Copy link
Owner

wolfpld commented Nov 15, 2024

How does this interact with previous Android SDKs, where the change was not needed?

@TroutZhang
Copy link
Contributor Author

My colleague uses tracy for profiling on Android, and we recently upgraded ndk from 25c to 27c, today when enabling tracy, the build will crash, hence the fix.

@TroutZhang
Copy link
Contributor Author

How does this interact with previous Android SDKs, where the change was not needed?

It seems previously the emulated TLS doesn't give error at all.

@wolfpld
Copy link
Owner

wolfpld commented Nov 15, 2024

Ah, it shouldn't matter, there are ifdefs against the SDK and NDK version.

@TroutZhang
Copy link
Contributor Author

My local fix is like the below:

#if defined(__ANDROID__) && __ANDROID_API__ >= 29 && defined(__NDK_MAJOR__) && __NDK_MAJOR__ >= 26
    #define TLS_MODEL __attribute__((tls_model("local-dynamic")))
#else
    #define TLS_MODEL
#endif

@TroutZhang
Copy link
Contributor Author

Ah, it shouldn't matter, there are ifdefs against the SDK and NDK version.

Yes, it should be empty, but since the code works just as is, so I didn't change much.

@TroutZhang
Copy link
Contributor Author

Basically, only one of our games, that has minSdk 29, triggered this. Others are fine when ndk 25c => 27c.

So, only ndk 26+ and minSdk 29+ will enable hardware ELF TLS, hence the crash in dlopen from Java's System.loadLibrary

@wolfpld wolfpld merged commit 5120ad8 into wolfpld:master Nov 16, 2024
6 checks passed
@TroutZhang TroutZhang deleted the patch-1 branch November 17, 2024 08:03
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.

2 participants