CI: add sanitizer build job (ASAN/UBSAN) - #131
Draft
mikeller wants to merge 2 commits into
Draft
Conversation
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The sanitizer flags are not reliably propagated to the make distcheck rebuild, so the new job may not actually run the distcheck build under ASAN/UBSAN as intended.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds a new GitHub Actions job intended to run the project’s build and make distcheck under AddressSanitizer/UndefinedBehaviorSanitizer to catch memory errors in CI.
Changes:
- Add a new
sanitizersjob onubuntu-latestusingclang. - Configure ASAN/UBSAN runtime options and run
make distcheckin CI.
File summaries
| File | Description |
|---|---|
| .github/workflows/build.yml | Adds a new CI job to build and run distcheck under sanitizers. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Add a 'sanitizers' job that builds with -fsanitize=address,undefined using clang on ubuntu-latest and runs make distcheck under the sanitizer. ASAN_OPTIONS=detect_leaks=1 and abort_on_error=1 ensure any detected leak or memory error fails the build immediately. Full parser coverage under the sanitizer requires sample dump fixtures and a parse harness to be wired into this job in a follow-up. Signed-off-by: Michael Keller <github@ike.ch>
mikeller
force-pushed
the
feat/ci-asan-#94
branch
from
September 9, 2026 02:52
99f7fdd to
f58dc0c
Compare
Add raw per-dive binary dumps (captured via dctool download) for three device families, together with a manifest and a shell harness that runs dctool parse over each fixture and fails on any non-zero exit. test/fixtures/manifest.txt Maps each blob to the exact dctool descriptor string used at capture and required at replay. Format: filename|vendor|product. test/fixtures/shearwater_petrel2-0001.bin (21 KB) Shearwater Petrel 2, SHEARWATER_PETREL family, CCR dive, no GPS. Descriptor: 'Shearwater Petrel 2', transport: bluetooth (classic). test/fixtures/hw_ostc5-0001.bin (15 KB) Heinrichs Weikamp OSTC 5, HW_OSTC3 family, CCR dive, no GPS. Descriptor: 'Heinrichs Weikamp OSTC 5', transport: bluetooth (classic). test/fixtures/garmin_descent_mk2i-0001.bin (12 KB) Garmin Descent Mk2i, GARMIN family, no GPS. Descriptor: 'Garmin Descent Mk2(i)/Mk3(i)(S)/G1/G2/X50i', transport: USB storage (MTP). All three families use dc_field_add_string / dc_field_cache, which is the code path affected by the field-cache leak reported upstream. Parsing all three fixtures under an ASAN build with detect_leaks=1 reproduces the leak without the fix applied; with the fix the run is clean. test/run-parse.sh Locates the built examples/dctool, reads the manifest, and runs 'dctool -d "<descriptor>" parse -o /dev/null <blob>' for each entry. Exits non-zero if any invocation returns non-zero. Under the sanitizer build the ASAN abort_on_error flag turns any detected error into a non-zero exit, so a leak or memory error surfaces as a failed parse step. Wire the harness into the sanitizers CI job with: - run: test/run-parse.sh No autotools make-check infrastructure is introduced; the script is a standalone entry point deliberately kept small and reviewable. Dive data is the device owner's own recordings, cleared for redistribution under the library licence. No GPS or personal location data is present in any of the committed blobs. Signed-off-by: Michael Keller <github@ike.ch>
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The sanitizer job wiring and the new fixture parse harness align with existing dctool descriptor selection behavior and don’t introduce any verified functional issues.
Review details
- Files reviewed: 3/6 changed files
- Comments generated: 0 new
- Review effort level: Lite
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.
Add a 'sanitizers' job that builds with -fsanitize=address,undefined
using clang on ubuntu-latest and runs make distcheck under the
sanitizer. ASAN_OPTIONS=detect_leaks=1 and abort_on_error=1 ensure any
detected leak or memory error fails the build immediately.
Full parser coverage under the sanitizer requires sample dump fixtures
and a parse harness to be wired into this job in a follow-up.