php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #79595 zend_init_fpu() alters FPU precision
Submitted: 2020-05-13 23:48 UTC Modified: 2020-05-26 16:23 UTC
From: v-yitam at microsoft dot com Assigned: cmb (profile)
Status: Closed Package: Math related
PHP Version: 7.4.5 OS: Alpine Linux
Private report: No CVE-ID: None
View Add Comment Developer Edit
Welcome! If you don't have a Git account, you can't do anything here.
You can add a comment by following this link or if you reported this bug, you can edit this bug over here.
(description)
Block user comment
Status: Assign to:
Package:
Bug Type:
Summary:
From: v-yitam at microsoft dot com
New email:
PHP Version: OS:

 

 [2020-05-13 23:48 UTC] v-yitam at microsoft dot com
Description:
------------
A PHP extension which depends on the FPU precision being the default of the ABI (80 bit) will produce subtly incorrect results with floating point number. 

In particular, it breaks any musl libc-linked binaries that use floating point strtod(). In Alpine Linux, musl's strtod (https://git.musl-libc.org/cgit/musl/tree/src/internal/floatscan.c) depends on long double, which requires FPU set to 80-bit precision to actually work

To illustrate, I wrote two simple test scripts (one cpp and one php), using the example provided in a similar issue (https://github.com/microsoft/WSL/issues/830):

The output from the test program: 1.00000000000000000e-05
The output from running php:      1.00000000000000008e-5

Please see below.

Test script:
---------------
alpine:~$ more test_print.cpp
#include <stdio.h>

int main()
{
  long double n = 1;
  printf("%.17Le\n", n / 100000);
}
alpine:~$ g++ -std=c++11 -static -o test_print test_print.cpp
alpine:~$ ./test_print
1.00000000000000000e-05

alpine:~$ more test_print.cpp
#include <stdio.h>

int main()
{
  long double n = 1;
  printf("%.17Le\n", n / 100000);
}
alpine:~$ g++ -std=c++11 -static -o test_print test_print.cpp
alpine:~$ ./test_print
1.00000000000000000e-05
alpine:~$ more test_print.php
<?php

$n = 1;
printf("%.17e\n", 1/100000);

?>

alpine:~$ php test_print.php
1.00000000000000008e-5



Expected result:
----------------
FPU precision should not be changed

Actual result:
--------------
Have to save/restore FPU state and force it back to 80-bit 

Patches

Add a Patch

Pull Requests

Pull requests:

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2020-05-14 17:40 UTC] v-yitam at microsoft dot com
I wanted to modify my original bug report but it didn't seem possible. Anyway, below is a more accurate example to illustrate the issue. It was copied when running the debugger and stepping through zend_init_fpu() in Zend/zend_float.c:


24	{
(gdb) s
28		if (!EG(saved_fpu_cw_ptr)) {
(gdb) n
29			EG(saved_fpu_cw_ptr) = (void*)&EG(saved_fpu_cw);
(gdb) n
31		XPFPA_STORE_CW(EG(saved_fpu_cw_ptr));
(gdb) n
32	long double n = 1;                                                           
(gdb) n
33	printf("Before %.17Le\n", n/100000);
(gdb) n
Before 1.00000000000000000e-05
35		XPFPA_SWITCH_DOUBLE();
(gdb) n
36	printf("After %.17Le\n", n/100000);
(gdb) n
After 1.00000000000000008e-05
40	}
(gdb)
 [2020-05-18 07:26 UTC] cmb@php.net
From a comment in zend_float.h:

| This header file defines several platform-dependent macros that
| ensure equal and deterministic floating point behaviour across
| several platforms, compilers and architectures.

So, apparently, this is done deliberately.  I'm not sure that this
is a good idea, though.
 [2020-05-19 17:20 UTC] v-yitam at microsoft dot com
Thanks for getting back to us. Indeed, I suspect so:

/* NOTE: This only sets internal precision. MSVC does NOT support double-
   extended precision! */
# define XPFPA_SWITCH_DOUBLE_EXTENDED()

If I changed the macro XPFPA_SWITCH_DOUBLE() to XPFPA_SWITCH_DOUBLE_EXTENDED(), the problem was resolved in Alpine Linux.

Hence, please investigate further why using XPFPA_SWITCH_DOUBLE() is necessary. On Linux the FPU should be left at 80-bit precision, especially Alpine, as musl libc's strtod() requires it to work properly, not to mention it also breaks any functions that expect the standard ABI on Linux.
 [2020-05-20 11:09 UTC] nikic@php.net
Are you interested in this for 32-bit or 64-bit builds?

I think we can safely disable this code if compiling under __SSE__, because we know that PHP itself will not use x87 FPU in that case.

For 32-bit builds targeting x87 FPU, I don't think we want to change this, as we do want normal floating point operations to be based on double precision.
 [2020-05-20 15:20 UTC] v-yitam at microsoft dot com
This floating point calculation issue happens in Alpine Linux only, so we are targeting 64-bit builds for Linux only. Thanks for considering!
 [2020-05-20 15:50 UTC] cmb@php.net
A possible fix might be <https://github.com/php/php-src/pull/5602>.
Could you please try that, @yitam?
 [2020-05-20 15:56 UTC] v-yitam at microsoft dot com
My pleasure! Will get back to you later.
 [2020-05-20 16:53 UTC] v-yitam at microsoft dot com
The patch seems to fix the issue after the preliminary round of testing in Alpine Linux with PHP 7.4.5. Congratulations! Please let us know when this patch will be merged to a stable release.
 [2020-05-22 07:38 UTC] cmb@php.net
-Assigned To: +Assigned To: cmb
 [2020-05-22 07:39 UTC] cmb@php.net
The following pull request has been associated:

Patch Name: Fix #79595: zend_init_fpu() alters FPU precision
On GitHub:  https://github.com/php/php-src/pull/5602
Patch:      https://github.com/php/php-src/pull/5602.patch
 [2020-05-22 13:48 UTC] cmb@php.net
Automatic comment on behalf of cmbecker69@gmx.de
Revision: http://git.php.net/?p=php-src.git;a=commit;h=88dfc475c5937822399843e8aed9b98a36a01813
Log: Fix #79595: zend_init_fpu() alters FPU precision
 [2020-05-22 13:48 UTC] cmb@php.net
-Status: Assigned +Status: Closed
 [2020-05-22 13:50 UTC] cmb@php.net
The fix is scheduled to be released with PHP 7.4.7.
 [2020-05-22 14:57 UTC] nikic@php.net
Automatic comment on behalf of nikita.ppv@gmail.com
Revision: http://git.php.net/?p=php-src.git;a=commit;h=10eb842a6434f7b05dbb8676da57ada445f4434f
Log: Revert &quot;Fix #79595: zend_init_fpu() alters FPU precision&quot;
 [2020-05-25 08:40 UTC] cmb@php.net
The following pull request has been associated:

Patch Name: Fix #79595: zend_init_fpu() alters FPU precision
On GitHub:  https://github.com/php/php-src/pull/5621
Patch:      https://github.com/php/php-src/pull/5621.patch
 [2020-05-25 08:41 UTC] cmb@php.net
-Status: Closed +Status: Re-Opened
 [2020-05-25 08:41 UTC] cmb@php.net
The fix had to be reverted due to unforeseen issues on
i386.  PR #5621 is hopefully a better fix.
 [2020-05-25 17:15 UTC] v-yitam at microsoft dot com
Thank you all for working hard on fixing this issue. I just tested the new proposed fix but unfortunately it didn't work in Alpine Linux. I added the following 'debugging' lines in zend_float.c:

ZEND_API void zend_init_fpu(void) /* {{{ */                                
{                                                                          
int x = -1;                                                                
#if defined(HAVE__CONTROLFP_S) && !defined(__x86_64__)                     
    x = 1;                                                                 
#elif defined(HAVE__CONTROLFP) && !defined(__x86_64__)                     
    x = 2;                                                                 
#elif defined(HAVE__FPU_SETCW)  && !defined(__x86_64__)                    
    x = 3;                                                                 
#elif defined(HAVE_FPSETPREC)  && !defined(__x86_64__)                     
    x = 4;                                                                 
#elif defined(HAVE_FPU_INLINE_ASM_X86)                                     
    x = 5;                                                                 
#else                                                  
    x = 0;                                             
#endif         


Then I ran the debugger:


Breakpoint 1, zend_init_fpu ()
    at /home/jennyt/php-7.4.6/Zend/zend_float.c:24
24	{
(gdb) s
25	int x = -1;
(gdb) s
36	    x = 5;
(gdb)



Hope this helps!
 [2020-05-26 07:03 UTC] cmb@php.net
Thanks for checking, and yes, this was helpful!  I had forgotten
to add the guard to that #elif[1].

[1] <https://github.com/php/php-src/pull/5621/commits/c3fe3f7424b7e9a793ef4b55f197de9e2b6593af>
 [2020-05-26 15:22 UTC] cmb@php.net
Automatic comment on behalf of cmbecker69@gmx.de
Revision: http://git.php.net/?p=php-src.git;a=commit;h=879004dae325e50d3e1a8f9477e66cdfeef0d366
Log: Fix #79595: zend_init_fpu() alters FPU precision
 [2020-05-26 15:22 UTC] cmb@php.net
-Status: Re-Opened +Status: Closed
 [2020-05-26 15:24 UTC] cmb@php.net
Note that the fix missed the deadline for PHP 7.4.7, so will have
to wait for PHP 7.4.8 (roughly mid July).
 [2020-05-26 16:23 UTC] v-yitam at microsoft dot com
> Note that the fix missed the deadline for PHP 7.4.7, so will have
to wait for PHP 7.4.8 (roughly mid July).

No worries. We will jot it down. A brief testing shows that the new fix works. Thank you all!
 
PHP Copyright © 2001-2020 The PHP Group
All rights reserved.
Last updated: Sat Jul 11 03:01:25 2020 UTC