[BUG] pthread_create() segfaults when called from a constructor in statically linked executables

[stevenskevin]

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

#include <pthread.h>
#include <unistd.h>

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:

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

#include <pthread.h>
#include <signal.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>

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, :597, :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 appears to be the regression point:

commit 2cb5f7f578ec682c2bf628f29ffe21de8ccbc917
Author: Wei Li <sirius.liwei@huawei.com>
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

• I have checked and the NDK I'm using is currently supported

Affected versions

r29, r27

[enh-google]

yeah, this is pretty much expected for the stated "you can't ensure that the libc constructors are ordered first by the static linker" reason.

you can probably work around this by using the optional integer "priority" argument to push your constructor later: __attribute__((__constructor__(666)) might work?

it's probably a bug that system property lookup crashes when given a null pointer. normally i'd add the missing "if null then abort_with_clear_message()", but in this case it might make sense to just return null: that way any too-early calls will just silently no-op (meaning here you just wouldn't get any logging). but that doesn't seem ideal from the perspective of diagnosing real errors...

i see ScopedTrace::ScopedTrace() has

  // Do not call should_trace if tracing is disabled, the call can crash if
  // done during initialization.

so potentially we could add something there.

but "bad things happen if i start spawning threads from elf constructors in static binaries" does feel borderline "working as intended" --- it's not like there's any test coverage for that, or any likelihood that we'd put much effort into the problem generally...