Fix: CI leak regressions silently disabled by detect_leaks=0 (false green)#1351
Merged
Conversation
…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>
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.
The bug
The CI tool-test job (
ci-iccdev-tool-tests.yml, "Run CTest tool suites" step) andICCDEV_TEST_ENVboth exportASAN_OPTIONS=...,detect_leaks=0to silence unrelated leaks in the smoke-tested tools. The C++ LeakSanitizer-based leak regressions inherit that and run theiradd_testbinary 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-setdetect_leaks=1internally 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 maskeddetect_leaks=1→ 608-byte leak reported, EXIT 1The 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 thecmmsearch-namedcolor-ownership(#1332) test, which is already on master; the siblingxform-create-tag-ownership(#1308) gets the identical fix in its own PR #1348.Two subtleties (both verified, both encoded in the comment):
detect_leaks=1. Appendinghalt_on_error=0makes the runtime print the leak but exit 0 — ctest would still pass.:—detect_leaks=1:...is mis-parsed and silently leaves leak detection off.Verification (clean build, CI env
detect_leaks=0exported)iccdev.cmmsearch-namedcolor-ownership:deletereverted → FAIL,568 byte(s) leaked in 10 allocationsSo the test now genuinely guards against a future regression instead of passing unconditionally.
🤖 Generated with Claude Code