Fix five correctness bugs from the audit (B1–B5)#12
Merged
Conversation
B1 (critical): *swap/csrot declared incX/incY as uint64_t but tested `if (incX < 0)` — always false, so a negative stride wrapped to a huge positive stride and read/wrote out of bounds. Change incX/incY (and the index accumulators) to int64_t, matching the already-correct saxpy family. Test: test_sswap_negstride (X reversed, Y forward). B2 (high): qaxpy canonicalized only QY[0]; non-canonical NaNs in QY[1..] escaped (SoftFloat propagates the quieted payload), breaking determinism for length>1. Canonicalize every written element. Test: test_qaxpy_nan_unify (non-canonical NaN at index 1 -> QUADNAN). B3 (high): FNEG used `1 << (bits-1)` — UB for 32-bit (1<<31), wrong for 64-bit (1<<63 exceeds int), and c128_conj didn't compile (xor on the uint64_t[2] array). Use a uint64_t mask; define c128_conj separately to flip only the high-word sign bit. Verified compiling + correct under -fsanitize=undefined. B4 (medium): i*amax returned a hardcoded 1 for N==1 while being 0-based for N>1. Return 0. Test: test_isamax_one. B5 (low): *nrm2 loop bound `1 + (N-1)*incX` could overflow -> early exit or OOB. Iterate by count (k < N, ix += incX). Covered by the existing nrm2 stride tests. 143/143 tests pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This was referenced May 30, 2026
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
1 participant
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.
Five behavior bugs the audit found. Stacks on #11 (base is
fix/test-suite-honesty); retarget tomasteronce #11 merges. Each fix ships with a regression test (except B5, covered by existing stride tests, and B3, verified standalone since complex isn't in the munit build).*swap/csrotdeclaredincX/incYuint64_tthen testedif (incX<0)(always false) → negative stride wrapped toUINT64_MAX→ OOBint64_tparams + index vars (matchessaxpy)test_sswap_negstrideqaxpycanonicalized onlyQY[0]; non-canonical NaNs inQY[1..]escaped (SoftFloat propagates the quieted payload) → determinism breaktest_qaxpy_nan_unifyFNEG1<<31UB,1<<63wrong,c128_conjdidn't compile (uint64_t[2] ^ int)uint64_tmask; separatec128_conjflippingv[1]sign-fsanitize=undefinedi*amaxreturned hardcoded1forN==1but 0-based forN>10test_isamax_one*nrm2bound1+(N-1)*incXcould overflow → early-exit/OOB143/143 pass. These are the bugs the Phase 1 honesty fixes (#11) make visible; B1/B4 specifically live in the swap-stride and
d/h/q-amax routines that #11 wired back in.🤖 Generated with Claude Code