Skip to content

Fix: CI leak regressions silently disabled by detect_leaks=0 (false green)#1351

Merged
xsscx merged 1 commit into
masterfrom
fix-ci-leak-regression-detect-leaks
Jun 14, 2026
Merged

Fix: CI leak regressions silently disabled by detect_leaks=0 (false green)#1351
xsscx merged 1 commit into
masterfrom
fix-ci-leak-regression-detect-leaks

Conversation

@colourbill-ctrl

Copy link
Copy Markdown
Contributor

The bug

The CI tool-test job (ci-iccdev-tool-tests.yml, "Run CTest tool suites" step) and ICCDEV_TEST_ENV both export ASAN_OPTIONS=...,detect_leaks=0 to silence unrelated leaks in the smoke-tested tools. The C++ LeakSanitizer-based leak regressions inherit that and run their add_test binary directly with no override — so LeakSanitizer is disabled for exactly the tests whose pass/fail signal is the at-exit leak scan. They pass even when the leak they exist to catch is present (a false green).

The script-based leak regressions (#1336/#1337) are unaffected because they re-set detect_leaks=1 internally before invoking the tool.

Proof

For iccdev.xform-create-tag-ownership (#1308) on pre-fix master, clean build:

  • detect_leaks=0 (CI's env) → EXIT 0, false PASS — 608-byte leak masked
  • detect_leaks=1 → 608-byte leak reported, EXIT 1

The fix

Launch each LSan leak regression through cmake -E env ASAN_OPTIONS=detect_leaks=1 (the same override the script tests use), forcing leak detection back on for that test's own process. This PR fixes the cmmsearch-namedcolor-ownership (#1332) test, which is already on master; the sibling xform-create-tag-ownership (#1308) gets the identical fix in its own PR #1348.

Two subtleties (both verified, both encoded in the comment):

  • Use only detect_leaks=1. Appending halt_on_error=0 makes the runtime print the leak but exit 0 — ctest would still pass.
  • Use a comma separator, not :detect_leaks=1:... is mis-parsed and silently leaves leak detection off.

Verification (clean build, CI env detect_leaks=0 exported)

iccdev.cmmsearch-namedcolor-ownership:

So the test now genuinely guards against a future regression instead of passing unconditionally.

🤖 Generated with Claude Code

…ion (#1332)

Same CI blind spot fixed for the #1308 regression in #1348: the tool-test job
and ICCDEV_TEST_ENV export ASAN_OPTIONS=...detect_leaks=0 to silence unrelated
tool leaks, so this LSan-based leak regression inherited detect_leaks=0 and
would PASS even if CIccCmmSearch::AddXform leaked the rejected NamedColor
profile -- the exact regression it exists to catch.

Launch it through `cmake -E env ASAN_OPTIONS=detect_leaks=1` so leak detection
is forced on for the test's own process.  Only detect_leaks=1: halt_on_error=0
would make the leak print but exit 0, and a ':' separator is mis-parsed and
silently disables the check.

Verified on a clean build under the CI env (detect_leaks=0 exported): the test
PASSES with the #1333 fix present and FAILS with the 568-byte leak when the
NamedColor-reject free is reverted -- i.e. it now actually guards against a
future regression.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added Testing CTest, regression, or test coverage Configuration Repository, CMake, YAML, JSON, or tool configuration Build Build system, CMake, compiler, or packaging pending CI checks still running labels Jun 14, 2026

@xsscx xsscx left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

2026-06-14 21:38:35 UTC

@github-actions github-actions Bot added passed All CI checks passed and removed pending CI checks still running labels Jun 14, 2026
@xsscx xsscx merged commit dba9896 into master Jun 14, 2026
30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Build Build system, CMake, compiler, or packaging Configuration Repository, CMake, YAML, JSON, or tool configuration passed All CI checks passed Testing CTest, regression, or test coverage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants