Skip to content

Commit abf3059

Browse files
committed
Fix AVX2 SIMD boundary overread in colorShuffle_avx2
This commit fixes memory overread issues in AVX2-optimized RGB/BGR color conversions that could cause crashes at certain image widths (e.g., 1280). Problem: - 3->3 conversion (RGB↔BGR): Reads 16 bytes but only needs 15, causing 1-byte overread at boundaries - 3->4 conversion (RGB→RGBA): Second read extends to offset+27, causing 4-byte overread at boundaries Root cause: SIMD intrinsics (_mm_loadu_si128) read fixed byte amounts that can exceed actual pixel data at image boundaries. Solution: Calculate proper loop boundary for each conversion type: - 3->4 conversions: patchSize + 2 (needs 2 extra pixels margin) - 3->3 conversions: patchSize + 1 (needs 1 extra pixel margin) - 4->4 and 4->3: patchSize (already safe) Testing: Added 78 comprehensive test cases covering: - Issue #30 exact scenario (1280x720) - Critical boundary widths (1272-1280, 1915-1920) - Small/tiny widths (1-10 pixels) - Non-contiguous memory (with stride padding) All tests pass on both CPU and AVX2 backends. Fixes #30
1 parent bed9a0e commit abf3059

3 files changed

Lines changed: 438 additions & 1 deletion

File tree

src/ccap_convert_avx2.cpp

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -181,11 +181,21 @@ AVX2_TARGET void colorShuffle_avx2(const uint8_t* src, int srcStride, uint8_t* d
181181
shuffle128 = _mm_load_si128((__m128i*)shuffleData);
182182
}
183183

184+
// Different cases require different boundary conditions to avoid reading beyond allocated memory:
185+
// - 3->4: reads 16 bytes from x*3+12, needs x*3+27 < width*3, i.e., x+9 < width
186+
// - 3->3: reads 16 bytes from x*3, needs x*3+15 < width*3, i.e., x+5 < width
187+
// - 4->3: reads 16 bytes from x*4+16, needs x*4+31 < width*4, i.e., x+8 <= width
188+
// - 4->4: reads 32 bytes from x*4, needs x*4+31 < width*4, i.e., x+8 <= width
189+
constexpr uint32_t loopBoundary = (inputChannels == 3 && outputChannels == 4) ? (patchSize + 2) :
190+
(inputChannels == 3 && outputChannels == 3) ? (patchSize + 1) :
191+
patchSize;
192+
184193
for (int y = 0; y < height; ++y) {
185194
const uint8_t* srcRow = src + y * srcStride;
186195
uint8_t* dstRow = dst + y * dstStride;
187196
uint32_t x = 0;
188-
while (x + patchSize <= (uint32_t)width) {
197+
198+
while (x + loopBoundary <= (uint32_t)width) {
189199
// _mm256_shuffle_epi8 can’t move these bytes across 16-byte lanes of the vector.
190200
// @see issue <https://stackoverflow.com/questions/77149094/how-to-use-mm256-shuffle-epi8-to-order-elements>
191201
if constexpr (outputChannels == 4 && inputChannels == 3) { // 3 -> 4, need to split channels

tests/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ add_executable(
7373
test_yuv_conversions.cpp
7474
test_platform_features.cpp
7575
test_frame_conversions.cpp
76+
test_boundary_conditions.cpp
7677
)
7778

7879
target_link_libraries(

0 commit comments

Comments
 (0)