Prevent potential race in thread startup and cleanup dead code (#399)

Author: zhengyu123

ddprof-lib/src/main/cpp/buffers.h

@@ -146,6 +146,7 @@ class Buffer {
       v >>= 7;
     }
     _data[_offset++] = (char)v;
+    assert(_offset < limit());
   }
 
   #ifdef __aarch64__
@@ -170,6 +171,7 @@ class Buffer {
       v >>= 7;
     }
     _data[_offset++] = (char)v;
+    assert(_offset < limit());
   }
 
   // the trickery of RecordingBuffer extending Buffer::_data array may trip off asan
@@ -205,6 +207,7 @@ class Buffer {
   __attribute__((no_sanitize("bounds")))
   #endif
   void putVar32(int offset, u32 v) {
+    assert(offset + 4 < limit());
     _data[offset] = v | 0x80;
     _data[offset + 1] = (v >> 7) | 0x80;
     _data[offset + 2] = (v >> 14) | 0x80;

ddprof-lib/src/main/cpp/guards.h

@@ -125,7 +125,7 @@ class SignalBlocker {
     sigaddset(&prof_signals, SIGPROF);     // Used by ITimer and CTimer
     sigaddset(&prof_signals, SIGVTALRM);   // Used by WallClock
 #ifdef __linux__
-    // Block RT signals used by TLS priming and potentially other profilers (Linux-only)
+    // Block RT signals (Linux-only)
     // This prevents any RT signal handler from interrupting TLS initialization
     for (int sig = SIGRTMIN; sig <= SIGRTMIN + 5 && sig <= SIGRTMAX; sig++) {
       sigaddset(&prof_signals, sig);

ddprof-lib/src/main/cpp/libraryPatcher_linux.cpp

@@ -3,6 +3,7 @@
 #ifdef __linux__
 #include "profiler.h"
 #include "vmStructs.h"
+#include "guards.h"
 
 #include <cassert>
 #include <dlfcn.h>
@@ -31,7 +32,7 @@ void LibraryPatcher::initialize() {
 class RoutineInfo {
 private:
   func_start_routine _routine;
-  void* const _args;
+  void* _args;
 public:
   RoutineInfo(func_start_routine routine, void* args) :
     _routine(routine), _args(args) {
@@ -41,23 +42,40 @@ class RoutineInfo {
     return _routine;
   }
 
-  void* const args() const {
+  void* args() const {
     return _args;
   }
 };
 
+#ifdef __aarch64__
+// Initialize the current thread's TLS with profiling signals blocked.
+// Kept in a separate noinline function so that SignalBlocker's 128-byte
+// sigset_t lives in its own frame and does not trigger stack-protector
+// canary placement in start_routine_wrapper on aarch64.
+__attribute__((noinline))
+static void init_thread_tls() {
+    // Block profiling signals during TLS initialization: TLS init
+    // (pthread_once, pthread_key_create, pthread_setspecific) is not
+    // async-signal-safe. A profiling signal here can corrupt
+    // internal TLS bookkeeping on the stack.
+    SignalBlocker blocker;
+    ProfiledThread::initCurrentThread();
+}
+
 // Wrapper around the real start routine.
 // The wrapper:
 // 1. Register the newly created thread to profiler
 // 2. Call real start routine
 // 3. Unregister the thread from profiler once the routine is completed.
-static void* start_routine_wrapper(void* args) {
+// This version is to workaround a precarious stack guard corruption,
+// which only happens in Linux/musl/aarch64/jdk11
+__attribute__((visibility("hidden")))
+static void* start_routine_wrapper_spec(void* args) {
     RoutineInfo* thr = (RoutineInfo*)args;
     func_start_routine routine = thr->routine();
-    void* const params = thr->args();
+    void* params = thr->args();
     delete thr;
-
-    ProfiledThread::initCurrentThread();
+    init_thread_tls();
     int tid = ProfiledThread::currentTid();
     Profiler::registerThread(tid);
     void* result = routine(params);
@@ -66,12 +84,59 @@ static void* start_routine_wrapper(void* args) {
     return result;
 }
 
+static int pthread_create_hook_spec(pthread_t* thread,
+                          const pthread_attr_t* attr,
+                          func_start_routine start_routine,
+                          void* args) {
+  RoutineInfo* thr = new RoutineInfo(start_routine, args);
+  int ret = pthread_create(thread, attr, start_routine_wrapper_spec, (void*)thr);
+  if (ret != 0) delete thr;
+  return ret;
+}
+
+#endif // __aarch64__
+
+static void thread_cleanup(void* arg) {
+    int tid = *static_cast<int*>(arg);
+    Profiler::unregisterThread(tid);
+    ProfiledThread::release();
+}
+
+// Wrapper around the real start routine.
+// See comments for start_routine_wrapper_spec() for details
+__attribute__((visibility("hidden")))
+static void* start_routine_wrapper(void* args) {
+    RoutineInfo* thr = (RoutineInfo*)args;
+    func_start_routine routine = thr->routine();
+    void* params = thr->args();
+    delete thr;
+    {
+        // Block profiling signals during TLS initialization: TLS init
+        // (pthread_once, pthread_key_create, pthread_setspecific) is not
+        // async-signal-safe. A profiling signal here can corrupt
+        // internal TLS bookkeeping on the stack.
+        SignalBlocker blocker;
+        ProfiledThread::initCurrentThread();
+    }
+    int tid = ProfiledThread::currentTid();
+    Profiler::registerThread(tid);
+    void* result = nullptr;
+    // Handle pthread_exit() bypass - the thread calls pthread_exit()
+    // instead of normal termination
+    pthread_cleanup_push(thread_cleanup, &tid);
+    result = routine(params);
+    pthread_cleanup_pop(1);
+    return result;
+}
+
 static int pthread_create_hook(pthread_t* thread,
                           const pthread_attr_t* attr,
                           func_start_routine start_routine,
                           void* args) {
   RoutineInfo* thr = new RoutineInfo(start_routine, args);
-  return pthread_create(thread, attr, start_routine_wrapper, (void*)thr);
+  int ret = pthread_create(thread, attr, start_routine_wrapper, (void*)thr);
+  if (ret != 0) delete thr;
+  return ret;
 }
 
 void LibraryPatcher::patch_libraries() {
@@ -105,10 +170,18 @@ void LibraryPatcher::patch_library_unlocked(CodeCache* lib) {
     }
   }
   TEST_LOG("Patching: %s", lib->name());
+  void* func = (void*)pthread_create_hook;
+
+#ifdef __aarch64__
+  // Workaround stack guard corruption in Linux/aarch64/musl/jdk11
+  if (VM::isHotspot() && OS::isMusl() && VM::java_version() == 11) {
+    func = (void*)pthread_create_hook_spec;
+  }
+#endif
   _patched_entries[_size]._lib = lib;
   _patched_entries[_size]._location = pthread_create_location;
   _patched_entries[_size]._func = (void*)__atomic_load_n(pthread_create_location, __ATOMIC_RELAXED);
-  __atomic_store_n(pthread_create_location, (void*)pthread_create_hook, __ATOMIC_RELAXED);
+  __atomic_store_n(pthread_create_location, func, __ATOMIC_RELAXED);
   _size++;
 }
 

ddprof-lib/src/main/cpp/os.h

@@ -159,14 +159,6 @@ class OS {
 
     static int truncateFile(int fd);
     static void mallocArenaMax(int arena_max);
-
-    // TLS priming support
-    static bool isTlsPrimingAvailable();
-    static int installTlsPrimeSignalHandler(SigHandler handler, int signal_offset = 4);
-    static void uninstallTlsPrimeSignalHandler(int signal_num);
-    static void enumerateThreadIds(const std::function<void(int)>& callback);
-    static void signalThread(int tid, int signum);
-    static int getThreadCount();
 };
 
 #endif // _OS_H

ddprof-lib/src/main/cpp/os_linux.cpp

@@ -726,91 +726,4 @@ SigAction OS::replaceSigbusHandler(SigAction action) {
     return old_action;
 }
 
-bool OS::isTlsPrimingAvailable() {
-    return true; // TLS priming supported on Linux
-}
-
-int OS::installTlsPrimeSignalHandler(SigHandler handler, int signal_offset) {
-    int signal_num = SIGRTMIN + signal_offset;
-    if (signal_num > SIGRTMAX) {
-        TEST_LOG("No available RT signal for TLS priming: offset %d exceeds range (SIGRTMIN=%d, SIGRTMAX=%d)",
-                 signal_offset, SIGRTMIN, SIGRTMAX);
-        return -1;
-    }
-
-    struct sigaction sa;
-    sa.sa_handler = handler;
-    sigemptyset(&sa.sa_mask);
-    sa.sa_flags = SA_RESTART;
-
-    if (sigaction(signal_num, &sa, NULL) != 0) {
-        TEST_LOG("Failed to install RT signal %d for TLS priming: %s (signal may be in use)",
-                 signal_num, strerror(errno));
-        return -1;
-    }
-
-    TEST_LOG("Successfully installed TLS priming handler on RT signal %d", signal_num);
-    return signal_num;
-}
-
-void OS::uninstallTlsPrimeSignalHandler(int signal_num) {
-    if (signal_num <= 0) {
-        return;
-    }
-
-    struct sigaction sa;
-    sa.sa_handler = SIG_DFL;
-    sigemptyset(&sa.sa_mask);
-    sa.sa_flags = 0;
-
-    if (sigaction(signal_num, &sa, NULL) != 0) {
-        TEST_LOG("Failed to uninstall RT signal %d for TLS priming: %s",
-                 signal_num, strerror(errno));
-    } else {
-        TEST_LOG("Successfully uninstalled TLS priming handler for RT signal %d", signal_num);
-    }
-}
-
-void OS::enumerateThreadIds(const std::function<void(int)>& callback) {
-    DIR* task_dir = opendir("/proc/self/task");
-    if (!task_dir) {
-        TEST_LOG("Failed to open /proc/self/task: %s", strerror(errno));
-        return;
-    }
-
-    struct dirent* entry;
-    while ((entry = readdir(task_dir)) != nullptr) {
-        if (entry->d_name[0] >= '1' && entry->d_name[0] <= '9') {
-            int tid = atoi(entry->d_name);
-            if (tid > 0) {
-                callback(tid);
-            }
-        }
-    }
-    closedir(task_dir);
-}
-
-void OS::signalThread(int tid, int signum) {
-    if (syscall(SYS_tgkill, getpid(), tid, signum) != 0 && errno != ESRCH) {
-        TEST_LOG("Failed to signal thread %d with signal %d: %s", tid, signum, strerror(errno));
-    }
-}
-
-int OS::getThreadCount() {
-    FILE* status = fopen("/proc/self/status", "r");
-    if (!status) {
-        return -1;
-    }
-
-    char line[256];
-    int thread_count = -1;
-    while (fgets(line, sizeof(line), status)) {
-        if (sscanf(line, "Threads:\t%d", &thread_count) == 1) {
-            break;
-        }
-    }
-    fclose(status);
-    return thread_count;
-}
-
 #endif // __linux__

ddprof-lib/src/main/cpp/os_macos.cpp

@@ -484,66 +484,4 @@ SigAction OS::replaceSigsegvHandler(SigAction action) {
     return old_action;
 }
 
-bool OS::isTlsPrimingAvailable() {
-    return false; // TLS priming disabled on macOS
-}
-
-int OS::installTlsPrimeSignalHandler(SigHandler handler, int signal_offset) {
-    return -1; // TLS priming not supported on macOS
-}
-
-void OS::uninstallTlsPrimeSignalHandler(int signal_num) {
-    // No-op on macOS since TLS priming is not supported
-}
-
-void OS::enumerateThreadIds(const std::function<void(int)>& callback) {
-    // Get current task
-    task_t task = mach_task_self();
-
-    // Get thread list
-    thread_act_array_t thread_list;
-    mach_msg_type_number_t thread_count;
-
-    kern_return_t kr = task_threads(task, &thread_list, &thread_count);
-    if (kr != KERN_SUCCESS) {
-        TEST_LOG("Failed to get thread list: %d", kr);
-        return;
-    }
-
-    // Call callback for each thread
-    for (mach_msg_type_number_t i = 0; i < thread_count; i++) {
-        callback(static_cast<int>(thread_list[i]));
-    }
-
-    // Clean up
-    vm_deallocate(task, (vm_address_t)thread_list, thread_count * sizeof(thread_t));
-}
-
-void OS::signalThread(int tid, int signum) {
-    // On macOS, tid is actually a mach thread port
-    thread_t thread = static_cast<thread_t>(tid);
-
-    // Convert mach thread to pthread for signaling
-    // This is a limitation - we can't easily signal arbitrary mach threads
-    // In practice, this is mainly used for TLS priming which is disabled on macOS
-    TEST_LOG("Thread signaling not fully supported on macOS (thread=%d, signal=%d)", tid, signum);
-}
-
-int OS::getThreadCount() {
-    task_t task = mach_task_self();
-    thread_act_array_t thread_list;
-    mach_msg_type_number_t thread_count;
-
-    kern_return_t kr = task_threads(task, &thread_list, &thread_count);
-    if (kr != KERN_SUCCESS) {
-        TEST_LOG("Failed to get thread count: %d", kr);
-        return 0;
-    }
-
-    // Clean up
-    vm_deallocate(task, (vm_address_t)thread_list, thread_count * sizeof(thread_t));
-
-    return static_cast<int>(thread_count);
-}
-
 #endif // __APPLE__