Bug #77742 [Ver->Csd]: bcpow() implementation related to gcc compiler optimization

Edit report at https://bugs.php.net/bug.php?id=77742&edit=1 ID: 77742 Updated by: nikic@php.net Reported by: samding at ca dot ibm dot com Summary: bcpow() implementation related to gcc compiler optimization -Status: Verified +Status: Closed Type: Bug Package: Unknown/Other Function Operating System: Linux PHP Version: 7.3.2 Block user comment: N Private report: N New Comment: 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 Previous Comments: ------------------------------------------------------------------------ [2019-03-14 16:24:03] samding at ca dot ibm dot com PHP version 7.3.2 ------------------------------------------------------------------------ [2019-03-14 16:19:01] 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:10:38] 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 ------------------------------------------------------------------------ -- Edit this bug report at https://bugs.php.net/bug.php?id=77742&edit=1