New issue
Advanced search Search tips

Issue 593512 link

Starred by 5 users

Issue metadata

Status: Available
Owner: ----
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug

Sign in to add a comment

Check strtod()/std::stod() implementations, see if dmg_fp can be removed

Project Member Reported by, Mar 9 2016

Issue description

From an email thread regarding whether we need dmg_fp:

Some Googling turns up , which ends with links to several articles about cases which various non-dmg_fp implementations got wrong at the time:

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.

Comment 1 by, Mar 10 2016


Comment 2 by, Mar 10 2016

Status: Assigned (was: Available)
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.

Comment 4 by, Mar 14 2016

Owner: ----
Status: Available (was: Assigned)
Ah okay.  I'm busy with other stuff, and it's a good firs bug.  Leaving this one open then.
Project Member

Comment 5 by, Jun 15 2016

The following revision refers to this bug:

commit 8d7634d843a6b981d4b9bebe8f94412b6bccc3be
Author: scottmg <>
Date: Wed Jun 15 17:52:45 2016

Add strtod test cases

Adds test cases from

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. suggests another solution where it'd
be good to have these tests.

BUG=593512,615142, 616062 , 588726 ,95729

Cr-Commit-Position: refs/heads/master@{#399948}


Comment 6 by, 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)

Project Member

Comment 7 by, Oct 14 2016

The following revision refers to this bug:

commit 7bd66b6c465395634479807d17b5d1cf9f5ac6d6
Author: kcwu <>
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).


Cr-Commit-Position: refs/heads/master@{#425496}


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.
Summary: Check strtod()/std::stod() implementations, see if dmg_fp can be removed (was: Check std::strtod() implementations, see if dmg_fp can be removed)
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.
Project Member

Comment 10 by, Nov 1 2017

Labels: Hotlist-Recharge-Cold
Status: Untriaged (was: Available)
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 - Your friendly Sheriffbot
Labels: -Hotlist-Recharge-Cold
Status: Available (was: Untriaged)

Sign in to add a comment