Conversation
2 tasks
Contributor
Scan-Build Report
Bug Summary
Reports
|
||||||||||||||||||||||||||||||||||||
Contributor
CI Test ResultsRun: #35281021055 | Commit:
Status Overview
Legend: ✅ passed | ❌ failed | ⚪ skipped | 🚫 cancelled Summary: Total: 32 | Passed: 32 | Failed: 0 Updated: 2026-09-17 22:32:47 UTC |
Contributor
jbachorik
added this pull request to stack #803
September 17, 2026 20:02
jbachorik
marked this pull request as ready for review
September 17, 2026 20:03
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. |
jbachorik
force-pushed
the
jb/rc-2-jfr-chain-events
branch
from
September 17, 2026 20:05
ab3b0eb to
4534a7f
Compare
![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: ab3b0ebbe1
ℹ️ 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.
Normal liveness events can write an unknown leak tag. A long reference chain can also exceed the fixed JFR buffer and damage memory.
🤖 Datadog Autotest · Commit ab3b0eb · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest
jbachorik
force-pushed
the
jb/rc-2-jfr-chain-events
branch
from
September 17, 2026 20:18
4534a7f to
71320e1
Compare
The null-pointer store is the deliberate never-returning crash, but clang scan-build reports a null dereference. Under __clang_analyzer__, replace it with __builtin_trap() - the analyzer understands the trap as a halt and explores no false path past it; the real build keeps the store.
Introduces the ReferenceChainEvent/ReferenceChainAbandonedEvent payloads (event.h), their JFR metadata (jfrMetadata.*), and the FlightRecorder emission paths that serialize chain events into JFR recording buffers, including the constant-pool handling for per-hop edge labels. Emission is pull-style: profiler.cpp snapshots events and hands them to FlightRecorder; this layer does not depend on the tracker itself.
jbachorik
force-pushed
the
jb/rc-2-jfr-chain-events
branch
from
September 17, 2026 20:20
71320e1 to
80173ad
Compare
Review findings on the JFR plumbing layer: - MAX_REFERENCE_CHAIN_EVENT_HOPS was a fixed 4096, permitting a ~438 KB worst-case event (near-limit edge labels) against a ~61 KB recording buffer - the reservation flushed first but the margin underflowed, so the write ran past the buffer (debug assert, release corruption). The cap is now derived from RECORDING_BUFFER_LIMIT minus the event's fixed fields, divided by the per-hop worst case, so a full-cap event always fits. - Truncation dropped ALL edge labels: the label count was gated on _edges.size() == emitted_size, which only holds for untruncated chains. Labels align with the chain's leaf-first element order, so truncation now emits the first emitted_size labels and loses only the root-side ones. - ObjectLivenessEvent::leak_tag is default-initialized to 0 so any construction path that forgets to set it serializes a defined untagged value (flush_table() overwrites it from the entry, which track() zeroes at insert). Moves the JFR round-trip and arguments parsing unit tests into this layer (they test exactly this code), rewrites the round-trip test to construct events directly instead of through the tracker, and adds byte-level boundary tests: oversize-chain truncation with label preservation, the size-prefix invariant, and the default leak tag.
jbachorik
force-pushed
the
jb/rc-2-jfr-chain-events
branch
from
September 17, 2026 22:14
339be6e to
d51dfb8
Compare
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
1 participant
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 the JFR plumbing for reference-chain events: the
ReferenceChainEvent/ReferenceChainAbandonedEventpayloads (event.h), their dynamic JFR metadata and constant-pool handling (jfrMetadata.*), the FlightRecorder emission paths that serialize chain events and per-hop edge labels into recording buffers (flightRecorder.*), and thereferencechainsArgumentsblock (arguments.*) that configures the feature.Motivation:
Part 2 of the stacked series for reference chains (PROF-15341). Emission is pull-style — the later tracker layer snapshots events and hands them to FlightRecorder — so this layer is independently reviewable and compiles/links without the tracker.
Additional Notes:
Stacked series for PROF-15341. Merge order: #804 (scan-build crashNow fix) -> this PR -> #797 -> #798 -> #799 -> #800 -> #801 -> #802. #795 (UBSan ELF fix) is already merged. Until #804 lands, this PR's diff additionally shows the small faultInjection.cpp change from #804.
How to test the change?:
The JFR round-trip unit tests for chain events land in the gtest PR later in this stack;
buildDebugcompiles and links this layer standalone.For Datadog employees: