Chorba: Fix edge case bug for >256KB input#2033
Merged
Dead2 merged 2 commits intozlib-ng:developfrom Dec 2, 2025
Merged
Conversation
Contributor
WalkthroughExtended the Chorba CRC-32 bitbuffer initialization loop from 60 to 64 entries and increased CRC32 test buffer sizes and test coverage (larger BUFSIZE and many additional large-length test cases). No public API/declaration changes. Changes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
arch/generic/crc32_chorba_c.c (1)
392-394: Zeroing 64 tail words matches pipeline depth; consider de‑duplicating magic constantsExtending the zeroing loop to cover
14870 .. 14870 + 64aligns the cleared region with the(14870 + 64)look‑ahead used in the main pass condition at Line 270, which should eliminate the stale bitbuffer state that caused CRC mismatches on large inputs. The index math stays well withinbitbuffersizezwordsand is safely wrapped with% bitbuffersizezwords, so there’s no new bounds or UB concern here.To avoid future off‑by‑N drift between these related constants, consider introducing named constants and reusing them in both places, e.g.:
-#define bitbuffersizeqwords (bitbuffersizebytes / sizeof(uint64_t)) +static const int chorba_118960_tail_base = 14870; +static const int chorba_118960_tail_words = 64; +#define bitbuffersizeqwords (bitbuffersizebytes / sizeof(uint64_t)) ... - for(; (i + (14870 + 64) * sizeof(z_word_t)) < len; i += (32 * sizeof(z_word_t))) { + for(; (i + (chorba_118960_tail_base + chorba_118960_tail_words) * sizeof(z_word_t)) < len; i += (32 * sizeof(z_word_t))) { ... - for (int j = 14870; j < 14870 + 64; j++) { - bitbuffer[(j + (i / sizeof(z_word_t))) % bitbuffersizezwords] = 0; - } + for (int j = chorba_118960_tail_base; + j < chorba_118960_tail_base + chorba_118960_tail_words; + j++) { + bitbuffer[(j + (i / sizeof(z_word_t))) % bitbuffersizezwords] = 0; + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
arch/generic/crc32_chorba_c.c(1 hunks)
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: KungFuJesus
Repo: zlib-ng/zlib-ng PR: 1872
File: arch/x86/chorba_sse2.c:0-0
Timestamp: 2025-02-23T16:49:52.043Z
Learning: In zlib-ng, bounds checking for CRC32 computation is handled by the caller, not within the individual CRC32 implementation functions like `crc32_chorba_sse2`.
Learnt from: KungFuJesus
Repo: zlib-ng/zlib-ng PR: 1872
File: arch/x86/chorba_sse2.c:0-0
Timestamp: 2025-02-21T01:42:40.488Z
Learning: In the SSE2-optimized Chorba CRC implementation (chorba_small_nondestructive_sse), the input buffer length is enforced to be a multiple of 16 bytes due to SSE2 operations, making additional checks for smaller alignments (like 8 bytes) redundant.
Learnt from: KungFuJesus
Repo: zlib-ng/zlib-ng PR: 1872
File: arch/x86/chorba_sse2.c:14-24
Timestamp: 2025-02-21T01:41:50.358Z
Learning: In zlib-ng's SSE2 vectorized Chorba CRC implementation, the code that calls READ_NEXT macro ensures 16-byte alignment, making explicit alignment checks unnecessary within the macro.
Learnt from: Dead2
Repo: zlib-ng/zlib-ng PR: 1837
File: arch/generic/crc32_c.c:19-29
Timestamp: 2025-01-23T22:01:53.422Z
Learning: The Chorba CRC32 functions (crc32_chorba_118960_nondestructive, crc32_chorba_32768_nondestructive, crc32_chorba_small_nondestructive, crc32_chorba_small_nondestructive_32bit) are declared in crc32_c.h.
Learnt from: samrussell
Repo: zlib-ng/zlib-ng PR: 1837
File: arch/generic/crc32_braid_c.c:596-597
Timestamp: 2024-12-25T19:45:06.009Z
Learning: We should verify out-of-bounds pointer arithmetic in chorba_118960_nondestructive() and related loops where "input + i" is used, especially when len < expected block sizes.
Learnt from: KungFuJesus
Repo: zlib-ng/zlib-ng PR: 1833
File: test/test_crc32.cc:0-0
Timestamp: 2024-12-22T20:40:03.280Z
Learning: In the crc32_align test class (test/test_crc32.cc), the alignment offset parameter is always guaranteed to be non-negative because the test parameters are controlled and it never makes sense to have a negative offset.
📚 Learning: 2025-02-21T01:42:40.488Z
Learnt from: KungFuJesus
Repo: zlib-ng/zlib-ng PR: 1872
File: arch/x86/chorba_sse2.c:0-0
Timestamp: 2025-02-21T01:42:40.488Z
Learning: In the SSE2-optimized Chorba CRC implementation (chorba_small_nondestructive_sse), the input buffer length is enforced to be a multiple of 16 bytes due to SSE2 operations, making additional checks for smaller alignments (like 8 bytes) redundant.
Applied to files:
arch/generic/crc32_chorba_c.c
📚 Learning: 2025-01-23T22:01:53.422Z
Learnt from: Dead2
Repo: zlib-ng/zlib-ng PR: 1837
File: arch/generic/crc32_c.c:19-29
Timestamp: 2025-01-23T22:01:53.422Z
Learning: The Chorba CRC32 functions (crc32_chorba_118960_nondestructive, crc32_chorba_32768_nondestructive, crc32_chorba_small_nondestructive, crc32_chorba_small_nondestructive_32bit) are declared in crc32_c.h.
Applied to files:
arch/generic/crc32_chorba_c.c
📚 Learning: 2025-02-23T16:49:52.043Z
Learnt from: KungFuJesus
Repo: zlib-ng/zlib-ng PR: 1872
File: arch/x86/chorba_sse2.c:0-0
Timestamp: 2025-02-23T16:49:52.043Z
Learning: In zlib-ng, bounds checking for CRC32 computation is handled by the caller, not within the individual CRC32 implementation functions like `crc32_chorba_sse2`.
Applied to files:
arch/generic/crc32_chorba_c.c
📚 Learning: 2025-02-21T01:41:50.358Z
Learnt from: KungFuJesus
Repo: zlib-ng/zlib-ng PR: 1872
File: arch/x86/chorba_sse2.c:14-24
Timestamp: 2025-02-21T01:41:50.358Z
Learning: In zlib-ng's SSE2 vectorized Chorba CRC implementation, the code that calls READ_NEXT macro ensures 16-byte alignment, making explicit alignment checks unnecessary within the macro.
Applied to files:
arch/generic/crc32_chorba_c.c
📚 Learning: 2024-12-25T19:45:06.009Z
Learnt from: samrussell
Repo: zlib-ng/zlib-ng PR: 1837
File: arch/generic/crc32_braid_c.c:596-597
Timestamp: 2024-12-25T19:45:06.009Z
Learning: We should verify out-of-bounds pointer arithmetic in chorba_118960_nondestructive() and related loops where "input + i" is used, especially when len < expected block sizes.
Applied to files:
arch/generic/crc32_chorba_c.c
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (82)
- GitHub Check: macOS Clang Native Instructions (Intel)
- GitHub Check: Windows GCC Compat No Opt
- GitHub Check: Windows MSVC 2022 v142 Win64
- GitHub Check: Windows MSVC ARM64 No Test
- GitHub Check: EL10 GCC S390X DFLTCC ASAN
- GitHub Check: Ubuntu GCC AARCH64 No NEON UBSAN
- GitHub Check: Ubuntu GCC AARCH64 ASAN
- GitHub Check: macOS GCC (ARM64)
- GitHub Check: macOS Clang Native Instructions (Intel)
- GitHub Check: Windows GCC Compat No Opt
- GitHub Check: Windows MSVC 2022 v142 Win64
- GitHub Check: Windows MSVC ARM64 No Test
- GitHub Check: EL10 GCC S390X DFLTCC ASAN
- GitHub Check: Ubuntu GCC AARCH64 No NEON UBSAN
- GitHub Check: Ubuntu GCC AARCH64 ASAN
- GitHub Check: macOS GCC (ARM64)
- GitHub Check: macOS Clang Native Instructions (Intel)
- GitHub Check: Windows GCC Compat No Opt
- GitHub Check: Windows MSVC 2022 v142 Win64
- GitHub Check: Windows MSVC ARM64 No Test
- GitHub Check: EL10 GCC S390X DFLTCC ASAN
- GitHub Check: Ubuntu GCC AARCH64 No NEON UBSAN
- GitHub Check: Ubuntu GCC AARCH64 ASAN
- GitHub Check: macOS GCC (ARM64)
- GitHub Check: macOS Clang Native Instructions (Intel)
- GitHub Check: Windows GCC Compat No Opt
- GitHub Check: Windows MSVC 2022 v142 Win64
- GitHub Check: Windows MSVC ARM64 No Test
- GitHub Check: EL10 GCC S390X DFLTCC ASAN
- GitHub Check: Ubuntu GCC AARCH64 No NEON UBSAN
- GitHub Check: Ubuntu GCC AARCH64 ASAN
- GitHub Check: macOS GCC (ARM64)
- GitHub Check: macOS Clang Native Instructions (Intel)
- GitHub Check: Windows GCC Compat No Opt
- GitHub Check: Windows MSVC 2022 v142 Win64
- GitHub Check: Windows MSVC ARM64 No Test
- GitHub Check: EL10 GCC S390X DFLTCC ASAN
- GitHub Check: Ubuntu GCC AARCH64 No NEON UBSAN
- GitHub Check: Ubuntu GCC AARCH64 ASAN
- GitHub Check: macOS GCC (ARM64)
- GitHub Check: macOS Clang Native Instructions (Intel)
- GitHub Check: Windows GCC Compat No Opt
- GitHub Check: Windows MSVC 2022 v142 Win64
- GitHub Check: Windows MSVC ARM64 No Test
- GitHub Check: EL10 GCC S390X DFLTCC ASAN
- GitHub Check: Ubuntu GCC AARCH64 No NEON UBSAN
- GitHub Check: Ubuntu GCC AARCH64 ASAN
- GitHub Check: macOS GCC (ARM64)
- GitHub Check: macOS Clang Native Instructions (Intel)
- GitHub Check: Windows GCC Compat No Opt
- GitHub Check: Windows MSVC 2022 v142 Win64
- GitHub Check: Windows MSVC ARM64 No Test
- GitHub Check: EL10 GCC S390X DFLTCC ASAN
- GitHub Check: Ubuntu GCC AARCH64 No NEON UBSAN
- GitHub Check: Ubuntu GCC AARCH64 ASAN
- GitHub Check: macOS GCC (ARM64)
- GitHub Check: macOS Clang Native Instructions (Intel)
- GitHub Check: Windows GCC Compat No Opt
- GitHub Check: Windows MSVC 2022 v142 Win64
- GitHub Check: Windows MSVC ARM64 No Test
- GitHub Check: EL10 GCC S390X DFLTCC ASAN
- GitHub Check: Ubuntu GCC AARCH64 No NEON UBSAN
- GitHub Check: Ubuntu GCC AARCH64 ASAN
- GitHub Check: macOS GCC (ARM64)
- GitHub Check: macOS Clang Native Instructions (Intel)
- GitHub Check: Windows GCC Compat No Opt
- GitHub Check: Windows MSVC 2022 v142 Win64
- GitHub Check: Windows MSVC ARM64 No Test
- GitHub Check: EL10 GCC S390X DFLTCC ASAN
- GitHub Check: Ubuntu GCC AARCH64 No NEON UBSAN
- GitHub Check: Ubuntu GCC AARCH64 ASAN
- GitHub Check: Ubuntu GCC -O3 OSB add_subdirectory
- GitHub Check: macOS GCC (ARM64)
- GitHub Check: macOS Clang Native Instructions (Intel)
- GitHub Check: Windows GCC Compat No Opt
- GitHub Check: Windows MSVC 2022 v142 Win64
- GitHub Check: Windows MSVC ARM64 No Test
- GitHub Check: EL10 GCC S390X DFLTCC ASAN
- GitHub Check: Ubuntu GCC AARCH64 No NEON UBSAN
- GitHub Check: Ubuntu GCC AARCH64 ASAN
- GitHub Check: Ubuntu GCC -O3 OSB add_subdirectory
- GitHub Check: macOS GCC (ARM64)
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #2033 +/- ##
===========================================
- Coverage 80.90% 80.88% -0.02%
===========================================
Files 163 163
Lines 12883 12883
Branches 3174 3174
===========================================
- Hits 10423 10421 -2
- Misses 1387 1423 +36
+ Partials 1073 1039 -34 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Add test case from @KungFuJesus and a few others in similar data lengths
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
test/crc32_test_strings_p.h (1)
4-13: Avoid duplicating the buffer size literal in the init loopYou now hard-code
615336both in the array size and the fill loop bound. To avoid future mismatches if this buffer grows again, consider driving the loop fromsizeof(crc32_fullwin_buf)instead of a magic constant (and using an index type that matches it):-ALIGNED_(16) static uint8_t crc32_fullwin_buf[615336]; - -static uint8_t* crc32_setup_buf() { - for (int i = 0; i < 615336; ++i) { +ALIGNED_(16) static uint8_t crc32_fullwin_buf[615336]; + +static uint8_t* crc32_setup_buf() { + for (size_t i = 0; i < sizeof(crc32_fullwin_buf); ++i) { uint8_t ic = (uint8_t)(i % 256); crc32_fullwin_buf[i] = ic; }This keeps the buffer length and init loop in lockstep with no extra maintenance.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
test/crc32_test_strings_p.h(2 hunks)test/test_crc32_fold_copy.cc(1 hunks)
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: KungFuJesus
Repo: zlib-ng/zlib-ng PR: 1872
File: arch/x86/chorba_sse2.c:0-0
Timestamp: 2025-02-23T16:49:52.043Z
Learning: In zlib-ng, bounds checking for CRC32 computation is handled by the caller, not within the individual CRC32 implementation functions like `crc32_chorba_sse2`.
Learnt from: KungFuJesus
Repo: zlib-ng/zlib-ng PR: 1872
File: arch/x86/chorba_sse2.c:0-0
Timestamp: 2025-02-21T01:42:40.488Z
Learning: In the SSE2-optimized Chorba CRC implementation (chorba_small_nondestructive_sse), the input buffer length is enforced to be a multiple of 16 bytes due to SSE2 operations, making additional checks for smaller alignments (like 8 bytes) redundant.
Learnt from: KungFuJesus
Repo: zlib-ng/zlib-ng PR: 1872
File: arch/x86/chorba_sse2.c:14-24
Timestamp: 2025-02-21T01:41:50.358Z
Learning: In zlib-ng's SSE2 vectorized Chorba CRC implementation, the code that calls READ_NEXT macro ensures 16-byte alignment, making explicit alignment checks unnecessary within the macro.
Learnt from: Dead2
Repo: zlib-ng/zlib-ng PR: 1837
File: arch/generic/crc32_c.c:19-29
Timestamp: 2025-01-23T22:01:53.422Z
Learning: The Chorba CRC32 functions (crc32_chorba_118960_nondestructive, crc32_chorba_32768_nondestructive, crc32_chorba_small_nondestructive, crc32_chorba_small_nondestructive_32bit) are declared in crc32_c.h.
Learnt from: samrussell
Repo: zlib-ng/zlib-ng PR: 1837
File: arch/generic/crc32_braid_c.c:596-597
Timestamp: 2024-12-25T19:45:06.009Z
Learning: We should verify out-of-bounds pointer arithmetic in chorba_118960_nondestructive() and related loops where "input + i" is used, especially when len < expected block sizes.
📚 Learning: 2025-02-21T01:42:40.488Z
Learnt from: KungFuJesus
Repo: zlib-ng/zlib-ng PR: 1872
File: arch/x86/chorba_sse2.c:0-0
Timestamp: 2025-02-21T01:42:40.488Z
Learning: In the SSE2-optimized Chorba CRC implementation (chorba_small_nondestructive_sse), the input buffer length is enforced to be a multiple of 16 bytes due to SSE2 operations, making additional checks for smaller alignments (like 8 bytes) redundant.
Applied to files:
test/crc32_test_strings_p.htest/test_crc32_fold_copy.cc
📚 Learning: 2025-01-23T22:01:53.422Z
Learnt from: Dead2
Repo: zlib-ng/zlib-ng PR: 1837
File: arch/generic/crc32_c.c:19-29
Timestamp: 2025-01-23T22:01:53.422Z
Learning: The Chorba CRC32 functions (crc32_chorba_118960_nondestructive, crc32_chorba_32768_nondestructive, crc32_chorba_small_nondestructive, crc32_chorba_small_nondestructive_32bit) are declared in crc32_c.h.
Applied to files:
test/crc32_test_strings_p.h
📚 Learning: 2024-12-22T20:40:03.280Z
Learnt from: KungFuJesus
Repo: zlib-ng/zlib-ng PR: 1833
File: test/test_crc32.cc:0-0
Timestamp: 2024-12-22T20:40:03.280Z
Learning: In the crc32_align test class (test/test_crc32.cc), the alignment offset parameter is always guaranteed to be non-negative because the test parameters are controlled and it never makes sense to have a negative offset.
Applied to files:
test/crc32_test_strings_p.htest/test_crc32_fold_copy.cc
📚 Learning: 2025-02-21T01:41:50.358Z
Learnt from: KungFuJesus
Repo: zlib-ng/zlib-ng PR: 1872
File: arch/x86/chorba_sse2.c:14-24
Timestamp: 2025-02-21T01:41:50.358Z
Learning: In zlib-ng's SSE2 vectorized Chorba CRC implementation, the code that calls READ_NEXT macro ensures 16-byte alignment, making explicit alignment checks unnecessary within the macro.
Applied to files:
test/crc32_test_strings_p.h
🧬 Code graph analysis (1)
test/test_crc32_fold_copy.cc (1)
test/benchmarks/benchmark_crc32_fold_copy.cc (7)
state(43-87)crc32_fold_copy_chorba_sse41(36-39)crc32_fold_copy_chorba(23-26)crc32_fold_copy_chorba_sse2(30-33)state(50-58)state(83-86)state(60-81)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (110)
- GitHub Check: macOS GCC (ARM64)
- GitHub Check: macOS GCC (Intel)
- GitHub Check: Ubuntu GCC ARM HF
- GitHub Check: macOS GCC UBSAN (ARM64)
- GitHub Check: Windows GCC Native Instructions (AVX)
- GitHub Check: Ubuntu MinGW i686
- GitHub Check: macOS Clang ASAN (ARM64)
- GitHub Check: Windows MSVC ARM64 No Test
- GitHub Check: Windows MSVC 2022 v143 Win64 Native Instructions (AVX)
- GitHub Check: EL10 GCC S390X DFLTCC UBSAN
- GitHub Check: Windows MSVC 2022 v141 Win64
- GitHub Check: macOS GCC (ARM64)
- GitHub Check: macOS GCC (Intel)
- GitHub Check: Ubuntu GCC ARM HF
- GitHub Check: macOS GCC UBSAN (ARM64)
- GitHub Check: Windows GCC Native Instructions (AVX)
- GitHub Check: Ubuntu MinGW i686
- GitHub Check: macOS Clang ASAN (ARM64)
- GitHub Check: Windows MSVC ARM64 No Test
- GitHub Check: Windows MSVC 2022 v143 Win64 Native Instructions (AVX)
- GitHub Check: EL10 GCC S390X DFLTCC UBSAN
- GitHub Check: Windows MSVC 2022 v141 Win64
- GitHub Check: macOS GCC (ARM64)
- GitHub Check: macOS GCC (Intel)
- GitHub Check: Ubuntu GCC ARM HF
- GitHub Check: macOS GCC UBSAN (ARM64)
- GitHub Check: Windows GCC Native Instructions (AVX)
- GitHub Check: Ubuntu MinGW i686
- GitHub Check: macOS Clang ASAN (ARM64)
- GitHub Check: Windows MSVC ARM64 No Test
- GitHub Check: Windows MSVC 2022 v143 Win64 Native Instructions (AVX)
- GitHub Check: EL10 GCC S390X DFLTCC UBSAN
- GitHub Check: Windows MSVC 2022 v141 Win64
- GitHub Check: macOS GCC (ARM64)
- GitHub Check: macOS GCC (Intel)
- GitHub Check: Ubuntu GCC ARM HF
- GitHub Check: macOS GCC UBSAN (ARM64)
- GitHub Check: Windows GCC Native Instructions (AVX)
- GitHub Check: Ubuntu MinGW i686
- GitHub Check: macOS Clang ASAN (ARM64)
- GitHub Check: Windows MSVC ARM64 No Test
- GitHub Check: Windows MSVC 2022 v143 Win64 Native Instructions (AVX)
- GitHub Check: EL10 GCC S390X DFLTCC UBSAN
- GitHub Check: Windows MSVC 2022 v141 Win64
- GitHub Check: macOS GCC (ARM64)
- GitHub Check: macOS GCC (Intel)
- GitHub Check: Ubuntu GCC ARM HF
- GitHub Check: macOS GCC UBSAN (ARM64)
- GitHub Check: Windows GCC Native Instructions (AVX)
- GitHub Check: Ubuntu MinGW i686
- GitHub Check: macOS Clang ASAN (ARM64)
- GitHub Check: Windows MSVC ARM64 No Test
- GitHub Check: Windows MSVC 2022 v143 Win64 Native Instructions (AVX)
- GitHub Check: EL10 GCC S390X DFLTCC UBSAN
- GitHub Check: Windows MSVC 2022 v141 Win64
- GitHub Check: macOS GCC (ARM64)
- GitHub Check: macOS GCC (Intel)
- GitHub Check: Ubuntu GCC ARM HF
- GitHub Check: macOS GCC UBSAN (ARM64)
- GitHub Check: Windows GCC Native Instructions (AVX)
- GitHub Check: Ubuntu MinGW i686
- GitHub Check: macOS Clang ASAN (ARM64)
- GitHub Check: Windows MSVC ARM64 No Test
- GitHub Check: Windows MSVC 2022 v143 Win64 Native Instructions (AVX)
- GitHub Check: EL10 GCC S390X DFLTCC UBSAN
- GitHub Check: Windows MSVC 2022 v141 Win64
- GitHub Check: macOS GCC (ARM64)
- GitHub Check: macOS GCC (Intel)
- GitHub Check: Ubuntu GCC ARM HF
- GitHub Check: macOS GCC UBSAN (ARM64)
- GitHub Check: Windows GCC Native Instructions (AVX)
- GitHub Check: Ubuntu MinGW i686
- GitHub Check: macOS Clang ASAN (ARM64)
- GitHub Check: Windows MSVC ARM64 No Test
- GitHub Check: Windows MSVC 2022 v143 Win64 Native Instructions (AVX)
- GitHub Check: EL10 GCC S390X DFLTCC UBSAN
- GitHub Check: Windows MSVC 2022 v141 Win64
- GitHub Check: macOS GCC (ARM64)
- GitHub Check: macOS GCC (Intel)
- GitHub Check: Ubuntu GCC ARM HF
- GitHub Check: macOS GCC UBSAN (ARM64)
- GitHub Check: Windows GCC Native Instructions (AVX)
- GitHub Check: Ubuntu MinGW i686
- GitHub Check: macOS Clang ASAN (ARM64)
- GitHub Check: Windows MSVC ARM64 No Test
- GitHub Check: Windows MSVC 2022 v143 Win64 Native Instructions (AVX)
- GitHub Check: EL10 GCC S390X DFLTCC UBSAN
- GitHub Check: Windows MSVC 2022 v141 Win64
- GitHub Check: macOS GCC (ARM64)
- GitHub Check: macOS GCC (Intel)
- GitHub Check: Ubuntu GCC ARM HF
- GitHub Check: macOS GCC UBSAN (ARM64)
- GitHub Check: Windows GCC Native Instructions (AVX)
- GitHub Check: Ubuntu MinGW i686
- GitHub Check: macOS Clang ASAN (ARM64)
- GitHub Check: Windows MSVC ARM64 No Test
- GitHub Check: Windows MSVC 2022 v143 Win64 Native Instructions (AVX)
- GitHub Check: EL10 GCC S390X DFLTCC UBSAN
- GitHub Check: Windows MSVC 2022 v141 Win64
- GitHub Check: macOS GCC (ARM64)
- GitHub Check: macOS GCC (Intel)
- GitHub Check: Ubuntu GCC ARM HF
- GitHub Check: macOS GCC UBSAN (ARM64)
- GitHub Check: Windows GCC Native Instructions (AVX)
- GitHub Check: Ubuntu MinGW i686
- GitHub Check: macOS Clang ASAN (ARM64)
- GitHub Check: Windows MSVC ARM64 No Test
- GitHub Check: Windows MSVC 2022 v143 Win64 Native Instructions (AVX)
- GitHub Check: EL10 GCC S390X DFLTCC UBSAN
- GitHub Check: Windows MSVC 2022 v141 Win64
🔇 Additional comments (2)
test/test_crc32_fold_copy.cc (1)
15-28: BUFSIZE bump is consistent with new max test length
BUFSIZE= 615336U matches the largestlenincrc32_tests(the 615336-bytebuf32kcase), andASSERT_LE(params.len, BUFSIZE)continues to guaranteedstbufis not overrun. This cleanly aligns the fold-copy tests with the expanded large-input coverage.test/crc32_test_strings_p.h (1)
180-191: New large buf32k test vectors are well-bounded and target the edge caseThe additional
buf32kentries up to length 615336:
- All use
buf32kpointing intocrc32_fullwin_buf[615336], solennever exceeds the backing buffer.- Stay within the updated
BUFSIZEused bytest_crc32_fold_copy(ASSERT_LE(params.len, BUFSIZE)still holds).- Cluster around large sizes (118960+512 and several values in the 6150xx–615336 range), which is exactly where the Chorba bitbuffer logic was problematic.
These look like solid coverage for the >256KB regression without introducing new OOB risks.
nmoinvaz
approved these changes
Dec 2, 2025
Member
|
@Dead2 probably needs to be backported with a new release? |
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
3 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.
Fixes #2029