Skip to content

Make the valgrind CI job actually check the test binaries - #1368

Open
ACSimon33 wants to merge 7 commits into
Reference-LAPACK:masterfrom
ACSimon33:valgrind_exec_wrapper
Open

Make the valgrind CI job actually check the test binaries#1368
ACSimon33 wants to merge 7 commits into
Reference-LAPACK:masterfrom
ACSimon33:valgrind_exec_wrapper

Conversation

@ACSimon33

Copy link
Copy Markdown
Collaborator

Summary

The memory-check CI job has been reporting a clean bill of health without ever
looking at a LAPACK binary. Every test is registered as a cmake -P runtest.cmake
wrapper invocation, so ctest -T memcheck ran valgrind on cmake, and the real
test executable — started by execute_process() inside the wrapper — was never
instrumented at all.

This PR fixes the instrumentation, adds an explicit LAPACK_MEMORY_CHECK option to
configure the test suite for it, and fixes the first three defects the working
memory check reported.

The fix

  • Under LAPACK_MEMORY_CHECK on Unix the command is sh -c 'exec <binary> …'.
    exec replaces the shell instead of forking it, so exactly one process ever
    writes to the log file, and that process is the test binary.
  • Everywhere else it keeps the existing runtest.cmake wrapper unchanged, so
    Windows, multi-config generators and failing-test output all behave as before.

The three call sites — BLAS/TESTING, CBLAS/testing and TESTING — now go
through this one helper instead of each spelling out the wrapper invocation, which
is also how the four near-duplicate add_test() blocks collapse.

New option: LAPACK_MEMORY_CHECK

  • hard-requires valgrind (REQUIRED) instead of quietly doing nothing;
  • forces LAPACK_TESTING_USE_PYTHON=OFF, because the Python summary is a ctest test of its own and would otherwise have valgrind profile the interpreter rather than LAPACK;
  • switches the test commands to the exec form described above;
  • adds --trace-children=yes, and narrows --show-leak-kinds from all to definite,indirect to keep the still-reachable noise out of the report.

The CI job in .github/workflows/special.yml now passes
-D LAPACK_MEMORY_CHECK:BOOL=ON in place of the old
-D LAPACK_TESTING_USE_PYTHON:BOOL=OFF.

Defects the working memory check found

cblas_sgemv read an uninitialized TA and called Fortran with it.
The column-major branch called cblas_xerbla() for an illegal TransA and then
fell straight through to F77_sgemv(F77_TA, …) with TA never assigned — because
return was missing. cblas_xerbla does not abort under the test harness, so this
was a live uninitialized read followed by a Fortran call with a garbage TRANS
character. The row-major branch of the same routine, and both branches of dgemv,
cgemv and zgemv, already had the return; only cblas_sgemv was missing it.

IWORK was leaked by all four LIN test drivers.
{s,d,c,z}chkaa.F allocate IWORK(34*NMAX) and deallocate E, S, WORK and
RWORK on exit but never IWORK.

@codecov

codecov Bot commented Aug 24, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 69.01%. Comparing base (dd51569) to head (3989868).
✅ All tests successful. No failed tests found.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1368   +/-   ##
=======================================
  Coverage   69.01%   69.01%           
=======================================
  Files        6122     6122           
  Lines      486123   486124    +1     
  Branches    23286    23286           
=======================================
+ Hits       335514   335515    +1     
  Misses     150420   150420           
  Partials      189      189           
Components Coverage Δ
BLAS 97.94% <ø> (ø)
CBLAS 96.98% <100.00%> (+<0.01%) ⬆️
LAPACK 82.37% <ø> (ø)
LAPACKE 0.10% <ø> (ø)
TMGLIB 55.69% <ø> (ø)
BLAS testing 88.33% <ø> (ø)
CBLAS testing 89.63% <ø> (ø)
LAPACK testing 82.35% <ø> (ø)
LAPACKE testing ∅ <ø> (∅)
Files with missing lines Coverage Δ
CBLAS/src/cblas_sgemv.c 100.00% <100.00%> (ø)
TESTING/LIN/cchkaa.F 74.32% <ø> (ø)
TESTING/LIN/dchkaa.F 73.91% <ø> (ø)
TESTING/LIN/schkaa.F 73.91% <ø> (ø)
TESTING/LIN/zchkaa.F 73.02% <ø> (ø)

Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dd51569...3989868. Read the comment docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant