Skip to content

fix(dev): stop preflight and the symbol check failing on out-of-scope input - #1475

Merged
lusoris merged 4 commits into
masterfrom
fix/preflight-m32-asan-symbols
Sep 18, 2026
Merged

lusoris merged 4 commits into
masterfrom
fix/preflight-m32-asan-symbols

Conversation

@lusoris

@lusoris lusoris commented Sep 18, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Three local checks were wrong. Two failed on input they should not judge and made make preflight report failures on a correct branch (#1474); the third never checked anything. None of them runs in CI.

  • scripts/dev/preflight.sh, 32-bit sweep. It ran gcc -m32 -fsyntax-only over every changed C file, including the arm64/, x86/ SIMD and GPU trees. The Ubuntu i686 gcc lane it mirrors never compiles those: it configures -Denable_asm=false and no GPU backend. So a change to adm_neon.c or the AVX ADM files failed the stage on arm_neon.h and _mm_extract_epi64. The missing-header filter also knew only clang's file not found and the German gcc text, but the script runs under LC_ALL=C, so gcc's No such file or directory counted as a failure. The stage now sweeps only the changed files the gcc stage's CPU build compiles (its compile_commands.json), which also leaves out the per-backend GPU tests that only build with a backend define, skips the ISA trees, and accepts that message. The gcc stage also reused any existing build/ directory, even one holding only other build trees (build/cuda, ...), and then failed at ninja; it now requires build/build.ninja.

  • core/test/check_exported_symbols.py. In a sanitizer build linked with GNU ld, libvmaf.so exports the linker-defined __start_asan_globals / __stop_asan_globals, the bounds of ASan's metadata section. lld, which the CI sanitizer lane uses, hides them. They are runtime artefacts, not API, so the checker now treats the __start_ / __stop_ bounds of the ASan, HWASan and SanitizerCoverage sections as runtime-owned. Any other section bound still fails. The file's three pre-existing ruff findings are cleared as well.

  • scripts/dev/check-cuda-extern-c.sh (ADR-0747). It never checked a kernel. Its regex expected cuModuleGetFunction(&fn, "name"), but every call passes the module first, so it collected nothing and then aborted on the empty list under set -u. Rewritten: it reads every call, including split ones, blanks comments and strings before counting braces, and names the 23 macro-generated kernels it cannot locate instead of passing them. It reports 48 of 71 located and 0 unwrapped, and fails with the kernel names when an extern "C" block is removed.

The real i686+asm limitation behind the first point (_mm_extract_epi64 in adm_avx2.c / adm_avx512.c) is unchanged and stays as ADR-0151 records it.

Type

  • feat — new feature
  • fix — bug fix
  • perf — performance improvement
  • refactor — no behavior change
  • docs — documentation only
  • test — test-only
  • build / ci — tooling / infra
  • port — cherry-pick from upstream Netflix/vmaf
  • sycl / cuda / simd — backend-specific

Checklist

  • Commits follow Conventional Commits (the commit-msg hook enforces this).
  • make format && make lint is green locally.
  • Unit tests pass: meson test -C build.
  • If I touched any SIMD/GPU code path, I ran /cross-backend-diff and the worst ULP is ≤ 2: no kernel code touched.
  • If I touched a feature extractor with SIMD/GPU twins, I either updated every twin or listed the gap under "Known follow-ups" below: none touched.
  • If I added a new .c / .cpp / .cu / .h / .hpp, it has the appropriate license header (see CONTRIBUTING.md): no new files.
  • If this is a breaking change, the commit message uses ! or BREAKING CHANGE: and the migration path is documented below: not breaking.
  • If this PR adds an ADR, the ADR row lives in docs/adr/_index_fragments/<NNNN-slug>.md and the slug is appended to docs/adr/_index_fragments/_order.txt: no ADR.

Bug-status hygiene (ADR-0165)

  • docs/state.md: T-PREFLIGHT-M32-FALSE-FAILURES-2026-09-18, T-EXPORTED-SYMBOLS-ASAN-GNU-LD-2026-09-18 and T-CUDA-EXTERN-C-CHECK-VACUOUS-2026-09-18, all closed.

Netflix golden-data gate (ADR-0024)

  • I did not modify any assertAlmostEqual(...) score in the Netflix golden Python tests.
  • If I believe a golden value must change, I have explained why below AND pinged @lusoris for a CODEOWNERS exception: none.

Deep-dive deliverables (ADR-0108)

  • Research digest — no digest needed: trivial.
  • Decision matrix — no alternatives: only-one-way fix.
  • AGENTS.md invariant note — no rebase-sensitive invariants.
  • Reproducer / smoke-test command — below.
  • CHANGELOG fragment — changelog.d/fixed/preflight-m32-asan-symbols.md, changelog.d/fixed/cuda-extern-c-check.md.
  • Rebase note — no rebase impact: fork-only tooling.

Reproducer

# on a branch that changes core/src/feature/arm64/adm_neon.c (e.g. #1474):
scripts/dev/preflight.sh --stage m32          # was FAIL on arm_neon.h, now PASS

# GNU-ld ASan build (CI links with lld, where the symbols stay hidden):
CC=clang meson setup build-asan core -Db_sanitize=address -Denable_cuda=false -Denable_sycl=false -Db_lto=false
ninja -C build-asan && meson test -C build-asan check_exported_symbols

Verified locally

Check Result
preflight.sh --stage m32 on #1474's and the GPU branch's trees FAIL before, PASS after; a planted 64-bit _Static_assert in a CPU file still fails it
check_exported_symbols.py on a GNU-ld ASan, a release and a SYCL libvmaf.so all pass
The new rule on __start_mysection, __stop_vmaf_table, a leaked function name all still reported
check-cuda-extern-c.sh on master / with one extern "C" block removed exit 0, 48 of 71 located / exit 1 naming both kernels
shellcheck, ruff, black, pre-commit on the changed files pass

Known follow-ups

  • The 23 macro-generated CUDA kernels are still verified only from the built PTX, not by the script.

… input

scripts/dev/preflight.sh swept every changed C file with gcc -m32, including
the arm64, x86 SIMD and GPU trees that the i686 lane never compiles (it
configures -Denable_asm=false and no GPU backend), and its missing-header
filter did not know gcc's English message although the script runs under
LC_ALL=C. A branch touching adm_neon.c or the AVX ADM files therefore failed
the stage on missing intrinsics and arm_neon.h. The stage now skips those
trees and accepts the message.

check_exported_symbols failed in a sanitizer build linked with GNU ld, which
exports the linker-defined __start_/__stop_ bounds of ASan's metadata
section (lld, used by the CI sanitizer lane, hides them). Those bounds are
now treated as runtime symbols; any other section bound still fails. The
file's three ruff findings are cleared too.
@github-actions github-actions Bot added the type:bug Something isn't working label Sep 18, 2026
… to check

check-cuda-extern-c.sh (ADR-0747) expected cuModuleGetFunction(&fn, "name"), but every call passes the module first, so it collected no names and then aborted on the empty associative array under set -u. The check is rewritten: it reads the name from every call, including split ones, blanks comments and strings before counting braces, and names the macro-generated kernels it cannot locate. 48 of 71 kernels located, 0 unwrapped; removing an extern "C" block makes it fail with the kernel names.
… reuse build/ only when configured

The m32 sweep still compiled per-backend GPU tests such as test_gpu_adm_tiny_frames.c, which only build with a backend define; it now takes the changed files the gcc stage's CPU build compiles (its compile_commands.json), minus the ISA trees the i686 lane leaves out. The gcc stage treated any existing build/ as configured and failed at ninja when build/ only held other build trees; it now checks for build/build.ninja. A planted 64-bit static assertion in a CPU file still fails the stage.
@lusoris
lusoris merged commit 7cc0cc9 into master Sep 18, 2026
86 of 87 checks passed
@lusoris
lusoris deleted the fix/preflight-m32-asan-symbols branch September 18, 2026 19:36
lusoris added a commit that referenced this pull request Sep 19, 2026
…rements

The CAMBI metric page's CPU SIMD section now has a per-stage table for AVX2,
AVX-512 and NEON and says why the two aarch64 stages that stay scalar (the
mask row and the mode filter) do: the compilers already vectorise those loops,
so a NEON kernel removes no work. It explains why the AVX-512 and NEON
c-values stage is so much faster than AVX2 (it skips the pixels that leave the
histogram unchanged), gives the measured per-stage and whole-frame speed-ups,
adds the aarch64 --cpumask example, and replaces the claim that the AVX-512
and NEON kernels predate the c-values layout, which was never the reason they
were undispatched. The arm backend overview lists CAMBI's NEON coverage as
full.

Research-2065 holds the method (real frames, interleaved best-of-N timing
under GCC, Clang and icx; qemu instruction counts for NEON), the per-stage and
whole-frame numbers, why the c-values walk was bound by per-column bookkeeping
rather than vector width, the register-pressure reason the AVX-512 scans stay
out of line, and the decision for each previously dead kernel.

docs/state.md closes T-CAMBI-SIMD-DEAD-KERNELS and
T-CAMBI-AVX2-PARITY-TEST-NOOP and opens T-CAMBI-AVX2-CVALUES-LLVM: in Clang
and icx builds, icx being the published container's compiler, the AVX2
c-values driver is about 0.8x of scalar. The two local-gate defects hit while
validating this branch (check_exported_symbols in an ASan build, and
preflight's 32-bit sweep on NEON sources) are fixed, with their own state
rows, in #1475. The changelog fragment, the rebase note (all of this is
fork-local; the dispatch additions must survive the next upstream rewrite of
init()) and the x86 and arm64 AGENTS.md invariants complete the set.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type:bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant