test: add fuzzer for thread local#645
Conversation
CI Test ResultsRun: #29124212473 | Commit:
Status Overview
Legend: ✅ passed | ❌ failed | ⚪ skipped | 🚫 cancelled Failed Testsglibc-aarch64/debug / 8-j9Job: View logs No detailed failure information available. Check the job logs. Summary: Total: 32 | Passed: 31 | Failed: 1 Updated: 2026-07-10 21:35:08 UTC |
There was a problem hiding this comment.
Pull request overview
Note
Copilot couldn't run its full agentic review because no GitHub Actions runner was available. Make sure your repository has a runner available to run Copilot's review, or add a copilot-setup-steps.yml file specifying one with the runs-on attribute. See the docs for more details.
Adds a new libFuzzer target to stress ThreadLocal behavior across a full thread lifecycle (including pthread TSD destructor teardown), and documents the new fuzz target.
Changes:
- Introduces
fuzz_threadLocal.cpp, a fuzz harness that spawns a fresh worker thread per input and asserts key invariants (create/free counts, value stability, bit-exact round trips). - Updates the fuzzing README to describe the new
fuzz_threadLocaltarget, its approach, and expected bug classes.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| ddprof-lib/src/test/fuzz/fuzz_threadLocal.cpp | New fuzzer target exercising ThreadLocal across thread start/exit and validating invariants. |
| ddprof-lib/src/test/fuzz/README.md | Documentation for the new fuzz target and its bug expectations. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Reliability & Chaos Results❌ 2 failure(s) detected Pipeline: https://gitlab.ddbuild.io/DataDog/java-profiler/-/pipelines/124117969 ❌ chaos: profiler jemalloc amd64 21 0 3 temXchaos❌ chaos: profiler tcmalloc amd64 21 0 3 temXchaos |
rkennke
left a comment
rkennke
left a comment
There was a problem hiding this comment.
Looks good! Thank you!
One thing you might want to add: Every other target has corpus/<target_name>/, but there's no corpus/fuzz_threadLocal/. Not required (the fuzzer will still run from an empty corpus), but a few seed inputs exercising each opcode range would speed up initial coverage discovery.
![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: 5726c0b33e
ℹ️ 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".
| Seed corpus files are in `corpus/<target_name>/`. These provide starting points | ||
| for the fuzzer to understand the expected input format. | ||
|
|
||
| `corpus/fuzz_threadLocal/` seeds a few inputs per opcode range documented in |
There was a problem hiding this comment.
The fuzz plugin strips the fuzz_ prefix when deriving fuzzName and passes File(corpusBaseDir, fuzzName) to libFuzzer, so fuzz_threadLocal.cpp runs with ddprof-lib/src/test/fuzz/corpus/threadLocal, not corpus/fuzz_threadLocal (see FuzzTargetsPlugin.kt lines 114-120 and 166-168). With the seeds committed and documented under corpus/fuzz_threadLocal, this new target silently starts from an empty corpus and never exercises the intended lifecycle/bit-pattern seeds unless the user manually points libFuzzer at the other directory.
Useful? React with 👍 / 👎.
Benchmark Results (commit 5726c0b)Pipeline: https://gitlab.ddbuild.io/DataDog/apm-reliability/benchmarking-platform/-/pipelines/124117972 Commit:
|
| Benchmark | JDK | Latest | Dev | Δ (dev vs latest) | Issues L/D |
|---|---|---|---|---|---|
| akka-uct | 21 | ✅ 10339 ms (21 iters) | ✅ 10326 ms (21 iters) | ≈ -0.1% (±11.8%) | — / — |
| akka-uct | 25 | ✅ 8820 ms (24 iters) | ✅ 8703 ms (24 iters) | ≈ -1.3% (±9.7%) | — / — |
| finagle-chirper | 21 | ✅ 5968 ms (33 iters) | ✅ 5943 ms (33 iters) | ≈ -0.4% (±25.3%) | |
| finagle-chirper | 25 | ✅ 5499 ms (36 iters) | ✅ 5510 ms (36 iters) | ≈ +0.2% (±24.5%) | |
| fj-kmeans | 21 | ✅ 2843 ms (66 iters) | ✅ 2778 ms (67 iters) | ≈ -2.3% (±2.6%) | — / — |
| future-genetic | 21 | ✅ 2027 ms (92 iters) | ✅ 2057 ms (90 iters) | ≈ +1.5% (±2.6%) | — / — |
| naive-bayes | 25 | ✅ 1016 ms (168 iters) | ✅ 1029 ms (167 iters) | ≈ +1.3% (±31.7%) | — / — |
| reactors | 21 | ✅ 16846 ms (15 iters) | ✅ 15044 ms (15 iters) | 🟢 -10.7% | — / — |
| reactors | 25 | ✅ 18785 ms (15 iters) | ✅ 18702 ms (15 iters) | ≈ -0.4% (±4.6%) | — / — |
Internal counter details (ddprof)
ddprof internal counters, latest / dev (✅ = 0, · = unavailable):
| Benchmark | JDK | Dropped rec | Dropped jvmti | Dropped trace | Skipped WC | AGCT fail | Unwind fail |
|---|---|---|---|---|---|---|---|
| akka-uct | 21 | ✅ / ✅ | ✅ / ✅ | 1 / 3 | 1948 / 2023 | ✅ / ✅ | ✅ / ✅ |
| akka-uct | 25 | ✅ / ✅ | ✅ / ✅ | 1 / 1 | 2314 / 2193 | ✅ / ✅ | ✅ / ✅ |
| finagle-chirper | 21 | ✅ / ✅ | ✅ / ✅ | 4 / 2 | 8575 / 8373 | ✅ / ✅ | ✅ / ✅ |
| finagle-chirper | 25 | ✅ / ✅ | ✅ / ✅ | ✅ / ✅ | 8534 / 8267 | ✅ / ✅ | ✅ / ✅ |
| fj-kmeans | 21 | ✅ / ✅ | ✅ / ✅ | 7 / 2 | 1267 / 1245 | ✅ / ✅ | ✅ / ✅ |
| fj-kmeans | 25 | ✅ / ✅ | ✅ / ✅ | 2 / 2 | 1261 / 1300 | ✅ / ✅ | ✅ / ✅ |
| future-genetic | 21 | ✅ / ✅ | ✅ / ✅ | 3 / ✅ | 2969 / 2968 | ✅ / ✅ | ✅ / ✅ |
| future-genetic | 25 | ✅ / ✅ | ✅ / ✅ | 2 / ✅ | 2886 / 2759 | ✅ / ✅ | ✅ / ✅ |
| naive-bayes | 21 | ✅ / ✅ | ✅ / ✅ | 5 / ✅ | 3495 / ✅ | ✅ / ✅ | ✅ / ✅ |
| naive-bayes | 25 | ✅ / ✅ | ✅ / ✅ | 7 / 6 | 3440 / 3479 | ✅ / ✅ | ✅ / ✅ |
| reactors | 21 | ✅ / ✅ | ✅ / ✅ | ✅ / ✅ | 1734 / 1502 | ✅ / ✅ | ✅ / ✅ |
| reactors | 25 | ✅ / ✅ | ✅ / ✅ | ✅ / 1 | 1952 / 1980 | ✅ / ✅ | ✅ / ✅ |
What does this PR do?:
Add fuzzing test for
ThreadLocalMotivation:
Verify the correctness of the implementation.
Additional Notes:
Target:
ThreadLocal<T, CREATE_FUNC, CLEAN_FUNC>(and thedouble/generic-pointerspecializations) - the pthread-TSD-based alternative to
thread_localused becausepthread_(get/set)specific()cannot be safely introduced mid-signal-handling on someplatforms (see the comment block in
threadLocal.h).Each input is replayed on a freshly spawned thread that is joined before returning, so
the pthread key destructor for whatever is left in a slot fires synchronously inside
that
join()- the one part of the lifecycle a persistent driver thread would neverexercise. A shadow model tracks the expected create/get/set/clear state and traps on
any mismatch.
Expected bugs: stale/mismatched values from
get(),create_tracked()/free_tracked()running the wrong number of times (double free or leak across
clear(), an overwritingset(), or thread-exit teardown),set(nullptr)failing to trigger a lazy recreate onthe next
get(), and non-bit-exact round-trips for thedoublespecialization (NaN,infinities, subnormals, -0.0).
How to test the change?:
./gradlew clean :ddprof-lib:fuzz:fuzz -Pfuzz-duration=30For 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.Unsure? Have a question? Request a review!