Add signal-reachability checker and source quality checks (qualityChecks task)

.github/workflows/ci.yml

@@ -156,6 +156,47 @@ jobs:
         run: |
           ./gradlew spotlessCheck --no-daemon --parallel --build-cache --no-watch-fs
 
+  quality-checks:
+    runs-on: ubuntu-22.04
+    needs: check-for-pr
+    if: needs.check-for-pr.outputs.skip != 'true'
+    steps:
+      - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
+        with:
+          fetch-depth: 0
+
+      - name: Setup Java
+        uses: actions/setup-java@b843ebf9b913146b38da2eaa7aeddf76074ae1ad # v6.1.0
+        with:
+          distribution: 'zulu'
+          java-version: '21'
+
+      - name: Setup OS
+        run: |
+          sudo apt-get update
+          sudo apt-get install -y curl zip unzip binutils g++ python3
+
+      - name: Cache Gradle Wrapper Binaries
+        uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0
+        with:
+          path: ~/.gradle/wrapper/dists
+          key: gradle-wrapper-${{ runner.os }}-${{ hashFiles('gradle/wrapper/gradle-wrapper.properties') }}
+          restore-keys: |
+            gradle-wrapper-${{ runner.os }}-
+
+      - name: Cache Gradle User Home
+        uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0
+        with:
+          path: ~/.gradle/caches
+          key: gradle-caches-${{ runner.os }}-${{ hashFiles('**/*.gradle*', '**/gradle-wrapper.properties') }}
+          restore-keys: |
+            gradle-caches-${{ runner.os }}-
+
+      - name: Quality checks
+        run: |
+          # Signal-reachability check (ELF debug objects + fixture self-test) and the utils/check-*.py source checks
+          ./gradlew :ddprof-lib:qualityChecks --no-daemon --build-cache --no-watch-fs
+
   check-javadoc:
     runs-on: ubuntu-22.04
     needs: check-for-pr

ddprof-lib/build.gradle.kts

@@ -183,6 +183,93 @@ afterEvaluate {
   }
 }
 
+val debugObjDir: File = layout.buildDirectory.dir("obj/main/debug").get().asFile
+val signalReachabilityScript: File = rootProject.file("utils/check-signal-reachability.py")
+
+// The reachability checker reads ELF objects only (Linux); Mach-O objects are skipped.
+fun hasElfObjects(dir: File): Boolean {
+  val obj = dir.listFiles { f -> f.name.endsWith(".o") }?.firstOrNull() ?: return false
+  val magic = ByteArray(4)
+  return obj.inputStream().use { it.read(magic) } == 4 &&
+    magic.contentEquals(byteArrayOf(0x7f, 'E'.code.toByte(), 'L'.code.toByte(), 'F'.code.toByte()))
+}
+
+// Static async-signal-safety reachability check over the debug objects; see
+// utils/check-signal-reachability.py. Not wired into check/build: run
+// `./gradlew :ddprof-lib:checkSignalReachability` (fails fast), or via
+// qualityChecks. The checker reads ELF objects only, so the task is skipped
+// when the debug objects are absent or not ELF.
+tasks.register<Exec>("checkSignalReachability") {
+  group = "verification"
+  description = "Checks that no async-signal-unsafe function is reachable from the signal handlers and malloc hooks"
+  // Compile task exists for every active configuration; named lookup fails loudly on rename.
+  dependsOn(tasks.named("compileDebug"))
+
+  commandLine("python3", signalReachabilityScript.absolutePath, "--obj-dir", debugObjDir.absolutePath)
+
+  onlyIf("debug objects are ELF") { hasElfObjects(debugObjDir) }
+}
+
+// Quality checks: the signal-reachability check (ELF hosts only) and its
+// fixture self-test (Linux only), plus the source-level utils/check-*.py
+// scripts, each also runnable on its own. Not wired into check/build; CI runs
+// `./gradlew :ddprof-lib:qualityChecks` in the quality-checks job of
+// .github/workflows/ci.yml. Runs every check, collects every failure, and
+// fails once at the end.
+interface QualityCheckServices {
+  @get:Inject val exec: ExecOperations
+}
+
+tasks.register("qualityChecks") {
+  group = "verification"
+  description = "Runs the signal-reachability check and the source-level quality checks in utils/"
+  // Depends on the debug objects, not on checkSignalReachability: that Exec
+  // task fails fast, which would stop the remaining checks from running.
+  dependsOn(tasks.named("compileDebug"))
+
+  val exec = objects.newInstance<QualityCheckServices>().exec
+  val root = rootProject.projectDir
+  val scripts = listOf(
+    "check-jvm-boundary-includes.py",
+    "check-doc-index.py",
+    "check-sigsetjmp-shape.py",
+    "check-tracked-artifacts.py",
+  ).map { File(root, "utils/$it") }
+
+  doLast {
+    val failed = mutableListOf<String>()
+    // Compile-and-assert the planted-violation fixture first: it pins the
+    // checker's most fragile assumptions (section symbols, ctor aliases, TLS,
+    // std-library attribution) and catches toolchain/objdump drift. The
+    // checker is ELF-only, so the self-test runs on Linux hosts only (macOS
+    // produces Mach-O fixture objects by design).
+    val onLinux = System.getProperty("os.name").lowercase().contains("linux")
+    if (onLinux) {
+      exec.exec {
+        commandLine("python3", signalReachabilityScript.absolutePath, "--self-test")
+        isIgnoreExitValue = true
+      }.exitValue.takeIf { it != 0 }?.let { failed += "self-test (check-signal-reachability.py --self-test)" }
+    }
+    if (hasElfObjects(debugObjDir)) {
+      exec.exec {
+        commandLine("python3", signalReachabilityScript.absolutePath, "--obj-dir", debugObjDir.absolutePath)
+        isIgnoreExitValue = true
+      }.exitValue.takeIf { it != 0 }?.let { failed += "check-signal-reachability.py" }
+    } else {
+      logger.lifecycle("qualityChecks: check-signal-reachability.py skipped (debug objects are not ELF)")
+    }
+    failed += scripts.filter { script ->
+      exec.exec {
+        commandLine("python3", script.absolutePath, "--root", root.absolutePath)
+        isIgnoreExitValue = true
+      }.exitValue != 0
+    }.map { it.name }
+    if (failed.isNotEmpty()) {
+      throw GradleException("quality checks failed: ${failed.joinToString { it }}")
+    }
+  }
+}
+
 // Add runBenchmarks task
 tasks.register("runBenchmarks") {
   dependsOn(":ddprof-lib:benchmarks:runBenchmark")

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

@@ -42,6 +42,7 @@
   X(DICTIONARY_ENDPOINTS_ARENA_WASTE_BYTES, "dictionary_endpoints_arena_waste_bytes") \
   X(DICTIONARY_CONTEXT_ARENA_WASTE_BYTES, "dictionary_context_arena_waste_bytes")     \
   X(DICTIONARY_DRAIN_TIMEOUTS, "dictionary_drain_timeouts")                    \
+  X(REFCOUNT_NESTED_OVERFLOW, "refcount_nested_overflow")                      \
   X(DICTIONARY_ID_CONFLICTS, "dictionary_id_conflicts")                        \
   X(CLASS_MAP_AT_CAPACITY, "class_map_at_capacity_drops")                      \
   X(CONTEXT_STORAGE_BYTES, "context_storage_bytes")                            \

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

@@ -18,11 +18,6 @@
 RefCountSlot RefCountGuard::refcount_slots[RefCountGuard::MAX_THREADS];
 int RefCountGuard::slot_owners[RefCountGuard::MAX_THREADS];
 
-// One-time warning latch: emit at most one Log::warn per process when
-// reentrant nesting exceeds NESTED_DEPTH and the scanner can no longer see
-// every nested resource on the slot.
-static std::atomic<bool> s_nested_overflow_warned{false};
-
 int RefCountGuard::getThreadRefCountSlot() {
     ProfiledThread* thrd = ProfiledThread::current();
     int tid = thrd != nullptr ? thrd->tid() : OS::threadId();
@@ -88,12 +83,13 @@ RefCountGuard::RefCountGuard(void* resource) : _active(true), _is_reentrant(fals
             __atomic_store_n(&refcount_slots[_my_slot].nested[idx], resource, __ATOMIC_SEQ_CST);
         } else {
             // Reentrant nesting deeper than NESTED_DEPTH; this guard's resource
-            // is invisible to the scanner.  Latch a single warning per process.
-            bool expected = false;
-            if (s_nested_overflow_warned.compare_exchange_strong(expected, true, std::memory_order_relaxed)) {
-                Log::warn("RefCountGuard reentrancy depth %u exceeds NESTED_DEPTH=%d; scanner may miss nested resources",
-                          static_cast<unsigned>(prev_count) + 1, NESTED_DEPTH);
-            }
+            // is invisible to the scanner.  Counted, not logged: the guard
+            // constructor runs in signal handlers, where stdio is not
+            // async-signal-safe (PROF-16314).  The counter is written to
+            // each JFR chunk as a datadog.ProfilerCounter event and exposed via
+            // JavaProfiler.getDebugCounters(); it advances once per overflowing
+            // guard.
+            Counters::increment(REFCOUNT_NESTED_OVERFLOW, 1);
         }
     } else {
         // Non-reentrant (count was 0): store pointer first so the scanner skips

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

@@ -34,8 +34,10 @@
  * it no matter how many nested guards start or end during the scan.  (When a
  * nested guard moved the outer resource out of active_ptr and back, a scan
  * could read each location while the resource was in the other one.)
- * nested is sized to NESTED_DEPTH; deeper nesting emits a one-time warning and
- * that guard's resource is invisible to the scanner (rare: it requires
+ * nested is sized to NESTED_DEPTH; deeper nesting increments the
+ * REFCOUNT_NESTED_OVERFLOW counter (async-signal-safe; the guard constructor
+ * runs in signal handlers, so it must not log) and that guard's resource is
+ * invisible to the scanner (rare: it requires
  * NESTED_DEPTH+1 nested signal deliveries on the same thread).  The unrecorded
  * guard is the innermost one, the handler running at that moment; recording
  * it by overwriting an entry would move a protected resource again.

doc/architecture/JvmSupportLayer.md

@@ -235,7 +235,7 @@ checked on a Zing JVM. Zing is also currently disabled in the CI JDK matrix
 
 ## Known boundary exceptions
 
-In these shared files, outside the abstraction files, `grep '#include "(hotspot|j9|zing)/'`
+In these shared files, outside the abstraction files, `grep -E '#include ["<](hotspot|j9|zing)/'`
 finds direct includes of implementation headers:
 
 | File | Includes |

doc/architecture/RefCountGuard.md

@@ -98,7 +98,7 @@ sequenceDiagram
     alt "prev_count - 1 fits in NESTED_DEPTH"
         Inner->>Slot: "store nested[prev_count - 1] := resource (seq_cst)"
     else "depth exceeds NESTED_DEPTH"
-        Inner->>Slot: "Log warn once per process; this resource invisible to scanner"
+        Inner->>Slot: "increment REFCOUNT_NESTED_OVERFLOW counter; this resource invisible to scanner"
     end
 ```
 
@@ -107,15 +107,25 @@ resource.  After construction the slot contains:
 - `count == prev_count + 1`
 - `active_ptr == ` the root guard's resource
 - `nested[0..min(prev_count, NESTED_DEPTH)-1]` holding the nested guards'
-  resources; a guard nested deeper than that is not recorded and triggers the
-  one-time overflow warning latched by `s_nested_overflow_warned`.  That
-  unrecorded guard is the innermost one - the handler running at that moment -
+  resources; a guard nested deeper than that is not recorded and increments
+  the `REFCOUNT_NESTED_OVERFLOW` counter (`refcount_nested_overflow`) once per
+  overflowing guard.  It is a counter rather than a log line because the
+  constructor runs in signal handlers, where logging is not
+  async-signal-safe.  That unrecorded guard is the innermost one - the handler
+  running at that moment -
   whereas the earlier displacing protocol lost track of a suspended outer one.
   Either way a drain can then free a resource the thread uses once it
   continues, and it takes `NESTED_DEPTH + 1` nested signal deliveries on one
   thread.  Recording the innermost guard by overwriting an entry would move a
   protected resource again, which is what this protocol avoids.
 
+The overflow counter is written with the other profiler counters as a
+`datadog.ProfilerCounter` event in every JFR chunk
+(`Recording::writeCounters()`), and is readable from Java through
+`JavaProfiler.getDebugCounters()` under `refcount_nested_overflow`.
+`Profiler::resetRecordingState()` zeroes it (`Counters::reset()`), so it counts
+overflows per recording.
+
 The `nested[]` store is `SEQ_CST`.  The caller re-checks its resource right
 after constructing the guard (`CallTraceStorage::put()` reloads
 `_active_storage`, `StringDictionary` reloads `_accepting`), while a drainer
@@ -231,13 +241,14 @@ that drain, with no waiting.
 | "Every resource on a reentrant chain is visible to the scanner up to NESTED_DEPTH" | `nested[prev_count-1]` write in ctor; conditional clear in dtor |
 | "No deadlock between drain and signal handler" | All scans are bounded (100 spins, then up to 5000 × 100 µs sleeps); a timeout is reported to the caller, and `waitForRefCountToClear()` also counts, logs and (DEBUG, non-gtest) aborts |
 | "Slot can be reclaimed after drain returns" | `slot_owners[i] = 0` is written only by the non-reentrant teardown path — destructor or move-assignment overwriting an active guard — and only after `count` has been decremented to 0 |
-| "Nesting beyond NESTED_DEPTH is observable" | One-time `Log::warn` latched via `s_nested_overflow_warned` |
+| "Nesting beyond NESTED_DEPTH is observable" | `REFCOUNT_NESTED_OVERFLOW` counter incremented by each overflowing guard; reported as a `datadog.ProfilerCounter` JFR event and via `JavaProfiler.getDebugCounters()` |
 
 ---
 
 ## Files
 
 - `ddprof-lib/src/main/cpp/refCountGuard.h` — class declaration, `RefCountSlot` layout, `NESTED_DEPTH`.
-- `ddprof-lib/src/main/cpp/refCountGuard.cpp` — implementation, drain loop, overflow warning.
+- `ddprof-lib/src/main/cpp/refCountGuard.cpp` — implementation, drain loop, nested-overflow counter.
+- `ddprof-lib/src/main/cpp/counters.h` — `REFCOUNT_NESTED_OVERFLOW` counter definition.
 - `ddprof-lib/src/main/cpp/stringDictionary.h` — primary user; see [StringDictionary](StringDictionary.md).
 - `ddprof-lib/src/main/cpp/callTraceStorage.cpp`, `callTraceHashTable.cpp` — secondary user; see [CallTraceStorage](CallTraceStorage.md).

utils/README.md

@@ -182,6 +182,36 @@ Examples:
 
 Single-value commands run one container test configuration immediately. When any dimension expands to multiple cells, the script prints a compact status table first; interactive terminals ask for confirmation, while non-interactive runs require `--run` to execute. Matrix execution prints the status table again after all cells finish and writes summaries to `build/reports/container-matrix/summary.md` and `build/reports/container-matrix/summary.json`. Matrix gtest runs require a short `--gtest-task` name so the task follows each cell's configuration. Cells not run because of `--fail-fast` are reported as cancelled separately from unsupported cells that are skipped.
 
+### `check-signal-reachability.py`
+
+Static async-signal-safety check. Builds a call graph from the relocations of the debug object files (`objdump -dr`) and reports every function reachable from a signal handler or malloc hook that calls an unsafe symbol (heap allocation, locks, stdio, dynamic-loader calls, dynamic TLS, logging). The entry points, unsafe symbols and known findings are data lists at the top of the script. ELF objects only (Linux x64/aarch64); needs GNU `objdump` and `c++filt`.
+
+**Usage:**
+```bash
+./utils/check-signal-reachability.py --obj-dir ddprof-lib/build/obj/main/debug [--allowlist <file>]
+./utils/check-signal-reachability.py --self-test   # planted violations in utils/tests/signal-reachability-fixture.cpp
+./gradlew :ddprof-lib:checkSignalReachability      # opt-in; SKIPPED when the debug objects are not ELF
+```
+
+Exit codes: `0` no findings outside the allowlist (or self-test passed), `1` new findings (one `<object>:<caller> -> <unsafe symbol> (via <path>)` line each), `2` usage/environment error. `--allowlist` replaces the built-in known findings; one entry per line: `<caller>[|<caller>...] -> <unsafe symbol>  # <JIRA-KEY>`.
+
+### Source quality checks
+
+Standalone, stdlib-only checks over the git-tracked files. Each prints one `<path>[:<line>]: <problem>` line per violation and exits `0` clean, `1` violations, `2` environment error; `--root` selects the checkout (default: the one containing the script).
+
+| Script | Checks |
+|---|---|
+| `check-jvm-boundary-includes.py` | Shared code in `ddprof-lib/src/main/cpp` does not `#include "hotspot/"`, `"j9/"` or `"zing/"` headers (quoted or angle-bracket form), except the boundary files and the known exceptions (mirrors `doc/architecture/JvmSupportLayer.md#known-boundary-exceptions`). |
+| `check-doc-index.py` | Every `doc/**/*.md` is linked from `doc/README.md` and has a PascalCase file name. |
+| `check-sigsetjmp-shape.py` | Every `sigsetjmp()` has a `JmpCtxScope` in effect, `savemask` 1, and a landing pad that starts with `SIGNAL_HANDLER_UNWIND_AFTER_LONGJMP();` then `<scope>.restore();` (the `guards.h` pattern). The glibc cancellation buffer in `libraryPatcher_linux.cpp` is the one exception. |
+| `check-tracked-artifacts.py` | No tracked path under `docs/sphinx/` or `.sphinx/`, and no `*.scratch*` path. |
+
+**Usage:**
+```bash
+./utils/check-doc-index.py                 # any one check
+./gradlew :ddprof-lib:qualityChecks        # signal-reachability check (+ --self-test on Linux) plus all four; run by CI (ci.yml quality-checks job)
+```
+
 ### `patch-dd-java-agent.sh`
 
 Patches a `dd-java-agent.jar` with a locally-built ddprof library for quick local testing without a full dd-trace-java rebuild.

utils/check-doc-index.py

@@ -0,0 +1,74 @@
+#!/usr/bin/env python3
+# Copyright 2026, Datadog, Inc
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+"""Check that every tracked doc/**/*.md is linked from doc/README.md and named in PascalCase.
+
+doc/README.md itself is the only exception to both rules.
+
+Exit codes: 0 clean, 1 violations, 2 environment error.
+"""
+
+import argparse
+import os
+import posixpath
+import re
+import subprocess
+import sys
+
+INDEX = "doc/README.md"
+_LINK = re.compile(r"\]\((<?[^)\s>]+>?)\)")
+_PASCAL = re.compile(r"^[A-Z][A-Za-z0-9]*\.md$")
+
+
+def main(argv):
+    ap = argparse.ArgumentParser(description=__doc__.split("\n\n")[0])
+    ap.add_argument("--root", default=os.path.dirname(os.path.dirname(os.path.abspath(__file__))),
+                    help="repository root (default: the checkout containing this script)")
+    root = ap.parse_args(argv).root
+
+    try:
+        docs = subprocess.run(["git", "-C", root, "ls-files", "--", "doc/*.md", "doc/**/*.md"],
+                              capture_output=True, text=True, check=True).stdout.splitlines()
+        with open(os.path.join(root, INDEX), encoding="utf-8") as f:
+            index = f.read()
+    except (OSError, subprocess.CalledProcessError) as e:
+        print("error: %s" % e, file=sys.stderr)
+        return 2
+
+    linked = set()
+    for target in _LINK.findall(index):
+        target = target.strip("<>").split("#", 1)[0]
+        if target and "://" not in target:
+            linked.add(posixpath.normpath(posixpath.join("doc", target)))
+
+    errors = []
+    tracked = set(docs)
+    for doc in sorted(tracked - {INDEX}):
+        if doc not in linked:
+            errors.append("%s: not linked from %s" % (doc, INDEX))
+        if not _PASCAL.match(posixpath.basename(doc)):
+            errors.append("%s: file name is not PascalCase (e.g. BuildSystemGuide.md)" % doc)
+    # Reverse direction: every relative index link must resolve to a tracked
+    # file, so renames that leave a dangling link fail here too.
+    for target in sorted(linked):
+        if target.startswith("doc/") and target not in tracked:
+            errors.append("%s: %s links to a file that is not tracked" % (INDEX, target))
+    for e in errors:
+        print(e)
+    print("doc index: %s" % ("%d problem(s)" % len(errors) if errors else "OK (%d docs)" % (len(docs) - 1)))
+    return 1 if errors else 0
+
+
+if __name__ == "__main__":
+    sys.exit(main(sys.argv[1:]))