[BUG] pthread_create() segfaults when called from a constructor in statically linked executables
- Dominant language
- No language data
- Stars
- 2.3k
- Forks
- 310
- PR merge metrics
- No merged PRs in 30d
Description
### Description
bionic segfaults when running `pthread_create()` from an ELF static initializer (`__attribute__((constructor))`) in a statically linked binary.
After analyzing the issue (see below), I expect it also affects `pthread_join()` and contended paths in the `pthread_mutex_*` functions. `pthread_create()` appears to be the only one currently reachable from constructors (the others both require other threads to already exist), but the underlying problem is that any libc call using `ScopedTrace` is unsafe in this context.
## Version
I ran the below test case on both the latest r29 stable and the latest r27d LTS (both downloaded today):
```
$ cat "$NDK/source.properties"
Pkg.Desc = Android NDK
Pkg.Revision = 29.0.14206865
Pkg.BaseRevision = 29.0.14206865
Pkg.ReleaseName = r29
```
```
$ cat "$NDK/source.properties"
Pkg.Desc = Android NDK
Pkg.Revision = 27.3.13750724
Pkg.BaseRevision = 27.3.13750724
Pkg.ReleaseName = r27d
```
## Test case
```c
#include
#include
static void *thread_fn(void *arg) {
(void)arg;
return NULL;
}
__attribute__((constructor)) static void ctor(void) {
pthread_t t;
write(1, "ctor: before\n", 13);
pthread_create(&t, NULL, thread_fn, NULL);
write(1, "ctor: after\n", 12);
}
int main(void) {
write(1, "main\n", 5);
return 0;
}
```
Built with `"$NDK/toolchains/llvm/prebuilt/linux-x86_64/bin/aarch64-linux-android30-clang" -static -o ctortest ctortest.c`, and tested on Android 14 in the Cuttlefish emulator (though the exact environment shouldn't matter much due to the static linking).
**Expected:** prints both messages in the constructor, then the one in `main()`, and exits cleanly
**Actual:** prints "`ctor: before`" and then segfaults
## Control cases
**No `-static`**: The same code now works as expected.
**Moving the `pthread_create()` call to `main()`**: Removing the constructor and changing `main` to this:
```c
int main(void) {
pthread_t t;
write(1, "main: before\n", 13);
pthread_create(&t, NULL, thread_fn, NULL);
write(1, "main: after\n", 12);
return 0;
}
```
...also works fine.
## Debugging
I added a sigaction SEGV handler to retrieve the crash address:
Modified test case including the SEGV handler
```c
#include
#include
#include
#include
#include
#include
static void segv(int sig, siginfo_t* info, void* uc_v);
static void *thread_fn(void *arg) {
(void)arg;
return NULL;
}
__attribute__((constructor)) static void ctor(void) {
struct sigaction sa;
memset(&sa, 0, sizeof(sa));
sa.sa_sigaction = segv;
sa.sa_flags = SA_SIGINFO;
sigemptyset(&sa.sa_mask);
if (sigaction(SIGSEGV, &sa, NULL) == -1) {
perror("sigaction");
exit(EXIT_FAILURE);
}
pthread_t t;
write(1, "ctor: before\n", 13);
pthread_create(&t, NULL, thread_fn, NULL);
write(1, "ctor: after\n", 12);
}
static void segv(int sig, siginfo_t* info, void* uc_v) {
ucontext_t* uc = uc_v;
char buf[256];
int n = snprintf(buf, sizeof(buf),
"SEGV addr=%p pc=%p lr=%p | ctor=%p pthread_create=%p thread_fn=%p\n",
info->si_addr, (void*)uc->uc_mcontext.pc,
(void*)uc->uc_mcontext.regs[30],
(void*)&ctor, (void*)&pthread_create, (void*)&thread_fn);
write(1, buf, n);
_exit(139);
}
int main(void) {
write(1, "main\n", 5);
return 0;
}
```
```
SEGV addr=0x0 pc=0x266970 lr=0x23f1c8 | ctor=0x21b18c pthread_create=0x232d60 thread_fn=0x21b2dc
```
Then I used Ghidra to find the PC and LR addresses in the test binary:
- PC landed in `__strchr_aarch64`
- LR landed in `PropertyInfoArea::GetPropertyInfoIndexes()` at its `strchr(name, '.')` call
Finally, I asked Claude Opus 5 to figure out the root cause based on that:
> bionic's `pthread_create` traces itself through a file-scope `CachedProperty` object. That object's C++ constructor is a dynamic initializer in libc's `.init_array`. In a static link, libc's `.init_array` entries are merged with the executable's, and their ordering depends on link order — and in this binary, `ctor` lands ahead of `bionic_systrace.cpp's` initializer, so the executable's constructors run before libc has initialised itself. The object is then still zero-filled BSS, `property_name_` is null, and `__system_property_find(nullptr)` walks down to `strchr(NULL, '.')`. Dynamic links are immune because libc.so's initializers always run at load time, long before the executable's constructors.
It also notes that not just `pthread_create`, but "***any* traced libc call is unsafe from a static executable's constructors.** The same `ScopedTrace` path is used by `pthread_join()` and the contended paths in `pthread_mutex` ([`libc/bionic/pthread_mutex.cpp:190`](https://android.googlesource.com/platform/bionic/+/731631f300090436d7f5df80d50b6275c8c60a93/libc/bionic/pthread_mutex.cpp#190), [`:597`](https://android.googlesource.com/platform/bionic/+/731631f300090436d7f5df80d50b6275c8c60a93/libc/bionic/pthread_mutex.cpp#597), [`:752`](https://android.googlesource.com/platform/bionic/+/731631f300090436d7f5df80d50b6275c8c60a93/libc/bionic/pthread_mutex.cpp#752))."
I can see in Ghidra that the custom "`ctor`" function did indeed happen to come before bionic_systrace.cpp's:
```
__init_array_start
0026c180 a4 11 22 00 00 00 00 00 addr init_have_lse_atomics
0026c188 4c 14 22 00 00 00 00 00 addr __init_cpu_features
0026c190 8c b1 21 00 00 00 00 00 addr ctor
0026c198 48 10 23 00 00 00 00 00 addr _GLOBAL__sub_I_release.cpp
0026c1a0 48 3c 23 00 00 00 00 00 addr _GLOBAL__sub_I_bionic_systrace.cpp
```
## Originating commit
[Commit `2cb5f7f`](https://android.googlesource.com/platform/bionic/+/2cb5f7f578ec682c2bf628f29ffe21de8ccbc917%5E%21/#F0) appears to be the regression point:
```diff
commit 2cb5f7f578ec682c2bf628f29ffe21de8ccbc917
Author: Wei Li
Date: Fri Jan 26 15:00:32 2018 +0800
Move static variable out of should_trace().
[...]
diff --git a/libc/bionic/bionic_systrace.cpp b/libc/bionic/bionic_systrace.cpp
index 970a92ba1..bac3d8802 100644
--- a/libc/bionic/bionic_systrace.cpp
+++ b/libc/bionic/bionic_systrace.cpp
@@ -29,12 +29,11 @@
#define WRITE_OFFSET 32
static Lock g_lock;
+static CachedProperty g_debug_atrace_tags_enableflags("debug.atrace.tags.enableflags");
+static uint64_t g_tags;
static int g_trace_marker_fd = -1;
static bool should_trace() {
- static CachedProperty g_debug_atrace_tags_enableflags("debug.atrace.tags.enableflags");
- static uint64_t g_tags;
-
g_lock.lock();
if (g_debug_atrace_tags_enableflags.DidChange()) {
g_tags = strtoull(g_debug_atrace_tags_enableflags.Get(), nullptr, 0);
```
That commit was to fix a deadlock, so reverting it wouldn't be the right fix. Claude suggests that "any fix needs to avoid reintroducing the `__cxa_guard_acquire` recursion from 2cb5f7f; constant-initialising the object would be one way."
### I am using a supported NDK
- [x] I have checked and the NDK I'm using is currently supported
### Affected versions
r29, r27
Contributor guide
Research direction
Reproduce the failure with the static constructor test case, then inspect libc/bionic/bionic_systrace.cpp and the referenced pthread_mutex.cpp paths, along with pthread_create() and pthread_join(). Trace how CachedProperty and ScopedTrace are initialized before executable constructors. Done means pthread_create() from a static constructor no longer segfaults and the related traced paths remain safe without reintroducing the reported deadlock.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- android, c, cpp
- Domain
- operating-systems
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100