Skip to content

Fix overflow issue in Number.prototype.toString for large numbers#2471

Merged
lahma merged 1 commit into
sebastienros:mainfrom
asheigithub:main
May 14, 2026
Merged

Fix overflow issue in Number.prototype.toString for large numbers#2471
lahma merged 1 commit into
sebastienros:mainfrom
asheigithub:main

Conversation

@asheigithub

Copy link
Copy Markdown
Contributor

Fixes a crash when calling Number.prototype.toString(radix) on very large numbers such as:

(-12345e+30).toString(2)

Summary

Use Math.Truncate(x) to avoid overflow when converting extremely large Numbers to string in non‑decimal radices.

Details

The previous implementation cast the Number directly to long:

var integer = (long)x;

This overflows for values larger than long.MaxValue (≈9e18), which is common for Numbers with large exponents such as -12345e+30.

  • Replace (long)x with Math.Truncate(x) to safely obtain the integer part without overflow.
  • When the integer part exceeds long.MaxValue, repeatedly divide by the radix until the value fits into long.
  • Append the corresponding number of zeros to reconstruct the correct magnitude.

Linked issue

Fixes # (no existing issue; this PR includes the fix)

Test plan

  • Added or updated unit tests in Jint.Tests
  • Ran dotnet test --configuration Release locally
  • For ECMAScript spec changes: ran Jint.Tests.Test262 and confirmed no regressions
  • For interop changes: covered in Jint.Tests/Runtime/Interop
  • For perf changes: included before/after numbers from Jint.Benchmark

Breaking change?

No.

@lahma

lahma commented May 14, 2026

Copy link
Copy Markdown
Collaborator

Please add test coverage.

(-12345e+30).toString(2) and similar calls used to cast x to long
directly, which overflowed for values beyond ~9.22e18 (long.MaxValue)
and silently returned wrong digits (or threw).

Use BigInteger for the integer part once the truncated value no longer
fits in long. This produces the mathematically exact base-r
representation of the stored double for every radix in 2..36, while
keeping the fast (long) path for small values.

Add ShouldWriteLargeNumbersUsingBasesWithoutOverflow covering
power-of-2 radices (bit-exact, matches V8), non-power-of-2 radices, the
long.MaxValue boundary, and small-value regression cases.

Co-Authored-By: Marko Lahma <marko.lahma@gmail.com>
@lahma

lahma commented May 14, 2026

Copy link
Copy Markdown
Collaborator

Hi @asheigithub — I rebased this onto current main and replaced the integer-overflow handling with a BigInteger-based path, because the original divide-and-pad approach was only correct for power-of-2 radices.

Why the original fix was incomplete

The "divide by radix in floating-point until it fits in long, then append k zeros" loop only produces correct digits for radices 2, 4, 8, 16, 32. For those:

  • IEEE 754 doubles store m × 2^e, and for any value > 2^63 we necessarily have e ≥ 11, so dividing by a power of 2 is exact bit-shifting.
  • The bits we drop are below the 53-bit mantissa precision, so they are already zero — the trailing k zero-padding reconstructs them faithfully.

For non-power-of-2 radices (3, 5, 6, 7, 9, 11–15, 17–31, 33–36) neither property holds:

  • Division by an odd radix in floating point rounds to the nearest representable double, accumulating error across iterations.
  • The trailing base-r digits of the true value are not generally zero, so padding with '0' silently produces wrong digits.

Concrete example with the old fix:

(1e+20).toString(3)
// old fix: 220200020122120112010222022122002000000000  (wrong)
// new fix: 220200020122120112010222022122002000100201  (exact)

What the new commit does

  • Keep the fast (long) path for truncated < 2^63.
  • Otherwise convert the truncated double directly to BigInteger (which preserves the exact stored value) and use a new ToBase(BigInteger, int) overload.
  • Tighten the boundary: the original if (integer > long.MaxValue) check is unsafe because (double)long.MaxValue rounds up to 2^63, so the boundary value itself overflows the subsequent (long)integer cast. The new code compares against 2^63 explicitly.

Test coverage

Added ShouldWriteLargeNumbersUsingBasesWithoutOverflow covering:

  • The original PR's crash case (-12345e+30).toString(2).
  • Power-of-2 radices (bit-exact, matches V8/SpiderMonkey).
  • Non-power-of-2 radices (3, 5, 7, 36) — outputs are the mathematically exact base-r representation of the stored double. These differ from V8's output for these radices, but V8's algorithm is "implementation-approximated" per spec and actually drifts away from the real stored value (e.g. for (1e+25).toString(36) V8 returns 198exbvshgps00000 which decodes back to 9999999999999999839010816, while the stored double is 10000000000000000905969664).
  • long.MaxValue boundary values and small-value regression cases.

All 2997 Jint.Tests pass and all 1232 Number-related test262 tests pass with zero regressions. Authorship is preserved on the squashed commit.

@lahma
lahma enabled auto-merge (squash) May 14, 2026 11:55
@lahma
lahma merged commit 50a8a44 into sebastienros:main May 14, 2026
4 checks passed
@asheigithub

Copy link
Copy Markdown
Contributor Author

thank you ! my English is not very good,
But I am very happy that my small patch helped you find the real problem.

Your BigInteger solution looks great.
Thank you for explaining everything so clearly. I learned a lot from your message.

I will keep learning and hope I can make better contributions in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants