Fix performance for Bcmath numbers with trailing zeros #2

jorgsowa · 2023-07-07T19:10:57Z

This PR improves the performance of the BC extension computations for the decimal numbers with trailing zeros. Specifically we get Memory limit error while we want to raise zero with many trailing zeros to large exponent. Example: https://onlinephp.io/c/dcd86
Also it may improve other special cases of operations with number zero, like multiplying with number zero.

Changes made:

while we transform the number from string to bc_num we strip of the trailing zeros lower the scale.

The typo in HAVE_PTHREAD_ATTR_GET_STACK (might be due to pthread_attr_get_np being different from Linux's pthread_getattr_np) led to this code path never get called on FreeBSD.

x8 being already reserved, we can only pull x18 to x27.

Fixes OSS Fuzz #60735

The normal xmlSetProp() function parses the name to find the namespace. But we don't care about the namespace in this case.

These checks will always be false because we're dealing with elements here. DOMElement always are of type XML_ELEMENT_NODE, so they don't need to be checked.

dom_get_doc_props_read_only() already does a NULL check.

* ext/bcmath: coding style: use indentation And add braces to block statements, as the current code was pretty much unreadable with how inconsistent it was. * ext/bcmath: Remove some useless header inclusions * ext/bcmath: Use standard C99 bool type instead of char * ext/bcmath: Include specific headers instead of config.h * Restructure definitions to reduce header inclusions * Use size_t as a more appropriate type * Remove unused variable full_scale * Refactor bc_raisemod() to get rid of Zend dependencies This separates the concerns of throwing exceptions back into the PHP_FUNCTION instead of being the responsibility of the library * Refactor bc_raise() to get rid of Zend dependencies This separates the concerns of throwing exceptions back into the PHP_FUNCTION instead of being the responsibility of the library * Refactor bc_divmod() and bc_modulo() to return bool Return false on division by 0 attempt instead of -1 and true on success instead of 0 * Refactor bc_divide() to return bool Return false on division by 0 attempt instead of -1 and true on success instead of 0

- publicId could crash PHP if none was provided - notationName never worked The fields of this classs were untested. This new test file changes that. Closes phpGH-11779.

…deNS() Closes phpGH-11776.

Previously, when replacing the node with itself (or contained within itself), the node disappeared. Closes phpGH-11770.

Closes phpGH-11768.

Fixes oss-fuzz #60741 Closes phpGH-11780

Fixes phpGH-11547 Closes phpGH-11583

…Immutable objects

even without sanitizers, it is reproducible but with the following ``` <?php $g = gmp_init(256); var_dump(gmp_pow($g, PHP_INT_MAX)); ``` we get this ``` AddressSanitizer:DEADLYSIGNAL ================================================================= ==286922==ERROR: AddressSanitizer: FPE on unknown address 0x03e8000460ca (pc 0x7faf6c69de5c bp 0x400000000000004 sp 0x7ffe9843c740 T0) #0 0x7faf6c69de5c in __pthread_kill_implementation nptl/pthread_kill.c:44 #1 0x7faf6c649c81 in __GI_raise ../sysdeps/posix/raise.c:26 #2 0x7faf6db9386c in __gmp_exception (/lib/x86_64-linux-gnu/libgmp.so.10+0xd86c) (BuildId: 1af68a49fe041a5bb48a2915c3d47541f713bb38) #3 0x7faf6db938d3 in __gmp_overflow_in_mpz (/lib/x86_64-linux-gnu/libgmp.so.10+0xd8d3) (BuildId: 1af68a49fe041a5bb48a2915c3d47541f713bb38) #4 0x7faf6dbac95c in __gmpz_realloc (/lib/x86_64-linux-gnu/libgmp.so.10+0x2695c) (BuildId: 1af68a49fe041a5bb48a2915c3d47541f713bb38) #5 0x7faf6dba9038 in __gmpz_n_pow_ui (/lib/x86_64-linux-gnu/libgmp.so.10+0x23038) (BuildId: 1af68a49fe041a5bb48a2915c3d47541f713bb38) #6 0x5565ae1ccd9f in zif_gmp_pow /home/dcarlier/Contribs/php-src/ext/gmp/gmp.c:1286 #7 0x5565aee96ea9 in ZEND_DO_ICALL_SPEC_RETVAL_USED_HANDLER /home/dcarlier/Contribs/php-src/Zend/zend_vm_execute.h:1312 #8 0x5565af144320 in execute_ex /home/dcarlier/Contribs/php-src/Zend/zend_vm_execute.h:56075 #9 0x5565af160f07 in zend_execute /home/dcarlier/Contribs/php-src/Zend/zend_vm_execute.h:60439 #10 0x5565aed6fafe in zend_execute_scripts /home/dcarlier/Contribs/php-src/Zend/zend.c:1842 #11 0x5565aeae70a8 in php_execute_script /home/dcarlier/Contribs/php-src/main/main.c:2578 #12 0x5565af532f4e in do_cli /home/dcarlier/Contribs/php-src/sapi/cli/php_cli.c:964 #13 0x5565af535877 in main /home/dcarlier/Contribs/php-src/sapi/cli/php_cli.c:1334 #14 0x7faf6c633d67 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58 #15 0x7faf6c633e24 in __libc_start_main_impl ../csu/libc-start.c:360 #16 0x5565adc04040 in _start (/home/dcarlier/Contribs/php-src/sapi/cli/php+0x2604040) (BuildId: 949049955bdf8b7197390b1978a1dfc3ef6fdf38) AddressSanitizer can not provide additional info. SUMMARY: AddressSanitizer: FPE nptl/pthread_kill.c:44 in __pthread_kill_implementation ==286922==ABORTING ```

even without sanitizers, it is reproducible but with the following ``` <?php $g = gmp_init(256); var_dump(gmp_pow($g, PHP_INT_MAX)); ``` we get this ``` AddressSanitizer:DEADLYSIGNAL ================================================================= ==286922==ERROR: AddressSanitizer: FPE on unknown address 0x03e8000460ca (pc 0x7faf6c69de5c bp 0x400000000000004 sp 0x7ffe9843c740 T0) #0 0x7faf6c69de5c in __pthread_kill_implementation nptl/pthread_kill.c:44 #1 0x7faf6c649c81 in __GI_raise ../sysdeps/posix/raise.c:26 #2 0x7faf6db9386c in __gmp_exception (/lib/x86_64-linux-gnu/libgmp.so.10+0xd86c) (BuildId: 1af68a49fe041a5bb48a2915c3d47541f713bb38) #3 0x7faf6db938d3 in __gmp_overflow_in_mpz (/lib/x86_64-linux-gnu/libgmp.so.10+0xd8d3) (BuildId: 1af68a49fe041a5bb48a2915c3d47541f713bb38) #4 0x7faf6dbac95c in __gmpz_realloc (/lib/x86_64-linux-gnu/libgmp.so.10+0x2695c) (BuildId: 1af68a49fe041a5bb48a2915c3d47541f713bb38) #5 0x7faf6dba9038 in __gmpz_n_pow_ui (/lib/x86_64-linux-gnu/libgmp.so.10+0x23038) (BuildId: 1af68a49fe041a5bb48a2915c3d47541f713bb38) #6 0x5565ae1ccd9f in zif_gmp_pow /home/dcarlier/Contribs/php-src/ext/gmp/gmp.c:1286 #7 0x5565aee96ea9 in ZEND_DO_ICALL_SPEC_RETVAL_USED_HANDLER /home/dcarlier/Contribs/php-src/Zend/zend_vm_execute.h:1312 #8 0x5565af144320 in execute_ex /home/dcarlier/Contribs/php-src/Zend/zend_vm_execute.h:56075 #9 0x5565af160f07 in zend_execute /home/dcarlier/Contribs/php-src/Zend/zend_vm_execute.h:60439 #10 0x5565aed6fafe in zend_execute_scripts /home/dcarlier/Contribs/php-src/Zend/zend.c:1842 #11 0x5565aeae70a8 in php_execute_script /home/dcarlier/Contribs/php-src/main/main.c:2578 #12 0x5565af532f4e in do_cli /home/dcarlier/Contribs/php-src/sapi/cli/php_cli.c:964 #13 0x5565af535877 in main /home/dcarlier/Contribs/php-src/sapi/cli/php_cli.c:1334 #14 0x7faf6c633d67 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58 #15 0x7faf6c633e24 in __libc_start_main_impl ../csu/libc-start.c:360 #16 0x5565adc04040 in _start (/home/dcarlier/Contribs/php-src/sapi/cli/php+0x2604040) (BuildId: 949049955bdf8b7197390b1978a1dfc3ef6fdf38) AddressSanitizer can not provide additional info. SUMMARY: AddressSanitizer: FPE nptl/pthread_kill.c:44 in __pthread_kill_implementation ==286922==ABORTING ``` close phpGH-16384

``` ext/gd/libgd/gd.c:2275:14: runtime error: signed integer overflow: 2147483647 + 1 cannot be represented in type 'int' #0 0x5d6a2103e1db in php_gd_gdImageCopy /home/dcarlier/Contribs/php-src/ext/gd/libgd/gd.c:2275 #1 0x5d6a210a2b63 in gdImageCrop /home/dcarlier/Contribs/php-src/ext/gd/libgd/gd_crop.c:57 #2 0x5d6a21018ca4 in zif_imagecrop /home/dcarlier/Contribs/php-src/ext/gd/gd.c:3575 #3 0x5d6a21e46e7a in ZEND_DO_ICALL_SPEC_RETVAL_USED_HANDLER /home/dcarlier/Contribs/php-src/Zend/zend_vm_execute.h:1337 #4 0x5d6a221188da in execute_ex /home/dcarlier/Contribs/php-src/Zend/zend_vm_execute.h:57246 #5 0x5d6a221366bd in zend_execute /home/dcarlier/Contribs/php-src/Zend/zend_vm_execute.h:61634 #6 0x5d6a21d107a6 in zend_execute_scripts /home/dcarlier/Contribs/php-src/Zend/zend.c:1895 #7 0x5d6a21a63409 in php_execute_script /home/dcarlier/Contribs/php-src/main/main.c:2529 #8 0x5d6a22516d5e in do_cli /home/dcarlier/Contribs/php-src/sapi/cli/php_cli.c:966 #9 0x5d6a2251981d in main /home/dcarlier/Contribs/php-src/sapi/cli/php_cli.c:1341 #10 0x7f10d002a3b7 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58 #11 0x7f10d002a47a in __libc_start_main_impl ../csu/libc-start.c:360 #12 0x5d6a20a06da4 in _start (/home/dcarlier/Contribs/php-src/sapi/cli/php+0x2806da4) (BuildId: d9a79c7e0e4872311439d7313cb3a81fe04190a2) ``` close phpGH-18006

github-actions bot added the Extension: bcmath label Jul 7, 2023

jorgsowa changed the title ~~Bcmath fix performance for numbers with trailing zeros~~ Fix performance for Bcmath numbers with trailing zeros Jul 8, 2023

jorgsowa added 5 commits July 26, 2023 00:05

Adjust bc_num to number without trailing zeros

7124c53

Improve tests for bcpow with zero

0653132

Fixed tests for 32bits systems

ec994a3

Fixed test case for maximum exponent for int to lower than max

f55eab1

Fix bcmath tests for number zero

d63ba2f

jorgsowa force-pushed the bcmath-fix-performance-for-numbers-with-trailing-zeros branch from 94cfed7 to d63ba2f Compare July 25, 2023 22:05

petk and others added 19 commits July 26, 2023 09:58

Remove unused ZEND_STACK_GROWS_DOWNWARDS constant (php#11762)

d812639

zend call stack fix freebsd code path. (php#11766)

8fbbfd6

The typo in HAVE_PTHREAD_ATTR_GET_STACK (might be due to pthread_attr_get_np being different from Linux's pthread_getattr_np) led to this code path never get called on FreeBSD.

zend vm savee registers support for riscv 64. (php#11773)

5d6d66b

x8 being already reserved, we can only pull x18 to x27.

Fixed incorrect QM_ASSIGN elimination

c6202fc

Fixes OSS Fuzz #60735

Use xmlSetNsProp when possible to prevent parsing the name

f8f3280

The normal xmlSetProp() function parses the name to find the namespace. But we don't care about the namespace in this case.

Remove useless readonly checks

1bd964c

These checks will always be false because we're dealing with elements here. DOMElement always are of type XML_ELEMENT_NODE, so they don't need to be checked.

Simplify configuration getters (php#11778)

a808b32

dom_get_doc_props_read_only() already does a NULL check.

[skip ci] Add myself to CODEOWNER for BCMath extension

acf1bf7

Fix DOMEntity field getter bugs

ac1fc0e

- publicId could crash PHP if none was provided - notationName never worked The fields of this classs were untested. This new test file changes that. Closes phpGH-11779.

Fix incorrect attribute existence check in DOMElement::setAttributeNo…

6da01ce

…deNS() Closes phpGH-11776.

Fix DOMCharacterData::replaceWith() with itself

058cfa5

Previously, when replacing the node with itself (or contained within itself), the node disappeared. Closes phpGH-11770.

Fix empty argument cases for DOMParentNode methods

05b4555

Closes phpGH-11768.

[skip ci] Don't render zend_vm_handlers.h in PRs

095be3e

Fix open_basedir leak

96b77f6

Fixes oss-fuzz #60741 Closes phpGH-11780

Call cast_object handler from get_properties_for

716f6b4

Fixes phpGH-11547 Closes phpGH-11583

CS

2372bf2

Make the new DatePeriod::createFromISO8601String method emit DateTime…

7ecb398

…Immutable objects

Adjust bc_num to number without trailing zeros

00163a3

github-actions bot added Category: Engine Category: Build System Extension: date labels Jul 26, 2023

github-actions bot added Extension: dom Category: Optimizer Extension: com_dotnet Extension: opcache Extension: simplexml labels Jul 26, 2023

jorgsowa closed this Aug 6, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix performance for Bcmath numbers with trailing zeros #2

Fix performance for Bcmath numbers with trailing zeros #2

Uh oh!

jorgsowa commented Jul 7, 2023 •

edited

Loading

Uh oh!

Uh oh!

Fix performance for Bcmath numbers with trailing zeros #2

Fix performance for Bcmath numbers with trailing zeros #2

Uh oh!

Conversation

jorgsowa commented Jul 7, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

jorgsowa commented Jul 7, 2023 •

edited

Loading