php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #77742 bcpow() implementation related to gcc compiler optimization
Submitted: 2019-03-14 16:10 UTC Modified: 2019-03-14 16:24 UTC
From: samding at ca dot ibm dot com Assigned:
Status: Closed Package: Unknown/Other Function
PHP Version: 7.3.2 OS: Linux
Private report: No CVE-ID: None
Welcome back! If you're the original bug submitter, here's where you can edit the bug or add additional notes.
If you forgot your password, you can retrieve your password here.
Password:
Status:
Package:
Bug Type:
Summary:
From: samding at ca dot ibm dot com
New email:
PHP Version: OS:

 

 [2019-03-14 16:10 UTC] samding at ca dot ibm dot com
Description:
------------
When testing case "ext/bcmath/tests/bcpow_error2.phpt", it is found that 
gcc compiler optimization ("-O2") skipped two statements in following implementation code on s390x:

46 long
 47 bc_num2long (num)
 48      bc_num num;
 49 {
 50   long val;
 51   char *nptr;
 52   int  index;
 53
 54   /* Extract the int value, ignore the fraction. */
 55   val = 0;
 56   nptr = num->n_value;
 57   for (index=num->n_len; (index>0) && (val<=(LONG_MAX/BASE)); index--)
 58     val = val*BASE + *nptr++;
 59
 60   /* Check for overflow.  If overflow, return zero. */
 61   if (index>0) val = 0;                          // when -O2 is used, these 2 lines (61 & 62) are skipped and causes  a wrong value returned.
 62   if (val < 0) val = 0;
....

Here is the comment from gcc compiler developer:

"this code appears to rely on a strictly defined signed overflow. But that's not the case with C/C++. By default GCC assumes that a signed overflow will never happen. So it will optimize away the val < 0 check if it is possible to prove that val will never be subtracted from. 

I think the reason why it hits us only on S390x is that we by default have an unsigned char. So on S/390 the loop constantly adds a positive value to val so GCC deduces that val can only be less than 0 if there was an overflow. 

But in the end the code is not correct I think. It relies on undefined behavior and therefore should be fixed. Perhaps val could be turned into an unsigned long where an overflow is well-defined?   
" 

A possible solution is to define "val" as "unsigned long" and 
change the line 62 as " if (val >=0x8000000000000000 ) val =0;", which does not
depend on the overflow but can detect if val >= 2**63

Test script:
---------------
sapi/cli/php run-tests.php -P ext/bcmath/tests/bcpow_error2.phpt

Expected result:
----------------
val == 0;

Actual result:
--------------
val != 0

Patches

Pull Requests

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2019-03-14 16:19 UTC] nikic@php.net
-Status: Open +Status: Verified
 [2019-03-14 16:19 UTC] nikic@php.net
Indeed, this is clearly UB and should be fixed, preferably by avoiding overflow in the first place. It already checks that val*BASE doesn't overflow, might as well check that the subsequent addition doesn't,
 [2019-03-14 16:24 UTC] samding at ca dot ibm dot com
-PHP Version: 7.3.3 +PHP Version: 7.3.2
 [2019-03-14 16:24 UTC] samding at ca dot ibm dot com
PHP version 7.3.2
 [2019-03-14 16:27 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=e7d40afb7a7984174eb132a14b7a6621c8e76258
Log: Fixed bug #77742
 [2019-03-14 16:27 UTC] nikic@php.net
-Status: Verified +Status: Closed
 
PHP Copyright © 2001-2025 The PHP Group
All rights reserved.
Last updated: Tue Jan 28 03:01:30 2025 UTC