From 3702f9783b6367b7c94e2c11ede0896352da5cfe Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Sat, 7 Dec 2024 12:39:07 +0100 Subject: [PATCH 1/9] opcache_get_configuration() properly reports jit_prof_threshold The `jit_prof_threshold` is a float, supposed to be in range [0, 1], and usually very small (the default is 0.005). Reporting it as int is meaningless. Closes GH-17077. --- NEWS | 3 +++ .../tests/opcache_jit_prof_threshold.phpt | 19 +++++++++++++++++++ ext/opcache/zend_accelerator_module.c | 2 +- 3 files changed, 23 insertions(+), 1 deletion(-) create mode 100644 ext/opcache/tests/opcache_jit_prof_threshold.phpt diff --git a/NEWS b/NEWS index 32d837a47b675..09abb4f1ab1d5 100644 --- a/NEWS +++ b/NEWS @@ -8,6 +8,9 @@ PHP NEWS - Iconv: . Fixed bug GH-17047 (UAF on iconv filter failure). (nielsdos) +- Opcache: + . opcache_get_configuration() properly reports jit_prof_threshold. (cmb) + - SimpleXML: . Fixed bug GH-17040 (SimpleXML's unset can break DOM objects). (nielsdos) diff --git a/ext/opcache/tests/opcache_jit_prof_threshold.phpt b/ext/opcache/tests/opcache_jit_prof_threshold.phpt new file mode 100644 index 0000000000000..f98c12b99ba91 --- /dev/null +++ b/ext/opcache/tests/opcache_jit_prof_threshold.phpt @@ -0,0 +1,19 @@ +--TEST-- +opcache_get_configuration() properly reports jit_prof_threshold +--EXTENSIONS-- +opcache +--SKIPIF-- + +--FILE-- + +--EXPECTF-- +float(0.0078125) +bool(true) diff --git a/ext/opcache/zend_accelerator_module.c b/ext/opcache/zend_accelerator_module.c index 5c69c9f7883a2..90936929f8447 100644 --- a/ext/opcache/zend_accelerator_module.c +++ b/ext/opcache/zend_accelerator_module.c @@ -849,7 +849,7 @@ ZEND_FUNCTION(opcache_get_configuration) add_assoc_long(&directives, "opcache.jit_max_recursive_returns", JIT_G(max_recursive_returns)); add_assoc_long(&directives, "opcache.jit_max_root_traces", JIT_G(max_root_traces)); add_assoc_long(&directives, "opcache.jit_max_side_traces", JIT_G(max_side_traces)); - add_assoc_long(&directives, "opcache.jit_prof_threshold", JIT_G(prof_threshold)); + add_assoc_double(&directives, "opcache.jit_prof_threshold", JIT_G(prof_threshold)); add_assoc_long(&directives, "opcache.jit_max_trace_length", JIT_G(max_trace_length)); #endif From 85731e8830f3279e65bfbb6bee9639653964944d Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Mon, 9 Dec 2024 00:01:37 +0100 Subject: [PATCH 2/9] [skip ci] Add nightly job for x64 Windows with ASan enabled It seems reasonable to have an ASan job on Windows, especially to be able to check Windows specific code. Since the tests may take about 70 minutes, it doesn't make sense to add these for pushes, but for a nightly job that should be okay. Closes GH-17087. --- .github/scripts/windows/build_task.bat | 1 + .github/scripts/windows/test_task.bat | 4 +++- .github/workflows/nightly.yml | 11 ++++++++++- 3 files changed, 14 insertions(+), 2 deletions(-) diff --git a/.github/scripts/windows/build_task.bat b/.github/scripts/windows/build_task.bat index 3c8dc04d59b40..fd9a956bd38fb 100644 --- a/.github/scripts/windows/build_task.bat +++ b/.github/scripts/windows/build_task.bat @@ -30,6 +30,7 @@ if %errorlevel% neq 0 exit /b 3 if "%THREAD_SAFE%" equ "0" set ADD_CONF=%ADD_CONF% --disable-zts if "%INTRINSICS%" neq "" set ADD_CONF=%ADD_CONF% --enable-native-intrinsics=%INTRINSICS% +if "%ASAN%" equ "1" set ADD_CONF=%ADD_CONF% --enable-sanitizer --enable-debug-pack set CFLAGS=/W1 /WX /w14013 diff --git a/.github/scripts/windows/test_task.bat b/.github/scripts/windows/test_task.bat index 0d6117324d63d..ee8a5f01b2c0c 100644 --- a/.github/scripts/windows/test_task.bat +++ b/.github/scripts/windows/test_task.bat @@ -133,9 +133,11 @@ for %%i in (ldap) do ( set TEST_PHPDBG_EXECUTABLE=%PHP_BUILD_DIR%\phpdbg.exe +if "%ASAN%" equ "1" set ASAN_OPTS=--asan + mkdir c:\tests_tmp -nmake test TESTS="%OPCACHE_OPTS% -g FAIL,BORK,LEAK,XLEAK --no-progress -q --offline --show-diff --show-slow 1000 --set-timeout 120 --temp-source c:\tests_tmp --temp-target c:\tests_tmp %PARALLEL%" +nmake test TESTS="%OPCACHE_OPTS% -g FAIL,BORK,LEAK,XLEAK %ASAN_OPTS% --no-progress -q --offline --show-diff --show-slow 1000 --set-timeout 120 --temp-source c:\tests_tmp --temp-target c:\tests_tmp %PARALLEL%" set EXIT_CODE=%errorlevel% diff --git a/.github/workflows/nightly.yml b/.github/workflows/nightly.yml index a0900f1f3d2c5..4041271f4d041 100644 --- a/.github/workflows/nightly.yml +++ b/.github/workflows/nightly.yml @@ -963,10 +963,18 @@ jobs: - x64: true zts: true opcache: true + asan: false - x64: false zts: false opcache: false - name: "WINDOWS_${{ matrix.x64 && 'X64' || 'X86' }}_${{ matrix.zts && 'ZTS' || 'NTS' }}" + asan: false + - x64: true + zts: true + opcache: true + asan: true + branch: 'master' + timeout: 120 + name: "WINDOWS_${{ matrix.x64 && 'X64' || 'X86' }}_${{ matrix.zts && 'ZTS' || 'NTS' }}${{ matrix.asan && '_ASAN' || ''}}" runs-on: windows-${{ inputs.windows_version }} env: PHP_BUILD_CACHE_BASE_DIR: C:\build-cache @@ -979,6 +987,7 @@ jobs: INTRINSICS: "${{ matrix.zts && 'AVX2' || '' }}" PARALLEL: -j2 OPCACHE: "${{ matrix.opcache && '1' || '0' }}" + ASAN: "${{ matrix.asan && '1' || '0' }}" steps: - name: git config run: git config --global core.autocrlf false && git config --global core.eol lf From 03731570cf2b2abdb0a01d4d30d7d320b886bb66 Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Mon, 2 Dec 2024 12:56:54 +0100 Subject: [PATCH 3/9] Fix GH-16843: Windows phpize builds ignore source subfolders phpize builds on Windows ignore the paths of extension sources, and build all object files in the same folder. This can't work if there are multiple source files with the same base name stored in separate folders and registered as such (e.g. cls/worker.c and src/worker.c). While extension authors can work around by avoiding duplicate base names, they may not even be aware of the problem because on POSIX systems, the object files are usually placed right besides the sources. Thus we take the relative path (from `configure_module_dirname`) of the source files into account even for phpize builds. Since this may break some extension builds (especially those which use Makefile fragments), we do not apply this fix to stable branches. Closes GH-17016. --- NEWS | 1 + UPGRADING | 4 ++++ win32/build/confutils.js | 11 +++++++++-- 3 files changed, 14 insertions(+), 2 deletions(-) diff --git a/NEWS b/NEWS index 171a437f53c06..b723e8a11662c 100644 --- a/NEWS +++ b/NEWS @@ -81,6 +81,7 @@ PHP NEWS - Windows: . Fixed bug GH-10992 (Improper long path support for relative paths). (cmb, nielsdos) + . Fixed bug GH-16843 (Windows phpize builds ignore source subfolders). (cmb) - XMLWriter: . Improved performance and reduce memory consumption. (nielsdos) diff --git a/UPGRADING b/UPGRADING index 37538739d6af6..4b37e3e41a4dc 100644 --- a/UPGRADING +++ b/UPGRADING @@ -166,6 +166,10 @@ PHP 8.5 UPGRADE NOTES PHP_RELEASE_VERSION are now always numbers. Previously, they have been strings for buildconf builds. +* phpize builds now reflect the source tree in the build dir (like that already + worked for in-tree builds); some extension builds (especially when using + Makefile.frag.w32) may need adjustments. + * --enable-sanitzer is now supported for MSVC builds. This enables ASan and debug assertions, and is supported as of MSVC 16.10 and Windows 10. diff --git a/win32/build/confutils.js b/win32/build/confutils.js index 438eb1d239c3f..cf1010266f2c4 100644 --- a/win32/build/confutils.js +++ b/win32/build/confutils.js @@ -1616,8 +1616,15 @@ function ADD_SOURCES(dir, file_list, target, obj_dir) if (obj_dir == null) { if (MODE_PHPIZE) { /* In the phpize mode, the subdirs are always relative to BUID_DIR. - No need to differentiate by extension, only one gets built. */ - var build_dir = (dirname ? dirname : "").replace(new RegExp("^..\\\\"), ""); + No need to differentiate by extension, only one gets built. + We still need to cater to subfolders, though. */ + if (dir.charAt(configure_module_dirname.length) === "\\" && + dir.substr(0, configure_module_dirname.length) === configure_module_dirname) { + var reldir = dir.substr(configure_module_dirname.length + 1); + var build_dir = (dirname ? (reldir + "\\" + dirname) : reldir).replace(new RegExp("^..\\\\"), ""); + } else { + var build_dir = (dirname ? dirname : "").replace(new RegExp("^..\\\\"), ""); + } } else { var build_dir = (dirname ? (dir + "\\" + dirname) : dir).replace(new RegExp("^..\\\\"), ""); } From 3bea6a2ddbe02cd9da10f66091f9996ae43de64e Mon Sep 17 00:00:00 2001 From: David Carlier Date: Thu, 28 Nov 2024 13:00:42 +0000 Subject: [PATCH 4/9] ext/sockets: socket_strerror follow-up on GH-16267 fix. boundaries should be INT_MIN <= val < INT_MAX in fact. close GH-16891 --- NEWS | 4 ++++ ext/sockets/sockets.c | 6 +++++- ext/sockets/tests/gh16267.phpt | 14 +++++--------- 3 files changed, 14 insertions(+), 10 deletions(-) diff --git a/NEWS b/NEWS index 09abb4f1ab1d5..2dee36b440be4 100644 --- a/NEWS +++ b/NEWS @@ -14,6 +14,10 @@ PHP NEWS - SimpleXML: . Fixed bug GH-17040 (SimpleXML's unset can break DOM objects). (nielsdos) +- Sockets: + . Fixed bug GH-16276 (socket_strerror overflow handling with INT_MIN). + (David Carlier / cmb) + - Streams: . Fixed bug GH-17037 (UAF in user filter when adding existing filter name due to incorrect error handling). (nielsdos) diff --git a/ext/sockets/sockets.c b/ext/sockets/sockets.c index 2f731b3b05acc..e1b350b4045a4 100644 --- a/ext/sockets/sockets.c +++ b/ext/sockets/sockets.c @@ -354,7 +354,11 @@ char *sockets_strerror(int error) /* {{{ */ #ifndef PHP_WIN32 if (error < -10000) { - error = -error - 10000; + if (error == INT_MIN) { + error = 2147473648; + } else { + error = -error - 10000; + } #ifdef HAVE_HSTRERROR buf = hstrerror(error); diff --git a/ext/sockets/tests/gh16267.phpt b/ext/sockets/tests/gh16267.phpt index d2462b3164530..de3e1b657fbc5 100644 --- a/ext/sockets/tests/gh16267.phpt +++ b/ext/sockets/tests/gh16267.phpt @@ -3,20 +3,16 @@ GH-16267 - overflow on socket_strerror argument --EXTENSIONS-- sockets --SKIPIF-- - + --FILE-- getMessage() . PHP_EOL; -} -try { - socket_strerror(PHP_INT_MAX); + socket_strerror(2147483648); } catch (\ValueError $e) { echo $e->getMessage() . PHP_EOL; } ?> --EXPECTF-- -socket_strerror(): Argument #1 ($error_code) must be between %s and %s -socket_strerror(): Argument #1 ($error_code) must be between %s and %s +string(%d) "%S" +socket_strerror(): Argument #1 ($error_code) must be between %i and %d From e675c1a467e05c44682106dcfd8f58bd7a373aee Mon Sep 17 00:00:00 2001 From: Ilija Tovilo Date: Mon, 9 Dec 2024 12:33:44 +0100 Subject: [PATCH 5/9] Drop intl on macOS + PHP 8.1 build Based on the discussion in GH-16286, drop the intl build from macOS + PHP 8.1, since we cannot build with supported intl versions without too many changes. Closes GH-17092 See GH-16286 --- .github/actions/brew/action.yml | 3 +-- .github/actions/configure-macos/action.yml | 1 - 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/.github/actions/brew/action.yml b/.github/actions/brew/action.yml index 3b36ec446ddca..5cd8d33cfa6b7 100644 --- a/.github/actions/brew/action.yml +++ b/.github/actions/brew/action.yml @@ -32,5 +32,4 @@ runs: libjpeg \ libxslt \ postgresql - brew reinstall icu4c@74 - brew link icu4c gettext --force + brew link gettext --force diff --git a/.github/actions/configure-macos/action.yml b/.github/actions/configure-macos/action.yml index b21f6466c444c..87627d4f84848 100644 --- a/.github/actions/configure-macos/action.yml +++ b/.github/actions/configure-macos/action.yml @@ -66,7 +66,6 @@ runs: --with-ffi \ --enable-zend-test \ --enable-dl-test=shared \ - --enable-intl \ --with-mhash \ --with-sodium \ --enable-dba \ From b0b39cdc3e5cf791ccd7f358b32ff1340a652d78 Mon Sep 17 00:00:00 2001 From: Dmitry Stogov Date: Tue, 19 Nov 2024 18:03:54 +0300 Subject: [PATCH 6/9] Backport JIT fix: set valid EX(opline) before calling gc_possible_root() (#16858) This will finally make the COMMUNTIY build of the PHP 8.1 build green. See https://github.com/php/php-src/pull/16858#issuecomment-2509010556 Closes GH-17091 --- ext/opcache/jit/zend_jit_arm64.dasc | 12 ++++++++++++ ext/opcache/jit/zend_jit_x86.dasc | 12 ++++++++++++ 2 files changed, 24 insertions(+) diff --git a/ext/opcache/jit/zend_jit_arm64.dasc b/ext/opcache/jit/zend_jit_arm64.dasc index 1583706855d41..b9a4fbc82e124 100644 --- a/ext/opcache/jit/zend_jit_arm64.dasc +++ b/ext/opcache/jit/zend_jit_arm64.dasc @@ -1533,6 +1533,9 @@ static bool logical_immediate_p(uint64_t value, uint32_t reg_size) || } | IF_GC_MAY_NOT_LEAK FCARG1x, >4, Rw(tmp_reg1), Rw(tmp_reg2) | // gc_possible_root(Z_COUNTED_P(z)) +|| if (opline) { +| SET_EX_OPLINE opline, TMP1 +|| } | EXT_CALL gc_possible_root, Rx(tmp_reg1) || } || if (cold && ((op_info) & ((MAY_BE_ANY|MAY_BE_UNDEF|MAY_BE_INDIRECT|MAY_BE_GUARD)-(MAY_BE_OBJECT|MAY_BE_RESOURCE))) != 0) { @@ -5972,6 +5975,9 @@ static int zend_jit_assign_to_variable(dasm_State **Dst, if (RC_MAY_BE_N(var_info) && (var_info & (MAY_BE_ARRAY|MAY_BE_OBJECT)) != 0) { |4: | IF_GC_MAY_NOT_LEAK FCARG1x, >8, TMP1w, TMP2w + if (opline) { + | SET_EX_OPLINE opline, REG0 + } | EXT_CALL gc_possible_root, REG0 if (in_cold) { | b >8 @@ -5999,6 +6005,9 @@ static int zend_jit_assign_to_variable(dasm_State **Dst, | GET_ZVAL_PTR FCARG1x, var_use_addr, TMP1 | GC_DELREF FCARG1x, TMP1w | IF_GC_MAY_NOT_LEAK FCARG1x, >5, TMP1w, TMP2w + if (opline) { + | SET_EX_OPLINE opline, TMP1 + } | EXT_CALL gc_possible_root, TMP1 if (Z_REG(var_use_addr) != ZREG_FP) { | ldr Rx(Z_REG(var_use_addr)), T1 // restore @@ -11980,6 +11989,9 @@ static int zend_jit_bind_global(dasm_State **Dst, const zend_op *opline, uint32_ |3: | // GC_ZVAL_CHECK_POSSIBLE_ROOT(variable_ptr) | IF_GC_MAY_NOT_LEAK FCARG1x, >5, TMP1w, TMP2w + if (opline) { + | SET_EX_OPLINE opline, REG0 + } | EXT_CALL gc_possible_root, REG0 | b >5 } diff --git a/ext/opcache/jit/zend_jit_x86.dasc b/ext/opcache/jit/zend_jit_x86.dasc index b3989a4ae254d..a1d0a8e098a2a 100644 --- a/ext/opcache/jit/zend_jit_x86.dasc +++ b/ext/opcache/jit/zend_jit_x86.dasc @@ -1443,6 +1443,9 @@ static size_t tsrm_tls_offset; |1: || } | IF_GC_MAY_NOT_LEAK FCARG1a, >4 +|| if (opline) { +| SET_EX_OPLINE opline, r0 +|| } | // gc_possible_root(Z_COUNTED_P(z)) | EXT_CALL gc_possible_root, r0 || } @@ -6526,6 +6529,9 @@ static int zend_jit_assign_to_variable(dasm_State **Dst, if (RC_MAY_BE_N(var_info) && (var_info & (MAY_BE_ARRAY|MAY_BE_OBJECT)) != 0) { |4: | IF_GC_MAY_NOT_LEAK FCARG1a, >8 + if (opline) { + | SET_EX_OPLINE opline, r0 + } | EXT_CALL gc_possible_root, r0 if (in_cold) { | jmp >8 @@ -6553,6 +6559,9 @@ static int zend_jit_assign_to_variable(dasm_State **Dst, | GET_ZVAL_PTR FCARG1a, var_use_addr | GC_DELREF FCARG1a | IF_GC_MAY_NOT_LEAK FCARG1a, >5 + if (opline) { + | SET_EX_OPLINE opline, r0 + } | EXT_CALL gc_possible_root, r0 if (Z_REG(var_use_addr) != ZREG_FP) { | mov Ra(Z_REG(var_use_addr)), T1 // restore @@ -12742,6 +12751,9 @@ static int zend_jit_bind_global(dasm_State **Dst, const zend_op *opline, uint32_ |3: | // GC_ZVAL_CHECK_POSSIBLE_ROOT(variable_ptr) | IF_GC_MAY_NOT_LEAK FCARG1a, >5 + if (opline) { + | SET_EX_OPLINE opline, r0 + } | EXT_CALL gc_possible_root, r1 | jmp >5 } From 84917300b234f898bd92a08fc3ab3b4a9f2246e9 Mon Sep 17 00:00:00 2001 From: Ilija Tovilo Date: Sun, 8 Dec 2024 20:02:26 +0100 Subject: [PATCH 7/9] Fix duplicate dynamic properties in hooked object iterator properties table Ouch, Z_TRY_ADDREF_P() uses pz twice... Also make sure we actually reserve enough Buckets for all dynamic properties. Fixes OSS-Fuzz #382922236 Closes GH-17085 --- NEWS | 4 ++++ .../property_hooks/oss-fuzz-382922236.phpt | 20 +++++++++++++++++++ Zend/zend_property_hooks.c | 7 +++++-- 3 files changed, 29 insertions(+), 2 deletions(-) create mode 100644 Zend/tests/property_hooks/oss-fuzz-382922236.phpt diff --git a/NEWS b/NEWS index 589e9277552f4..58e692610249a 100644 --- a/NEWS +++ b/NEWS @@ -7,6 +7,10 @@ PHP NEWS . Fixed bug GH-17061 (Now Number::round() does not remove trailing zeros). (Saki Takamachi) +- Core: + . Fixed bug OSS-Fuzz #382922236 (Duplicate dynamic properties in hooked object + iterator properties table). (ilutov) + - DBA: . Skip test if inifile is disabled. (orlitzky) diff --git a/Zend/tests/property_hooks/oss-fuzz-382922236.phpt b/Zend/tests/property_hooks/oss-fuzz-382922236.phpt new file mode 100644 index 0000000000000..f87b70ea5a7a7 --- /dev/null +++ b/Zend/tests/property_hooks/oss-fuzz-382922236.phpt @@ -0,0 +1,20 @@ +--TEST-- +OSS-Fuzz #382922236: Duplicate dynamic properties in hooked object iterator properties table +--FILE-- + 42; + } +} + +$obj = new C(); +$b = &$obj->b; +unset($b); +echo json_encode($obj); + +?> +--EXPECT-- +{"a":42,"b":null} diff --git a/Zend/zend_property_hooks.c b/Zend/zend_property_hooks.c index 82ddf2f8835a0..d1db597d3a32d 100644 --- a/Zend/zend_property_hooks.c +++ b/Zend/zend_property_hooks.c @@ -44,7 +44,9 @@ static uint32_t zho_num_backed_props(zend_object *zobj) static zend_array *zho_build_properties_ex(zend_object *zobj, bool check_access, bool force_ptr, bool include_dynamic_props) { zend_class_entry *ce = zobj->ce; - zend_array *properties = zend_new_array(ce->default_properties_count); + zend_array *properties = zend_new_array(include_dynamic_props && zobj->properties + ? zend_hash_num_elements(zobj->properties) + : ce->default_properties_count); zend_hash_real_init_mixed(properties); /* Build list of parents */ @@ -105,7 +107,8 @@ static zend_array *zho_build_properties_ex(zend_object *zobj, bool check_access, zend_string *prop_name; zval *prop_value; ZEND_HASH_FOREACH_STR_KEY_VAL_FROM(zobj->properties, prop_name, prop_value, zho_num_backed_props(zobj)) { - Z_TRY_ADDREF_P(_zend_hash_append(properties, prop_name, prop_value)); + zval *tmp = _zend_hash_append(properties, prop_name, prop_value); + Z_TRY_ADDREF_P(tmp); } ZEND_HASH_FOREACH_END(); } From 792f63df45b62cbba530df2f6c7dd82bf0667706 Mon Sep 17 00:00:00 2001 From: Ilija Tovilo Date: Tue, 3 Dec 2024 18:08:33 +0100 Subject: [PATCH 8/9] Fix unstable get_iterator pointer for hooked classes in shm on Windows Closes GH-17034 --- NEWS | 2 ++ Zend/zend_compile.c | 7 +++++ Zend/zend_inheritance.c | 31 +++++++++++++++++++++- Zend/zend_portability.h | 7 +++++ ext/opcache/tests/dump_property_hooks.phpt | 6 +++++ 5 files changed, 52 insertions(+), 1 deletion(-) diff --git a/NEWS b/NEWS index 58e692610249a..e1a2a5c491d99 100644 --- a/NEWS +++ b/NEWS @@ -10,6 +10,8 @@ PHP NEWS - Core: . Fixed bug OSS-Fuzz #382922236 (Duplicate dynamic properties in hooked object iterator properties table). (ilutov) + . Fixed unstable get_iterator pointer for hooked classes in shm on Windows. + (ilutov) - DBA: . Skip test if inifile is disabled. (orlitzky) diff --git a/Zend/zend_compile.c b/Zend/zend_compile.c index 2d1a50191ff03..456c0b8f410b4 100644 --- a/Zend/zend_compile.c +++ b/Zend/zend_compile.c @@ -8575,10 +8575,13 @@ static void zend_compile_property_hooks( ce->num_hooked_props++; + /* See zend_link_hooked_object_iter(). */ +#ifndef ZEND_OPCACHE_SHM_REATTACHMENT if (!ce->get_iterator) { /* Will be removed again, in case of Iterator or IteratorAggregate. */ ce->get_iterator = zend_hooked_object_get_iterator; } +#endif if (!prop_info->ce->parent_name) { zend_verify_hooked_property(ce, prop_info, prop_name); @@ -9104,6 +9107,10 @@ static void zend_compile_class_decl(znode *result, zend_ast *ast, bool toplevel) /* We currently don't early-bind classes that implement interfaces or use traits */ if (!ce->num_interfaces && !ce->num_traits && !ce->num_hooked_prop_variance_checks +#ifdef ZEND_OPCACHE_SHM_REATTACHMENT + /* See zend_link_hooked_object_iter(). */ + && !ce->num_hooked_props +#endif && !(CG(compiler_options) & ZEND_COMPILE_WITHOUT_EXECUTION)) { if (toplevel) { if (extends_ast) { diff --git a/Zend/zend_inheritance.c b/Zend/zend_inheritance.c index 98ada65cd3f64..8197e61628439 100644 --- a/Zend/zend_inheritance.c +++ b/Zend/zend_inheritance.c @@ -1746,6 +1746,27 @@ ZEND_API inheritance_status zend_verify_property_hook_variance(const zend_proper return zend_perform_covariant_type_check(ce, prop_info->type, ce, value_arg_info->type); } +#ifdef ZEND_OPCACHE_SHM_REATTACHMENT +/* Hooked properties set get_iterator, which causes issues on for shm + * reattachment. Avoid early-binding on Windows and set get_iterator during + * inheritance. The linked class may not use inheritance cache. */ +static void zend_link_hooked_object_iter(zend_class_entry *ce) { + if (!ce->get_iterator && ce->num_hooked_props) { + ce->get_iterator = zend_hooked_object_get_iterator; + ce->ce_flags &= ~ZEND_ACC_CACHEABLE; + if (CG(current_linking_class) == ce) { +# if ZEND_DEBUG + /* This check is executed before inheriting any elements that can + * track dependencies. */ + HashTable *ht = (HashTable*)ce->inheritance_cache; + ZEND_ASSERT(!ht); +# endif + CG(current_linking_class) = NULL; + } + } +} +#endif + ZEND_API void zend_do_inheritance_ex(zend_class_entry *ce, zend_class_entry *parent_ce, bool checked) /* {{{ */ { zend_property_info *property_info; @@ -3405,7 +3426,7 @@ static zend_class_entry *zend_lazy_class_load(zend_class_entry *pce) return ce; } -#ifndef ZEND_WIN32 +#ifndef ZEND_OPCACHE_SHM_REATTACHMENT # define UPDATE_IS_CACHEABLE(ce) do { \ if ((ce)->type == ZEND_USER_CLASS) { \ is_cacheable &= (ce)->ce_flags; \ @@ -3550,6 +3571,10 @@ ZEND_API zend_class_entry *zend_do_link_class(zend_class_entry *ce, zend_string zend_enum_register_funcs(ce); } +#ifdef ZEND_OPCACHE_SHM_REATTACHMENT + zend_link_hooked_object_iter(ce); +#endif + if (parent) { if (!(parent->ce_flags & ZEND_ACC_LINKED)) { add_dependency_obligation(ce, parent); @@ -3838,6 +3863,10 @@ ZEND_API zend_class_entry *zend_try_early_bind(zend_class_entry *ce, zend_class_ zend_begin_record_errors(); } +#ifdef ZEND_OPCACHE_SHM_REATTACHMENT + zend_link_hooked_object_iter(ce); +#endif + zend_do_inheritance_ex(ce, parent_ce, status == INHERITANCE_SUCCESS); if (parent_ce && parent_ce->num_interfaces) { zend_do_inherit_interfaces(ce, parent_ce); diff --git a/Zend/zend_portability.h b/Zend/zend_portability.h index 5be8d7e4f5ced..e4195402fc294 100644 --- a/Zend/zend_portability.h +++ b/Zend/zend_portability.h @@ -863,4 +863,11 @@ static zend_always_inline uint64_t ZEND_BYTES_SWAP64(uint64_t u) } #endif +#ifdef ZEND_WIN32 +/* Whether it's allowed to reattach to a shm segment from different processes on + * this platform. This prevents pointing to internal structures from shm due to + * ASLR. Currently only possible on Windows. */ +# define ZEND_OPCACHE_SHM_REATTACHMENT 1 +#endif + #endif /* ZEND_PORTABILITY_H */ diff --git a/ext/opcache/tests/dump_property_hooks.phpt b/ext/opcache/tests/dump_property_hooks.phpt index d8727e873b85f..5083ad385f31f 100644 --- a/ext/opcache/tests/dump_property_hooks.phpt +++ b/ext/opcache/tests/dump_property_hooks.phpt @@ -6,6 +6,12 @@ opcache.enable_cli=1 opcache.opt_debug_level=0x20000 --EXTENSIONS-- opcache +--SKIPIF-- + --FILE-- Date: Tue, 3 Dec 2024 14:12:46 +0100 Subject: [PATCH 9/9] Fix enum to bool comparison The compiler compiles $value == true to ZEND_BOOL, which always returns true for objects (with the default cast_object handler). However, when compared to a statically unknown rhs $value == $true, the resulting opcode ZEND_IS_EQUAL would call the objects compare handler. The zend_objects_not_comparable() handler, which is installed for enums and other internal classes, blanketly returns false. This does not match the ZEND_BOOL semantics. Object to boolean comparison is now handled directly in zend_compare(), analogous to object to null comparison. It continuous to call the cast_object handler, but guarantees consistent behavior across ZEND_BOOL and ZEND_IS_EQUAL. Fixes GH-16954 Closes GH-17031 --- UPGRADING | 6 ++++ Zend/tests/enum/comparison.phpt | 4 +-- Zend/tests/gh16954.phpt | 50 +++++++++++++++++++++++++++++++++ Zend/zend_object_handlers.c | 5 ++-- Zend/zend_operators.c | 30 +++++++++++++++----- 5 files changed, 84 insertions(+), 11 deletions(-) create mode 100644 Zend/tests/gh16954.phpt diff --git a/UPGRADING b/UPGRADING index 4b37e3e41a4dc..6446beec2d868 100644 --- a/UPGRADING +++ b/UPGRADING @@ -28,6 +28,12 @@ PHP 8.5 UPGRADE NOTES - Core: . It is no longer possible to use "array" and "callable" as class alias names in class_alias(). + . Loosely comparing uncomparable objects (e.g. enums, \CurlHandle and other + internal classes) to booleans was previously inconsistent. If compared to a + boolean literal $object == true, it would behave the same way as (bool) + $object. If compared to a statically unknown value $object == $true, it + would always return false. This behavior was consolidated to always follow + the behavior of (bool) $object. - Intl: . The extension now requires at least ICU 57.1. diff --git a/Zend/tests/enum/comparison.phpt b/Zend/tests/enum/comparison.phpt index 5df2f282ec064..778e9a48ef5e6 100644 --- a/Zend/tests/enum/comparison.phpt +++ b/Zend/tests/enum/comparison.phpt @@ -53,5 +53,5 @@ bool(false) bool(false) bool(false) bool(false) -bool(false) -bool(false) +bool(true) +bool(true) diff --git a/Zend/tests/gh16954.phpt b/Zend/tests/gh16954.phpt new file mode 100644 index 0000000000000..18a650dd73dcc --- /dev/null +++ b/Zend/tests/gh16954.phpt @@ -0,0 +1,50 @@ +--TEST-- +GH-16954: Enum to bool comparison is inconsistent +--FILE-- + +--EXPECT-- +bool(true) +bool(true) +bool(true) +bool(true) +bool(false) +bool(false) +bool(false) +bool(false) +bool(false) +bool(false) +bool(false) +bool(false) +bool(true) +bool(true) +bool(true) +bool(true) diff --git a/Zend/zend_object_handlers.c b/Zend/zend_object_handlers.c index f08685092745a..0709d05580dc8 100644 --- a/Zend/zend_object_handlers.c +++ b/Zend/zend_object_handlers.c @@ -2063,8 +2063,9 @@ ZEND_API int zend_std_compare_objects(zval *o1, zval *o2) /* {{{ */ object_lhs = false; } ZEND_ASSERT(Z_TYPE_P(value) != IS_OBJECT); - uint8_t target_type = (Z_TYPE_P(value) == IS_FALSE || Z_TYPE_P(value) == IS_TRUE) - ? _IS_BOOL : Z_TYPE_P(value); + uint8_t target_type = Z_TYPE_P(value); + /* Should be handled in zend_compare(). */ + ZEND_ASSERT(target_type != IS_FALSE && target_type != IS_TRUE); if (Z_OBJ_HT_P(object)->cast_object(Z_OBJ_P(object), &casted, target_type) == FAILURE) { // TODO: Less crazy. if (target_type == IS_LONG || target_type == IS_DOUBLE) { diff --git a/Zend/zend_operators.c b/Zend/zend_operators.c index c2fbc0ee110c9..879141d1a139e 100644 --- a/Zend/zend_operators.c +++ b/Zend/zend_operators.c @@ -2333,13 +2333,29 @@ ZEND_API int ZEND_FASTCALL zend_compare(zval *op1, zval *op2) /* {{{ */ } if (Z_TYPE_P(op1) == IS_OBJECT - && Z_TYPE_P(op2) == IS_OBJECT - && Z_OBJ_P(op1) == Z_OBJ_P(op2)) { - return 0; - } else if (Z_TYPE_P(op1) == IS_OBJECT) { - return Z_OBJ_HANDLER_P(op1, compare)(op1, op2); - } else if (Z_TYPE_P(op2) == IS_OBJECT) { - return Z_OBJ_HANDLER_P(op2, compare)(op1, op2); + || Z_TYPE_P(op2) == IS_OBJECT) { + zval *object, *other; + if (Z_TYPE_P(op1) == IS_OBJECT) { + object = op1; + other = op2; + } else { + object = op2; + other = op1; + } + if (EXPECTED(Z_TYPE_P(other) == IS_OBJECT)) { + if (Z_OBJ_P(object) == Z_OBJ_P(other)) { + return 0; + } + } else if (Z_TYPE_P(other) == IS_TRUE || Z_TYPE_P(other) == IS_FALSE) { + zval casted; + if (Z_OBJ_HANDLER_P(object, cast_object)(Z_OBJ_P(object), &casted, _IS_BOOL) == FAILURE) { + return object == op1 ? 1 : -1; + } + int ret = object == op1 ? zend_compare(&casted, other) : zend_compare(other, &casted); + ZEND_ASSERT(!Z_REFCOUNTED_P(&casted)); + return ret; + } + return Z_OBJ_HANDLER_P(object, compare)(op1, op2); } if (!converted) {