fix(sigaction): prevent infinite loop in signal handler chaining

build-logic/conventions/src/main/kotlin/com/datadoghq/profiler/ProfilerTestPlugin.kt

@@ -560,21 +560,22 @@ class ProfilerTestPlugin : Plugin<Project> {
             }
         }
 
-        // Wire up assemble/gtest dependencies
+        // Wire up gtest -> test dependencies (C++ tests run before Java tests)
         project.gradle.projectsEvaluated {
             configNames.forEach { cfgName ->
-                val testTask = project.tasks.findByName("test$cfgName")
+                val capitalizedCfgName = cfgName.replaceFirstChar { it.uppercaseChar() }
+                val testTaskName = "test$capitalizedCfgName"
+                val testTask = project.tasks.findByName(testTaskName)
                 val profilerLibProject = project.rootProject.findProject(profilerLibProjectPath)
 
-                if (profilerLibProject != null) {
-                    val assembleTask = profilerLibProject.tasks.findByName("assemble${cfgName.replaceFirstChar { it.uppercaseChar() }}")
-                    if (testTask != null && assembleTask != null) {
-                        assembleTask.dependsOn(testTask)
-                    }
-
-                    val gtestTask = profilerLibProject.tasks.findByName("gtest${cfgName.replaceFirstChar { it.uppercaseChar() }}")
-                    if (testTask != null && gtestTask != null) {
+                if (profilerLibProject != null && testTask != null) {
+                    // Wire test -> gtest dependency (C++ unit tests run before Java integration tests)
+                    val gtestTaskName = "gtest${capitalizedCfgName}"
+                    try {
+                        val gtestTask = profilerLibProject.tasks.named(gtestTaskName)
                         testTask.dependsOn(gtestTask)
+                    } catch (e: org.gradle.api.UnknownTaskException) {
+                        project.logger.info("Task $gtestTaskName not found in $profilerLibProjectPath - gtest may not be available")
                     }
                 }
             }

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

@@ -739,6 +739,11 @@ static SigAction _protected_bus_handler = nullptr;
 static volatile SigAction _segv_chain_target = nullptr;
 static volatile SigAction _bus_chain_target = nullptr;
 
+// Original handlers (JVM's) saved before we install ours - used for oldact in sigaction hook
+static struct sigaction _orig_segv_sigaction;
+static struct sigaction _orig_bus_sigaction;
+static bool _orig_handlers_saved = false;
+
 // Real sigaction function pointer (resolved via dlsym)
 typedef int (*real_sigaction_t)(int, const struct sigaction*, struct sigaction*);
 static real_sigaction_t _real_sigaction = nullptr;
@@ -748,6 +753,14 @@ void OS::protectSignalHandlers(SigAction segvHandler, SigAction busHandler) {
     if (_real_sigaction == nullptr) {
         _real_sigaction = (real_sigaction_t)dlsym(RTLD_DEFAULT, "sigaction");
     }
+    // Save the current (JVM's) signal handlers BEFORE we install ours.
+    // These will be returned as oldact when we intercept other libraries' sigaction calls,
+    // so they chain to JVM instead of back to us (which would cause infinite loops).
+    if (!_orig_handlers_saved && _real_sigaction != nullptr) {
+        _real_sigaction(SIGSEGV, nullptr, &_orig_segv_sigaction);
+        _real_sigaction(SIGBUS, nullptr, &_orig_bus_sigaction);
+        _orig_handlers_saved = true;
+    }
     _protected_segv_handler = segvHandler;
     _protected_bus_handler = busHandler;
 }
@@ -780,7 +793,10 @@ static int sigaction_hook(int signum, const struct sigaction* act, struct sigact
                     // Save their handler as our chain target
                     __atomic_exchange_n(&_segv_chain_target, new_handler, __ATOMIC_ACQ_REL);
                     if (oldact != nullptr) {
-                        _real_sigaction(signum, nullptr, oldact);
+                        // Return the original (JVM's) handler, not our handler.
+                        // This way, when the intercepted library chains to "previous",
+                        // it goes to JVM, not back to us (which would cause infinite loops).
+                        *oldact = _orig_segv_sigaction;
                     }
                     Counters::increment(SIGACTION_INTERCEPTED);
                     // Don't actually install their handler - keep ours on top
@@ -794,7 +810,8 @@ static int sigaction_hook(int signum, const struct sigaction* act, struct sigact
                 if (new_handler != _protected_bus_handler) {
                     __atomic_exchange_n(&_bus_chain_target, new_handler, __ATOMIC_ACQ_REL);
                     if (oldact != nullptr) {
-                        _real_sigaction(signum, nullptr, oldact);
+                        // Return the original (JVM's) handler, not our handler.
+                        *oldact = _orig_bus_sigaction;
                     }
                     Counters::increment(SIGACTION_INTERCEPTED);
                     return 0;

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

@@ -496,4 +496,8 @@ SigAction OS::getBusChainTarget() {
     return nullptr;
 }
 
+void* OS::getSigactionHook() {
+    return nullptr;  // No sigaction interception on macOS
+}
+
 #endif // __APPLE__

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

@@ -51,6 +51,7 @@ static void (*orig_trapHandler)(int signo, siginfo_t *siginfo, void *ucontext);
 static void (*orig_segvHandler)(int signo, siginfo_t *siginfo, void *ucontext);
 static void (*orig_busHandler)(int signo, siginfo_t *siginfo, void *ucontext);
 
+
 static Engine noop_engine;
 static PerfEvents perf_events;
 static WallClockASGCT wall_asgct_engine;
@@ -1044,93 +1045,121 @@ void Profiler::disableEngines() {
 }
 
 void Profiler::segvHandler(int signo, siginfo_t *siginfo, void *ucontext) {
-  if (!crashHandler(signo, siginfo, ucontext)) {
-    // Check dynamic chain target first (set by intercepted sigaction calls)
-    SigAction chain = OS::getSegvChainTarget();
-    if (chain != nullptr) {
-      chain(signo, siginfo, ucontext);
-    } else if (orig_segvHandler != nullptr) {
-      orig_segvHandler(signo, siginfo, ucontext);
-    }
+  if (crashHandlerInternal(signo, siginfo, ucontext)) {
+    return;  // Handled
+  }
+  // Not handled, chain to next handler
+  SigAction chain = OS::getSegvChainTarget();
+  if (chain != nullptr) {
+    chain(signo, siginfo, ucontext);
+  } else if (orig_segvHandler != nullptr) {
+    orig_segvHandler(signo, siginfo, ucontext);
   }
 }
 
 void Profiler::busHandler(int signo, siginfo_t *siginfo, void *ucontext) {
-  if (!crashHandler(signo, siginfo, ucontext)) {
-    // Check dynamic chain target first (set by intercepted sigaction calls)
-    SigAction chain = OS::getBusChainTarget();
-    if (chain != nullptr) {
-      chain(signo, siginfo, ucontext);
-    } else if (orig_busHandler != nullptr) {
-      orig_busHandler(signo, siginfo, ucontext);
-    }
+  if (crashHandlerInternal(signo, siginfo, ucontext)) {
+    return;  // Handled
+  }
+  // Not handled, chain to next handler
+  SigAction chain = OS::getBusChainTarget();
+  if (chain != nullptr) {
+    chain(signo, siginfo, ucontext);
+  } else if (orig_busHandler != nullptr) {
+    orig_busHandler(signo, siginfo, ucontext);
   }
 }
 
-bool Profiler::crashHandler(int signo, siginfo_t *siginfo, void *ucontext) {
+// Returns: 0 = not handled (chain to next handler), non-zero = handled
+int Profiler::crashHandlerInternal(int signo, siginfo_t *siginfo, void *ucontext) {
   ProfiledThread* thrd = ProfiledThread::currentSignalSafe();
-  if (thrd != nullptr && !thrd->enterCrashHandler()) {
-    // we are already in a crash handler; don't recurse!
-    return false;
-  }
 
+  // First, try to handle safefetch - this doesn't need TLS or any protection
+  // because it directly checks the PC and modifies ucontext to skip the fault.
+  // This must be checked first before any reentrancy checks.
   if (SafeAccess::handle_safefetch(signo, ucontext)) {
-    if (thrd != nullptr) {
-      thrd->exitCrashHandler();
+    return 1;  // handled
+  }
+
+  // Reentrancy protection: use TLS-based tracking if available.
+  // If TLS is not available, we can only safely handle faults that we can
+  // prove are from our protected code paths (checked via sameStack heuristic
+  // in StackWalker::checkFault). For anything else, we must chain immediately
+  // to avoid claiming faults that aren't ours.
+  bool have_tls_protection = false;
+  if (thrd != nullptr) {
+    if (!thrd->enterCrashHandler()) {
+      // we are already in a crash handler; don't recurse!
+      return 0;  // not handled, safe to chain
     }
-    return true;
+    have_tls_protection = true;
   }
+  // If thrd == nullptr, we proceed but with limited handling capability.
+  // Only StackWalker::checkFault (which has its own sameStack fallback)
+  // and the JDK-8313796 workaround can safely handle faults without TLS.
 
-  uintptr_t fault_address = (uintptr_t)siginfo->si_addr;
   StackFrame frame(ucontext);
   uintptr_t pc = frame.pc();
+
+  uintptr_t fault_address = (uintptr_t)siginfo->si_addr;
   if (pc == fault_address) {
     // it is 'pc' that is causing the fault; can not access it safely
-    if (thrd != nullptr) {
+    if (have_tls_protection) {
       thrd->exitCrashHandler();
     }
-    return false;
+    return 0;  // not handled, safe to chain
   }
 
   if (WX_MEMORY && Trap::isFaultInstruction(pc)) {
-    if (thrd != nullptr) {
+    if (have_tls_protection) {
       thrd->exitCrashHandler();
     }
-    return true;
+    return 1;  // handled
   }
 
   if (VM::isHotspot()) {
     // the following checks require vmstructs and therefore HotSpot
 
+    // StackWalker::checkFault has its own fallback for when TLS is unavailable:
+    // it uses sameStack() heuristic to check if we're in a protected stack walk.
+    // If the fault is from our protected walk, it will longjmp and never return.
+    // If it returns, the fault wasn't from our code.
     StackWalker::checkFault(thrd);
 
     // Workaround for JDK-8313796 if needed. Setting cstack=dwarf also helps
     if (_need_JDK_8313796_workaround &&
         VMStructs::isInterpretedFrameValidFunc((const void *)pc) &&
         frame.skipFaultInstruction()) {
-      if (thrd != nullptr) {
+      if (have_tls_protection) {
         thrd->exitCrashHandler();
       }
-      return true;
+      return 1;  // handled
     }
   }
 
-  if (thrd != nullptr) {
+  if (have_tls_protection) {
     thrd->exitCrashHandler();
   }
-  return false;
+  return 0;  // not handled, safe to chain
+}
+
+// Legacy wrapper for external callers (if any)
+bool Profiler::crashHandler(int signo, siginfo_t *siginfo, void *ucontext) {
+  return crashHandlerInternal(signo, siginfo, ucontext) > 0;
 }
 
 void Profiler::setupSignalHandlers() {
   // Do not re-run the signal setup (run only when VM has not been loaded yet)
   if (__sync_bool_compare_and_swap(&_signals_initialized, false, true)) {
       if (VM::isHotspot() || VM::isOpenJ9()) {
         // HotSpot and J9 tolerate interposed SIGSEGV/SIGBUS handler; other JVMs probably not
+        // IMPORTANT: protectSignalHandlers must be called BEFORE replaceSigsegvHandler so that
+        // the original (JVM's) handlers are saved before we install ours. This way, when we
+        // intercept other libraries' sigaction calls and return oldact, we return the JVM's
+        // handler (not ours), preventing infinite chaining loops.
+        OS::protectSignalHandlers(segvHandler, busHandler);
         orig_segvHandler = OS::replaceSigsegvHandler(segvHandler);
         orig_busHandler = OS::replaceSigbusHandler(busHandler);
-        // Protect our handlers from being overwritten by other libraries (e.g., wasmtime).
-        // Their handlers will be stored as chain targets and called from our handlers.
-        OS::protectSignalHandlers(segvHandler, busHandler);
         // Patch sigaction GOT in libraries with broken signal handlers (already loaded)
         LibraryPatcher::patch_sigaction();
       }

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

@@ -194,6 +194,7 @@ class alignas(alignof(SpinLock)) Profiler {
   void unlockAll();
 
   static bool crashHandler(int signo, siginfo_t *siginfo, void *ucontext);
+  static int crashHandlerInternal(int signo, siginfo_t *siginfo, void *ucontext);
   static void check_JDK_8313796_workaround();
 
   static Profiler *const _instance;