Skip to content

guard non-positive requested digits in DoubleToAscii and BignumDtoa - #318

Open
jdymitarai wants to merge 1 commit into
google:masterfrom
jdymitarai:guard-negative-digits
Open

guard non-positive requested digits in DoubleToAscii and BignumDtoa#318
jdymitarai wants to merge 1 commit into
google:masterfrom
jdymitarai:guard-negative-digits

Conversation

@jdymitarai

Copy link
Copy Markdown
Contributor

Problem

Following the hardening of DigitGenCounted (#313), GenerateCountedDigits (#314), and FastFixedDtoa (#315) against non-positive/negative counts, entry points and bignum utilities still had edge cases in release builds (-DNDEBUG):

  1. DoubleToStringConverter::DoubleToAscii:

    • DoubleToAscii checked if (mode == PRECISION && requested_digits == 0) to return an empty representation (*length = 0, *point = 0).
    • When called with requested_digits < 0 in release builds (where DOUBLE_CONVERSION_ASSERT is compiled out), this check failed. FastDtoa returned false (guard non-positive requested_digits in DigitGenCounted #313), and execution proceeded to BignumDtoa, setting *length = 0 (guard non-positive count in GenerateCountedDigits #314) but leaving *point at an unreset, non-zero value, after doing redundant bignum work.
    • For mode == FIXED with requested_digits < 0, DoubleToAscii fell through to BignumDtoa, where BignumToFixed could emit truncated integer digits into buffer when (*decimal_point) + requested_digits > 0.
  2. BignumDtoa:

    • For mode == BIGNUM_DTOA_PRECISION && requested_digits <= 0 or mode == BIGNUM_DTOA_FIXED && requested_digits < 0, BignumDtoa lacked early return guards.
  3. Bignum Shift and Exponent Operations:

    • Bignum::ShiftLeft(shift_amount): if shift_amount <= 0, non-positive shifts are a no-op, but previously called BigitsShiftLeft(local_shift) which triggered undefined behavior on negative shifts.
    • Bignum::MultiplyByPowerOfTen(exponent): checked if (exponent == 0) return;. For exponent < 0, it bypassed the multiplier loops and called ShiftLeft(exponent) with a negative shift amount.
    • Bignum::AssignPowerUInt16(base, power_exponent): checked if (power_exponent == 0) return;. Negative exponents bypassed the loops and called ShiftLeft(shifts * power_exponent).

Solution

  1. In DoubleToStringConverter::DoubleToAscii, guard (mode == PRECISION && requested_digits <= 0) || (mode == FIXED && requested_digits < 0) by returning early with an empty string, *length = 0, and *point = 0.
  2. In BignumDtoa, return early with an empty string, *length = 0, and *decimal_point = 0 for (mode == BIGNUM_DTOA_PRECISION && requested_digits <= 0) || (mode == BIGNUM_DTOA_FIXED && requested_digits < 0).
  3. In Bignum::ShiftLeft, return early if shift_amount <= 0.
  4. In Bignum::MultiplyByPowerOfTen and Bignum::AssignPowerUInt16, change checks to <= 0.
  5. Add regression test cases across test/cctest/test-dtoa.cc, test/cctest/test-bignum-dtoa.cc, and test/cctest/test-bignum.cc.

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.

1 participant