Check strtod()/std::stod() implementations, see if dmg_fp can be removed |
|||||||||
Issue descriptionFrom an email thread regarding whether we need dmg_fp: Some Googling turns up http://www.exploringbinary.com/how-strtod-works-and-sometimes-doesnt/ , which ends with links to several articles about cases which various non-dmg_fp implementations got wrong at the time: http://www.exploringbinary.com/incorrectly-rounded-conversions-in-visual-c-plus-plus/ http://www.exploringbinary.com/incorrectly-rounded-conversions-in-gcc-and-glibc/ http://www.exploringbinary.com/incorrect-decimal-to-floating-point-conversion-in-sqlite/ http://www.exploringbinary.com/double-rounding-errors-in-floating-point-conversions/ I would say we should do the following: * Go through the examples on those pages and see if std::strtod() is correct on all the current toolchains we support. The articles suggest, for example, that GCC/glibc have been fixed as of December 2013 (which may be too new for us to have everywhere) while MSVC had not as of 2013. * If these are not correct, then also check that we have the latest dmg_fp sources. In the process of writing the article the author found various bugs in dmg_fp that were subsequently fixed.
,
Mar 10 2016
,
Mar 10 2016
Yes, it's only ~9k binary for strtod (might be more on other platforms?), but that code is a bit weird and has been a bit of a hassle (warnings, fuzzing, etc.). So if we can get rid of it now, that would be nice.
,
Mar 14 2016
Ah okay. I'm busy with other stuff, and it's a good firs bug. Leaving this one open then.
,
Jun 15 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8d7634d843a6b981d4b9bebe8f94412b6bccc3be commit 8d7634d843a6b981d4b9bebe8f94412b6bccc3be Author: scottmg <scottmg@chromium.org> Date: Wed Jun 15 17:52:45 2016 Add strtod test cases Adds test cases from http://www.exploringbinary.com/how-strtod-works-and-sometimes-doesnt/. These currently fail on (see ps#1): - linux_chromium_gn_chromeos_rel - linux_chromium_asan_rel_ng - linux_chromium_chromeos_ozone_rel_ng - linux_chromium_rel_ng - cast_shell_linux but despite history appear to be fine on Windows (as of VS2015). As they were a bit of fiddling to get up and running, and we don't appear to have any similar tests for dmg_fp, add these tests so that we can hopefully drop dmg_fp in the future once std::strtod works more consistently everywhere. https://crbug.com/95729 suggests another solution where it'd be good to have these tests. BUG=593512,615142, 616062 , 588726 ,95729 Review-Url: https://codereview.chromium.org/2044643005 Cr-Commit-Position: refs/heads/master@{#399948} [modify] https://crrev.com/8d7634d843a6b981d4b9bebe8f94412b6bccc3be/base/strings/string_number_conversions_unittest.cc
,
Sep 26 2016
If we want to replace dmg_fp, be careful about feature mismatch between libraries. For example, std::strtod accepts leading spaces but dmg_fp doesn't. std::strtod accepts hex numbers (0x prefix) but dmg_fp doesn't (by our setup)
,
Oct 14 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7bd66b6c465395634479807d17b5d1cf9f5ac6d6 commit 7bd66b6c465395634479807d17b5d1cf9f5ac6d6 Author: kcwu <kcwu@chromium.org> Date: Fri Oct 14 22:37:28 2016 Add boundary test case to StringToDouble The max presentable double is HUGE_VAL=(2^53-1)*(2^971). And (2^53)*(2^971) is considered overflow. Added cases above and below (2^53-0.5)*(2^971). The value above is overflow and the value below is not. std::strtod on linux doesn't pass the above case (it returns inf correctly but doesn't set errno=ERANGE). BUG=593512 Review-Url: https://codereview.chromium.org/2415353002 Cr-Commit-Position: refs/heads/master@{#425496} [modify] https://crrev.com/7bd66b6c465395634479807d17b5d1cf9f5ac6d6/base/strings/string_number_conversions_unittest.cc
,
Oct 21 2016
Note that this would likely be web exposed if we did it too, we need to be cautious about the edge cases like whitespace and hex as kcwu@ mentions.
,
Oct 31 2016
I just noticed today that strtod() is the C function and std::stod() is the C++11 one. They look to have a slightly different interface (the latter expects std::string, among other things); I don't know if they have different conversino behavior.
,
Nov 1 2017
This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue. Sorry for the inconvenience if the bug really should have been left as Available. If you change it back, also remove the "Hotlist-Recharge-Cold" label. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Nov 1 2017
,
Nov 1
This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue. Sorry for the inconvenience if the bug really should have been left as Available. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Nov 1
|
|||||||||
►
Sign in to add a comment |
|||||||||
Comment 1 by hua...@chromium.org
, Mar 10 2016