SR-13015: Decimal calculation incorrect on Linux #2827
Conversation
|
@swift-ci test |
|
Updated with correct overflow check. |
| @@ -1025,7 +1025,7 @@ fileprivate func integerMultiply<T:VariableLengthNumber>(_ big: inout T, | |||
| accumulator = UInt32(carry) + bigij + UInt32(right[j]) * UInt32(left[i]) | |||
| carry = UInt16(truncatingIfNeeded:accumulator >> 16) | |||
| big[i+j] = UInt16(truncatingIfNeeded:accumulator) | |||
| } else if carry != 0 || (right[j] == 0 && left[j] == 0) { | |||
| } else if carry != 0 || (right[j] > 0 && left[i] > 0) { | |||
There was a problem hiding this comment.
This will overflow if the accumulator above would be non-zero, ie if there's a carry or both right and left elements that are multiplied are non-zero.
|
Could you add in the extra tests in https://gist.githubusercontent.com/spevans/a5c9da89e91538abc688c5498fe4ed28/raw/e23ca9cdc21b87e2bcc1880ddafc5e01624bd9ce/sr13015.diff and squash it into one commit for easier back porting |
|
@swift-ci test linux |
|
Sure, will do. |
Fix typo and logic error in the overflow check in integerMultiply(), which was screwing up Decimal calculations under certain conditions.
|
Rather than checking the same Decimal normalization in three different places, I removed my test and added your more extended tests instead. |
|
@swift-ci test linux |
|
@swift-ci test |
|
You mentioned backporting this, should I submit a pull for the 5.3 branch? I presume 5.2 releases are done, or will that take this too? |
|
Lets get a backport for 5.3 going, Im not sure if it will make it into the release or will end up in 5.3.1 but having it ready to go will be useful, thanks. |
Fix typo and logic error in the overflow check in
integerMultiply(), which was screwing up Decimal calculations under certain conditions.I was able to reproduce on Android and spent hours tracking this down on there. I'm guessing it only worked on macOS because this code is unused on there.