fix: correct wasm div of negative high-word dividend by -1#138
Open
spokodev wants to merge 1 commit into
Open
Conversation
The wasm-backed signed division short-circuits and returns the dividend unchanged whenever this.high === 0x80000000 and the divisor is -1. That guard is meant to catch only the true two's-complement overflow of MIN_VALUE / -1, but it omits the low-word check, so any negative dividend sharing that high word (for example -9223372036854775807) is wrongly returned unchanged instead of being negated. Add this.low === 0 so only the exact MIN_VALUE / -1 overflow is special cased; every other value falls through to wasm.div_s. This matches the non-wasm path, which guards on this.eq(MIN_VALUE).
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.
Problem
The wasm-backed signed division path returns the wrong value when dividing a negative dividend whose high 32-bit word is
0x80000000but whose low word is non-zero, by-1.Root cause
The overflow guard in
LongPrototype.divide(wasm branch) is meant to catch the single true two's-complement overflow,MIN_VALUE / -1, where the result would be one larger thanMAX_VALUE. It fires wheneverthis.high === -0x80000000 && divisor === -1but never checks the low word:MIN_VALUEislow = 0, high = 0x80000000. Any other negative value sharing that high word (for example-9223372036854775807=low = 1, high = 0x80000000) also matches the guard and is short-circuited back unchanged, so it is never negated.The non-wasm path does not have this issue: it guards on
this.eq(MIN_VALUE), which checks both words, so it only special-cases the exact overflow. The wasm and non-wasm paths therefore disagree for these inputs.Oracle
64-bit two's-complement signed division,
BigInt.asIntN(64, a / b):BigInt.asIntN(64, a / -1n)-9223372036854775807 / -1-92233720368547758079223372036854775807-9223372036854775296 / -1-92233720368547752969223372036854775296-9223372036854775808 / -1(MIN_VALUE)-9223372036854775808-9223372036854775808Only the exact
MIN_VALUE / -1is a genuine overflow that must wrap toMIN_VALUE; that behavior is preserved.Fix
Add
this.low === 0to the guard so only the exactMIN_VALUEis special-cased; every other value falls through towasm.div_s. This brings the wasm path in line with the non-wasm path'sthis.eq(MIN_VALUE)check.Tests
Added
testSignedNegHighWordDivNegOne, which asserts the two negated results above and re-asserts thatMIN_VALUE / -1still returnsMIN_VALUE. The test fails onmain(returns the unchanged dividend) and passes with the fix. The full unit suite, including the closure-library tests, passes against both the ESM source and the UMD build. A 200k-case differential against theBigInt.asIntN(64, a / b)oracle, biased toward the0x80000000high-word edge, matches on every case.