Repository navigation
fix(ci): repair master red — TSan operator-new dup + revert #1060 ARM golden drift - #1063
Merged
Merged
Conversation
… golden drift Two regressions rode into master via the 2026-06-27 admin-merge batch (per-PR CI was bypassed). Both are required-check failures. 1. Sanitizers (TSan) link failure. The R2-9 OOM-injection test `test_gpu_dispatch_env_oom.cpp` replaces the global `operator new` / `operator delete`; under TSan/ASan/MSan the sanitizer runtimes interpose their own, so lld reports `duplicate symbol: operator new(unsigned long)`. Guard the override and self-skip the test under `__SANITIZE_THREAD__`/`__SANITIZE_ADDRESS__`/`__SANITIZE_MEMORY__` and the Clang `__has_feature` equivalents. The R2-9 slot-poisoning check still runs in every non-sanitized suite, so coverage is retained. 2. ARM golden drift. PR #1060's aarch64-only `-ffp-contract=off` guard on the scalar `adm_dwt2_s` / `adm_dwt2_lo_s` shifted the akiyo `disable_enhn_gain` ADM score on the ARM build matrix (88.030463 -> 88.030322), failing the `vmafexec_test.py` golden assertions. x86 D24 was unaffected (the guard was `ARCH_AARCH64`-gated), which is why it was not caught pre-merge. Per global rule #1 (immutable golden; fix the code, never the assertion) the scalar guard is reverted: `adm_tools.c` returns to its pre-#1060 FMA-default scalar arithmetic on aarch64 (byte-identical to 2d2f452^), restoring 88.030463. The now-stale `test_float_adm_simd.c` parity test and its meson registration are removed. The NEON-vs-scalar parity gap returns to the undispatched/FMA-free state of ADR-1057; T-NEON-FMA-FLOAT-ADM-DWT2-2026-06-06 stays Open for a correct re-attempt (make the NEON side match the scalar FMA reference; validate against the full ARM quality suite, not only x86 golden). Docs: ADR-1057 Update (2026-06-27) records the reverted follow-up and the lesson; docs/state.md adds the Recently-closed row (and fixes a stale ADR-1057 link); changelog fragment replaced; arm64 AGENTS.md + research digest restored. Verified: CPU build OK; fast suite 105/105; `test_gpu_dispatch_env_oom` builds and passes (override active in the non-sanitized config); `git diff 2d2f452^ -- core/src/feature/adm_tools.c` empty; clang-format, assertion-density, check-copyright clean on touched files. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
lusoris
force-pushed
the
fix/master-ci-tsan-arm-golden
branch
from
June 27, 2026 19:46
bf57164 to
edbc0f1
Compare
…supported_arguments
The third required-check failure on master (pre-existing): the
`test_gpu_dispatch_env_oom` meson target passed the raw GCC/Clang flag
`-Wno-write-strings` unconditionally. MSVC `cl` parses `/W` as a warning-level
prefix, so it rejects the flag with
`D8021: invalid numeric argument '/Wno-write-strings'`, failing
`Build — Windows MSVC + CUDA (build only)`. Route the flag through
`cxx.get_supported_arguments('-Wno-write-strings')` so MSVC drops it while
GCC/Clang keep it (the flag is only needed to silence -Wwrite-strings from the
shared upstream `mu_assert` macro on the C++ TU).
A full grep confirms `-Wno-write-strings` was the only raw `-Wno-*` reaching the
compiler directly; `-ffp-contract=off` elsewhere is only a D9002 warning on MSVC
(ignored), and the lone `-Wl,-framework` is macOS-gated.
Verified: CPU build OK; fast suite 105/105; the flag still applies on Linux clang
(get_supported_arguments returns it). Completes the master-CI repair started in
this PR (Sanitizers-thread + D24 golden already green on the PR run).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…] (supersede flag hack) Follow-up to the get_supported_arguments(-Wno-write-strings) change: dropping the flag on MSVC merely exposed the real, hard error it was masking — test_gpu_dispatch_env_oom.cpp(145): error C2440: 'return': cannot convert from 'const char [70]' to 'char *' test.h's `mu_assert` does `return message;`. The harness is C-first, where a string literal -> `char *` is legal; this is the ONLY test whose body is a C++ TU using mu_assert, and under MSVC /std:c++latest that conversion is a hard C2440 (no flag silences an error). The other `.cpp` test executables either build from `.c` bodies (test_dict/model/feature) or pull in library `.cpp` sources with no mu_assert, so they are unaffected — this file is uniquely exposed. Fix: hold the two assert messages in `static char[]` buffers. A mutable array decays to `char *` with no conversion (no C2440, no -Wwrite-strings), and static storage keeps the returned pointer valid after the function returns. The obsolete `-Wno-write-strings` cpp_args is removed. Compiles identically on MSVC / GCC / Clang. Verified: CPU build OK; fast suite green; test_gpu_dispatch_env_oom builds + passes; clang-format clean. Completes the master-CI repair (all three required failures: TSan dup-symbol, ARM golden drift, Windows MSVC C2440). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
4 of 7 tasks
lusoris
added a commit
that referenced
this pull request
Aug 30, 2026
…57) (#1134) The aarch64 Netflix golden failure (akiyo 88.030322 vs 88.030463, delta 1.41e-04, three failing assertions on Build - Ubuntu ARM clang (CPU)) is an integer off-by-one, not the FMA-contraction problem ADR-1057 describes. adm_dwt2_8_neon special-cases the j = 0 output column because its source indices are mirrored ({1, 0, 1, 2} rather than the regular 2j-1..2j+2 stride). That special case accumulated with for (int idx = 0; idx < 3; idx++) while the scalar reference adm_dwt2_8 in core/src/feature/integer_adm.c multiplies all four taps (filter_lo[0..3] against s0..s3 from ind_x[0..3][j]). The NEON path therefore dropped ind_x[3][0] and its coefficient -4240 from the first output column of every DWT2 row, on every scale. Fix: idx < 4. Why ADR-1057's diagnosis was wrong: integer ADM accumulates in int64, so FP contraction cannot affect it. Building the entire tree with -ffp-contract=off on aarch64 leaves the score at 88.030322, byte-for-byte unchanged. That is why PR #1060's fp-contract approach could not have worked and #1063 reverted it. Reproduced and fixed locally without a CI round-trip, using an aarch64-linux-gnu-gcc cross-build under qemu-aarch64-static: NEON integer_adm2=1.116698 adm3=1.052571 scale3=1.13728 -Denable_asm=false integer_adm2=1.116701 adm3=1.052573 scale3=1.13729 NEON + -ffp-contract=off 88.030322 (unchanged - rules out FP contraction) after this fix 38 passed, 1 skipped, 0 failed on aarch64 (vmafexec_test.py + result_test.py, all akiyo golden assertions included) x86 is unaffected: the change is confined to core/src/feature/arm64/adm_neon.c. No NEON-vs-scalar parity test covers the ADM DWT2 kernels on any architecture - test_integer_adm_simd.c only exercises adm_cm_avx2 - which is how a three-vs-four tap error shipped. Only the ARM golden leg caught it, and that leg is not a required check, so it stayed red. Both gaps are worth closing separately. Co-authored-by: Lusoris <lusoris@pm.me> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
5 of 9 tasks
lusoris
added a commit
that referenced
this pull request
Sep 16, 2026
22 inline code-scanning findings on #1425, triaged one at a time rather than dismissed or suppressed in bulk. Five are real: - `read_luma8()` guarded `shift == 0` after the `bitdepth == 8` case had already returned, so the arm was unreachable. Removing it also makes visible that `shift` is 2, 4 or 8 there and `shift - 1U` cannot underflow. - `check_filtered_center()` and `check_spatial_mask_first_image()` took `VmafPicture` by value and then took its address; both take a pointer. - `test_vif_lifecycle.c` and `test_vmaf_roi_bounds.c` compared floats with `==`. Exact equality is the *intent* — bit-exact frame differencing, exactly symmetric saliency — so loosening them to a tolerance would hide the defect they exist to catch. They now compare bit patterns, the same shape `test_feature_isa_invariance.c` already documents for the same reason. Fifteen are false positives, each checked rather than assumed: - every `cpp/unused-static-function` hit is called. The `pdjson.c` helpers are called directly; `json_begin_error` only through the `json_error()` macro; the thread-pool helpers through `#define pthread_create observed_create` combined with `#include "../src/thread_pool.c"`, an interposition CodeQL cannot follow. gcc with `-Wunused-function` reports none of them. - the `cpp/constant-comparison` hits in `feature_extractor.cpp` and `fex_ctx_vector_internal.h` are `SIZE_MAX / sizeof(T)` overflow guards. They are provably dead on 64-bit, which is what CodeQL sees, and live on i686 — a target `libvmaf-build-matrix.yml` gates. Deleting them to quiet a note would be a real 32-bit regression. The two Semgrep findings are #1062/#1063, already dismissed earlier; their PR comments are stale, because GitHub applies an in-source suppression only at alert-creation time. 148/148 tests pass. Co-Authored-By: Claude Opus 5 (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
1 participant
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.
Summary
Two regressions rode into
mastervia the 2026-06-27 admin-merge batch (per-PR CI bypassed). Both are required-check failures; this PR fixes both at the root.Sanitizers (TSan) link failure. The R2-9 OOM-injection test
core/test/test_gpu_dispatch_env_oom.cppreplaces the globaloperator new/operator delete. Under TSan/ASan/MSan the sanitizer runtimes interpose their own, so lld reportsduplicate symbol: operator new(unsigned long). The override + the test now self-skip under__SANITIZE_THREAD__/__SANITIZE_ADDRESS__/__SANITIZE_MEMORY__and the Clang__has_featureequivalents. The R2-9 slot-poisoning check still runs in every non-sanitized suite, so coverage is retained.ARM golden drift. PR fix(arm64): FMA-safe float-ADM DWT2 bit-exactness — guard scalar adm_dwt2_s (closes T-NEON-FMA, ADR-1057) #1060's aarch64-only
-ffp-contract=offguard on the scalaradm_dwt2_s/adm_dwt2_lo_sshifted the akiyodisable_enhn_gainADM score on the ARM build matrix (88.030463→88.030322), failingvmafexec_test.pygolden assertions. x86 D24 was unaffected (the guard wasARCH_AARCH64-gated → not caught pre-merge). Per global rule chore(meta): post-cutover URL sweep — lusoris/vmaf → VMAFx/vmafx #1 (immutable golden; fix the code, never the assertion) the scalar guard is reverted —adm_tools.cis byte-identical to its pre-fix(arm64): FMA-safe float-ADM DWT2 bit-exactness — guard scalar adm_dwt2_s (closes T-NEON-FMA, ADR-1057) #1060 state (2d2f45283e^), restoring88.030463. The now-staletest_float_adm_simd.cparity test + its meson registration are removed. The NEON-vs-scalar parity gap returns to the undispatched/FMA-free state of ADR-1057;T-NEON-FMA-FLOAT-ADM-DWT2-2026-06-06stays Open for a correct re-attempt.Reproducer / smoke-test
Deep-dive deliverables (ADR-0108)
core/src/feature/arm64/AGENTS.mdrestored to pre-fix(arm64): FMA-safe float-ADM DWT2 bit-exactness — guard scalar adm_dwt2_s (closes T-NEON-FMA, ADR-1057) #1060; the "don't alter the scalar golden path to chase NEON parity" lesson is recorded in ADR-1057 Update (2026-06-27).changelog.d/fixed/master-ci-tsan-arm-golden.md.adm_tools.c(reduces rebase surface, adds none).state.md
T-MASTER-CI-TSAN-ARM-GOLDEN-2026-06-27Recently-closed row added;T-NEON-FMA-FLOAT-ADM-DWT2-2026-06-06confirmed still Open; stale ADR-1057 link fixed.Docs / golden
no docs needed: internal test/SIMD-parity revert with no user-visible delta.🤖 Generated with Claude Code