Skip to content

fix(ctrace): support PC sampling status markers - #2629

Merged
thorstendb-ARM merged 4 commits into
mainfrom
fix/ctrace-pc-sampling-markers
Sep 22, 2026
Merged

thorstendb-ARM merged 4 commits into
mainfrom
fix/ctrace-pc-sampling-markers

Conversation

@thorstendb-ARM

Copy link
Copy Markdown
Contributor

Fixes

  • Treat the valid Armv8-M one-byte DWT PC-sampling payload 0xFF as Trace prohibited, not a decoder error.
  • Add the missing CPU Sleeping CSV note for the one-byte 0x00 marker, following the
    PC-sampling marker specification.
  • Avoid empty Trace Compass analyses: the consumer rejects an XML stateProvider without graphical content.

Changes

  • Replace the semantic sleep boolean with explicit PC, sleep, and trace-prohibited states. Four-byte payloads remain
    PC addresses, including 0x000000FF; unsupported payloads remain errors. Arbitrary raw 0xFF bytes are not markers.
  • Preserve route, timestamp, quality, and the existing pcsample selection. CSV leaves the PC column empty for markers
    and writes the specified note.
  • Add CTF event PC_SAMPLE_PROHIBITED (ID 10) with the common sample-quality fields. The existing PC_SAMPLE binary
    layout, event IDs, and profile version remain unchanged; its state field still determines a zero-or-one PC array.
  • Close any open route-local Sleep interval at the prohibited marker without inferring running state or a prohibited
    duration. Omit companion XML when no graphical topics remain, remove stale XML, and retain the CTF event table.
  • Add a synthetic marker fixture, negative/routing/filter tests, SWO and formatted TB integration coverage, a real
    Babeltrace consumer check, and documentation of the schema and Trace Compass acceptance.

Scope is limited to ctrace. No OpenCSD dependency or shared workflow changes.

Validation

  • macOS Release: all 7 CTest entries passed.
  • Linux/GCC 13: all 8 CTest entries passed, including 358 unit tests, 34 integration tests, and Babeltrace 2.0.5.
  • CI-compatible source-line coverage: 6410/6410 lines (100%).
  • Actual Trace Compass Server 0.17.0 / TSP 0.6.0: PC → Sleep → Prohibited → PC at 1/3/6/10 microseconds; Sleep spans
    exactly [3, 6) microseconds. Marker-only input retains its table event, emits no empty XML or graphical view, and
    removes stale XML. Testing used an isolated server instance.
  • Fixture integrity, Markdownlint, and git diff --check passed.

Checklist

  • 🤖 This change is covered by unit tests.
  • 🤹 Manual consumer testing has been performed.
  • 🛡️ Security impacts have been considered; marker handling neither reconstructs a hidden PC nor bypasses payload validation.
  • 📖 Documentation updates are complete.
  • 🧠 Third-party dependencies and TPIP reviewed; no updates required.

Recognize the Armv8-M trace-prohibited marker and preserve the distinct PC, sleep, and prohibited states through CSV and CTF output.

Add PC_SAMPLE_PROHIBITED without changing the existing PC_SAMPLE wire layout. Close sleep state at the marker and omit empty Trace Compass XML analyses.

Cover SWO and formatted TB decoding, output filters, negative payload cases, consumer compatibility, and source-line coverage. Document the event schema and Trace Compass acceptance.
@github-actions

github-actions Bot commented Sep 21, 2026 •

Copy link
Copy Markdown

Test Results

    6 files  +  1      6 suites   - 33   6s ⏱️ - 3m 40s
  392 tests +207    392 ✅ +224   0 💤  - 17  0 ❌ ±0 
1 176 runs  +667  1 162 ✅ +704  14 💤  - 37  0 ❌ ±0 

Results for commit 749de96. ± Comparison against base commit 16b8fe1.

This pull request removes 185 and adds 392 tests. Note that renamed tests count towards both.
AuxCmdTests ‑ MkdirCmdTest
AuxCmdTests ‑ RmdirCmdTest
AuxCmdTests ‑ TouchCmdTest
BuildSystemGeneratorTests ‑ GenAuditFile
BuildSystemGeneratorTests ‑ GenAuditFile_WithOut_Existing_Audit_File
BuildSystemGeneratorTests ‑ GenAuditFile_With_Existing_Audit_File
BuildSystemGeneratorTests ‑ GetString
BuildSystemGeneratorTests ‑ StrConv
BuildSystemGeneratorTests ‑ StrNorm
CBuildGCCTests ‑ Asm
…
CtraceIntegTests ‑ AbortsAllOutputsOnFormattedDataBeforeFirstSourceId
CtraceIntegTests ‑ AppliesMultiValueTypeAndStreamUnionsAndTheirIntersection
CtraceIntegTests ‑ AppliesTraceRunConfiguration
CtraceIntegTests ‑ CompletesHealthyBackendWhenOtherOutputTargetHasWrongType
CtraceIntegTests ‑ ConvertsBlinkyFixtureToGoldenOutputsAndSkipsLegacyExcludedTraceBusInput
CtraceIntegTests ‑ ConvertsCapturedDwtEventCountersAcrossOverflow
CtraceIntegTests ‑ ConvertsDwtMatchAcrossCsvAndCtf
CtraceIntegTests ‑ ConvertsPcSamplingMarkersFromSwoAndFormattedTbInEveryOutputMode
CtraceIntegTests ‑ ConvertsReconstructedFormattedTraceBusFixture
CtraceIntegTests ‑ DecodesDeterministicSyntheticFormattedPacketFamiliesOnAnchorAndFallbackRoutes
…

♻️ This comment has been updated with latest results.

@jreineckearm

Copy link
Copy Markdown

@JonatanAntoni , could you please run a copilot review?

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

jreineckearm
jreineckearm previously approved these changes Sep 22, 2026

@jreineckearm jreineckearm left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, cannot manually test though as I don't have reproducers for the special PC sample packets.

Please see some comments, can be addressed in following PRs if deemed worthy.

Comment thread tools/ctrace/src/decode/DwtPacketDecoder.cpp
Comment thread tools/ctrace/test/data/trace-pc-sample/README.md Outdated
Comment thread tools/ctrace/test/integration/src/ValidateFixtureIntegrity.cmake
Comment thread tools/ctrace/README.md Outdated
Distinguish four-byte PC zero from the one-byte sleep marker in decoder, CSV and pipeline tests. Remove duplicate marker documentation and exclude Markdown from fixture checksums while retaining full test-data inventory validation. Correct the outdated CSV address-column specification note.

@jreineckearm jreineckearm left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the updates, good to merge.

Comment thread tools/ctrace/test/integration/src/ValidateFixtureIntegrity.cmake
Comment thread tools/ctrace/src/decode/DwtPacketDecoder.cpp
@codecov

codecov Bot commented Sep 22, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.51%. Comparing base (16b8fe1) to head (749de96).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2629      +/-   ##
==========================================
+ Coverage   74.48%   74.51%   +0.02%     
==========================================
  Files         197      197              
  Lines       32524    32562      +38     
  Branches    19018    19034      +16     
==========================================
+ Hits        24224    24262      +38     
+ Misses       6000     5997       -3     
- Partials     2300     2303       +3     
Flag Coverage Δ
ctrace-cov 100.00% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
tools/ctrace/src/decode/DwtPacketDecoder.cpp 100.00% <100.00%> (ø)
tools/ctrace/src/model/TraceEvent.h 100.00% <ø> (ø)
tools/ctrace/src/output/csv/CsvRowMapper.cpp 100.00% <100.00%> (ø)
tools/ctrace/src/output/ctf/CtfBundleOutput.cpp 100.00% <100.00%> (ø)
tools/ctrace/src/output/ctf/CtfEncoder.cpp 100.00% <100.00%> (ø)
tools/ctrace/src/output/ctf/CtfMetadataWriter.cpp 100.00% <100.00%> (ø)
tools/ctrace/src/output/ctf/CtfSchema.h 100.00% <100.00%> (ø)
...ls/ctrace/src/output/ctf/TraceCompassXmlWriter.cpp 100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@thorstendb-ARM
thorstendb-ARM merged commit 071e08f into main Sep 22, 2026
28 checks passed
@thorstendb-ARM
thorstendb-ARM deleted the fix/ctrace-pc-sampling-markers branch September 22, 2026 14:28
thorstendb-ARM added a commit that referenced this pull request Sep 22, 2026
Preserve TB byte-skip diagnostics while incorporating PC-sampling markers from #2629. Resolve fixture inventory, route-selection test and documentation conflicts. Verify the marker-only CSV output including NULL-source padding annotations and remove obsolete marker limitations.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants