Skip to content

Fix work buffer overflow in threaded SYRK/HERK for large N - #6235

Open
apullin wants to merge 5 commits into
OpenMathLib:developfrom
apullin:issue6224
Open

apullin wants to merge 5 commits into
OpenMathLib:developfrom
apullin:issue6224

Conversation

@apullin

@apullin apullin commented Nov 7, 2026

Copy link
Copy Markdown
Contributor

Fixes #6224.

Each thread of the threaded SYRK/HERK driver packs one K block of op(A) for its whole row range behind sb in its work area.
For the widest thread that is about GEMM_Q * N / sqrt(nthreads) elements.
Nothing limited it to the space left in the BUFFER_SIZE work area, so for large N it wrote past the end (in #6224: into C and its malloc header).
Threaded ?POTRF uses the same driver and was affected too.

The threshold scales with BUFFER_SIZE: 32 MiB in scipy-openblas (BUFFERSIZE=20), 128 MiB by default.
A default x86_64 build overflows too, at larger N (Haswell kernels, 2 threads: from N ≈ 92,000).

The fix shortens the K block until the widest thread's panels fit behind sb.
All threads use the same blocking, since they read each other's panels.
Where the panels already fit, nothing changes.

Tests:

  • utest/test_syrk_thread.c checks threaded ?SYRK/?HERK (all variants) and ?POTRF against an exact integer reference; with a work area of 16 MiB or less (32-bit x86) it also runs a ZHERK that overflowed.
  • utest/test_syrk_thread_workarea.c calls the threaded driver with a work area the test owns, at the smallest N that overflowed (a few seconds natively; needs the static library; skipped above OPENBLAS_UTEST_TIME_LIMIT, default 10 s, e.g. under qemu). It uses driver internals, so it is a separate commit you can drop.

Tested on develop c2fc6cf, before → after:

  • the issue's program (BUFFERSIZE=20 INTERFACE64=1, SkylakeX kernels under Intel SDE, 12 threads): SIGSEGV → all elements correct
  • default x86_64 build, DSYRK N=93000, 2 threads: SIGSEGV → all elements correct
  • 32-bit x86, 2 threads: ZHERK N=5500 heap corruption → correct; ZPOTRF N=6000 crash → correct
  • work_area_bound: fails → passes on x86_64 and 32-bit x86, and with the time limit lifted on arm64, POWER, riscv64 and LoongArch (qemu)
  • no other test result changes: utest on 10 architectures; BLAS, CBLAS and LAPACK tests on x86_64; Make and CMake (static/shared), OpenMP, USE_THREAD=0, ILP64

Where SYRK is already broken (SVE wider than the kernels assume, #6100; RISCV64_ZVL256B at VLEN ≥ 512; 32-bit Penryn/Dunnington), the new exactness tests fail before and after, like existing tests.

@apullin

apullin commented Nov 7, 2026

Copy link
Copy Markdown
Contributor Author

The two msys2 CLANG64 failures are not from this change.
The new utest is the first to run SSYRK/SGEMM with K > 341 on clang/Windows, where the AVX512 fallback kernel overflows its stack buffer: the SGEMM twin of #6232.
Fix: #6236. With it and #6234, the CLANG64 jobs pass (checked on my fork).
I'll update this branch once both are in develop.

@martin-frbg martin-frbg added this to the 0.3.35 milestone Nov 7, 2026
Each thread of the threaded SYRK/HERK driver packs one K block of op(A)
for its whole row range behind sb, and nothing limited that to the
space left in the BUFFER_SIZE work area, so for large N it wrote past
the end (also through threaded ?POTRF). Shorten the K block until the
panels of the widest thread fit; all threads use the same blocking.

Fixes OpenMathLib#6224
Threaded ?SYRK/?HERK (all variants, several thread partitions and K
blocks) and threaded ?POTRF must reproduce an exact integer reference
and leave the other triangle, the padding and the guard zones
untouched. With a work area of 16 MiB or less (e.g. 32-bit x86) a
large ZHERK also checks the shortened K blocks of OpenMathLib#6224.
Call the threaded DSYRK/ZSYRK driver directly with a work area owned by
the test, followed by memory that must stay zero, at the smallest n
where the unfixed driver of OpenMathLib#6224 writes at least 64 KiB past
BUFFER_SIZE. With k=1 and ldc=0 this costs about n*n/2 multiply-adds
and a few MB. Skipped where it would take longer than
OPENBLAS_UTEST_TIME_LIMIT seconds (default 10). Needs the static
library (driver symbols).
clock_gettime() needs -lrt with glibc before 2.17, which broke linking openblas_utest in the manylinux1 CI job.

This branch has not been deployed

No deployments
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.

Threaded DSYRK on SkylakeX returns incorrect results and corrupts heap for large N (ILP64)

2 participants