Repository navigation
Conversation
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.
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).
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 <scope>.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.
…, self-test wiring, doc-index reverse check
…ontext 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.
jbachorik
marked this pull request as ready for review
October 23, 2026 16:06
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Collaborator
Author
|
@DataDog please fix my ci |
|
I can only run on private repositories. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 03a07a75a6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![datadog-official[bot]](https://avatars.githubusercontent.com/in/1358509?s=60&v=4){kind=small}
There was a problem hiding this comment.
Linux qualityChecks fails on a healthy checkout because its self-test treats stale allowlist entries as findings. The source checks also reject valid brace initialization and miss angle-bracket JVM includes.
🤖 Bits Code Review · Commit 03a07a7
Contributor
CI Test ResultsRun: #39993890183 | Commit:
Status Overview
Legend: ✅ passed | ❌ failed | ⚪ skipped | 🚫 cancelled Summary: Total: 32 | Passed: 32 | Failed: 0 Updated: 2026-10-23 17:52:36 UTC |
Contributor
Collaborator
Author
|
Auto-expanded scope: 2 sibling issue(s) fixed in the same pass:
|
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Adds deterministic quality checks enforcing the AGENTS.md signal-safety and
documentation rules:
utils/check-signal-reachability.py: builds a per-function call graph fromthe compiled ELF objects (function symbols + call/TLS relocations, COMDAT
sections, C1/C2 constructor aliases) and reports every path from a signal
handler or malloc/free hook to an async-signal-unsafe sink (allocation,
blocking locks, TLS access, stdio, loader/JNI). Found the two real
violations now tracked as PROF-16313 and PROF-16314 (both in the built-in
allowlist until fixed).
utils/check-{jvm-boundary-includes,doc-index,sigsetjmp-shape,tracked-artifacts}.py:the cheap source-level checks — JVM boundary includes against the documented
exception list, doc index coverage + PascalCase naming in both directions,
sigsetjmplanding-pad shape, and no tracked session artifacts.utils/tests/signal-reachability-fixture.cpp: planted-violation fixture;--self-testcompiles it and asserts every planted violation is reported,allowlisted ones suppressed, negative controls clean. Wired into
qualityChecks(Linux only; the checker is ELF-only).:ddprof-lib:checkSignalReachability(runs on the debugobjects; SKIPPED on non-ELF/macOS) and
:ddprof-lib:qualityChecksaggregating everything. Neither is wired into
check/buildyet, becausePROF-16313/PROF-16314 are open.
Motivation
The AGENTS.md invariants (async-signal safety, JVM abstraction boundary, doc
index rules) had no mechanical enforcement — violations surfaced only through
manual review. A per-file regex lint is useless here (measured ~99% false
positives: a file is not a context; reachability is), so the core check works
on the call graph instead. The checker found 2 real signal-path violations on
its first run across macOS arm64, Linux arm64 and Linux x64 objects.
Additional Notes
2 environment error.
--self-testkeeps the checker honest acrosstoolchain/objdump changes.
debug objects; allowlist-emptied task fails; mutation-injected violations in
every check are caught.
findings; the 3 HIGH ones were untracked scratch never committed; the rest
addressed in
c966c5ea7(sink coverage for aligned new/delete and libcaliases, allowlist encoding/strictness, self-test wiring, doc-index reverse
check,
CXXargument handling,docs/sphinxboundary, named-task wiring).assuming they initialize before handlers are installed; that ordering is not
verifiable from relocations and is documented in the script.
How to test the change?
./gradlew :ddprof-lib:qualityChecks(source checks everywhere; self-test onLinux) and
./gradlew :ddprof-lib:checkSignalReachability(needs ELF debugobjects). Both are opt-in. Mutation testing: inject
malloc, athread_local,or
SpinLock::lock()into any signal-handler-reachable function and thechecker reports it.
For Datadog employees:
credentials of any kind, I've requested a security review (run the
dd:platform-security-reviewskill, or file a request via the PSEC review form).
bewairealso runs automatically on every PR.check.