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) {
+                    // gtest runs before test (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.h

@@ -157,6 +157,7 @@ class OS {
     static SigAction getSegvChainTarget();
     static SigAction getBusChainTarget();
     static void* getSigactionHook();
+    static void resetSignalHandlersForTesting();
 
     static int getMaxThreadId(int floor) {
         int maxThreadId = getMaxThreadId();

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 (!__atomic_load_n(&_orig_handlers_saved, __ATOMIC_ACQUIRE) && _real_sigaction != nullptr) {
+        _real_sigaction(SIGSEGV, nullptr, &_orig_segv_sigaction);
+        _real_sigaction(SIGBUS, nullptr, &_orig_bus_sigaction);
+        __atomic_store_n(&_orig_handlers_saved, true, __ATOMIC_RELEASE);
+    }
     _protected_segv_handler = segvHandler;
     _protected_bus_handler = busHandler;
 }
@@ -760,7 +773,67 @@ SigAction OS::getBusChainTarget() {
     return __atomic_load_n(&_bus_chain_target, __ATOMIC_ACQUIRE);
 }
 
-// sigaction hook - called via GOT patching to intercept sigaction calls
+// sigaction_hook - intercepts sigaction(2) calls from any library via GOT patching.
+//
+// PROBLEM SOLVED
+// ==============
+// Without interception, a library (e.g. wasmtime) can overwrite our SIGSEGV handler:
+//
+//   Before:   kernel --> our_handler --> JVM_handler
+//   After lib calls sigaction(SIGSEGV, lib_handler, &oldact):
+//             kernel --> lib_handler
+//   lib_handler stores oldact = our_handler as its chain target
+//   => when lib chains on unhandled fault: lib_handler --> our_handler --> lib_handler --> ...
+//      INFINITE LOOP
+//
+// HANDLER CHAIN AFTER SETUP
+// ==========================
+//
+//   protectSignalHandlers()          replaceSigsegvHandler()     LibraryPatcher::patch_sigaction()
+//         |                                  |                            |
+//         v                                  v                            v
+//   save JVM handler               install our_handler         GOT-patch sigaction
+//   into _orig_segv_sigaction      as real OS handler          => all future sigaction()
+//                                                                 calls go through us
+//
+//   Signal delivery chain:
+//
+//     kernel
+//       |
+//       v
+//     our_handler  (installed via replaceSigsegvHandler, never displaced)
+//       |
+//       +-- handled by us? --> done
+//       |
+//       v (not handled)
+//     _segv_chain_target  (lib_handler, set when we intercepted lib's sigaction call)
+//       |
+//       +-- handled by lib? --> done
+//       |
+//       v (lib chains to its saved oldact)
+//     _orig_segv_sigaction  (JVM's original handler, what we returned as oldact to lib)
+//       |
+//       v
+//     JVM handles or terminates
+//
+// INTERCEPTION LOGIC (this function)
+// ===================================
+// Case 1 - Install call [act != nullptr, SA_SIGINFO]:
+//   - Save lib's handler as _segv_chain_target (we'll call it if we can't handle)
+//   - Return _orig_segv_sigaction as oldact (NOT our handler, to break the loop)
+//   - Do NOT actually install lib's handler (keep ours on top)
+//
+// Case 2 - Query-only call [act == nullptr, oldact != nullptr]:
+//   - Return _orig_segv_sigaction as oldact (same reason: lib must not see our handler)
+//   - A lib that queries, stores the result, then uses it as a chain target would
+//     loop if we returned our handler here.
+//
+// Case 3 - 1-arg handler [act != nullptr, no SA_SIGINFO]:
+//   - Pass through: we cannot safely chain 1-arg handlers (different calling convention)
+//
+// Case 4 - Any other signal, or protection not yet active:
+//   - Pass through to real sigaction unchanged.
+//
 static int sigaction_hook(int signum, const struct sigaction* act, struct sigaction* oldact) {
     // _real_sigaction must be resolved before any GOT patching happens
     if (_real_sigaction == nullptr) {
@@ -769,37 +842,53 @@ static int sigaction_hook(int signum, const struct sigaction* act, struct sigact
     }
 
     // If this is SIGSEGV or SIGBUS and we have protected handlers installed,
-    // intercept the call to keep our handler on top
-    if (act != nullptr) {
-        if (signum == SIGSEGV && _protected_segv_handler != nullptr) {
-            // Only intercept SA_SIGINFO handlers (3-arg form) for safe chaining
+    // intercept the call to keep our handler on top.
+    // We intercept both install calls (act != nullptr) and query-only calls (act == nullptr)
+    // to ensure callers always see the JVM's original handler, never ours.
+    // A caller that gets our handler as oldact and later chains to it would cause an
+    // infinite loop: us -> them -> us -> ...
+    if (signum == SIGSEGV && _protected_segv_handler != nullptr) {
+        if (act != nullptr) {
+            // Install call: only intercept SA_SIGINFO handlers (3-arg form) for safe chaining
             if (act->sa_flags & SA_SIGINFO) {
                 SigAction new_handler = act->sa_sigaction;
                 // Don't intercept if it's our own handler being installed
                 if (new_handler != _protected_segv_handler) {
                     // 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 ours, to prevent
+                        // the caller from chaining back to us.
+                        *oldact = _orig_segv_sigaction;
                     }
                     Counters::increment(SIGACTION_INTERCEPTED);
                     // Don't actually install their handler - keep ours on top
                     return 0;
                 }
             }
             // Let 1-arg handlers (without SA_SIGINFO) pass through - we can't safely chain them
-        } else if (signum == SIGBUS && _protected_bus_handler != nullptr) {
+        } else if (oldact != nullptr) {
+            // Query-only call: return the JVM's original handler, not ours.
+            // Same reason: a caller that stores our handler and later chains to it causes loops.
+            *oldact = _orig_segv_sigaction;
+            return 0;
+        }
+    } else if (signum == SIGBUS && _protected_bus_handler != nullptr) {
+        if (act != nullptr) {
             if (act->sa_flags & SA_SIGINFO) {
                 SigAction new_handler = act->sa_sigaction;
                 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);
+                        *oldact = _orig_bus_sigaction;
                     }
                     Counters::increment(SIGACTION_INTERCEPTED);
                     return 0;
                 }
             }
+        } else if (oldact != nullptr) {
+            *oldact = _orig_bus_sigaction;
+            return 0;
         }
     }
 
@@ -811,4 +900,15 @@ void* OS::getSigactionHook() {
     return (void*)sigaction_hook;
 }
 
+void OS::resetSignalHandlersForTesting() {
+    __atomic_store_n(&_orig_handlers_saved, false, __ATOMIC_RELEASE);
+    memset(&_orig_segv_sigaction, 0, sizeof(_orig_segv_sigaction));
+    memset(&_orig_bus_sigaction, 0, sizeof(_orig_bus_sigaction));
+    _protected_segv_handler = nullptr;
+    _protected_bus_handler = nullptr;
+    __atomic_store_n(&_segv_chain_target, (SigAction)nullptr, __ATOMIC_RELEASE);
+    __atomic_store_n(&_bus_chain_target, (SigAction)nullptr, __ATOMIC_RELEASE);
+    // _real_sigaction is intentionally not reset: safe to reuse across tests
+}
+
 #endif // __linux__

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

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

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

@@ -1044,93 +1044,116 @@ 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
 }
 
 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

@@ -193,7 +193,7 @@ class alignas(alignof(SpinLock)) Profiler {
   void lockAll();
   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;