Repository navigation
Fix AVX-512 AudioChannelMixer tail over-read; guard-page buffers in kernel tests - #1633
Merged
Merged
Conversation
The tail of mix_channels_x64_AVX512<2, 1> loaded input channel 1 with a plain _mm512_loadu_ps(). When fewer than 16 samples remain, that reads up to 60 bytes past the end of the channel. If the channel ends next to an unmapped page, this faults. That is the intermittent segfault in the "x64_AVX512(2 -> 1 x 18)" unit test on CI runners with AVX512, and the same path runs for stereo -> mono mixing in ChannelMixer. Use _mm512_maskz_loadu_ps() like the other tails. Masked-off lanes of a masked load never fault. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…hannel. Add GuardPageBuffer: an array whose last element sits right before a no-access page (mmap + mprotect, or VirtualAlloc + VirtualProtect on Windows). Reading or writing even one element past the end faults right away. The AudioChannelMixer tests now put the input and the output of the kernel under test in such buffers. A SIMD tail that loads or stores past the end then crashes on every run. Before, it only crashed when the heap happened to end there, which is how the AVX512 2 -> 1 over-read slipped through as a rare CI segfault. The tests also compared only output channel 0. Compare all of them. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Use GuardPageBuffer for the buffers that the kernels under test read and write, so a tail that reads or writes past the end faults on every run: - AbsFFT, AudioChannelInterleave, AudioStreamConverter, ImageStats (pixel_sum_sqr, sum_sqr_deviation): inputs and outputs. - SpikeConvolution: "in", "kernel" and the ISA output, which is now exactly the documented size (outputs rounded up to 16 floats). The buffers are mapped once per test, and each input length uses the end of them. - ScaleInvariantMatrixMatch: all matrices. The tail padding is gone, so the last row ends at the guard page. - AudioPipeline and AudioResampler: the inputs, and the resampler block kernel outputs. GuardPageBuffer gets an optional alignment (AbsFFT needs 64-byte aligned buffers, SpikeConvolution a 64-byte aligned output) and move support. The image filter, BinaryMatrix and Waterfill tests are unchanged: they run on image files through ImageRGB32, which owns its memory. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Add GuardPageImageRGB32 (Tests/GuardPageImage.h): an RGB32 image with no row padding (bytes_per_row = width * 4) whose last row ends right before a no-access page. ImageRGB32 pads rows to 64 bytes, so tests on it never notice a kernel that reads or writes a little past the end of a row. The ImageFilters, BinaryMatrix and Waterfill tests now copy the test image into one, and the filter outputs are guarded images too. These tests are not registered yet (their add_tests functions are empty), so this only takes effect once they are. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Mysticial
reviewed
Oct 21, 2026
Mysticial
reviewed
Oct 21, 2026
Mysticial
approved these changes
Oct 21, 2026
Mysticial
left a comment
Mysticial
left a comment
Collaborator
There was a problem hiding this comment.
Approved, after addressing comments.
This was referenced Oct 21, 2026
Review feedback on PR 1633. The Default implementation is a kernel as well, so its output in the AudioChannelMixer and SpikeConvolution tests now ends at a guard page too. SpikeConvolution gives both outputs the documented output size, so OUTPUT_PAD is gone. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Review request on PR 1633. The comments say why the class exists, how it places the data next to the guard page, where the tests use it, and its limits. No functional change. Co-Authored-By: Claude Opus 5.5 <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
2 participants
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.
Fix a read past the end of the input in the AVX-512 kernel that mixes 2 audio channels into 1. Change the SIMD kernel tests to use guard-page buffers, so that they find this type of bug in each run.
GuardPageBufferis a test array that ends right before a page that the program cannot access. A read or write past its end crashes the test at once.The bug crashed the unit tests at random on CI runners with AVX-512, for example in #1619. It can also crash the program when it mixes stereo audio to mono. The bug is from #1551.
Other changes:
0 test files added, 15 test files changed:
Test results: before the fix, the new tests crashed each time on an AVX-512 runner (run). With the fix, all tests pass with AVX-512 (Ubuntu clang, Windows MSVC) and without it (Windows clang).
🤖 Generated with Claude Code