C++ Standard Library
  1. C++ Standard Library
  2. STDCXX-2

[MSVC] std::num_put bad formatting of 0.0 with precision and showpoint

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 4.1.2, 4.1.3, 4.1.4, 4.2.0
    • Fix Version/s: 4.2.1
    • Component/s: 22. Localization
    • Labels:
      None
    • Environment:

      Windows/MSVC

    • Severity:
      Incorrect Behavior

      Description

      When compiled with MSVC (any version), the program below aborts at runtime.

       
      $ cat t.cpp && cl  -D_RWCONFIG=11s_msvc_7_1 -Ic:/contrib/cygwin/build/sebor/dev-hal/include -I./../../../../include -Ic:/contrib/cygwin/build/sebor/dev-hal/examples/stdlib/manual/../include  -Ic:/contrib/cygwin/build/sebor/dev-hal/include/ansi -I./../../../..
      -Ic:/contrib/cygwin/build/sebor/dev-hal -Ic:/contrib/cygwin/build/sebor/dev-hal/examples/stdlib/manual -I. -nologo -GX -MLd -W3 -Zi -GA -GR -GF -GZ  -c t.cpp && link  -nologo /NODEFAULTLIB:libcpd /debug /LIBPATH:./../../../../lib /OUT:t.exe t.obj  std11s_msvc_7_1.lib user32.lib t.cpp && ./t.exe
      
      #include <cassert>
      #include <sstream>
      
      int main ()
      {
          std::ostringstream strm;
      
          strm.setf (strm.showpoint);
          strm.precision (2);
      
          strm << 0.0;
      
          assert ("0.0" == strm.str ());
      }
      
      Assertion failed: "0.0" == strm.str (), file t.cpp, line 13
      

        Issue Links

          Activity

          Hide
          Martin Sebor added a comment -

          Assigning to Farid – whenever you have a moment.

          Show
          Martin Sebor added a comment - Assigning to Farid – whenever you have a moment.
          Hide
          Farid Zaripov added a comment -

          The problem in the MSVC CRT.

          The expression strm << 0.0; is invokes the __rw_put_num() function, where used the CRT function snprintf() (num_put.cpp, line 752):

          snprintf (buf, size, "%#.2lg", 0.0);

          The result of the snprintf() call above is: "0.00".

          Info from MSDN:
          -----------------------------
          Flag '#' means: When used with the g or G format, the # flag forces the output value to contain a decimal point in all cases and prevents the truncation of trailing zeros.

          The precision (".2") means: The precision specifies the maximum number of significant digits printed.
          -----------------------------

          The prefix 'l' before type character 'g' here should point to the long double type, but passed double number (0.0). Anyway this is not causes any problem because of the types double and long double have the same internal representation on the MSVC.

          Show
          Farid Zaripov added a comment - The problem in the MSVC CRT. The expression strm << 0.0; is invokes the __rw_put_num() function, where used the CRT function snprintf() (num_put.cpp, line 752): snprintf (buf, size, "%#.2lg", 0.0); The result of the snprintf() call above is: "0.00". Info from MSDN: ----------------------------- Flag '#' means: When used with the g or G format, the # flag forces the output value to contain a decimal point in all cases and prevents the truncation of trailing zeros. The precision (".2") means: The precision specifies the maximum number of significant digits printed. ----------------------------- The prefix 'l' before type character 'g' here should point to the long double type, but passed double number (0.0). Anyway this is not causes any problem because of the types double and long double have the same internal representation on the MSVC.
          Hide
          Farid Zaripov added a comment -

          We need to fix the bug (avoid to use snprintf on the MSVC).

          Show
          Farid Zaripov added a comment - We need to fix the bug (avoid to use snprintf on the MSVC).
          Hide
          Andrew Black added a comment -

          Updating affects/fix versions to reflect STDCXX-497 (Should this be closed as a duplicate?)

          Show
          Andrew Black added a comment - Updating affects/fix versions to reflect STDCXX-497 (Should this be closed as a duplicate?)
          Hide
          Martin Sebor added a comment -

          Usually the more recently opened issue is closed as a duplicate of a pre-existing one, so we should probably close STDCXX-497.

          Show
          Martin Sebor added a comment - Usually the more recently opened issue is closed as a duplicate of a pre-existing one, so we should probably close STDCXX-497 .
          Hide
          Martin Sebor added a comment -

          Added 4.2.0 to the set of Affected Versions and set Severity to Incorrect Behavior.

          Show
          Martin Sebor added a comment - Added 4.2.0 to the set of Affected Versions and set Severity to Incorrect Behavior.
          Hide
          Farid Zaripov added a comment -

          Attached patch and regression test.

          Show
          Farid Zaripov added a comment - Attached patch and regression test.
          Hide
          Farid Zaripov added a comment -

          Resolving issue to fix the spent time.

          Show
          Farid Zaripov added a comment - Resolving issue to fix the spent time.
          Hide
          Farid Zaripov added a comment -

          Reopening issue since the patch is not commited

          Show
          Farid Zaripov added a comment - Reopening issue since the patch is not commited
          Hide
          Martin Sebor added a comment -

          Wouldn't the __rw_fix_flt function be a better place for the workaround?

          Show
          Martin Sebor added a comment - Wouldn't the __rw_fix_flt function be a better place for the workaround?
          Hide
          Martin Sebor added a comment -

          Btw., I note __rw_fix_flt uses the _WIN32 macro to enable another workaround for a bug (or "feature," if you work for Microsoft) in the MSVC CRT, while the workaround in your patch is guarded with _MSC_VER. Even though the latter is arguably ore appropriate, I suspect they are both subtly wrong: the former will apply the workaround for gcc on Cygwin (which, presumably, doesn't need it because it uses its own C library), while the latter will (likely) fail to MinGW which I believe does sit on top of the Microsoft libc.

          Show
          Martin Sebor added a comment - Btw., I note __rw_fix_flt uses the _WIN32 macro to enable another workaround for a bug (or "feature," if you work for Microsoft) in the MSVC CRT, while the workaround in your patch is guarded with _MSC_VER . Even though the latter is arguably ore appropriate, I suspect they are both subtly wrong: the former will apply the workaround for gcc on Cygwin (which, presumably, doesn't need it because it uses its own C library), while the latter will (likely) fail to MinGW which I believe does sit on top of the Microsoft libc.
          Hide
          Farid Zaripov added a comment -

          The gcc on cygwin doesn't defines the _WIN32 macro.

          So I suppose that we can replace everywhere _MSC_VER to _WIN32 where we suppose to use Microsoft libc.

          Show
          Farid Zaripov added a comment - The gcc on cygwin doesn't defines the _WIN32 macro. So I suppose that we can replace everywhere _MSC_VER to _WIN32 where we suppose to use Microsoft libc.
          Hide
          Farid Zaripov added a comment -

          __rw_fix_flt() is the better place of course and I thought of using it, to fix this issue.
          But this workaround is intended for zero float value only, but the value doesn't passed to the __rw_fix_flt().

          If we could extend the parameters list of the __rw_fix_flt() to append the bool value indicating that val is zero (see the attached num_put.diff2 patch).

          Show
          Farid Zaripov added a comment - __rw_fix_flt() is the better place of course and I thought of using it, to fix this issue. But this workaround is intended for zero float value only, but the value doesn't passed to the __rw_fix_flt(). If we could extend the parameters list of the __rw_fix_flt() to append the bool value indicating that val is zero (see the attached num_put.diff2 patch).
          Hide
          Farid Zaripov added a comment -

          The another patch is attached.

          Show
          Farid Zaripov added a comment - The another patch is attached.
          Hide
          Martin Sebor added a comment -

          I thought the macro was defined in the Windows headers but after reading the Predefined Macros it looks like I thought wrong.

          I don't think replacing _MSC_VER with _WIN32 would be safe, even if _WIN32 is defined by Visual Studio and not by gcc on Cygwin. There might be compilers out there (maybe gcc on MinGW, or Borland C++, or Digital Mars) that do define it.

          IMO, I see a need to distinguish between the following platforms/environments:

          • the compiler (we have that)
          • the language runtime library independent of the compiler to handle cases where the same library is shared by multiple compilers (such as gcc's libsupc++ that's used by Intel C++ and IBM XLC++ on Linux but not by Sun C++)
          • the C library (such as the one used by Visual Studio and Intel C++ on Windows, but not by gcc on Cygwin)
          • the OS itself (we have that)
          Show
          Martin Sebor added a comment - I thought the macro was defined in the Windows headers but after reading the Predefined Macros it looks like I thought wrong. I don't think replacing _MSC_VER with _WIN32 would be safe, even if _WIN32 is defined by Visual Studio and not by gcc on Cygwin. There might be compilers out there (maybe gcc on MinGW, or Borland C++, or Digital Mars) that do define it. IMO, I see a need to distinguish between the following platforms/environments: the compiler (we have that) the language runtime library independent of the compiler to handle cases where the same library is shared by multiple compilers (such as gcc's libsupc++ that's used by Intel C++ and IBM XLC++ on Linux but not by Sun C++) the C library (such as the one used by Visual Studio and Intel C++ on Windows, but not by gcc on Cygwin) the OS itself (we have that)
          Hide
          Martin Sebor added a comment -

          I started to document the ways we distinguish between the various platforms on the Platforms page on our Wiki. The table is still incomplete so if you have additional info it'd be great if you could help complete it

          Show
          Martin Sebor added a comment - I started to document the ways we distinguish between the various platforms on the Platforms page on our Wiki. The table is still incomplete so if you have additional info it'd be great if you could help complete it
          Hide
          Martin Sebor added a comment -

          Unfortunately, using equality comparison on floats is problematic due to rounding errors and floating point exceptions triggered by NaNs. If this is a problem only for the case of "0.0" maybe we could test the string instead to see if the value is 0?

          Show
          Martin Sebor added a comment - Unfortunately, using equality comparison on floats is problematic due to rounding errors and floating point exceptions triggered by NaNs. If this is a problem only for the case of "0.0" maybe we could test the string instead to see if the value is 0?
          Hide
          Martin Sebor added a comment -

          Btw., changing the file suffix from .diff to .diff2 prevents some browsers from recognizing the file type. If Jira requires the names of attachments to be unique, or if we want to make them unique to preserve the chronological order in which they were attached, I suggest appending the number to the base name of the file. I.e., go with file.diff, file-2.diff, file-3.diff, etc.

          Show
          Martin Sebor added a comment - Btw., changing the file suffix from .diff to .diff2 prevents some browsers from recognizing the file type. If Jira requires the names of attachments to be unique, or if we want to make them unique to preserve the chronological order in which they were attached, I suggest appending the number to the base name of the file. I.e., go with file.diff , file-2.diff , file-3.diff , etc.
          Show
          Farid Zaripov added a comment - Fixed thus: http://svn.apache.org/viewcvs?view=rev&rev=629100 and http://svn.apache.org/viewcvs?view=rev&rev=631678 Regression test added thus: http://svn.apache.org/viewcvs?view=rev&rev=628071 Changes merged in 4.2.x branch thus: http://svn.apache.org/viewcvs?view=rev&rev=637239

            People

            • Assignee:
              Farid Zaripov
              Reporter:
              Martin Sebor
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Time Tracking

                Estimated:
                Original Estimate - 4h
                4h
                Remaining:
                Time Spent - 2.5h Remaining Estimate - 1.5h
                1.5h
                Logged:
                Time Spent - 2.5h Remaining Estimate - 1.5h
                2.5h

                  Development