feat(profiler): async-signal-safety sanitizer

.github/dependabot.yml

@@ -12,6 +12,14 @@ updates:
       - "no-release-notes"
       - "no-review"
     ignore:
+      # JUnit 5.10+ dropped Java 8 support; 5.12+ dropped Java 11; 6.x requires Java 17.
+      # CI targets on Java 8 and 11 (HotSpot and J9) run the Gradle test worker on the
+      # test JDK itself (the profiler attaches to its own process), so the JUnit Platform
+      # classes must be loadable by Java 8/11. Keep the entire JUnit stack at 5.9.x / 1.9.x.
+      - dependency-name: "org.junit.jupiter:*"
+        versions: [">=5.10.0"]
+      - dependency-name: "org.junit.platform:*"
+        versions: [">=1.10.0"]
       - dependency-name: "org.junit-pioneer:junit-pioneer"
         versions: [">=2.0.0"]
     groups:

.github/workflows/test_workflow.yml

@@ -164,6 +164,13 @@ jobs:
         with:
           name: (test-reports) test-linux-glibc-amd64 (${{ matrix.java_version }}, ${{ matrix.config }})
           path: test-reports
+      - name: Upload signal-safety violation log
+        uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1
+        if: failure()
+        with:
+          name: signal-safety-violation-glibc-${{ matrix.java_version }}-${{ matrix.config }}-amd64
+          path: /tmp/signal-safety-violation.txt
+          if-no-files-found: ignore
 
   test-linux-musl-amd64:
     needs: [cache-jdks, filter-musl-configs]
@@ -283,6 +290,13 @@ jobs:
         with:
           name: (test-reports) test-linux-musl-amd64 (${{ matrix.java_version }}, ${{ matrix.config }})
           path: test-reports
+      - name: Upload signal-safety violation log
+        uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1
+        if: failure()
+        with:
+          name: signal-safety-violation-musl-${{ matrix.java_version }}-${{ matrix.config }}-amd64
+          path: /tmp/signal-safety-violation.txt
+          if-no-files-found: ignore
 
   test-linux-glibc-aarch64:
     needs: cache-jdks
@@ -443,6 +457,13 @@ jobs:
         with:
           name: (test-reports) test-linux-glibc-aarch64 (${{ matrix.java_version }}, ${{ matrix.config }})
           path: test-reports
+      - name: Upload signal-safety violation log
+        uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1
+        if: failure()
+        with:
+          name: signal-safety-violation-glibc-${{ matrix.java_version }}-${{ matrix.config }}-aarch64
+          path: /tmp/signal-safety-violation.txt
+          if-no-files-found: ignore
 
   test-linux-musl-aarch64:
      needs: [cache-jdks, filter-musl-configs]
@@ -540,3 +561,10 @@ jobs:
          with:
            name: (test-reports) test-linux-musl-aarch64 (${{ matrix.java_version }}, ${{ matrix.config }})
            path: test-reports
+       - name: Upload signal-safety violation log
+         uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1
+         if: failure()
+         with:
+           name: signal-safety-violation-musl-${{ matrix.java_version }}-${{ matrix.config }}-aarch64
+           path: /tmp/signal-safety-violation.txt
+           if-no-files-found: ignore

build-logic/conventions/src/main/kotlin/com/datadoghq/native/gtest/GtestPlugin.kt

@@ -188,12 +188,46 @@ class GtestPlugin : Plugin<Project> {
         val compiler = findCompiler(project)
         val includeFiles = extension.includes.plus(project.files(getGtestIncludes(extension)))
 
-        // Create per-config aggregation task
+        // Create per-config aggregation task (compile + link + run)
         val gtestConfigTask = project.tasks.register("gtest${config.capitalizedName()}") {
             group = "verification"
             description = "Run all Google Tests for the ${config.name} build of the library"
         }
 
+        // Per-config build-only aggregation task (compile + link, no run).
+        // Useful in CI where binaries are executed directly without going
+        // through Gradle's logging infrastructure.
+        val buildGtestConfigTask = project.tasks.register("buildGtest${config.capitalizedName()}") {
+            group = "build"
+            description = "Compile and link all Google Tests for the ${config.name} build (no run)"
+        }
+
+        // Compile all library sources ONCE for this config.  Each test
+        // binary only compiles its own test file and links against these
+        // shared objects, reducing compilations from O(n_tests × n_sources)
+        // to O(n_sources + n_tests).
+        val sharedBuilder = GtestTaskBuilder(project, extension, config)
+            .withCompiler(compiler)
+            .withIncludes(includeFiles)
+            .onlyIfGtest(hasGtest)
+        val sharedCompilerArgs = sharedBuilder.sharedCompilerArgs()
+        val sharedLibCompileTask = project.tasks.register(
+            "compileGtestLibrary${config.capitalizedName()}",
+            com.datadoghq.native.tasks.NativeCompileTask::class.java
+        ) {
+            onlyIf { hasGtest && !sharedBuilder.skipConditions() }
+            group = "build"
+            description = "Compile shared library sources for ${config.name} gtest binaries"
+
+            this.compiler.set(compiler)
+            this.compilerArgs.set(sharedCompilerArgs)
+            sources.from(project.fileTree(extension.mainSourceDir.get()) { include("**/*.cpp") })
+            includes.from(includeFiles)
+            objectFileDir.set(project.file(
+                "${project.layout.buildDirectory.get()}/obj/gtest/${config.name}/lib"
+            ))
+        }
+
         // Discover and create tasks for each test file using builder
         val testDir = extension.testSourceDir.get().asFile
         if (!testDir.exists()) {
@@ -206,11 +240,16 @@ class GtestPlugin : Plugin<Project> {
                 .forTest(testFile)
                 .withCompiler(compiler)
                 .withIncludes(includeFiles)
+                .withSharedLibObjects(sharedLibCompileTask)
                 .onlyIfGtest(hasGtest)
                 .build()
 
             gtestConfigTask.configure { dependsOn(executeTask) }
             gtestAll.configure { dependsOn(executeTask) }
+            // buildGtest depends on the link task, not the run task
+            buildGtestConfigTask.configure {
+                dependsOn("linkGtest${config.capitalizedName()}_${testFile.nameWithoutExtension}")
+            }
         }
     }
 

build-logic/conventions/src/main/kotlin/com/datadoghq/native/gtest/GtestTaskBuilder.kt

@@ -37,6 +37,7 @@ class GtestTaskBuilder(
     private lateinit var compiler: String
     private lateinit var includeFiles: FileCollection
     private var hasGtest: Boolean = true
+    private var sharedLibCompileTask: TaskProvider<NativeCompileTask>? = null
 
     private val configName: String get() = config.capitalizedName()
 
@@ -73,6 +74,23 @@ class GtestTaskBuilder(
         return this
     }
 
+    /**
+     * Provide the shared library compile task whose objects are linked into
+     * every test binary.  Allows the library sources to be compiled once
+     * instead of once per test file.
+     */
+    fun withSharedLibObjects(task: TaskProvider<NativeCompileTask>): GtestTaskBuilder {
+        sharedLibCompileTask = task
+        return this
+    }
+
+    /**
+     * Returns the compiler args used for compiling library and test sources.
+     * Exposed so GtestPlugin can configure the shared library compile task
+     * with identical flags without duplicating the adjustment logic.
+     */
+    fun sharedCompilerArgs(): List<String> = adjustCompilerArgs()
+
     /**
      * Build all tasks (compile, link, execute) and return the execute task provider.
      */
@@ -94,10 +112,16 @@ class GtestTaskBuilder(
             this.compiler.set(this@GtestTaskBuilder.compiler)
             this.compilerArgs.set(compilerArgs)
 
-            sources.from(
-                project.fileTree(extension.mainSourceDir.get()) { include("**/*.cpp") },
-                testFile
-            )
+            // When a shared library compile task is provided, library sources
+            // are compiled once there.  Only compile the test file itself here.
+            if (sharedLibCompileTask != null) {
+                sources.from(testFile)
+            } else {
+                sources.from(
+                    project.fileTree(extension.mainSourceDir.get()) { include("**/*.cpp") },
+                    testFile
+                )
+            }
             includes.from(includeFiles)
             objectFileDir.set(objDir)
         }
@@ -117,6 +141,10 @@ class GtestTaskBuilder(
             linker.set(compiler)
             this.linkerArgs.set(linkerArgs)
             objectFiles.from(project.fileTree(objDir) { include("*.o") })
+            sharedLibCompileTask?.let { sharedTask ->
+                dependsOn(sharedTask)
+                objectFiles.from(sharedTask.map { it.objectFileDir.get().asFileTree.matching { include("*.o") } })
+            }
             outputFile.set(binary)
 
             // Add gtest library paths
@@ -171,7 +199,7 @@ class GtestTaskBuilder(
         }
     }
 
-    private fun skipConditions(): Boolean {
+    fun skipConditions(): Boolean {
         return project.hasProperty("skip-tests") ||
                project.hasProperty("skip-native") ||
                project.hasProperty("skip-gtest")

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

@@ -202,6 +202,7 @@ Error CTimerJvmti::start(Arguments &args) {
 }
 
 void CTimerJvmti::signalHandler(int signo, siginfo_t *siginfo, void *ucontext) {
+  SIGNAL_HANDLER_GUARD();
   if (!OS::shouldProcessSignal(siginfo, SI_TIMER, SignalCookie::cpu())) {
     Counters::increment(CTIMER_SIGNAL_FOREIGN);
     OS::forwardForeignSignal(signo, siginfo, ucontext);
@@ -248,6 +249,7 @@ void CTimerJvmti::signalHandler(int signo, siginfo_t *siginfo, void *ucontext) {
 }
 
 void CTimer::signalHandler(int signo, siginfo_t *siginfo, void *ucontext) {
+  SIGNAL_HANDLER_GUARD();
   // Reject signals that did not originate from our timer_create timers.
   // This guards against Go's process-wide setitimer(ITIMER_PROF) and other
   // foreign SIGPROF sources that would otherwise drive our handler onto

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

@@ -17,6 +17,7 @@
 #include "dictionary.h"
 #include "arch.h"
 #include "counters.h"
+#include "signalSafety.h"
 #include <climits>
 #include <stdlib.h>
 #include <string.h>
@@ -41,6 +42,7 @@ Dictionary::~Dictionary() {
 }
 
 void Dictionary::clear() {
+  DEBUG_ASSERT_NOT_IN_SIGNAL();
   clear(_table, _id);
   memset(_table, 0, sizeof(DictTable));
   _table->base_index = _base_index = 1;
@@ -88,6 +90,15 @@ unsigned int Dictionary::lookup(const char *key, size_t length) {
 
 unsigned int Dictionary::lookup(const char *key, size_t length, bool for_insert,
                                 unsigned int sentinel) {
+  // The insert path mallocs (allocateKey) and may calloc a DictTable —
+  // both AS-unsafe.  Read-only lookups (for_insert == false, used by
+  // check() and bounded_lookup at capacity) only touch already-allocated
+  // memory and are AS-safe.  Assert here rather than in the overloads
+  // so bounded_lookup's runtime-decided for_insert is also covered.
+  if (for_insert) {
+    DEBUG_ASSERT_NOT_IN_SIGNAL();
+  }
+
   DictTable *table = _table;
   unsigned int h = hash(key, length);
 

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

@@ -19,6 +19,7 @@
 #include "jniHelper.h"
 #include "os.h"
 #include "profiler.h"
+#include "signalSafety.h"
 #include "rustDemangler.h"
 #include "safeAccess.h"
 #include "spinLock.h"
@@ -1411,6 +1412,7 @@ void Recording::writeMethods(Buffer *buf, Lookup *lookup) {
 }
 
 void Recording::writeClasses(Buffer *buf, Lookup *lookup) {
+  DEBUG_ASSERT_NOT_IN_SIGNAL();
   std::map<u32, const char *> classes;
   // Hold classMapSharedGuard() for the full function. The const char* pointers
   // stored in classes point into dictionary row storage; clear() frees that
@@ -1720,10 +1722,11 @@ void Recording::recordCpuLoad(Buffer *buf, float proc_user, float proc_system,
 // assumption is that we hold the lock (with lock_index)
 void Recording::addThread(int lock_index, int tid) {
     int active = _active_index.load(std::memory_order_acquire);
-    _thread_ids[lock_index][active].insert(tid);
+    _thread_ids[lock_index][active].insert(tid);  // ThreadIdTable::insert is signal-safe (atomics only)
 }
 
 Error FlightRecorder::start(Arguments &args, bool reset) {
+  DEBUG_ASSERT_NOT_IN_SIGNAL();
   ExclusiveLockGuard locker(&_rec_lock);
   const char *file = args.file();
   if (file == NULL || file[0] == 0) {
@@ -1751,6 +1754,7 @@ Error FlightRecorder::newRecording(bool reset) {
 }
 
 void FlightRecorder::stop() {
+  DEBUG_ASSERT_NOT_IN_SIGNAL();
   ExclusiveLockGuard locker(&_rec_lock);
   Recording* rec = _rec;
   if (rec != nullptr) {
@@ -1761,6 +1765,7 @@ void FlightRecorder::stop() {
 }
 
 Error FlightRecorder::dump(const char *filename, const int length) {
+  DEBUG_ASSERT_NOT_IN_SIGNAL();
   assert(length >= 0);
   ExclusiveLockGuard locker(&_rec_lock);
   Recording* rec = _rec;
@@ -1781,6 +1786,7 @@ Error FlightRecorder::dump(const char *filename, const int length) {
 }
 
 void FlightRecorder::flush() {
+  DEBUG_ASSERT_NOT_IN_SIGNAL();
   ExclusiveLockGuard locker(&_rec_lock);
   Recording* rec = _rec;
   if (rec != nullptr) {
@@ -1845,6 +1851,7 @@ void FlightRecorder::recordQueueTime(int lock_index, int tid,
 void FlightRecorder::recordDatadogSetting(int lock_index, int length,
                                           const char *name, const char *value,
                                           const char *unit) {
+  DEBUG_ASSERT_NOT_IN_SIGNAL();
   OptionalSharedLockGuard locker(&_rec_lock);
   if (locker.ownsLock()) {
     Recording* rec = _rec;
@@ -1856,6 +1863,7 @@ void FlightRecorder::recordDatadogSetting(int lock_index, int length,
 }
 
 void FlightRecorder::recordHeapUsage(int lock_index, long value, bool live) {
+  DEBUG_ASSERT_NOT_IN_SIGNAL();
   OptionalSharedLockGuard locker(&_rec_lock);
   if (locker.ownsLock()) {
     Recording* rec = _rec;

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

@@ -19,6 +19,58 @@
 #include "os.h"
 #include "thread.h"
 
+// Signal-context tracking — backed by ProfiledThread::_signal_depth; see
+// the comment block in guards.h for the rationale (initial-exec TLS was
+// rejected because of the static TLS surplus on Graal).
+
+int getInSignalDepth() {
+    ProfiledThread *pt = ProfiledThread::currentSignalSafe();
+    return pt != nullptr ? static_cast<int>(pt->signalDepth()) : 0;
+}
+
+bool isInTrackedSignalContext() {
+    ProfiledThread *pt = ProfiledThread::currentSignalSafe();
+    // null ProfiledThread = no thread context; the SignalHandlerScope
+    // never ran, so we have no positive evidence of a signal frame.
+    // See header comment for the rationale of returning false here.
+    return pt != nullptr && pt->signalDepth() != 0;
+}
+
+SignalHandlerScope::SignalHandlerScope() : _active(true) {
+    ProfiledThread *pt = ProfiledThread::currentSignalSafe();
+    if (pt != nullptr) {
+        pt->enterSignalScope();
+    } else {
+        // No thread context: nothing to update; mark inactive so destructor
+        // and release() are no-ops.
+        _active = false;
+    }
+}
+
+SignalHandlerScope::~SignalHandlerScope() {
+    if (!_active) return;
+    ProfiledThread *pt = ProfiledThread::currentSignalSafe();
+    if (pt != nullptr) {
+        pt->exitSignalScope();
+    }
+}
+
+void SignalHandlerScope::release() {
+    if (!_active) return;
+    ProfiledThread *pt = ProfiledThread::currentSignalSafe();
+    if (pt != nullptr) {
+        pt->exitSignalScope();
+    }
+    _active = false;
+}
+
+void signalHandlerUnwindAfterLongjmp() {
+    ProfiledThread *pt = ProfiledThread::currentSignalSafe();
+    if (pt != nullptr) {
+        pt->exitSignalScope();
+    }
+}
+
 // Static bitmap storage for fallback cases
 uint64_t CriticalSection::_fallback_bitmap[CriticalSection::FALLBACK_BITMAP_WORDS] = {};