Spl dir optimizations #28
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
Extension: standard
Extension: phar
and removed
Extension: fileinfo
labels
Normally, PHP evaluates all expressions in offsets (property or array), as well as the right hand side of assignments before actually fetching the offsets. This is well explained in this blog post. https://www.npopov.com/2017/04/14/PHP-7-Virtual-machine.html#writes-and-memory-safety For ??= we have a bit of a problem in that the rhs must only be evaluated if the lhs is null or undefined. Thus, we have to first compile the lhs with BP_VAR_IS, conditionally run the rhs and then re-fetch the lhs with BP_VAR_W to to make sure the offsets are valid if they have been invalidated. However, we don't want to just re-evaluate the entire lhs because it may contain side-effects, as in $array[$x++] ??= 42;. In this case, we don't want to re-evaluate $x++ because it would result in writing to a different offset than was previously tested. The same goes for function calls, like $array[foo()] ??= 42;, where the second call to foo() might result in a different value. PHP behaves correctly in these cases. This is implemented by memoizing sub-expressions in the lhs of ??= and reusing them when compiling the lhs for the second time. This is done for any expression that isn't a variable, i.e. anything that can (potentially) be written to. Unfortunately, this also means that function calls are considered writable due to their return-by-reference semantics, and will thus not be memoized. The expression foo()['bar'] ??= 42; will invoke foo() twice. Even worse, foo(bar()) ??= 42; will call both foo() and bar() twice, but foo(bar() + 1) ??= 42; will only call foo() twice. This is likely not by design, and was just overlooked in the implementation. The RFC does not specify how function calls in the lhs of the coalesce assignment behaves. This should probably be improved in the future. Now, the problem this commit actually fixes is that ??= may memoize expressions inside assert() function calls that may not actually execute. This is not only an issue when using the VAR in the second expression (which would usually also be skipped) but also when freeing the VAR. For this reason, it is not safe to memoize assert() sub-expressions. There are two possible solutions: 1. Don't memoize any sub-expressions of assert(), meaning they will execute twice. 2. Throw a compile error. Option 2 is not quite simple, because we can't disallow all memoization inside assert(), as that would break assertions like assert($array[foo()] ??= 'bar');. Code like this is highly unlikely (and dubious) but possible. In this case, we would need to make sure that a memoized value could not be used across the assert boundary it was created in. The complexity for this is not worthwhile. So we opt for option 1 and disable memoization immediately inside assert(). Fixes phpGH-11580 Closes phpGH-11581
* PHP-8.1: Fix use-of-uninitialized-value with ??= on assert
* PHP-8.2: Fix use-of-uninitialized-value with ??= on assert
This re-uses the already used for automatic retesting. That's certainly better than XFAIL. Closes phpGH-11325
* PHP-8.1: Implement flaky test section
* PHP-8.2: Implement flaky test section
This change extends supported parameter when generating EC keys. Specifically following parameters are now supported: p, a, b, order, generator, seed, cofactory, g_x, g_y, x, y and d. Those parameters can be passed to ec field in openssl_pkey_new options. It also fixes some issues openssl_pkey_get_details related to SM2 support. Closes phpGH-9991
This only takes up space and time.
On POSIX systems, we can use the maximum file length instead of maximum path length. This saves space and time for clearing the memory.
ndossche
added a commit
that referenced
this pull request
Jan 24, 2026
When certificate `cert` exists, but is not added to the store, it causes
memory leaks. The error handling was already existing but the freeing
only happened on the success case.
One could also ponder whether it is necessary to inform the user when
adding a certificate failed or signal this in some way.
Part of the leak report:
```
Direct leak of 384 byte(s) in 1 object(s) allocated from:
#0 0x7fdbf1f9e9c7 in malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:69
#1 0x7fdbf183a7c4 in CRYPTO_zalloc (/lib/x86_64-linux-gnu/libcrypto.so.3+0x2237c4) (BuildId: 0698e1ff610cb3c6993dccbd82c1281b1b4c5ade)
#2 0x7fdbf16f9d13 (/lib/x86_64-linux-gnu/libcrypto.so.3+0xe2d13) (BuildId: 0698e1ff610cb3c6993dccbd82c1281b1b4c5ade)
#3 0x7fdbf16f9e19 in ASN1_item_new_ex (/lib/x86_64-linux-gnu/libcrypto.so.3+0xe2e19) (BuildId: 0698e1ff610cb3c6993dccbd82c1281b1b4c5ade)
#4 0x7fdbf19a59f9 in X509_new_ex (/lib/x86_64-linux-gnu/libcrypto.so.3+0x38e9f9) (BuildId: 0698e1ff610cb3c6993dccbd82c1281b1b4c5ade)
#5 0x5575bcd295cb in php_openssl_pem_read_bio_x509 /work/php-src/ext/openssl/openssl_backend_v3.c:876
#6 0x5575bcd2ef3d in php_openssl_load_stream_cafile /work/php-src/ext/openssl/xp_ssl.c:855
#7 0x5575bcd2f4da in php_openssl_enable_peer_verification /work/php-src/ext/openssl/xp_ssl.c:912
#8 0x5575bcd33104 in php_openssl_setup_crypto /work/php-src/ext/openssl/xp_ssl.c:1610
#9 0x5575bcd39c18 in php_openssl_sockop_set_option /work/php-src/ext/openssl/xp_ssl.c:2512
#10 0x5575bdb4c610 in _php_stream_set_option /work/php-src/main/streams/streams.c:1466
#11 0x5575bdb5557d in php_stream_xport_crypto_setup /work/php-src/main/streams/transports.c:367
#12 0x5575bcd39f11 in php_openssl_sockop_set_option /work/php-src/ext/openssl/xp_ssl.c:2540
#13 0x5575bdb4c610 in _php_stream_set_option /work/php-src/main/streams/streams.c:1466
#14 0x5575bdb54655 in php_stream_xport_connect /work/php-src/main/streams/transports.c:248
#15 0x5575bdb5365d in _php_stream_xport_create /work/php-src/main/streams/transports.c:145
#16 0x5575bd8d30b1 in php_stream_url_wrap_http_ex /work/php-src/ext/standard/http_fopen_wrapper.c:490
#17 0x5575bd8d857e in php_stream_url_wrap_http /work/php-src/ext/standard/http_fopen_wrapper.c:1204
#18 0x5575bdb5073d in _php_stream_open_wrapper_ex /work/php-src/main/streams/streams.c:2270
#19 0x5575bd878fa6 in zif_file_get_contents /work/php-src/ext/standard/file.c:409
#20 0x5575bd5bfe39 in zif_phar_file_get_contents /work/php-src/ext/phar/func_interceptors.c:226
#21 0x5575bdab7ed2 in zend_test_execute_internal /work/php-src/ext/zend_test/observer.c:306
#22 0x5575bdde024a in ZEND_DO_FCALL_SPEC_RETVAL_USED_HANDLER /work/php-src/Zend/zend_vm_execute.h:2154
#23 0x5575bdf40995 in execute_ex /work/php-src/Zend/zend_vm_execute.h:116519
#24 0x5575bdf558b0 in zend_execute /work/php-src/Zend/zend_vm_execute.h:121962
#25 0x5575be0ba0ab in zend_execute_script /work/php-src/Zend/zend.c:1980
#26 0x5575bdaec8bb in php_execute_script_ex /work/php-src/main/main.c:2645
#27 0x5575bdaecccb in php_execute_script /work/php-src/main/main.c:2685
#28 0x5575be0bfc16 in do_cli /work/php-src/sapi/cli/php_cli.c:951
#29 0x5575be0c21e3 in main /work/php-src/sapi/cli/php_cli.c:1362
... etc ...
```
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.