msvcrt:scanf float conversion behavior

[Piotr Caban]

On 12/27/19 4:41 AM, Erich E. Hoover wrote:
> On Thu, Dec 26, 2019 at 7:05 AM Piotr Caban <piotr.caban at gmail.com> wrote:
>> ...
>> I was working on different strtod implementation (ucrtbase (as well as
>> glibc) uses sub 0.5 ulp precision algorithm). It's not ready yet. Also
>> it's not something that can be added during code-freeze.
> 
> Yeah, I was struggling to get something that passes this scanf test
> (without breaking the more numerous strtod tests):
> ===
> ret = vsscanf_wrapper(tests[i], "1.1e-30", "%lf", &double_res);
> ok(ret == 1, "sscanf returned %d for flags %#x\n", ret, tests[i]);
> ok(double_res == 1.1e-30, "got wrong double %.16e for flags %#x\n",
> double_res, tests[i]);
> ===
> I finally came up with the attached the other day, which seems to do
> the trick*.  If you think AJ might be okay with it then would only
> leave webservices/reader.c:str_to_double with "long double" usage,
> which I now have a patch for implementing with scanf.

The 1.1e-30 tests is an example of a test that stopped working in strtod 
with your patches. While working on new implementation of strtod I was 
running Chromium string conversion tests (it's a 100k semi random 
doubles conversion test). Here are some results:
ucrtbase:
  - Windows: 0 failures
  - Current 32-bit Wine: 32062 failures, biggest error: 3 ulp
  - 6e7f357 32-bit Wine: 36 failures, biggest error: 1 ulp
msvcrt:
  - Windows 10: 16 failures

ucrtbase scanf tests:
  - Windows: 0 failures
  - Current 32-bit Wine: 36 failures, biggest error: 1 ulp
  - Current 32-bit Wine with the patch you have attached: 49779 
failures, the exponent is sometimes off by 1, e.g. on 
"1.5475148155234508e-252"

The test that produces the biggest error for strtod function is:
4.0621786324484882e-053

I'm afraid that your patches will possibly cause tons of regressions and 
I'm not sure yet how to proceed with it during the code-freeze. There's 
a quite simple algorithm that should have similar results as msvcrt (I 
will need to implement it in order to know) that can possibly go into 
Wine during code-freeze. On the other hand it will need to be rewritten 
after the code-freeze so I'm not a fun of it. The proper algorithm I was 
working on will definitely exceed 1k lines of code.

> 
>> I was not working on scanf changes. It definitely makes sense to not
>> duplicate the code.
> 
> I've been poking at this a little bit and it looks like scanf supports
> everything that strtod and wcstod need.  There are a couple things
> that our scanf implementation doesn't support yet (inf/nan,
> Fortran-style exponent notation, and hex notation for floats), but
> once those are in place we should be good to go.  So, it may make
> sense to centralize everything in scanf (rather than the other way
> around) so that there isn't any duplication for wide character
> support.

I was thinking about having a common helper that will be used both in 
strtod and scanf. Proper string to double conversion is complicated and 
I don't think it should be mixed with scanf implementation.

Changing the strtod_helper signature to something like:
double strtod_helper(MSVCRT_wchar_t get(void *ctx), void 
unget(MSVCRT_wchar_t c, void *ctx), void *ctx, MSVCRT__locale_t locale, 
int *err)
should make it usable both for strtod like functions and scanf.

> * Also works well enough that the new compare_double should not be
> necessary, unless you've come up with some more test cases ;)
It will be still needed in string.c:3074. The test is written in a way 
that exact comparison will not work on Windows. It would probably also 
make sense to add most conversion tests to ucrtbase because other 
versions have bugs that we probably don't want to reproduce.

Thanks,
Piotr