Skip to content

Commit

Permalink
Avoid uint64_t overflow in Decimal::operator/() and fix static MaxCoe…
Browse files Browse the repository at this point in the history
…fficient value

Avoid uint64_t overflow in Decimal::operator/() and fix static MaxCoefficient value
https://bugs.webkit.org/show_bug.cgi?id=248784

Reviewed by Darin Adler.

Merge - https://src.chromium.org/viewvc/blink?view=revision&revision=191294 & https://src.chromium.org/viewvc/blink?view=revision&revision=174294

This patch changes Decimal::operator/() not to cause uint64_t overflow in
multiplication and addition during calculating quotient. Original code
wrongly assumed each division loop generate at most two digits, by
|MaxCoefficient < 100|, however this assumption is wrong such as
50,000 / 99,9999.

This patch also fixes assertion failure |n> Precision|, where |Precision| == 18, in |scaleUp(x, n)| via |Decimal::ceil()|.
Before this patch, we don't have 18 digits quotient with 10^-18 exponent from result of division operator.

Additionally, it also update MaxCoefficient static variable value in Decimal.cpp to match the Precision
and the corresponding comment about using 18 as precision.

* Source/WebCore/platform/Decimal.cpp:
- Update "MaxCoefficient" static variable
(Decimal:operator/): Update "divisor" function to account for overflow in multiplication and addition
(Decimal::ceiling): Fixes assertion failure
* LayoutTests/fast/range/input-range-progress-indicator-overflow.html: Add Test Case
* LayoutTests/fast/range/input-range-progress-indicator-overflow-expected.html: Add Test Case Expectation

Canonical link: https://commits.webkit.org/258174@main
  • Loading branch information
Ahmad-S792 authored and Ahmad Saleem committed Dec 21, 2022
1 parent e65bb2e commit d16c8e2
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 7 deletions.
@@ -0,0 +1,9 @@
<!DOCTYPE html>
<body>
<input id='test' type='range' min="1" max='9999999' />
</body>
<script>
function test() {
var x = document.getElementById("test").min = "5000000";
}
</script>
@@ -0,0 +1,4 @@
<!DOCTYPE html>
<body>
<input type='range' min='0' max='9999999' value="5000000" />
</body>
19 changes: 12 additions & 7 deletions Source/WebCore/platform/Decimal.cpp
@@ -1,6 +1,6 @@
/*
* Copyright (C) 2012 Google Inc. All rights reserved.
* Copyright (C) 2013 Apple Inc. All rights reserved.
* Copyright (C) 2012-2015 Google Inc. All rights reserved.
* Copyright (C) 2022 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions are
Expand Down Expand Up @@ -48,7 +48,7 @@ static int const ExponentMax = 1023;
static int const ExponentMin = -1023;
static int const Precision = 18;

static const uint64_t MaxCoefficient = UINT64_C(0x16345785D89FFFF); // 999999999999999999 == 18 9's
static const uint64_t MaxCoefficient = UINT64_C(0XDE0B6B3A763FFFF); // 999999999999999999 == 18 9's

// This class handles Decimal special values.
class SpecialValueHandler {
Expand Down Expand Up @@ -491,13 +491,18 @@ Decimal Decimal::operator/(const Decimal& rhs) const
uint64_t remainder = lhs.m_data.coefficient();
const uint64_t divisor = rhs.m_data.coefficient();
uint64_t result = 0;
while (result < MaxCoefficient / 100) {
while (remainder < divisor) {
for (;;) {
while (remainder < divisor && result < MaxCoefficient / 10) {
remainder *= 10;
result *= 10;
--resultExponent;
}
result += remainder / divisor;
if (remainder < divisor)
break;
uint64_t quotient = remainder / divisor;
if (result > MaxCoefficient - quotient)
break;
result += quotient;
remainder %= divisor;
if (!remainder)
break;
Expand Down Expand Up @@ -627,7 +632,7 @@ Decimal Decimal::ceiling() const
uint64_t result = m_data.coefficient();
const int numberOfDigits = countDigits(result);
const int numberOfDropDigits = -exponent();
if (numberOfDigits < numberOfDropDigits)
if (numberOfDigits <= numberOfDropDigits)
return isPositive() ? Decimal(1) : zero(Positive);

result = scaleDown(result, numberOfDropDigits - 1);
Expand Down

0 comments on commit d16c8e2

Please sign in to comment.