Repository navigation
fix(dom): mark mantissa truncated on the slow path when an exponent follows - #154
Open
linhongyu510 wants to merge 1 commit into
Open
linhongyu510 wants to merge 1 commit into
linhongyu510 wants to merge 1 commit into
Conversation
When parsing a number whose integer part is longer than 19 digits on the slow path, digits beyond the first 19 are dropped from 'man' and counted into 'exp10', but 'trunc' was never set. Without an explicit exponent the code falls through the Integer section which sets 'trunc' before the fast path, but with an explicit exponent it jumps straight to 'double_exp' and then to ParseFloatingNormalFast, which requires an exact significand. The truncated 19-digit mantissa is then treated as exact, producing a result that is 1 ULP below the correctly rounded value. Setting 'trunc' on the slow path makes the Eisel-Lemire consistency check kick in and fall back to the native parser when the truncated mantissa is not exactly representable, matching correctly-rounded strtod output. Adds a regression test with bit-exact expectations (EXPECT_DOUBLE_EQ tolerates a 4-ULP difference and would not catch this).
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #154 +/- ##
=======================================
Coverage 69.67% 69.67%
=======================================
Files 27 27
Lines 3706 3707 +1
Branches 1189 1189
=======================================
+ Hits 2582 2583 +1
Misses 211 211
Partials 913 913 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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.
Summary
parseNumberhas a slow path for integers longer than 19 digits: the first 19 digits are kept inman, and every remaining digit incrementsexp10. The dropped digits were never reflected intrunc.trunc = 1before the fast path — so results are correct....e0,...E+7, …), control jumps todouble_expand skips that assignment.truncstays 0, soParseFloatingNormalFastreceives a truncated significand but treats it as exact, producing a value 1 ULP below the correctly rounded result.Setting
trunc = 1on the slow path makes the Eisel–Lemire consistency check trigger the fallback to the native parser whenever the truncated mantissa cannot be rounded exactly.Evidence
Differential audit vs correctly-rounded
strtod(Pythonfloat()is bit-identical):kParseErrorInfinity).-32707769411490445312(no exponent) → correct;-32707769411490445312e0→ 1 ULP low.Test
Added
ParserTest.LongMantissaWithExponentwith 7 inputs that reproduced the bug, asserted bit-exact (viaEXPECT_EQon the raw uint64 bits).EXPECT_DOUBLE_EQwould not catch this — it tolerates a 4-ULP difference.300 testsin the suite pass.Checklist
cmake --build(AppleClang 21,-O3 -DNDEBUG -std=c++17 -arch arm64 -mcpu=apple-m1)clang-formatclean