Skip to content

ci: add Valgrind analysis tooling - #2591

Merged
xsscx merged 1 commit into
masterfrom
ci-qa-pr-docker-testing
Sep 18, 2026
Merged

xsscx merged 1 commit into
masterfrom
ci-qa-pr-docker-testing

Conversation

@xsscx

@xsscx xsscx commented Sep 18, 2026

Copy link
Copy Markdown
Member

PR Summary

Resolves: #2590

Tested: CI Boot Image

2026-09-18 14:31:22 UTC

CI Repro on ICS w/ V5 Seeding

git clone https://github.com/xsscx/research.git
cd ~/research
rm -rf iccDEV
rm -rf ics
git clone https://github.com/InternationalColorConsortium/ics.git
git clone https://github.com/InternationalColorConsortium/iccDEV.git
cd iccDEV
cd Build
git branch && echo "---" && git log --oneline -1
CC=clang CXX=clang++ CXXFLAGS="-fsanitize=address,undefined,integer,bounds,null,float-divide-by-zero,alignment,vla-bound -fno-omit-frame-pointer -g -O0" LDFLAGS="-fsanitize=address,undefined,integer,bounds,null,float-divide-by-zero,alignment,vla-bound" cmake Cmake -DCMAKE_BUILD_TYPE=Debug -DENABLE_ASAN=ON -DENABLE_UBSAN=ON -DENABLE_INTEGER_SANITIZER=ON -DENABLE_FLOAT_SANITIZER=ON -DSANITIZER_RECOVER=ON -DUBSAN_IGNORELIST="$PWD/../.github/ci/ubsan-ignorelist.txt" -DENABLE_TOOLS=ON
make -j32
cd ../Testing
export PATH="$PWD/../Build/Tools/IccApplyNamedCmm:$PWD/../Build/Tools/IccApplyProfiles:$PWD/../Build/Tools/IccApplySearch:$PWD/../Build/Tools/IccApplyToLink:$PWD/../Build/Tools/IccDumpProfile:$PWD/../Build/Tools/IccFromCube:$PWD/../Build/Tools/IccFromJson:$PWD/../Build/Tools/IccFromXml:$PWD/../Build/Tools/IccJpegDump:$PWD/../Build/Tools/IccPawgReport:$PWD/../Build/Tools/IccPngDump:$PWD/../Build/Tools/IccRoundTrip:$PWD/../Build/Tools/IccSpecSepToTiff:$PWD/../Build/Tools/IccTiffDump:$PWD/../Build/Tools/IccToJson:$PWD/../Build/Tools/IccToXml:$PWD/../Build/Tools/IccV5DspObsToV4Dsp:$PWD/../Build/Tools/IccProfileVisualize:$PATH"
export ASAN_OPTIONS='print_scariness=1:halt_on_error=1:detect_leaks=0'
export UBSAN_OPTIONS="halt_on_error=0:print_stacktrace=1:suppressions=$PWD/silence.txt"
./CreateAllProfiles.sh
./RunTests.sh
cd HDR && ./mkprofiles.sh
cd ../Display && ./RunProtoTests.sh
cd ../hybrid && ./BuildAndTest.sh
cd ../CalcTest && ./checkInvalidProfiles.sh
cd ../mcs && ./updateprev.sh
bash ./updateprevWithBkgd.sh
cd ~/research/ics/packages/ColorimetricEncoding
bash ./BuildAndTest.sh
cd ../HybridMultiSpectralEncoding
bash ./BuildAndTest.sh
cd ../HybridPrinterWithReflectance
bash ./BuildAndTest.sh
cd /home/xss/research &&
git pull --ff-only &&
git -C ics diff --quiet &&
git -C ics diff --cached --quiet &&
git -C ics pull --ff-only &&
rm -rf -- /home/xss/research/afl/iccDEV /home/xss/research/afl/bin &&
./afl/build.sh &&
ICS_ROOT=/home/xss/research/ics bash .github/scripts/validate-ics-fuzzer-seeds.sh &&
AFL_ICS_ROOT=/home/xss/research/ics ./afl/start.sh applynamedcmm --fresh --reseed --seed-order sorted --mode rare

Local Testing

2026-09-18 14:14:55 UTC

  • Success
  • Updated Valgrind helper: iccdev-valgrind-build / iccdev-valgrind-run
  • Maintainer QA tools on PATH: clang-tidy cppcheck scan-build hadolint zizmor valgrind gdb lcov gcovr shellcheck afl-fuzz
Image

Checklist

  • Signed all Commits in PR
  • Built locally according to docs/build.md
  • Followed the guidelines in Contributing document
  • Ran relevant CTest/profile tests from docs/ctest.md
  • Updated documentation for user-visible behavior changes
  • Ran sanitizer coverage for memory-safety or parser changes
  • Added or updated regression coverage for behavior changes
  • Attached a base...HEAD contract matrix for cross-cutting changes:
    producer, consumer, build/runtime behavior, platform/toolchain boundary,
    CI trigger, dependency owner, and local evidence
  • Reviewed active and suppressed automated findings from review threads and summaries
  • For Python package changes, followed docs/python-packaging-release.md for PR and merge requirements
  • Did not change maintainer-owned workflow, CTest, CPack, sanitizer, release, or security infrastructure unless requested by an iccDEV maintainer
  • New source files include the ICC copyright and BSD 3-Clause license header
  • Code style matches nearby code: 2-space indent, K&R braces, m_ members

Legal Requirements

All official software projects hosted by the International Color Consortium (ICC)
follows the open source software best practice policies. The International Color Consortium IP policy governs ICC specification development and contributions to ICC open source software. Software contributions are also covered by the Contributor License Agreement (CLA).

Contributor License Agreements

Developers who wish to contribute code to be considered for inclusion
in ICC software must first complete a Contributor License Agreement
(CLA)
.

There is no cost or membership requirement to sign the ICC Contributor License Agreement (CLA). Please note that this is different from membership in the International Color Consortium (ICC). If your organization relies on our projects, please become a member. Membership dues are an essential source of funding and investment for these projects.

  • If you are an individual writing the code on your own time and you are SURE you are the sole owner of any intellectual property you contribute, you can sign the CLA as an individual contributor.

  • If you are writing the code as part of your job, or if there is any possibility that your employer might think they own any intellectual property you create, then you should use the Corporate Contributor Licence Agreement

License

ICC software is licensed under the BSD 3-Clause "New" or "Revised" License. Contributions to ICC software projects should abide by that license unless otherwised specified or approved by the ICC.

Copyright Notices

All new source files must begin with the ICC Copyright notice and include or reference the BSD 3-Clause "New" or "Revised" License.

INTELLECTUAL PROPERTY & PATENTS

Participation in ICC's development activities is subject to ICC's Patent Policy.

Maintainer Review Required

If you have questions, contact a listed Maintainer.

@xsscx xsscx self-assigned this Sep 18, 2026
@xsscx xsscx added this to the v2.3.2.4 milestone Sep 18, 2026
@github-actions github-actions Bot added pending CI checks still running Documentation Documentation-only or documentation-related change Scripts Shell, PowerShell, or repository automation scripts Configuration Repository, CMake, YAML, JSON, or tool configuration Docker Dockerfile, container, or image workflow changes ci Continuous integration workflow changes github-actions GitHub Actions workflow or action configuration Governance Maintainer-owned policy, CODEOWNERS, prompts, or skills Unix Linux, macOS, Bash, or POSIX shell scope labels Sep 18, 2026
@xsscx
xsscx requested a balanced review from Copilot September 18, 2026 14:33

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.

🟡 Changes recommended

Sanitizer detection misses static Clang runtimes, and CI does not execute an end-to-end Valgrind lane.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Adds maintained Valgrind-family analysis tooling, container integration, and maintainer guidance.

Changes:

  • Adds a 13-target Valgrind registry and evidence workflow.
  • Exposes helpers through the unified container.
  • Documents Memcheck, Helgrind, DRD, Massif, and Callgrind usage.
File summaries
File Description
docs/valgrind-analysis.md Documents analysis workflows.
docs/regression-container.md Links container tooling.
docs/index.md Adds documentation navigation.
Dockerfile Installs Valgrind helper commands.
AGENTS.md Adds maintainer navigation.
.github/workflows/ci-docker.yml Checks container integration.
.github/skills/valgrind-analysis/SKILL.md Defines analysis guidance.
.github/skills/README.md Registers the skill.
.github/prompts/valgrind-analysis.prompt.md Adds task template.
.github/copilot-instructions.md Registers guidance resources.
.github/ci/valgrind/validate.sh Validates target definitions.
.github/ci/valgrind/targets.sh Defines analysis targets.
.github/ci/valgrind/status.sh Summarizes evidence.
.github/ci/valgrind/run.sh Runs analyzers and records results.
.github/ci/valgrind/README.md Describes the component.
.github/ci/valgrind/build.sh Builds isolated Debug targets.
.github/ci/docker/iccdev-banner.sh Advertises container helpers.
.github/agents/valgrind-analysis.agent.md Adds agent workflow guidance.
Review details
  • Files reviewed: 18/18 changed files
  • Comments generated: 2
  • Review effort level: Balanced

Comment thread .github/ci/valgrind/build.sh Outdated
Comment thread .github/workflows/ci-docker.yml
@xsscx xsscx added Copilot Copilot use indicated by Maintainer Codex Codex use indicated by Maintainer gpt-daybreak-blue Maintainer indicates use of Codex gpt-daybreak-blue Model labels Sep 18, 2026
@xsscx
xsscx requested a balanced review from Copilot September 18, 2026 14:58
@xsscx

xsscx commented Sep 18, 2026

Copy link
Copy Markdown
Member Author

Pre Merge Report

2026-09-18 14:59:24 UTC

PR bumps the Configs for QA Tooling for recent updates seen in git log

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.

🟡 Changes recommended

Target selection, failure classification, and build isolation contain unresolved correctness issues.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (2)

Previously missed (2) — in code that hasn't changed since the last review.

.github/ci/valgrind/run.sh:159

  • --allow-findings currently turns every nonzero child exit into success, not just Valgrind findings. A malformed target invocation, loader failure, abort, or crash is labeled finding here and can be accepted, contrary to the documented analyzer-only exception. Reserve the exception for Valgrind's exit code 86 (or a zero-status parsed error count), and always fail other child exits.
    .github/ci/valgrind/targets.sh:161
  • This lane exits before exercising threaded row application. MS_smCowsIcon.tif is an 8-channel multispectral image (Testing/README.md:145-149), but the first sRGB profile has three source channels, so iccApplyProfiles rejects the sample-count mismatch at Tools/CmdLine/IccApplyProfiles/iccApplyProfiles.cpp:623-640. Use the tracked 3-channel RGB TIFF for this lane; keep the multispectral TIFF for tiffdump.
  • Files reviewed: 19/19 changed files
  • Comments generated: 1
  • Review effort level: Balanced

Comment thread .github/ci/valgrind/build.sh
@xsscx

xsscx commented Sep 18, 2026

Copy link
Copy Markdown
Member Author

Codex Reply: Suppressed-comment disposition for review 5249327097 (tested against ab088fd, repaired in 636f8b9):\n\n- run.sh:159 was valid. A clean Valgrind execution with child exit 1 and zero analyzer errors was accepted by --allow-findings. It now records target-error and returns 1; only exit 86 or a zero-status parsed analyzer error count can be allowed.\n- targets.sh:161 was not valid. MS_smCowsIcon.tif declares 8 samples as RGB plus 5 extra samples. The existing compatibility path accepts the 3 color samples, and the exact applyprofiles-row Helgrind lane reached 100%, produced applied.tif, and finished rc=0/errors=0 with four worker threads. The lane is retained.\n\nNo further cloud review is being requested.

@xsscx
xsscx deployed to ghcr-publish September 18, 2026 15:19 — with GitHub Actions Active
@github-actions github-actions Bot added passed All CI checks passed and removed pending CI checks still running labels Sep 18, 2026
@xsscx
xsscx force-pushed the ci-qa-pr-docker-testing branch from 636f8b9 to 6e98ec6 Compare September 18, 2026 16:24
@github-actions github-actions Bot added pending CI checks still running passed All CI checks passed and removed passed All CI checks passed labels Sep 18, 2026
@xsscx

xsscx commented Sep 18, 2026

Copy link
Copy Markdown
Member Author

PR QA Report

2026-09-18 16:27:08 UTC

Note: Codex generated:

DRD applyprofiles-row exits 86 with 4 errors from 2 contexts. It reports condition-variable notification without the associated mutex at existing ICC library code:

  • IccProfLib/IccCmmThread.cpp:168, CIccApplyThreadedCmmPool::Apply(),
    notify_one() path.
  • IccProfLib/IccCmmThread.cpp:113,
    CIccApplyThreadedCmmPool::~CIccApplyThreadedCmmPool(), notify_all() path.

The corresponding wait paths are at lines 210-211. Helgrind reports the same registered workload clean. DRD and Helgrind generated byte-identical output TIFFs (79,224 bytes; SHA-256 b76c7642b741ffb3554ae7c3cbb2319210cb22eb04b7301e1b2b1ec0e5385b3).

This is not introduced by PR 2591. It was retained as a finding and not suppressed.

PR QA Summary

Surface Producer Consumer Boundary and behavior Trigger/owner Local proof
Target registry targets.sh build/run/validate helpers Names resolve to existing tools and durable inputs Valgrind QA maintainers 13-target validation PASS
Isolated build build.sh, common.sh Valgrind runners Separate non-sanitized Debug cache; unsafe/main paths fail closed Local/container QA path, cache, and binary probes PASS
Run evidence run.sh status.sh, users, CI Complete TSV classification; findings differ from target failures Local/container QA clean, finding, with-log, and no-log cases PASS
Self-test fixture and self-test.sh Docker workflow and maintainers Controlled subprocess behavior in source tree and installed image ci-docker.yml both execution modes PASS
Image install Dockerfile symlinks and healthcheck interactive and CI containers Installed command resolves to repository-owned scripts Image maintainers local image health/inventory PASS
Workflow gate ci-docker.yml published image Build dump lane, reject sanitizer binary, run Memcheck Docker workflow local equivalent PASS
Guidance docs, prompt, skill, agent files contributors and agents One consistent non-sanitized workflow and evidence contract Documentation owners ASCII/link/static checks PASS

@xsscx xsscx mentioned this pull request Sep 18, 2026
@xsscx
xsscx deployed to ghcr-publish September 18, 2026 16:30 — with GitHub Actions Active
@xsscx

xsscx commented Sep 18, 2026

Copy link
Copy Markdown
Member Author

Pre Merge Note (2)

2026-09-18 16:37:49 UTC

Intent

Merge

  1. Note: This PR reproduces the Mutex Issue I'd remarked in Maintainer email last week
  2. Next Steps: Finish the PoC now the Image can be burned to :latest then handoff to a Developer

@xsscx

xsscx commented Sep 18, 2026

Copy link
Copy Markdown
Member Author

Pre Merge Note (3)

2026-09-18 16:44:39 UTC

QA PR Summary

Job Success: https://github.com/InternationalColorConsortium/iccDEV/actions/runs/35368857378
Commit: 6e98ec6
Rebase: 4cd0f96
Post Merge Punch List Issue: #2592

  • Addressed the Copilot Reviews, Codex Reviews then local QA issues identified

Intent

  1. Merge
  2. Complete the Mutex PoC(s)
  3. Update Docs, Prompts, Skills, Agent Files & Artifacts

@xsscx
xsscx merged commit 8e52ccb into master Sep 18, 2026
21 checks passed
@xsscx
xsscx deleted the ci-qa-pr-docker-testing branch September 18, 2026 16:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci Continuous integration workflow changes Codex Codex use indicated by Maintainer Configuration Repository, CMake, YAML, JSON, or tool configuration Copilot Copilot use indicated by Maintainer Docker Dockerfile, container, or image workflow changes Documentation Documentation-only or documentation-related change github-actions GitHub Actions workflow or action configuration Governance Maintainer-owned policy, CODEOWNERS, prompts, or skills gpt-daybreak-blue Maintainer indicates use of Codex gpt-daybreak-blue Model passed All CI checks passed pending CI checks still running Scripts Shell, PowerShell, or repository automation scripts Unix Linux, macOS, Bash, or POSIX shell scope

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CI: Bump: AFL++, libFuzzer, CFL, Valgrind, memcheck, Helgrind, DRD, Massif & Callgrind

2 participants