From 8f686659f8587f68b440db69d5516e8fd94519ca Mon Sep 17 00:00:00 2001 From: Jaroslav Bachorik Date: Fri, 23 Oct 2026 16:24:30 +0200 Subject: [PATCH 1/6] Add static signal-reachability checker for native objects utils/check-signal-reachability.py builds a call graph from the relocations of ELF object files (objdump -dr), walks it from the signal handlers and malloc hooks, and reports every reachable function that calls an async-signal-unsafe symbol (heap, locks, stdio, dynamic loader, dynamic TLS, logging). Calls through section symbols are resolved to the local function they target, and edges are attributed to constructor and destructor aliases. Entry points, unsafe symbols and the known findings (PROF-16313, PROF-16314) are data lists at the top of the script; --allowlist replaces the known findings from a file. --self-test compiles utils/tests/signal-reachability-fixture.cpp and checks its planted violations, the allowlist and negative controls. Exit codes: 0 clean, 1 new findings, 2 usage or environment error. --- utils/README.md | 12 + utils/check-signal-reachability.py | 456 ++++++++++++++++++++ utils/tests/signal-reachability-fixture.cpp | 151 +++++++ 3 files changed, 619 insertions(+) create mode 100755 utils/check-signal-reachability.py create mode 100644 utils/tests/signal-reachability-fixture.cpp diff --git a/utils/README.md b/utils/README.md index bb506349b4..9ddac4d3df 100644 --- a/utils/README.md +++ b/utils/README.md @@ -182,6 +182,18 @@ 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 ] +./utils/check-signal-reachability.py --self-test # planted violations in utils/tests/signal-reachability-fixture.cpp +``` + +Exit codes: `0` no findings outside the allowlist (or self-test passed), `1` new findings (one `: -> (via )` line each), `2` usage/environment error. `--allowlist` replaces the built-in known findings; one entry per line: `[|...] -> # `. + ### `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. diff --git a/utils/check-signal-reachability.py b/utils/check-signal-reachability.py new file mode 100755 index 0000000000..fb26f166a5 --- /dev/null +++ b/utils/check-signal-reachability.py @@ -0,0 +1,456 @@ +#!/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. +"""Static async-signal-safety reachability check for the profiler's native code. + +Builds a call graph from the relocations of ELF object files (`objdump -dr`), +walks it from the signal-handler / malloc-hook entry points in ROOTS and reports +every function that calls into an UNSAFE_SINKS symbol (heap allocation, locks, +stdio, dynamic-loader calls, dynamic TLS, logging, ...). + +The graph only contains direct calls, so the objects must be built with +-ffunction-sections (every function gets its own section, so every call carries +a relocation) - the debug configuration (build/obj/main/debug) does that. +Indirect calls (function pointers, virtual dispatch) are not followed. + +Exit codes: + 0 no findings outside the allowlist (or --self-test passed) + 1 one or more new findings, one line each: + : -> (via ) + 2 usage / environment error (missing or non-ELF objects, no GNU objdump, + self-test fixture does not compile, malformed allowlist) +""" + +import argparse +import bisect +import collections +import os +import re +import shutil +import subprocess +import sys +import tempfile +from concurrent.futures import ThreadPoolExecutor + +# --------------------------------------------------------------------------- +# Entry points: mangled-symbol regexes (full match) of the functions that run +# in async-signal context or as a malloc interposer. Extend as needed. +# +# The NativeSocketSampler::{send,recv,write,read}_hook interposers are +# deliberately NOT listed: they run on the calling Java thread, not in a +# signal handler, and are allowed to allocate and lock +# (see ddprof-lib/src/main/cpp/nativeSocketSampler.h). +# --------------------------------------------------------------------------- +ROOTS = [ + # Signal handlers + r"_ZN6CTimer13signalHandlerE\w+", + r"_ZN11CTimerJvmti13signalHandlerE\w+", + r"_ZN6ITimer13signalHandlerE\w+", + r"_ZN11ITimerJvmti13signalHandlerE\w+", + r"_ZN10PerfEvents13signalHandlerE\w+", + r"_ZN14WallClockASGCT19sharedSignalHandlerE\w+", + r"_ZN14WallClockASGCT13signalHandlerE\w+", + r"_ZN14WallClockJvmti19sharedSignalHandlerE\w+", + r"_ZN14WallClockJvmti13signalHandlerE\w+", + r"_ZN8Profiler11segvHandlerE\w+", + r"_ZN8Profiler10busHandlerE\w+", + r"_ZL13wakeupHandleri", + # malloc interposers (must not re-enter the allocator) + r"malloc_hook", + r"calloc_hook", + r"realloc_hook", + r"posix_memalign_hook", + r"aligned_alloc_hook", + r"_ZL18nested_malloc_hookm", + r"_ZL25nested_aligned_alloc_hookmm", +] + +# --------------------------------------------------------------------------- +# Unsafe sinks: mangled-symbol regexes (full match) that must not be reached +# from a root. "TLS_ACCESS:" is the synthetic node for a thread_local +# access; the library is -fPIC, so every access is a TLS descriptor call that +# can allocate the DTV slot lazily. +# --------------------------------------------------------------------------- +UNSAFE_SINKS = [ + r"TLS_ACCESS:.*", + r"__tls_get_addr", + # heap + r"malloc", r"calloc", r"realloc", r"free", r"aligned_alloc", r"posix_memalign", r"strdup", + r"_Znwm", r"_Znam", r"_ZnwmRKSt9nothrow_t", r"_ZnamRKSt9nothrow_t", + r"_ZdlPv", r"_ZdaPv", r"_ZdlPvm", r"_ZdaPvm", + # locks and one-time initialization + r"pthread_mutex_lock", r"__cxa_guard_acquire", r"__cxa_atexit", + r"_ZN8SpinLock4lockEv", r"_ZN8SpinLock10lockSharedEv", r"_ZN5Mutex4lockEv", + r"_ZN15SharedLockGuardC\dEP8SpinLock", r"_ZN18ExclusiveLockGuardC\dEP8SpinLock", + r"_ZN13MutexLockerC\d\w*", + # stdio + r"vsnprintf", r"snprintf", r"fprintf", r"printf", r"fflush", r"fopen", + # dynamic loader / unwinder + r"dlsym", r"dladdr", r"dlopen", r"dl_iterate_phdr", r"backtrace", + # profiler internals that allocate or lock + r"_ZN3Log\w+", + r"_ZN14ProfiledThread17initCurrentThreadEv", # initCurrentThreadSignalSafe() is fine +] + +# Functions whose callees are not followed. The singleton accessors guard a +# function-local static; the guard is initialized at startup, before any +# handler or hook is installed. +BARRIERS = [ + r"_ZN8Counters8instanceEv", + r"_ZN9Libraries8instanceEv", + r"_ZN8Profiler8instanceEv", +] + +# --------------------------------------------------------------------------- +# Known findings: (caller symbol alternatives, unsafe symbol, tracking issue). +# Symbols are demangled names without parameter lists, exactly as printed in +# the report. --allowlist FILE replaces this list; the file holds one entry per +# line in the same shape: +# [|...] -> # [note] +# --------------------------------------------------------------------------- +ALLOWLIST = [ + (("JitCodeCache::findRuntimeStub", "JitCodeCache::findRuntimeStubInfo"), + "SpinLock::lockShared", "PROF-16313"), + (("RefCountGuard::RefCountGuard",), "Log::warn", "PROF-16314"), +] + +SELF_TEST_FIXTURE = os.path.join(os.path.dirname(os.path.abspath(__file__)), + "tests", "signal-reachability-fixture.cpp") + +# --------------------------------------------------------------------------- + +_SYM_LINE = re.compile(r"^([0-9a-f]+) (.{7}) (\S+)\s+[0-9a-f]+ (?:\.hidden |\.protected |\.internal )?(\S+)$") +# Direct calls, plus PC-relative address materialization (clang -O0 on aarch64 +# calls hidden functions via adrp+add+blr; on x86 PC32 covers lea/call alike). +# Taking a function's address is conservatively treated as calling it. +_CALL_RELOC = re.compile(r"^\s+([0-9a-f]+):\s+(R_AARCH64_(?:CALL26|JUMP26|ADR_PREL_PG_HI21|ADR_GOT_PAGE)|R_X86_64_(?:PLT32|PC32|GOTPCREL|GOTPCRELX|REX_GOTPCRELX))\s+(\S+?)([+-]0x[0-9a-f]+)?$") +_TLS_RELOC = re.compile(r"^\s+([0-9a-f]+):\s+(R_AARCH64_TLS\w*|R_X86_64_(?:TLSGD|TLSLD|GOTTPOFF|TPOFF32|GOTPC32_TLSDESC|TLSDESC_CALL))\s+(\S+?)([+-]0x[0-9a-f]+)?$") +_SECTION_HEADER = re.compile(r"^Disassembly of section (\S+):$") +_TLS_SECTION = re.compile(r"^\.(?:tbss|tdata)\.") + + +class EnvError(Exception): + """Usage or environment problem: exit code 2.""" + + +def _full(patterns): + return re.compile("^(?:" + "|".join(patterns) + ")$") + + +ROOT_RE, SINK_RE, BARRIER_RE = _full(ROOTS), _full(UNSAFE_SINKS), _full(BARRIERS) + + +def _symbol(node): + """Node id -> symbol name. Local symbols are qualified as 'sym@object'.""" + return node.rsplit("@", 1)[0] if "@" in node and not node.startswith("TLS_ACCESS:") else node + + +def _run(cmd, **kw): + try: + return subprocess.run(cmd, capture_output=True, text=True, **kw) + except OSError as e: + raise EnvError("cannot run %s: %s" % (cmd[0], e)) + + +def check_tools(): + out = _run(["objdump", "--version"]) + if out.returncode != 0 or "GNU objdump" not in out.stdout: + raise EnvError("GNU objdump is required (found: %s)" % + (out.stdout.splitlines()[0] if out.stdout else "none")) + if not shutil.which("c++filt"): + raise EnvError("c++filt is required") + + +def list_objects(obj_dir): + if not os.path.isdir(obj_dir): + raise EnvError("object directory not found: %s" % obj_dir) + objs = sorted(os.path.join(obj_dir, f) for f in os.listdir(obj_dir) if f.endswith(".o")) + if not objs: + raise EnvError("no .o files in %s" % obj_dir) + for o in objs: + with open(o, "rb") as f: + if f.read(4) != b"\x7fELF": + raise EnvError("%s is not an ELF object; only Linux (ELF) objects are supported" % o) + return objs + + +def _parse_object(obj): + """Returns (defined: {node: obj}, edges: [(caller_node, callee_node)]).""" + name = os.path.basename(obj) + syms = _run(["objdump", "-t", obj]) + dis = _run(["objdump", "-dr", "--no-show-raw-insn", obj]) + if syms.returncode != 0 or dis.returncode != 0: + raise EnvError("objdump failed on %s: %s" % (obj, (syms.stderr or dis.stderr).strip())) + + local = set() + by_section = collections.defaultdict(list) # section -> [(offset, node)] + for line in syms.stdout.splitlines(): + m = _SYM_LINE.match(line) + if not m: + continue + flags, section, sym = m.group(2), m.group(3), m.group(4) + if flags[0] == "l": + local.add(sym) + if flags[6] == "F" and section.startswith(".text"): + node = sym + "@" + name if flags[0] == "l" else sym + by_section[section].append((int(m.group(1), 16), node)) + for funcs in by_section.values(): + funcs.sort() + + def resolve(target, addend, x86): + if target.startswith(".text"): + # Call to a local function through its section symbol. + funcs = by_section.get(target) + if not funcs: + return None + off = addend + (4 if x86 else 0) + best = funcs[0][1] + for start, node in funcs: + if start <= off: + best = node + return best + if target.startswith("."): + return None # data section (.bss, .rodata, ...) + return target + "@" + name if target in local else target + + defined = {n: name for funcs in by_section.values() for _, n in funcs} + edges = [] + funcs, starts = [], [] + for line in dis.stdout.splitlines(): + h = _SECTION_HEADER.match(line) + if h: + funcs = by_section.get(h.group(1), []) + starts = [s for s, _ in funcs] + continue + if not funcs: + continue + m = _CALL_RELOC.match(line) + tls = None if m else _TLS_RELOC.match(line) + if not (m or tls): + continue + g = m or tls + off = int(g.group(1), 16) + # Owning function(s): the last start <= off, plus aliases at the same + # address (e.g. C1/C2 constructor and D1/D2 destructor variants). + i = bisect.bisect_right(starts, off) - 1 + if i < 0: + continue + addend = int(g.group(4), 16) if g.group(4) else 0 + if tls: + callee = "TLS_ACCESS:" + _TLS_SECTION.sub("", g.group(3)) + else: + callee = resolve(g.group(3), addend, g.group(2).startswith("R_X86_64")) + if callee is None: + continue + j = i + while j >= 0 and starts[j] == starts[i]: + edges.append((funcs[j][1], callee)) + j -= 1 + return defined, edges + + +def build_graph(objs): + defined, graph = {}, collections.defaultdict(set) + with ThreadPoolExecutor(max_workers=min(16, (os.cpu_count() or 4))) as pool: + for d, edges in pool.map(_parse_object, objs): + for n, o in d.items(): + defined.setdefault(n, o) + for caller, callee in edges: + graph[caller].add(callee) + return defined, graph + + +def demangle_all(nodes): + nodes = sorted(nodes) + raw = [_symbol(n).replace("TLS_ACCESS:", "") for n in nodes] + out = _run(["c++filt"], input="\n".join(raw) + "\n") + lines = out.stdout.splitlines() + if out.returncode != 0 or len(lines) != len(nodes): + raise EnvError("c++filt failed") + return {n: ("TLS_ACCESS:" if n.startswith("TLS_ACCESS:") else "") + _strip_params(d) + for n, d in zip(nodes, lines)} + + +def _strip_params(name): + """'void ns::f(int) const' -> 'void ns::f'""" + s = name.rstrip() + for q in (" const", " volatile", " &&", " &"): + if s.endswith(q): + s = s[: -len(q)] + if not s.endswith(")"): + return s + depth = 0 + for i in range(len(s) - 1, -1, -1): + if s[i] == ")": + depth += 1 + elif s[i] == "(": + depth -= 1 + if depth == 0: + return s[:i] if i else s + return s + + +_LIBRARY_FRAME = re.compile(r"^(?:.*\s)?(?:std|__gnu_cxx|__cxxabiv1)::") + + +def find_violations(defined, graph): + """BFS from ROOTS; returns [(object, caller, sink, path)] with demangled names.""" + roots = sorted(n for n in set(defined) | set(graph) if ROOT_RE.match(_symbol(n))) + parent = {r: None for r in roots} + queue = collections.deque(roots) + while queue: + u = queue.popleft() + s = _symbol(u) + if SINK_RE.match(s) or BARRIER_RE.match(s): + continue + for v in sorted(graph.get(u, ())): + if v not in parent: + parent[v] = u + queue.append(v) + + names = demangle_all(parent) + found, seen = [], set() + for u in parent: # BFS order: shortest path first + s = _symbol(u) + if SINK_RE.match(s) or BARRIER_RE.match(s): + continue + for v in sorted(graph.get(u, ())): + if not SINK_RE.match(_symbol(v)): + continue + path, x = [], u + while x is not None: + path.append(x) + x = parent[x] + path.reverse() + # Attribute calls made inside the C++ standard library to the + # profiler function that called into it. + caller = u + for p in reversed(path): + if not _LIBRARY_FRAME.match(names[p]): + caller = p + break + key = (names[caller], names[v]) + if key in seen: + continue + seen.add(key) + found.append((defined.get(caller, "?"), names[caller], names[v], + " -> ".join(names[p] for p in path))) + return roots, found + + +_ALLOW_LINE = re.compile(r"^(?P.+?)\s+->\s+(?P.+?)\s+#\s*(?P[A-Z][A-Z0-9]+-\d+)\b.*$") + + +def load_allowlist(path): + entries = [] + with open(path) as f: + for n, line in enumerate(f, 1): + if not line.strip() or line.lstrip().startswith("#"): + continue + m = _ALLOW_LINE.match(line.strip()) + if not m: + raise EnvError("%s:%d: expected '[|...] -> # '" % (path, n)) + entries.append((tuple(c.strip() for c in m.group("callers").split("|")), + m.group("sink").strip(), m.group("key"))) + return entries + + +def check(obj_dir, allowlist): + objs = list_objects(obj_dir) + check_tools() + defined, graph = build_graph(objs) + roots, found = find_violations(defined, graph) + if not roots: + raise EnvError("none of the ROOTS symbols is defined in %s" % obj_dir) + + new, used = [], set() + for obj, caller, sink, path in found: + hit = next((i for i, (callers, s, _) in enumerate(allowlist) if caller in callers and sink == s), None) + if hit is None: + new.append("%s:%s -> %s (via %s)" % (obj, caller, sink, path)) + else: + used.add(hit) + print("allowlisted [%s]: %s:%s -> %s" % (allowlist[hit][2], obj, caller, sink)) + for i, (callers, sink, key) in enumerate(allowlist): + if i not in used: + print("note: allowlist entry [%s] %s -> %s no longer matches; remove it" + % (key, "|".join(callers), sink), file=sys.stderr) + print("%d objects, %d roots, %d findings (%d allowlisted, %d new)" + % (len(objs), len(roots), len(found), len(found) - len(new), len(new))) + for line in new: + print(line) + return new, found + + +def self_test(): + """Compile the planted-violation fixture and check its EXPECT* annotations.""" + with open(SELF_TEST_FIXTURE) as f: + src = f.read() + + def expectations(tag): + return set(re.findall(r"^// %s: (.+?) -> (.+?)\s*$" % tag, src, re.M)) + + expected, allowed, forbidden = (expectations(t) for t in ("EXPECT", "EXPECT-ALLOWLISTED", "EXPECT-NOT")) + if not expected or not allowed or not forbidden: + raise EnvError("fixture %s lacks EXPECT / EXPECT-ALLOWLISTED / EXPECT-NOT lines" % SELF_TEST_FIXTURE) + cxx = os.environ.get("CXX", "c++") + with tempfile.TemporaryDirectory() as tmp: + obj = os.path.join(tmp, "fixture.o") + # Same codegen-relevant flags as the debug configuration. + cc = _run([cxx, "-O0", "-g", "-fPIC", "-fvisibility=hidden", "-ffunction-sections", + "-fdata-sections", "-std=c++17", "-c", SELF_TEST_FIXTURE, "-o", obj]) + if cc.returncode != 0: + raise EnvError("fixture compile failed:\n" + cc.stderr) + allowlist = [((c,), s, "SELFTEST-1") for c, s in allowed] + new, found = check(tmp, allowlist) + got = {(c, s) for _, c, s, _ in found} + reported = {(c, s) for c, s in got if (c, s) not in allowed} + errors = (["MISSED %s -> %s" % e for e in sorted(expected - reported)] + + ["NOT FOUND (allowlisted) %s -> %s" % e for e in sorted(allowed - got)] + + ["FALSE POSITIVE %s -> %s" % e for e in sorted(forbidden & got)]) + if len(new) != len(reported): + errors.append("allowlist did not suppress exactly the EXPECT-ALLOWLISTED findings") + for e in errors: + print("self-test: " + e) + print("self-test %s: %d/%d planted violations reported, %d/%d allowlisted suppressed, %d/%d negative controls clean" + % ("FAILED" if errors else "PASSED", len(expected & reported), len(expected), + len(allowed & got), len(allowed), len(forbidden - got), len(forbidden))) + return 1 if errors else 0 + + +def main(argv): + ap = argparse.ArgumentParser(description=__doc__.split("\n\n")[0], + formatter_class=argparse.RawDescriptionHelpFormatter, + epilog=__doc__.split("\n\n", 1)[1]) + ap.add_argument("--obj-dir", help="directory with the ELF .o files to check (e.g. ddprof-lib/build/obj/main/debug)") + ap.add_argument("--allowlist", help="known-findings file replacing the built-in ALLOWLIST") + ap.add_argument("--self-test", action="store_true", help="check the planted-violation fixture instead") + args = ap.parse_args(argv) + try: + if args.self_test: + return self_test() + if not args.obj_dir: + ap.print_usage(sys.stderr) + print("error: --obj-dir is required", file=sys.stderr) + return 2 + allowlist = load_allowlist(args.allowlist) if args.allowlist else ALLOWLIST + new, _ = check(args.obj_dir, allowlist) + return 1 if new else 0 + except (EnvError, OSError) as e: + print("error: %s" % e, file=sys.stderr) + return 2 + + +if __name__ == "__main__": + sys.exit(main(sys.argv[1:])) diff --git a/utils/tests/signal-reachability-fixture.cpp b/utils/tests/signal-reachability-fixture.cpp new file mode 100644 index 0000000000..f9182c7bec --- /dev/null +++ b/utils/tests/signal-reachability-fixture.cpp @@ -0,0 +1,151 @@ +/* + * 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. + */ + +// Planted violations for `utils/check-signal-reachability.py --self-test`. +// Compiled on its own into a temporary directory; never linked or run. +// Class and function names mirror the checker's ROOTS / UNSAFE_SINKS. +// +// EXPECT: must be reported as a new finding +// EXPECT-ALLOWLISTED: must be found and suppressed by the self-test allowlist +// EXPECT-NOT: must not be reported +// Entries are " -> " exactly as the checker prints them. + +struct siginfo_t; + +extern "C" { +void *malloc(unsigned long size); +int snprintf(char *buf, unsigned long size, const char *fmt, ...); +} + +class Log { +public: + static void warn(const char *fmt, ...); +}; + +class SpinLock { +public: + void lock(); +}; + +class ProfiledThread { +public: + static ProfiledThread *initCurrentThread(); + static ProfiledThread *initCurrentThreadSignalSafe(); +}; + +// Direct libc call from a signal handler. +// EXPECT: CTimer::signalHandler -> malloc +class CTimer { +public: + static void signalHandler(int signo, siginfo_t *info, void *ucontext); +}; +void CTimer::signalHandler(int, siginfo_t *, void *) { (void)malloc(16); } + +// Transitive call through static helpers: the calls go through section +// symbols (.text._ZL...), not function symbols. +// EXPECT: formatSlowPath -> snprintf +static void formatSlowPath(char *buf) { snprintf(buf, 8, "%d", 1); } +static void formatValue(char *buf) { formatSlowPath(buf); } +class ITimer { +public: + static void signalHandler(int signo, siginfo_t *info, void *ucontext); +}; +void ITimer::signalHandler(int, siginfo_t *, void *) { + char buf[8]; + formatValue(buf); +} + +// Lock taken in a constructor: the call site references the complete-object +// constructor (C1), whose body is emitted as an alias of the base-object +// constructor (C2). +// EXPECT: LockGuard::LockGuard -> SpinLock::lock +class LockGuard { +public: + explicit LockGuard(SpinLock *lock); +}; +LockGuard::LockGuard(SpinLock *lock) { lock->lock(); } +class PerfEvents { +public: + static void signalHandler(int signo, siginfo_t *info, void *ucontext); +}; +void PerfEvents::signalHandler(int, siginfo_t *, void *) { + static SpinLock lock; + LockGuard guard(&lock); +} + +// thread_local access (dynamic TLS) from a signal handler. +// EXPECT: Profiler::segvHandler -> TLS_ACCESS:t_depth +// A singleton accessor is a barrier: its callees are not followed. +// EXPECT-NOT: Profiler::instance -> malloc +thread_local int t_depth; +class Profiler { +public: + static Profiler *instance(); + static bool segvHandler(int signo, siginfo_t *info, void *ucontext); +}; +Profiler *Profiler::instance() { return static_cast(malloc(1)); } +bool Profiler::segvHandler(int, siginfo_t *, void *) { + Profiler::instance(); + return ++t_depth > 1; +} + +// Heap allocation from a malloc interposer. +// EXPECT: malloc_hook -> operator new[] +extern "C" __attribute__((visibility("default"))) void *malloc_hook(unsigned long size) { + return new char[size]; +} + +// Allocation inside a standard-library template is attributed to the +// profiler function calling into it. +// EXPECT: CTimerJvmti::signalHandler -> operator new +namespace std { +template struct fixture_box { + static T *make() { return new T(); } +}; +} // namespace std +class CTimerJvmti { +public: + static void signalHandler(int signo, siginfo_t *info, void *ucontext); +}; +void CTimerJvmti::signalHandler(int, siginfo_t *, void *) { + (void)std::fixture_box::make(); +} + +// Only the non-signal-safe ProfiledThread initializer is a sink. +// EXPECT: WallClockJvmti::sharedSignalHandler -> ProfiledThread::initCurrentThread +// EXPECT-NOT: WallClockASGCT::sharedSignalHandler -> ProfiledThread::initCurrentThreadSignalSafe +class WallClockJvmti { +public: + static void sharedSignalHandler(int signo, siginfo_t *info, void *ucontext); +}; +void WallClockJvmti::sharedSignalHandler(int, siginfo_t *, void *) { + ProfiledThread::initCurrentThread(); +} +class WallClockASGCT { +public: + static void sharedSignalHandler(int signo, siginfo_t *info, void *ucontext); +}; +void WallClockASGCT::sharedSignalHandler(int, siginfo_t *, void *) { + ProfiledThread::initCurrentThreadSignalSafe(); +} + +// Static (local-symbol) root; finding suppressed by the self-test allowlist. +// EXPECT-ALLOWLISTED: wakeupHandler -> Log::warn +__attribute__((used)) static void wakeupHandler(int) { Log::warn("wakeup"); } + +// Not reachable from any root. +// EXPECT-NOT: unrelatedFunction -> malloc +void unrelatedFunction() { (void)malloc(1); } From 5f6cc8fb6f01b32d282086dce4ec8957cbc6c065 Mon Sep 17 00:00:00 2001 From: Jaroslav Bachorik Date: Fri, 23 Oct 2026 16:24:36 +0200 Subject: [PATCH 2/6] Add opt-in checkSignalReachability Gradle task Registers :ddprof-lib:checkSignalReachability (verification group), which runs utils/check-signal-reachability.py on the debug objects after compileDebug. It is not wired into check or build. The task is skipped when the debug objects are missing or not ELF (e.g. macOS builds). --- ddprof-lib/build.gradle.kts | 22 ++++++++++++++++++++++ utils/README.md | 1 + 2 files changed, 23 insertions(+) diff --git a/ddprof-lib/build.gradle.kts b/ddprof-lib/build.gradle.kts index ff194cccf6..6823b1831b 100644 --- a/ddprof-lib/build.gradle.kts +++ b/ddprof-lib/build.gradle.kts @@ -183,6 +183,28 @@ afterEvaluate { } } +// Static async-signal-safety reachability check over the debug objects; see +// utils/check-signal-reachability.py. Opt-in (not wired into check/build): run +// `./gradlew :ddprof-lib:checkSignalReachability`. The checker reads ELF objects +// only, so the task is skipped when the debug objects are absent or not ELF. +tasks.register("checkSignalReachability") { + group = "verification" + description = "Checks that no async-signal-unsafe function is reachable from the signal handlers and malloc hooks" + dependsOn(tasks.matching { it.name == "compileDebug" }) + + val objDir = layout.buildDirectory.dir("obj/main/debug").get().asFile + val script = rootProject.file("utils/check-signal-reachability.py") + commandLine("python3", script.absolutePath, "--obj-dir", objDir.absolutePath) + + onlyIf("debug objects are ELF") { + val obj = objDir.listFiles { f -> f.name.endsWith(".o") }?.firstOrNull() + val magic = ByteArray(4) + obj != null && + obj.inputStream().use { it.read(magic) } == 4 && + magic.contentEquals(byteArrayOf(0x7f, 'E'.code.toByte(), 'L'.code.toByte(), 'F'.code.toByte())) + } +} + // Add runBenchmarks task tasks.register("runBenchmarks") { dependsOn(":ddprof-lib:benchmarks:runBenchmark") diff --git a/utils/README.md b/utils/README.md index 9ddac4d3df..5b98b9223b 100644 --- a/utils/README.md +++ b/utils/README.md @@ -190,6 +190,7 @@ Static async-signal-safety check. Builds a call graph from the relocations of th ```bash ./utils/check-signal-reachability.py --obj-dir ddprof-lib/build/obj/main/debug [--allowlist ] ./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 `: -> (via )` line each), `2` usage/environment error. `--allowlist` replaces the built-in known findings; one entry per line: `[|...] -> # `. From 5cb8d2d29865b45a264031ccdaf2f09fec3a682b Mon Sep 17 00:00:00 2001 From: Jaroslav Bachorik Date: Fri, 23 Oct 2026 16:31:17 +0200 Subject: [PATCH 3/6] Add source quality checks and opt-in qualityChecks task Four standalone stdlib-only checks under utils/, each printing one line per violation and exiting 0 clean, 1 violations, 2 environment error: - check-jvm-boundary-includes.py: shared code in ddprof-lib/src/main/cpp includes no hotspot/, j9/ or zing/ header outside the boundary files and the known exceptions, which mirror the JvmSupportLayer.md table; stale exceptions are reported too. - check-doc-index.py: every tracked 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 .restore(). The glibc cancellation buffer in libraryPatcher_linux.cpp is the one exception. - check-tracked-artifacts.py: no tracked docs/sphinx/, .sphinx/ or *.scratch* path. :ddprof-lib:qualityChecks depends on checkSignalReachability, runs all four and fails if any of them fails. It is not wired into check or build. To make the doc index check pass, index the NativeMemoryProfiling, SanityChecks, SigsegvPatching and Versioning docs in doc/README.md, and rename the ThreadContext benchmark report to ThreadContextBenchmark.md under a new Reports section. --- ddprof-lib/build.gradle.kts | 34 ++++++ utils/README.md | 17 +++ utils/check-doc-index.py | 68 +++++++++++ utils/check-jvm-boundary-includes.py | 101 ++++++++++++++++ utils/check-sigsetjmp-shape.py | 174 +++++++++++++++++++++++++++ utils/check-tracked-artifacts.py | 62 ++++++++++ 6 files changed, 456 insertions(+) create mode 100755 utils/check-doc-index.py create mode 100755 utils/check-jvm-boundary-includes.py create mode 100755 utils/check-sigsetjmp-shape.py create mode 100755 utils/check-tracked-artifacts.py diff --git a/ddprof-lib/build.gradle.kts b/ddprof-lib/build.gradle.kts index 6823b1831b..501bd0a3e0 100644 --- a/ddprof-lib/build.gradle.kts +++ b/ddprof-lib/build.gradle.kts @@ -205,6 +205,40 @@ tasks.register("checkSignalReachability") { } } +// Source-level quality checks (utils/check-*.py), each also runnable on its own. +// Opt-in: `./gradlew :ddprof-lib:qualityChecks`. Runs every check, then fails +// if any of them failed. +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/" + dependsOn("checkSignalReachability") + + val exec = objects.newInstance().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 = scripts.filter { script -> + exec.exec { + commandLine("python3", script.absolutePath, "--root", root.absolutePath) + isIgnoreExitValue = true + }.exitValue != 0 + } + if (failed.isNotEmpty()) { + throw GradleException("quality checks failed: ${failed.joinToString { it.name }}") + } + } +} + // Add runBenchmarks task tasks.register("runBenchmarks") { dependsOn(":ddprof-lib:benchmarks:runBenchmark") diff --git a/utils/README.md b/utils/README.md index 5b98b9223b..3f6b4a83b4 100644 --- a/utils/README.md +++ b/utils/README.md @@ -195,6 +195,23 @@ Static async-signal-safety check. Builds a call graph from the relocations of th Exit codes: `0` no findings outside the allowlist (or self-test passed), `1` new findings (one `: -> (via )` line each), `2` usage/environment error. `--allowlist` replaces the built-in known findings; one entry per line: `[|...] -> # `. +### Source quality checks + +Standalone, stdlib-only checks over the git-tracked files. Each prints one `[:]: ` 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, 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 `.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 # opt-in; checkSignalReachability plus all four +``` + ### `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. diff --git a/utils/check-doc-index.py b/utils/check-doc-index.py new file mode 100755 index 0000000000..e4bc7af7f6 --- /dev/null +++ b/utils/check-doc-index.py @@ -0,0 +1,68 @@ +#!/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.split("#", 1)[0] + if target and "://" not in target: + linked.add(posixpath.normpath(posixpath.join("doc", target))) + + errors = [] + for doc in sorted(set(docs) - {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) + 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:])) diff --git a/utils/check-jvm-boundary-includes.py b/utils/check-jvm-boundary-includes.py new file mode 100755 index 0000000000..6223a9692b --- /dev/null +++ b/utils/check-jvm-boundary-includes.py @@ -0,0 +1,101 @@ +#!/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 shared native code reaches JVM implementations only through the boundary. + +Shared files under ddprof-lib/src/main/cpp must not #include "hotspot/...", +"j9/..." or "zing/..." headers. Allowed: the implementation directories +themselves, the boundary files, and the known exceptions listed below. + +Exit codes: 0 clean, 1 violations or stale exceptions, 2 environment error. +""" + +import argparse +import os +import re +import subprocess +import sys + +SRC = "ddprof-lib/src/main/cpp" +IMPLEMENTATION_DIRS = ("hotspot/", "j9/", "zing/") + +# The abstraction files that dispatch on the detected VM. +BOUNDARY_FILES = { + "jvmSupport.h", "jvmSupport.inline.h", "jvmSupport.cpp", + "jvmThread.h", "jvmThread.cpp", + "vmEntry.h", "vmEntry.cpp", + "javaApi.cpp", +} + +# MUST match doc/architecture/JvmSupportLayer.md#known-boundary-exceptions; +# update both together. New code should not add to this list. +KNOWN_EXCEPTIONS = { + "profiler.cpp": {"hotspot/vmStructs.inline.h", "hotspot/hotspotSupport.h", + "j9/j9Support.h", "j9/j9WallClock.h"}, + "flightRecorder.cpp": {"hotspot/vmStructs.h"}, + "livenessTracker.cpp": {"hotspot/vmStructs.h", "hotspot/vmStructs.inline.h"}, + "sanityCheck.cpp": {"hotspot/vmStructs.h", "hotspot/vmStructs.inline.h"}, + "threadState.inline.h": {"hotspot/vmStructs.h"}, +} + +_INCLUDE = re.compile(r'^\s*#\s*include\s+"((?:hotspot|j9|zing)/[^"]+)"') + + +def tracked_files(root, path): + try: + out = subprocess.run(["git", "-C", root, "ls-files", "--", path], + capture_output=True, text=True, check=True).stdout + except (OSError, subprocess.CalledProcessError) as e: + print("error: git ls-files failed: %s" % e, file=sys.stderr) + return None + return out.splitlines() + + +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 + + files = tracked_files(root, SRC) + if files is None: + return 2 + errors, seen = [], set() + for path in files: + rel = path[len(SRC) + 1:] + if rel.startswith(IMPLEMENTATION_DIRS) or rel in BOUNDARY_FILES: + continue + with open(os.path.join(root, path), encoding="utf-8", errors="replace") as f: + for n, line in enumerate(f, 1): + m = _INCLUDE.match(line) + if not m: + continue + header = m.group(1) + if header in KNOWN_EXCEPTIONS.get(rel, ()): + seen.add((rel, header)) + else: + errors.append('%s:%d: shared code includes "%s"; call JVMSupport/JVMThread/VM instead ' + "(see doc/architecture/JvmSupportLayer.md)" % (path, n, header)) + for rel, headers in sorted(KNOWN_EXCEPTIONS.items()): + for header in sorted(headers - {h for r, h in seen if r == rel}): + errors.append('%s/%s: known exception "%s" is no longer included; remove it from ' + "KNOWN_EXCEPTIONS and doc/architecture/JvmSupportLayer.md" % (SRC, rel, header)) + for e in errors: + print(e) + print("jvm boundary includes: %s" % ("%d problem(s)" % len(errors) if errors else "OK")) + return 1 if errors else 0 + + +if __name__ == "__main__": + sys.exit(main(sys.argv[1:])) diff --git a/utils/check-sigsetjmp-shape.py b/utils/check-sigsetjmp-shape.py new file mode 100755 index 0000000000..c16719df5e --- /dev/null +++ b/utils/check-sigsetjmp-shape.py @@ -0,0 +1,174 @@ +#!/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 crash-protection sigsetjmp() follows the JmpCtxScope pattern (guards.h). + +For each sigsetjmp( / __sigsetjmp( call in tracked files under ddprof-lib/src/main/cpp: + 1. a JmpCtxScope is declared before it, in the same or an enclosing block; + 2. savemask is the literal 1; + 3. the landing pad `if (sigsetjmp(...) != 0) {` starts with + SIGNAL_HANDLER_UNWIND_AFTER_LONGJMP(); followed by .restore(); + +Exit codes: 0 clean, 1 violations or stale exceptions, 2 environment error. +""" + +import argparse +import os +import re +import subprocess +import sys + +SRC = "ddprof-lib/src/main/cpp" + +# (file, first argument) of calls that are not crash protection. +# libraryPatcher_linux.cpp: glibc pthread cancellation buffer. glibc's unwinder +# siglongjmps here on pthread_exit/cancel (not checkFault() from segvHandler), +# so there is no signal-handler state to unwind and no signal mask to restore. +KNOWN_EXCEPTIONS = { + ("libraryPatcher_linux.cpp", "(struct __jmp_buf_tag*)(void*)cancel_buf.__cancel_jmp_buf"), +} + +_CALL = re.compile(r"\b(?:__)?sigsetjmp\s*\(") +_SCOPE = re.compile(r"\bJmpCtxScope\s+(\w+)\s*[({]") +_PAD = re.compile(r"\s*!=\s*0\s*\)\s*\{\s*SIGNAL_HANDLER_UNWIND_AFTER_LONGJMP\s*\(\s*\)\s*;\s*(\w+)\s*\.\s*restore\s*\(\s*\)\s*;") +_COMMENT_OR_STRING = re.compile(r'//[^\n]*|/\*.*?\*/|"(?:\\.|[^"\\\n])*"|\'(?:\\.|[^\'\\\n])*\'', re.S) + + +def blank(m): + # Keep offsets and line numbers; keep string quotes so code shape is intact. + t = m.group(0) + if t[0] in "\"'": + return t[0] + " " * (len(t) - 2) + t[-1] + return re.sub(r"[^\n]", " ", t) + + +def split_args(code, open_paren): + """Top-level arguments of the call whose '(' is at open_paren; returns (args, end).""" + depth, start, args = 0, open_paren + 1, [] + for i in range(open_paren, len(code)): + c = code[i] + if c in "([{": + depth += 1 + elif c in ")]}": + depth -= 1 + if depth == 0: + args.append(code[start:i].strip()) + return args, i + 1 + elif c == "," and depth == 1: + args.append(code[start:i].strip()) + start = i + 1 + return None, len(code) + + +def scope_in_effect(code, pos): + """Name of the nearest JmpCtxScope declared before pos in the same or an enclosing block.""" + depth = 0 + visible = [] # code ranges at the call's block nesting + end = pos + for i in range(pos - 1, -1, -1): + c = code[i] + if c == "}": + if depth == 0: + visible.append((i + 1, end)) + depth += 1 + elif c == "{": + if depth == 0: + visible.append((i + 1, end)) + end = i + else: + depth -= 1 + if depth == 0: + end = i + if depth == 0: + visible.append((0, end)) + for a, b in visible: + names = _SCOPE.findall(code[a:b]) + if names: + return names[-1] + return None + + +def check_file(path, text): + code = _COMMENT_OR_STRING.sub(blank, text) + rel = path[len(SRC) + 1:] + errors, exceptions = [], set() + for m in _CALL.finditer(code): + line = code.count("\n", 0, m.start()) + 1 + where = "%s:%d" % (path, line) + args, end = split_args(code, m.end() - 1) + if args is None or len(args) != 2: + errors.append("%s: cannot parse the sigsetjmp() arguments" % where) + continue + first = " ".join(args[0].split()) + key = (rel, first) + if key in KNOWN_EXCEPTIONS: + exceptions.add(key) + continue + scope = scope_in_effect(code, m.start()) + if scope is None: + errors.append("%s: no JmpCtxScope declared before sigsetjmp(%s, ...); see guards.h" % (where, first)) + if args[1] != "1": + errors.append("%s: savemask is '%s', must be 1 (SIGSEGV stays blocked after siglongjmp otherwise)" + % (where, args[1])) + pad = _PAD.match(code, end) + if pad is None: + errors.append("%s: landing pad must be `if (sigsetjmp(...) != 0) {` starting with " + "SIGNAL_HANDLER_UNWIND_AFTER_LONGJMP(); then .restore();" % where) + elif scope is not None and pad.group(1) != scope: + errors.append("%s: landing pad restores '%s' but the JmpCtxScope in effect is '%s'" + % (where, pad.group(1), scope)) + return errors, exceptions + + +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)") + ap.add_argument("files", nargs="*", help="check these files instead of the tracked sources") + args = ap.parse_args(argv) + root = args.root + files = args.files + if not files: + try: + files = subprocess.run(["git", "-C", root, "ls-files", "--", SRC], + capture_output=True, text=True, check=True).stdout.splitlines() + except (OSError, subprocess.CalledProcessError) as e: + print("error: git ls-files failed: %s" % e, file=sys.stderr) + return 2 + + errors, seen, calls = [], set(), 0 + for path in files: + try: + with open(os.path.join(root, path), encoding="utf-8", errors="replace") as f: + text = f.read() + except OSError as e: + print("error: %s" % e, file=sys.stderr) + return 2 + calls += len(_CALL.findall(_COMMENT_OR_STRING.sub(blank, text))) + e, s = check_file(path, text) + errors += e + seen |= s + if not args.files: + for rel, arg in sorted(KNOWN_EXCEPTIONS - seen): + errors.append("%s/%s: known exception __sigsetjmp(%s, ...) no longer found; remove it from " + "KNOWN_EXCEPTIONS" % (SRC, rel, arg)) + for e in errors: + print(e) + print("sigsetjmp shape: %s" % ("%d problem(s)" % len(errors) if errors + else "OK (%d calls, %d known exception(s))" % (calls, len(seen)))) + return 1 if errors else 0 + + +if __name__ == "__main__": + sys.exit(main(sys.argv[1:])) diff --git a/utils/check-tracked-artifacts.py b/utils/check-tracked-artifacts.py new file mode 100755 index 0000000000..9a895d1038 --- /dev/null +++ b/utils/check-tracked-artifacts.py @@ -0,0 +1,62 @@ +#!/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 no session/tool artifacts are tracked by git. + +Rejected paths: anything under docs/sphinx/ or a .sphinx/ directory, and any +path component matching *.scratch* (e.g. .scratch/, notes.scratch.md). + +Exit codes: 0 clean, 1 violations, 2 environment error. +""" + +import argparse +import fnmatch +import os +import subprocess +import sys + + +def reason(path): + parts = path.split("/") + for i, part in enumerate(parts): + if part == "docs" and i + 1 < len(parts) - 1 and parts[i + 1] == "sphinx": + return "docs/sphinx/ holds review-session artifacts" + if part == ".sphinx" and i < len(parts) - 1: + return ".sphinx/ holds review-session state" + if fnmatch.fnmatchcase(part, "*.scratch*"): + return "*.scratch* paths are local scratch files" + return None + + +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: + files = subprocess.run(["git", "-C", root, "ls-files"], + capture_output=True, text=True, check=True).stdout.splitlines() + except (OSError, subprocess.CalledProcessError) as e: + print("error: git ls-files failed: %s" % e, file=sys.stderr) + return 2 + + errors = ["%s: tracked artifact (%s); git rm it" % (f, r) for f in files for r in [reason(f)] if r] + for e in errors: + print(e) + print("tracked artifacts: %s" % ("%d problem(s)" % len(errors) if errors else "OK")) + return 1 if errors else 0 + + +if __name__ == "__main__": + sys.exit(main(sys.argv[1:])) From c966c5ea7af3896044cd43c5c651451cf5aaa8ff Mon Sep 17 00:00:00 2001 From: Jaroslav Bachorik Date: Fri, 23 Oct 2026 17:23:10 +0200 Subject: [PATCH 4/6] Harden quality checks per review: sink coverage, allowlist strictness, self-test wiring, doc-index reverse check --- ddprof-lib/build.gradle.kts | 22 ++++++++++++++---- utils/check-doc-index.py | 12 +++++++--- utils/check-signal-reachability.py | 37 ++++++++++++++++++++++-------- utils/check-tracked-artifacts.py | 2 +- 4 files changed, 55 insertions(+), 18 deletions(-) diff --git a/ddprof-lib/build.gradle.kts b/ddprof-lib/build.gradle.kts index 501bd0a3e0..408510fdc3 100644 --- a/ddprof-lib/build.gradle.kts +++ b/ddprof-lib/build.gradle.kts @@ -190,7 +190,8 @@ afterEvaluate { tasks.register("checkSignalReachability") { group = "verification" description = "Checks that no async-signal-unsafe function is reachable from the signal handlers and malloc hooks" - dependsOn(tasks.matching { it.name == "compileDebug" }) + // Compile task exists for every active configuration; named lookup fails loudly on rename. + dependsOn(tasks.named("compileDebug")) val objDir = layout.buildDirectory.dir("obj/main/debug").get().asFile val script = rootProject.file("utils/check-signal-reachability.py") @@ -227,14 +228,27 @@ tasks.register("qualityChecks") { ).map { File(root, "utils/$it") } doLast { - val failed = scripts.filter { script -> + val failed = mutableListOf() + // 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", File(root, "utils/check-signal-reachability.py").absolutePath, "--self-test") + isIgnoreExitValue = true + }.exitValue.takeIf { it != 0 }?.let { failed += "self-test (check-signal-reachability.py --self-test)" } + } + 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.name }}") + throw GradleException("quality checks failed: ${failed.joinToString { it }}") } } } diff --git a/utils/check-doc-index.py b/utils/check-doc-index.py index e4bc7af7f6..de6b9d9143 100755 --- a/utils/check-doc-index.py +++ b/utils/check-doc-index.py @@ -27,7 +27,7 @@ import sys INDEX = "doc/README.md" -_LINK = re.compile(r"\]\(([^)\s]+)") +_LINK = re.compile(r"\]\((]+>?)\)") _PASCAL = re.compile(r"^[A-Z][A-Za-z0-9]*\.md$") @@ -48,16 +48,22 @@ def main(argv): linked = set() for target in _LINK.findall(index): - target = target.split("#", 1)[0] + target = target.strip("<>").split("#", 1)[0] if target and "://" not in target: linked.add(posixpath.normpath(posixpath.join("doc", target))) errors = [] - for doc in sorted(set(docs) - {INDEX}): + 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))) diff --git a/utils/check-signal-reachability.py b/utils/check-signal-reachability.py index fb26f166a5..902299d947 100755 --- a/utils/check-signal-reachability.py +++ b/utils/check-signal-reachability.py @@ -33,6 +33,7 @@ """ import argparse +import shlex import bisect import collections import os @@ -87,7 +88,13 @@ r"__tls_get_addr", # heap r"malloc", r"calloc", r"realloc", r"free", r"aligned_alloc", r"posix_memalign", r"strdup", + r"__libc_malloc", r"__libc_calloc", r"__libc_free", r"__libc_realloc", r"_Znwm", r"_Znam", r"_ZnwmRKSt9nothrow_t", r"_ZnamRKSt9nothrow_t", + # over-aligned operator new/delete (C++17 -faligned-new) + r"_ZnwmSt11align_val_t", r"_ZnamSt11align_val_t", + r"_ZnwmSt11align_val_tRKSt9nothrow_t", r"_ZnamSt11align_val_tRKSt9nothrow_t", + r"_ZdlPvSt11align_val_t", r"_ZdaPvSt11align_val_t", + r"_ZdlPvmSt11align_val_t", r"_ZdaPvmSt11align_val_t", r"_ZdlPv", r"_ZdaPv", r"_ZdlPvm", r"_ZdaPvm", # locks and one-time initialization r"pthread_mutex_lock", r"__cxa_guard_acquire", r"__cxa_atexit", @@ -106,6 +113,10 @@ # Functions whose callees are not followed. The singleton accessors guard a # function-local static; the guard is initialized at startup, before any # handler or hook is installed. +# BARRIERS note: stopping at these singletons assumes their first (lazy) call +# happens before any handler or hook is installed. The walk cannot verify that +# ordering from relocations, so a first-call-in-signal-context would pass +# unseen -- keep the singletons initialized during profiler startup. BARRIERS = [ r"_ZN8Counters8instanceEv", r"_ZN9Libraries8instanceEv", @@ -354,7 +365,7 @@ def find_violations(defined, graph): def load_allowlist(path): entries = [] - with open(path) as f: + with open(path, encoding="utf-8") as f: for n, line in enumerate(f, 1): if not line.strip() or line.lstrip().startswith("#"): continue @@ -382,15 +393,12 @@ def check(obj_dir, allowlist): else: used.add(hit) print("allowlisted [%s]: %s:%s -> %s" % (allowlist[hit][2], obj, caller, sink)) - for i, (callers, sink, key) in enumerate(allowlist): - if i not in used: - print("note: allowlist entry [%s] %s -> %s no longer matches; remove it" - % (key, "|".join(callers), sink), file=sys.stderr) + stale = [(callers, sink, key) for i, (callers, sink, key) in enumerate(allowlist) if i not in used] print("%d objects, %d roots, %d findings (%d allowlisted, %d new)" % (len(objs), len(roots), len(found), len(found) - len(new), len(new))) for line in new: print(line) - return new, found + return new, stale def self_test(): @@ -404,11 +412,12 @@ def expectations(tag): expected, allowed, forbidden = (expectations(t) for t in ("EXPECT", "EXPECT-ALLOWLISTED", "EXPECT-NOT")) if not expected or not allowed or not forbidden: raise EnvError("fixture %s lacks EXPECT / EXPECT-ALLOWLISTED / EXPECT-NOT lines" % SELF_TEST_FIXTURE) - cxx = os.environ.get("CXX", "c++") + # CXX may be a command with arguments ("ccache g++"); split it the way a shell would. + cxx = shlex.split(os.environ.get("CXX", "c++")) with tempfile.TemporaryDirectory() as tmp: obj = os.path.join(tmp, "fixture.o") # Same codegen-relevant flags as the debug configuration. - cc = _run([cxx, "-O0", "-g", "-fPIC", "-fvisibility=hidden", "-ffunction-sections", + cc = _run(cxx + ["-O0", "-g", "-fPIC", "-fvisibility=hidden", "-ffunction-sections", "-fdata-sections", "-std=c++17", "-c", SELF_TEST_FIXTURE, "-o", obj]) if cc.returncode != 0: raise EnvError("fixture compile failed:\n" + cc.stderr) @@ -445,9 +454,17 @@ def main(argv): print("error: --obj-dir is required", file=sys.stderr) return 2 allowlist = load_allowlist(args.allowlist) if args.allowlist else ALLOWLIST - new, _ = check(args.obj_dir, allowlist) + new, stale = check(args.obj_dir, allowlist) + if stale: + # An allowlist entry suppresses findings by name; one that no longer + # matches can silently mask a reappeared violation, so it fails the + # run like the sibling checkers do. + for callers, sink, key in stale: + print("error: stale allowlist entry [%s] %s -> %s; remove it" + % (key, "|".join(callers), sink), file=sys.stderr) + return 1 return 1 if new else 0 - except (EnvError, OSError) as e: + except (EnvError, OSError, ValueError) as e: print("error: %s" % e, file=sys.stderr) return 2 diff --git a/utils/check-tracked-artifacts.py b/utils/check-tracked-artifacts.py index 9a895d1038..41a1488c4d 100755 --- a/utils/check-tracked-artifacts.py +++ b/utils/check-tracked-artifacts.py @@ -30,7 +30,7 @@ def reason(path): parts = path.split("/") for i, part in enumerate(parts): - if part == "docs" and i + 1 < len(parts) - 1 and parts[i + 1] == "sphinx": + if part == "docs" and i + 1 < len(parts) and parts[i + 1] == "sphinx": return "docs/sphinx/ holds review-session artifacts" if part == ".sphinx" and i < len(parts) - 1: return ".sphinx/ holds review-session state" From 03a07a75a6697ce8281ba99a97e767d741c887c5 Mon Sep 17 00:00:00 2001 From: Jaroslav Bachorik Date: Fri, 23 Oct 2026 17:41:56 +0200 Subject: [PATCH 5/6] RefCountGuard: count nested-overflow instead of logging from signal context The guard constructor runs in signal handlers; its NESTED_DEPTH-overflow warning used Log::warn (stdio, not async-signal-safe). Replace the once-per-process latch with the REFCOUNT_NESTED_OVERFLOW counter, surfaced in the stop-time report like every other signal-path anomaly (PROF-16314). Drop the now-fixed entry from the checker allowlist. --- ddprof-lib/src/main/cpp/counters.h | 1 + ddprof-lib/src/main/cpp/refCountGuard.cpp | 17 ++++++----------- ddprof-lib/src/main/cpp/refCountGuard.h | 6 ++++-- utils/check-signal-reachability.py | 1 - 4 files changed, 11 insertions(+), 14 deletions(-) diff --git a/ddprof-lib/src/main/cpp/counters.h b/ddprof-lib/src/main/cpp/counters.h index 950ce1fb49..e602a5da4d 100644 --- a/ddprof-lib/src/main/cpp/counters.h +++ b/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") \ diff --git a/ddprof-lib/src/main/cpp/refCountGuard.cpp b/ddprof-lib/src/main/cpp/refCountGuard.cpp index a86fd26869..c4fa804d12 100644 --- a/ddprof-lib/src/main/cpp/refCountGuard.cpp +++ b/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 s_nested_overflow_warned{false}; - int RefCountGuard::getThreadRefCountSlot() { ProfiledThread* thrd = ProfiledThread::current(); int tid = thrd != nullptr ? thrd->tid() : OS::threadId(); @@ -88,12 +83,12 @@ 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(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 shows up in the + // stop-time report; the overflow stays latched per guard by the + // count itself, so the counter only advances on real overflows. + Counters::increment(REFCOUNT_NESTED_OVERFLOW, 1); } } else { // Non-reentrant (count was 0): store pointer first so the scanner skips diff --git a/ddprof-lib/src/main/cpp/refCountGuard.h b/ddprof-lib/src/main/cpp/refCountGuard.h index 1b1aef8501..13a69081f7 100644 --- a/ddprof-lib/src/main/cpp/refCountGuard.h +++ b/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. diff --git a/utils/check-signal-reachability.py b/utils/check-signal-reachability.py index 902299d947..5c3d2dd798 100755 --- a/utils/check-signal-reachability.py +++ b/utils/check-signal-reachability.py @@ -133,7 +133,6 @@ ALLOWLIST = [ (("JitCodeCache::findRuntimeStub", "JitCodeCache::findRuntimeStubInfo"), "SpinLock::lockShared", "PROF-16313"), - (("RefCountGuard::RefCountGuard",), "Log::warn", "PROF-16314"), ] SELF_TEST_FIXTURE = os.path.join(os.path.dirname(os.path.abspath(__file__)), From 2ee769c4fa9c638476397c8ba8f12dede3211524 Mon Sep 17 00:00:00 2001 From: Jaroslav Bachorik Date: Fri, 23 Oct 2026 19:17:33 +0200 Subject: [PATCH 6/6] sphinx: address review feedback on PR #893 --- .github/workflows/ci.yml | 41 +++ ddprof-lib/build.gradle.kts | 53 ++-- ddprof-lib/src/main/cpp/refCountGuard.cpp | 7 +- doc/architecture/JvmSupportLayer.md | 2 +- doc/architecture/RefCountGuard.md | 23 +- utils/README.md | 4 +- utils/check-jvm-boundary-includes.py | 9 +- utils/check-signal-reachability.py | 3 +- utils/check-sigsetjmp-shape.py | 2 +- utils/tests/test_quality_check_scripts.sh | 368 ++++++++++++++++++++++ 10 files changed, 476 insertions(+), 36 deletions(-) create mode 100755 utils/tests/test_quality_check_scripts.sh diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a4cc37a483..2903e26c89 100644 --- a/.github/workflows/ci.yml +++ b/.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 diff --git a/ddprof-lib/build.gradle.kts b/ddprof-lib/build.gradle.kts index 408510fdc3..7554c4e872 100644 --- a/ddprof-lib/build.gradle.kts +++ b/ddprof-lib/build.gradle.kts @@ -183,32 +183,39 @@ 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. Opt-in (not wired into check/build): run -// `./gradlew :ddprof-lib:checkSignalReachability`. The checker reads ELF objects -// only, so the task is skipped when the debug objects are absent or not ELF. +// 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("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")) - val objDir = layout.buildDirectory.dir("obj/main/debug").get().asFile - val script = rootProject.file("utils/check-signal-reachability.py") - commandLine("python3", script.absolutePath, "--obj-dir", objDir.absolutePath) + commandLine("python3", signalReachabilityScript.absolutePath, "--obj-dir", debugObjDir.absolutePath) - onlyIf("debug objects are ELF") { - val obj = objDir.listFiles { f -> f.name.endsWith(".o") }?.firstOrNull() - val magic = ByteArray(4) - obj != null && - obj.inputStream().use { it.read(magic) } == 4 && - magic.contentEquals(byteArrayOf(0x7f, 'E'.code.toByte(), 'L'.code.toByte(), 'F'.code.toByte())) - } + onlyIf("debug objects are ELF") { hasElfObjects(debugObjDir) } } -// Source-level quality checks (utils/check-*.py), each also runnable on its own. -// Opt-in: `./gradlew :ddprof-lib:qualityChecks`. Runs every check, then fails -// if any of them failed. +// 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 } @@ -216,7 +223,9 @@ interface QualityCheckServices { tasks.register("qualityChecks") { group = "verification" description = "Runs the signal-reachability check and the source-level quality checks in utils/" - dependsOn("checkSignalReachability") + // 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().exec val root = rootProject.projectDir @@ -237,10 +246,18 @@ tasks.register("qualityChecks") { val onLinux = System.getProperty("os.name").lowercase().contains("linux") if (onLinux) { exec.exec { - commandLine("python3", File(root, "utils/check-signal-reachability.py").absolutePath, "--self-test") + 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) diff --git a/ddprof-lib/src/main/cpp/refCountGuard.cpp b/ddprof-lib/src/main/cpp/refCountGuard.cpp index c4fa804d12..379d85d09f 100644 --- a/ddprof-lib/src/main/cpp/refCountGuard.cpp +++ b/ddprof-lib/src/main/cpp/refCountGuard.cpp @@ -85,9 +85,10 @@ RefCountGuard::RefCountGuard(void* resource) : _active(true), _is_reentrant(fals // Reentrant nesting deeper than NESTED_DEPTH; this guard's resource // 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 shows up in the - // stop-time report; the overflow stays latched per guard by the - // count itself, so the counter only advances on real overflows. + // 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 { diff --git a/doc/architecture/JvmSupportLayer.md b/doc/architecture/JvmSupportLayer.md index 09c8cb47ca..8a0ecccac3 100644 --- a/doc/architecture/JvmSupportLayer.md +++ b/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 | diff --git a/doc/architecture/RefCountGuard.md b/doc/architecture/RefCountGuard.md index 16c8862f8e..02eab4a8de 100644 --- a/doc/architecture/RefCountGuard.md +++ b/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). diff --git a/utils/README.md b/utils/README.md index 3f6b4a83b4..a77620dfe9 100644 --- a/utils/README.md +++ b/utils/README.md @@ -201,7 +201,7 @@ Standalone, stdlib-only checks over the git-tracked files. Each prints one `.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. | @@ -209,7 +209,7 @@ Standalone, stdlib-only checks over the git-tracked files. Each prints one `]+)>)') def tracked_files(root, path): @@ -81,7 +82,7 @@ def main(argv): m = _INCLUDE.match(line) if not m: continue - header = m.group(1) + header = m.group(1) or m.group(2) if header in KNOWN_EXCEPTIONS.get(rel, ()): seen.add((rel, header)) else: diff --git a/utils/check-signal-reachability.py b/utils/check-signal-reachability.py index 5c3d2dd798..580d7d1f4a 100755 --- a/utils/check-signal-reachability.py +++ b/utils/check-signal-reachability.py @@ -421,7 +421,8 @@ def expectations(tag): if cc.returncode != 0: raise EnvError("fixture compile failed:\n" + cc.stderr) allowlist = [((c,), s, "SELFTEST-1") for c, s in allowed] - new, found = check(tmp, allowlist) + new, _ = check(tmp, allowlist) + _, found = find_violations(*build_graph([obj])) got = {(c, s) for _, c, s, _ in found} reported = {(c, s) for c, s in got if (c, s) not in allowed} errors = (["MISSED %s -> %s" % e for e in sorted(expected - reported)] + diff --git a/utils/check-sigsetjmp-shape.py b/utils/check-sigsetjmp-shape.py index c16719df5e..94e1b1c7ca 100755 --- a/utils/check-sigsetjmp-shape.py +++ b/utils/check-sigsetjmp-shape.py @@ -89,7 +89,7 @@ def scope_in_effect(code, pos): else: depth -= 1 if depth == 0: - end = i + end = i + 1 if depth == 0: visible.append((0, end)) for a, b in visible: diff --git a/utils/tests/test_quality_check_scripts.sh b/utils/tests/test_quality_check_scripts.sh new file mode 100755 index 0000000000..900f52cd94 --- /dev/null +++ b/utils/tests/test_quality_check_scripts.sh @@ -0,0 +1,368 @@ +#!/usr/bin/env bash + +# Copyright 2026, Datadog, Inc + +# Hermetic tests for the utils/check-*.py quality-check scripts run by +# `./gradlew :ddprof-lib:qualityChecks`. +# Run with: bash utils/tests/test_quality_check_scripts.sh +# +# Linux-only cases (they compile the signal-reachability fixture and need GNU +# objdump) print "SKIP: (not Linux)" elsewhere; a skip counts as a pass. + +set -euo pipefail + +SCRIPT_DIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) +ROOT=$(cd "$SCRIPT_DIR/../.." && pwd) +SIGSETJMP="$ROOT/utils/check-sigsetjmp-shape.py" +BOUNDARY="$ROOT/utils/check-jvm-boundary-includes.py" +REACHABILITY="$ROOT/utils/check-signal-reachability.py" +FIXTURE="$ROOT/utils/tests/signal-reachability-fixture.cpp" +SRC=ddprof-lib/src/main/cpp +TESTS=0 +FAILED=0 + +# Importing the scripts as modules must not leave __pycache__ in the tree. +export PYTHONDONTWRITEBYTECODE=1 + +TEMP_DIR=$(mktemp -d) +trap 'rm -rf "$TEMP_DIR"' EXIT + +pass() { + TESTS=$((TESTS + 1)) +} + +fail() { + TESTS=$((TESTS + 1)) + FAILED=$((FAILED + 1)) + echo "FAIL: $*" >&2 +} + +skip() { + TESTS=$((TESTS + 1)) + echo "SKIP: $*" +} + +on_linux() { + [ "$(uname -s)" = "Linux" ] +} + +# expect [] +expect() { + local name=$1 want_rc=$2 rc=$3 out=$4 want_out=${5:-} + if [ "$rc" -ne "$want_rc" ]; then + fail "$name: exit $rc, expected $want_rc; output:"$'\n'"$out" + elif [ -n "$want_out" ] && [[ "$out" != *"$want_out"* ]]; then + fail "$name: output lacks '$want_out'; output:"$'\n'"$out" + else + pass + fi +} + +# --- T1: check-sigsetjmp-shape.py scope recognition --------------------------- + +SJ_ROOT="$TEMP_DIR/sigsetjmp" +mkdir -p "$SJ_ROOT/$SRC" +PAD_A='if (sigsetjmp(ctx, 1) != 0) { SIGNAL_HANDLER_UNWIND_AFTER_LONGJMP(); jmp_scope.restore(); return; }' + +# sigsetjmp_case ; prints output, returns the checker's exit code. +sigsetjmp_case() { + local name=$1 body=$2 + printf 'void f(ProfiledThread* pt, bool x) {\n%s\n}\n' "$body" > "$SJ_ROOT/$SRC/$name.cpp" + python3 "$SIGSETJMP" --root "$SJ_ROOT" "$SRC/$name.cpp" 2>&1 +} + +rc=0; out=$(sigsetjmp_case brace_init " JmpCtxScope jmp_scope{pt}; + $PAD_A") || rc=$? +expect "T1a brace-init guard" 0 "$rc" "$out" "OK (1 calls" + +rc=0; out=$(sigsetjmp_case paren_init " JmpCtxScope jmp_scope(pt); + $PAD_A") || rc=$? +expect "T1b paren-init guard" 0 "$rc" "$out" "OK (1 calls" + +rc=0; out=$(sigsetjmp_case block_paren " { JmpCtxScope jmp_scope(pt); } + $PAD_A") || rc=$? +expect "T1c paren-init guard in completed block" 1 "$rc" "$out" "no JmpCtxScope declared" + +rc=0; out=$(sigsetjmp_case block_brace " { JmpCtxScope jmp_scope{pt}; } + $PAD_A") || rc=$? +expect "T1d brace-init guard in completed block" 1 "$rc" "$out" "no JmpCtxScope declared" + +rc=0; out=$(sigsetjmp_case enclosing_brace " JmpCtxScope jmp_scope{pt}; + if (x) { + $PAD_A + }") || rc=$? +expect "T1e brace-init guard in enclosing block" 0 "$rc" "$out" "OK (1 calls" + +rc=0; out=$(sigsetjmp_case pad_mismatch " JmpCtxScope a{pt}; + if (sigsetjmp(ctx, 1) != 0) { SIGNAL_HANDLER_UNWIND_AFTER_LONGJMP(); b.restore(); return; }") || rc=$? +expect "T1f brace-init guard, pad name mismatch" 1 "$rc" "$out" \ + "restores 'b' but the JmpCtxScope in effect is 'a'" + +rc=0; out=$(python3 "$SIGSETJMP" 2>&1) || rc=$? +expect "T1g tracked tree" 0 "$rc" "$out" "OK (7 calls, 1 known exception(s))" + +# --- T2: check-jvm-boundary-includes.py include forms -------------------------- + +# boundary_repo [ ...]; prints the repo dir. +boundary_repo() { + local repo="$TEMP_DIR/boundary/$1" + shift + mkdir -p "$repo/$SRC" + git -C "$repo" init -q + while [ $# -gt 0 ]; do + mkdir -p "$(dirname "$repo/$SRC/$1")" + printf '%s\nvoid f() {}\n' "$2" > "$repo/$SRC/$1" + git -C "$repo" add "$SRC/$1" + shift 2 + done + echo "$repo" +} + +# run_boundary +run_boundary() { + python3 - "$BOUNDARY" "$1" "$2" <<'EOF' 2>&1 +import ast, importlib.util, sys +spec = importlib.util.spec_from_file_location("check_jvm_boundary_includes", sys.argv[1]) +mod = importlib.util.module_from_spec(spec) +spec.loader.exec_module(mod) +mod.KNOWN_EXCEPTIONS = ast.literal_eval(sys.argv[3]) +sys.exit(mod.main(["--root", sys.argv[2]])) +EOF +} + +repo=$(boundary_repo angle shared.cpp '#include ') +rc=0; out=$(run_boundary "$repo" '{}') || rc=$? +expect "T2a angle-bracket include" 1 "$rc" "$out" "shared.cpp:1: shared code includes \"hotspot/vmStructs.h\"" + +repo=$(boundary_repo quoted shared.cpp '#include "hotspot/vmStructs.h"') +rc=0; out=$(run_boundary "$repo" '{}') || rc=$? +expect "T2b quoted include" 1 "$rc" "$out" "shared.cpp:1: shared code includes \"hotspot/vmStructs.h\"" + +repo=$(boundary_repo j9zing a.cpp '#include ' b.cpp '# include ') +rc=0; out=$(run_boundary "$repo" '{}') || rc=$? +expect "T2c j9 angle include" 1 "$rc" "$out" "a.cpp:1: shared code includes \"j9/j9Support.h\"" +expect "T2c zing angle include" 1 "$rc" "$out" "b.cpp:1: shared code includes \"zing/x.h\"" + +repo=$(boundary_repo mixed1 shared.cpp '#include "hotspot/vmStructs.h>') +rc=0; out=$(run_boundary "$repo" '{}') || rc=$? +expect "T2d mixed \"...> delimiters not matched" 0 "$rc" "$out" "jvm boundary includes: OK" + +repo=$(boundary_repo mixed2 shared.cpp '#include ') +rc=0; out=$(run_boundary "$repo" '{"shared.cpp": {"hotspot/vmStructs.h"}}') || rc=$? +expect "T2f known exception via angle include" 0 "$rc" "$out" "jvm boundary includes: OK" + +repo=$(boundary_repo stale shared.cpp '#include ') +rc=0; out=$(run_boundary "$repo" '{"shared.cpp": {"hotspot/vmStructs.h"}}') || rc=$? +expect "T2g stale known exception" 1 "$rc" "$out" "no longer included" + +repo=$(boundary_repo boundary_file jvmSupport.cpp '#include ') +rc=0; out=$(run_boundary "$repo" '{}') || rc=$? +expect "T2h boundary file exempt" 0 "$rc" "$out" "jvm boundary includes: OK" + +repo=$(boundary_repo impl_dir hotspot/x.cpp '#include ') +rc=0; out=$(run_boundary "$repo" '{}') || rc=$? +expect "T2i implementation dir exempt" 0 "$rc" "$out" "jvm boundary includes: OK" + +rc=0; out=$(python3 "$BOUNDARY" 2>&1) || rc=$? +expect "T2j tracked tree" 0 "$rc" "$out" "jvm boundary includes: OK" + +# --- T3: check-signal-reachability.py --self-test (Linux) ---------------------- + +# self_test_copy []; prints stdout, +# stderr goes to $TEMP_DIR/sr-.err, returns the self-test's exit code. +self_test_copy() { + local dir="$TEMP_DIR/sr-$1" + mkdir -p "$dir/tests" + cp "$REACHABILITY" "$dir/" + cp "$FIXTURE" "$dir/tests/" + if [ -n "${2:-}" ]; then + printf '%s\n' "$2" >> "$dir/tests/signal-reachability-fixture.cpp" + fi + python3 "$dir/check-signal-reachability.py" --self-test 2> "$TEMP_DIR/sr-$1.err" +} + +if on_linux; then + rc=0; out=$(self_test_copy healthy) || rc=$? + expect "T3a healthy self-test" 0 "$rc" "$out" "self-test PASSED" + if [[ "$out" == *MISSED* ]]; then fail "T3a healthy self-test reports MISSED:"$'\n'"$out"; else pass; fi + + rc=0; out=$(self_test_copy missed '// EXPECT: CTimer::signalHandler -> fopen') || rc=$? + expect "T3b missed expectation" 1 "$rc" "$out" "MISSED CTimer::signalHandler -> fopen" + + rc=0; out=$(self_test_copy stale '// EXPECT-ALLOWLISTED: CTimer::signalHandler -> dlopen') || rc=$? + expect "T3c unused allowlist entry" 1 "$rc" "$out" "NOT FOUND (allowlisted) CTimer::signalHandler -> dlopen" + if grep -q 'not enough values to unpack' "$TEMP_DIR/sr-stale.err"; then + fail "T3c unused allowlist entry crashed: $(cat "$TEMP_DIR/sr-stale.err")" + else + pass + fi + + rc=0; out=$(self_test_copy false_positive '// EXPECT-NOT: CTimer::signalHandler -> malloc') || rc=$? + expect "T3d false positive" 1 "$rc" "$out" "FALSE POSITIVE CTimer::signalHandler -> malloc" +else + skip "T3a-T3d fixture self-test (not Linux)" +fi + +# --- T3h: self_test() evaluates the real findings (every host) ---------------- +# +# Stubs the compiler run and the object analysis so self_test()'s own logic is +# exercised without an ELF toolchain: findings must come from +# find_violations(*build_graph([obj])) inside the temp dir, and check()'s +# stale-entry list must never be treated as findings. + +run_self_test_harness() { + python3 - "$REACHABILITY" "$1" <<'EOF' 2>&1 +import contextlib, importlib.util, io, os, re, sys, types + +path, case = sys.argv[1], sys.argv[2] +spec = importlib.util.spec_from_file_location("check_signal_reachability", path) +mod = importlib.util.module_from_spec(spec) +spec.loader.exec_module(mod) + +with open(mod.SELF_TEST_FIXTURE) as f: + src = f.read() + +def pairs(tag): + return set(re.findall(r"^// %s: (.+?) -> (.+?)\s*$" % tag, src, re.M)) + +E, A, N = pairs("EXPECT"), pairs("EXPECT-ALLOWLISTED"), pairs("EXPECT-NOT") +FP = ("Profiler::instance", "malloc") +MISS = ("CTimer::signalHandler", "malloc") +if not (E and A and N) or (E & A) or (E & N) or (A & N) or FP not in N or MISS not in E: + print("FAIL: %s: fixture precondition broken (E=%r A=%r N=%r)" % (case, E, A, N)) + sys.exit(1) + +cases = { + # name findings stale new findings + "healthy": (E | A, [], E), + "stale": (E | A, [(("NoSuch::caller",), "malloc", "SELFTEST-1")], E), + "allowlisted_absent": (E, [], E), + "false_positive": (E | A | {FP}, [], E | {FP}), + "missed": ((E - {MISS}) | A, [], E - {MISS}), +} +F, stale, E_new = cases[case] + +DEFINED, GRAPH = object(), object() +state = {"obj": None, "fv_calls": 0, "problems": []} + +def fake_run(cmd, **kw): + state["obj"] = cmd[cmd.index("-o") + 1] + return types.SimpleNamespace(returncode=0, stdout="", stderr="") + +def fake_check(obj_dir, allowlist): + return ["x:%s -> %s (via y)" % p for p in sorted(E_new)], stale + +def fake_build_graph(objs): + if objs != [state["obj"]]: + state["problems"].append("build_graph(%r), expected [%r]" % (objs, state["obj"])) + if not os.path.isdir(os.path.dirname(objs[0])): + state["problems"].append("build_graph called after the temp dir was removed") + return DEFINED, GRAPH + +def fake_find_violations(defined, graph): + if defined is not DEFINED or graph is not GRAPH: + state["problems"].append("find_violations not called with build_graph's result") + state["fv_calls"] += 1 + return {"root"}, [("fixture.o", c, s, "path") for c, s in sorted(F)] + +mod._run, mod.check = fake_run, fake_check +mod.build_graph, mod.find_violations = fake_build_graph, fake_find_violations + +buf = io.StringIO() +try: + with contextlib.redirect_stdout(buf): + rc = mod.self_test() +except Exception as e: + print("FAIL: %s: self_test raised %s: %s" % (case, type(e).__name__, e)) + sys.exit(1) +if state["fv_calls"] != 1: + state["problems"].append("find_violations called %d times, expected 1" % state["fv_calls"]) +print(buf.getvalue(), end="") +for p in state["problems"]: + print("FAIL: %s: %s" % (case, p)) +print("rc=%d" % rc) +sys.exit(1 if state["problems"] else 0) +EOF +} + +# self_test_harness_case [] +self_test_harness_case() { + local name=$1 want=$2 want_out=${3:-} + local rc=0 out + out=$(run_self_test_harness "$name") || rc=$? + if [ "$rc" -ne 0 ]; then + fail "T3h-$name: harness failed:"$'\n'"$out" + elif [[ "$out" != *"rc=$want"* ]]; then + fail "T3h-$name: self_test() did not return $want:"$'\n'"$out" + elif [ -n "$want_out" ] && [[ "$out" != *"$want_out"* ]]; then + fail "T3h-$name: output lacks '$want_out':"$'\n'"$out" + else + pass + fi +} + +self_test_harness_case healthy 0 "self-test PASSED" +self_test_harness_case stale 0 "self-test PASSED" +self_test_harness_case allowlisted_absent 1 "NOT FOUND (allowlisted) wakeupHandler -> Log::warn" +self_test_harness_case false_positive 1 "FALSE POSITIVE Profiler::instance -> malloc" +self_test_harness_case missed 1 "MISSED CTimer::signalHandler -> malloc" + +# --- T4: tree-level script health and doc/build wiring ------------------------ + +rc=0; out=$(python3 "$ROOT/utils/check-doc-index.py" 2>&1) || rc=$? +expect "T4 check-doc-index.py" 0 "$rc" "$out" + +rc=0; out=$(python3 "$ROOT/utils/check-tracked-artifacts.py" 2>&1) || rc=$? +expect "T4 check-tracked-artifacts.py" 0 "$rc" "$out" + +# Built without the literal so this file never matches itself. +pat=s_nested_overflow; pat="${pat}_warned" +if out=$(git -C "$ROOT" grep -n "$pat" -- doc ddprof-lib AGENTS.md README.md utils/README.md); then + fail "T4 removed overflow-warning latch still documented:"$'\n'"$out" +else + pass +fi + +if grep -q 'opt-in; checkSignalReachability plus all four' "$ROOT/utils/README.md"; then + fail "T4 utils/README.md still calls qualityChecks opt-in" +else + pass +fi + +if grep -n 'Opt-in' "$ROOT/ddprof-lib/build.gradle.kts"; then + fail "T4 ddprof-lib/build.gradle.kts still says Opt-in" +else + pass +fi + +GRADLE="$ROOT/ddprof-lib/build.gradle.kts" +n=$(grep -c 'hasElfObjects(debugObjDir)' "$GRADLE" || true) +if [ "$n" -ne 2 ]; then fail "T4 hasElfObjects(debugObjDir) used $n times, expected 2"; else pass; fi +n=$(grep -c '0x7f' "$GRADLE" || true) +fun_line=$(grep -n '^fun hasElfObjects' "$GRADLE" | head -1 | cut -d: -f1 || true) +magic_line=$(grep -n '0x7f' "$GRADLE" | head -1 | cut -d: -f1 || true) +if [ "$n" -ne 1 ] || [ -z "$fun_line" ] || [ -z "$magic_line" ] || + [ "$magic_line" -le "$fun_line" ] || [ $((magic_line - fun_line)) -gt 6 ]; then + fail "T4 ELF magic must appear once, inside fun hasElfObjects (count=$n, fun=$fun_line, magic=$magic_line)" +else + pass +fi + +n=$(grep -cxF './gradlew :ddprof-lib:checkSignalReachability # opt-in; SKIPPED when the debug objects are not ELF' \ + "$ROOT/utils/README.md" || true) +if [ "$n" -ne 1 ]; then fail "T4 utils/README.md standalone checkSignalReachability line changed"; else pass; fi + +n=$(grep -c 'REFCOUNT_NESTED_OVERFLOW' "$ROOT/doc/architecture/RefCountGuard.md" || true) +if [ "$n" -lt 2 ]; then fail "T4 RefCountGuard.md mentions REFCOUNT_NESTED_OVERFLOW $n times, expected >= 2"; else pass; fi + +# --- Summary ----------------------------------------------------------------- + +if [ "$FAILED" -gt 0 ]; then + echo "FAIL: $FAILED/$TESTS test groups failed" >&2 + exit 1 +fi +echo "PASS: $TESTS test groups"