Fix out-of-bounds write in Newton-Raphson division, Improve normalize - #553
Merged
Conversation
These macros left the exponent of the previous value. Zero, Infinity and NaN have no meaningful exponent and every reader (VpExponent10, VpAsgn, ...) already ignores it, but the internal inconsistency reached slice_copy in Newton-Raphson division: BigDecimal_fix of a tiny positive value (a quotient block that is zero) produced a zero with a negative exponent, and slice_copy underflowed the copy length and wrote outside of the quotient buffer. It corrupted the heap for e.g. BigDecimal(y * 10**1800 + 5).divmod(BigDecimal(y)) with y = 10**1000 + 7, and rake test with NEWTON_RAPHSON_DIVISION_THRESHOLD=1 crashed in GC about once in four runs. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The range can be entirely above the most significant word or below the least significant word of the source. Clamp both ends instead of adjusting the start afterwards, so that the copy never exceeds dest[0, length). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
VpDivdNewtonInner ignored the result of AddExponent and kept adding to the exponent after an overflow turned the quotient into Infinity, and also added to the exponent of a zero quotient or remainder. Apply the total shift in one call and skip it for zero. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Rounding a small value with ROUND_CEILING or ROUND_FLOOR beyond its first digit can result in zero, and the exponent compensation for the rounding position was applied to that zero. Co-Authored-By: Claude Fable 5.1 <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
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.
VpSetZero,VpSetInfandVpSetNaNleft the exponent of the previous value.VpNmlzwasn'y normalizing the exponent.Zero, Infinity and NaN have no meaningful exponent and every reader already
ignores it, but the stale exponent reached
slice_copyin Newton-Raphsondivision. When a quotient block is zero,
BigDecimal_fixof the tiny productx * invproduces a zero that still has a negative exponent.slice_copythen underflows the copy length and writes one word outside of the quotient
buffer, corrupting the heap.
Reproduction (with the default threshold):
The result is correct, but GC can crash later. With
NEWTON_RAPHSON_DIVISION_THRESHOLD=1,rake testsegfaulted intest_bigmathabout once in four runs.Changes:
exponentinVpSetZero,VpSetInfandVpSetNaN. This alonefixes the crash.
VpNmlzto normalize exponentslice_copyto copy only the intersection of the requested rangeand the existing words, so it can never write outside
dest[0, length).