From bf3673a4151c97db539edb7be866da77462e71a6 Mon Sep 17 00:00:00 2001 From: David Carlier Date: Thu, 19 Dec 2024 02:12:14 +0000 Subject: [PATCH 1/3] ext/intl: TimeZone address todo to throw exceptions on error. close GH-17215 --- NEWS | 2 ++ UPGRADING | 12 ++++--- ext/intl/tests/bug62017.phpt | 17 ++++++---- .../tests/calendar_createInstance_error.phpt | 11 ++++--- .../dateformat___construct_bad_tz_cal.phpt | 3 +- .../tests/dateformat_setTimeZone_error.phpt | 20 +++++++----- .../tests/dateformat_set_timezone_id3.phpt | 11 ++++--- .../dateformat_set_timezone_id_icu72-1.phpt | 11 ++++--- ext/intl/timezone/timezone_class.cpp | 32 ++++--------------- 9 files changed, 61 insertions(+), 58 deletions(-) diff --git a/NEWS b/NEWS index 1d18cbd732f3a..ad99a49e6bb65 100644 --- a/NEWS +++ b/NEWS @@ -28,6 +28,8 @@ PHP NEWS - Intl: . Bumped ICU requirement to ICU >= 57.1. (cmb) + . IntlDateFormatter::setTimeZone()/datefmt_set_timezone() throws an exception + with uninitialised classes or clone failure. (David Carlier) - MySQLnd: . Added mysqlnd.collect_memory_statistics to ini quick reference. diff --git a/UPGRADING b/UPGRADING index 28805e0f824fd..1bdca204a0f35 100644 --- a/UPGRADING +++ b/UPGRADING @@ -100,10 +100,9 @@ PHP 8.5 UPGRADE NOTES 5. Changed Functions ======================================== -- Zlib: - . The "use_include_path" argument for the - gzfile, gzopen and readgzfile functions had been changed - from int to boolean. +- Intl: + . IntlDateFormatter::setTimeZone()()/datefmt_set_timezone + throws an IntlException on uninitialised classes/clone failures. - PCNTL: . pcntl_exec() now has a formal return type of false. @@ -125,6 +124,11 @@ PHP 8.5 UPGRADE NOTES . posix_fpathconf checks invalid file descriptors and sets last_error to EBADF and raises an E_WARNING message. +- Zlib: + . The "use_include_path" argument for the + gzfile, gzopen and readgzfile functions had been changed + from int to boolean. + ======================================== 6. New Functions ======================================== diff --git a/ext/intl/tests/bug62017.phpt b/ext/intl/tests/bug62017.phpt index be91594ada521..53b0deb4fa7d8 100644 --- a/ext/intl/tests/bug62017.phpt +++ b/ext/intl/tests/bug62017.phpt @@ -5,19 +5,22 @@ intl --FILE-- getMessage() . " in " . $e->getFile() . " on line " . $e->getLine() . PHP_EOL; +} + try { - var_dump( new IntlDateFormatter('', IntlDateFormatter::NONE, IntlDateFormatter::NONE, "Europe/Lisbon", - IntlDateFormatter::GREGORIAN, "\x80")); + IntlDateFormatter::GREGORIAN, "\x80"); } catch (IntlException $e) { echo PHP_EOL."Exception: " . $e->getMessage() . " in " . $e->getFile() . " on line " . $e->getLine() . PHP_EOL; } ?> --EXPECTF-- -Warning: datefmt_create(): datefmt_create: Time zone identifier given is not a valid UTF-8 string in %s on line %d -NULL -Exception: IntlDateFormatter::__construct(): datefmt_create: error converting pattern to UTF-16 in %sbug62017.php on line %d +Exception: datefmt_create: Time zone identifier given is not a valid UTF-8 string in %s on line %d + +Exception: IntlDateFormatter::__construct(): datefmt_create: error converting pattern to UTF-16 in %s on line %d diff --git a/ext/intl/tests/calendar_createInstance_error.phpt b/ext/intl/tests/calendar_createInstance_error.phpt index 91c904f32b4c4..a48b6b12c2d13 100644 --- a/ext/intl/tests/calendar_createInstance_error.phpt +++ b/ext/intl/tests/calendar_createInstance_error.phpt @@ -10,8 +10,11 @@ class X extends IntlTimeZone { function __construct() {} } -var_dump(intlcal_create_instance(new X, NULL)); +try { + intlcal_create_instance(new X, NULL); +} catch (IntlException $e) { + echo $e->getMessage(); +} ?> ---EXPECTF-- -Warning: intlcal_create_instance(): intlcal_create_instance: passed IntlTimeZone is not properly constructed in %s on line %d -NULL +--EXPECT-- +intlcal_create_instance: passed IntlTimeZone is not properly constructed diff --git a/ext/intl/tests/dateformat___construct_bad_tz_cal.phpt b/ext/intl/tests/dateformat___construct_bad_tz_cal.phpt index c0c5412e4ab4c..ca7cd5a13dee4 100644 --- a/ext/intl/tests/dateformat___construct_bad_tz_cal.phpt +++ b/ext/intl/tests/dateformat___construct_bad_tz_cal.phpt @@ -29,7 +29,8 @@ try { } ?> --EXPECTF-- -Exception: IntlDateFormatter::__construct(): datefmt_create: No such time zone: 'bad timezone' in %s on line %d + +Exception: datefmt_create: No such time zone: 'bad timezone' in %s on line %d Exception: IntlDateFormatter::__construct(): datefmt_create: Invalid value for calendar type; it must be one of IntlDateFormatter::TRADITIONAL (locale's default calendar) or IntlDateFormatter::GREGORIAN. Alternatively, it can be an IntlCalendar object in %s on line %d diff --git a/ext/intl/tests/dateformat_setTimeZone_error.phpt b/ext/intl/tests/dateformat_setTimeZone_error.phpt index 4467204134062..b911c4b41ec0c 100644 --- a/ext/intl/tests/dateformat_setTimeZone_error.phpt +++ b/ext/intl/tests/dateformat_setTimeZone_error.phpt @@ -10,15 +10,19 @@ ini_set("date.timezone", 'Atlantic/Azores'); $df = new IntlDateFormatter(NULL, 0, 0); -var_dump($df->setTimeZone(array())); -var_dump($df->setTimeZone('non existing timezone')); +try { + $df->setTimeZone(array()); +} catch (IntlException $e) { + echo $e->getMessage() . PHP_EOL; +} +try { + $df->setTimeZone('non existing timezone'); +} catch (IntlException $e) { + echo $e->getMessage(); +} ?> --EXPECTF-- Warning: Array to string conversion in %s on line %d - -Warning: IntlDateFormatter::setTimeZone(): datefmt_set_timezone: No such time zone: 'Array' in %s on line %d -bool(false) - -Warning: IntlDateFormatter::setTimeZone(): datefmt_set_timezone: No such time zone: 'non existing timezone' in %s on line %d -bool(false) +datefmt_set_timezone: No such time zone: 'Array' +datefmt_set_timezone: No such time zone: 'non existing timezone' diff --git a/ext/intl/tests/dateformat_set_timezone_id3.phpt b/ext/intl/tests/dateformat_set_timezone_id3.phpt index a964bb620bc1f..16c0bb05b9be3 100644 --- a/ext/intl/tests/dateformat_set_timezone_id3.phpt +++ b/ext/intl/tests/dateformat_set_timezone_id3.phpt @@ -37,7 +37,11 @@ function ut_main() $res_str .= "-----------"; $res_str .= "\nTrying to set timezone_id= $timezone_id_entry"; - if (ut_datefmt_set_timezone_id( $fmt , $timezone_id_entry ) !== $result) die("ut_datefmt_set_timezone_id failed"); + try { + ut_datefmt_set_timezone_id( $fmt , $timezone_id_entry ); + } catch (IntlException $e) { + echo $e->getMessage() . PHP_EOL; + } $timezone_id = ut_datefmt_get_timezone_id( $fmt ); $res_str .= "\nAfter call to set_timezone_id : timezone_id= $timezone_id"; $formatted = ut_datefmt_format( $fmt, 0); @@ -58,9 +62,8 @@ include_once( 'ut_common.inc' ); ut_run(); ?> --EXPECTF-- -Warning: IntlDateFormatter::setTimeZone(): datefmt_set_timezone: No such time zone: 'CN' in %sut_common.inc on line %d - -Warning: datefmt_set_timezone(): datefmt_set_timezone: No such time zone: 'CN' in %sut_common.inc on line %d +datefmt_set_timezone: No such time zone: 'CN' +datefmt_set_timezone: No such time zone: 'CN' After creation of the dateformatter : timezone_id= US/Pacific ----------- diff --git a/ext/intl/tests/dateformat_set_timezone_id_icu72-1.phpt b/ext/intl/tests/dateformat_set_timezone_id_icu72-1.phpt index 79953ab26b465..eebc6f0973a05 100644 --- a/ext/intl/tests/dateformat_set_timezone_id_icu72-1.phpt +++ b/ext/intl/tests/dateformat_set_timezone_id_icu72-1.phpt @@ -37,7 +37,11 @@ function ut_main() $res_str .= "-----------"; $res_str .= "\nTrying to set timezone_id= $timezone_id_entry"; - if (ut_datefmt_set_timezone_id( $fmt , $timezone_id_entry ) !== $result) die("ut_datefmt_set_timezone_id failed"); + try { + ut_datefmt_set_timezone_id( $fmt , $timezone_id_entry ); + } catch (IntlException $e) { + echo $e->getMessage() . PHP_EOL; + } $timezone_id = ut_datefmt_get_timezone_id( $fmt ); $res_str .= "\nAfter call to set_timezone_id : timezone_id= $timezone_id"; $formatted = ut_datefmt_format( $fmt, 0); @@ -58,9 +62,8 @@ include_once( 'ut_common.inc' ); ut_run(); ?> --EXPECTF-- -Warning: IntlDateFormatter::setTimeZone(): datefmt_set_timezone: No such time zone: 'CN' in %sut_common.inc on line %d - -Warning: datefmt_set_timezone(): datefmt_set_timezone: No such time zone: 'CN' in %sut_common.inc on line %d +datefmt_set_timezone: No such time zone: 'CN' +datefmt_set_timezone: No such time zone: 'CN' After creation of the dateformatter : timezone_id= US/Pacific ----------- diff --git a/ext/intl/timezone/timezone_class.cpp b/ext/intl/timezone/timezone_class.cpp index f7b3d4eeb08f2..9d477aaeca83b 100644 --- a/ext/intl/timezone/timezone_class.cpp +++ b/ext/intl/timezone/timezone_class.cpp @@ -22,6 +22,7 @@ #include #include #include "../intl_convertcpp.h" +#include "../intl_common.h" #include "../common/common_date.h" @@ -147,24 +148,15 @@ U_CFUNC TimeZone *timezone_process_timezone_argument(zval *zv_timezone, instanceof_function(Z_OBJCE_P(zv_timezone), TimeZone_ce_ptr)) { TimeZone_object *to = Z_INTL_TIMEZONE_P(zv_timezone); - /* TODO Throw proper Error exceptions for uninitialized classes and failure to clone */ if (to->utimezone == NULL) { - spprintf(&message, 0, "%s: passed IntlTimeZone is not " + zend_throw_error(IntlException_ce_ptr, "%s: passed IntlTimeZone is not " "properly constructed", func); - if (message) { - intl_errors_set(outside_error, U_ILLEGAL_ARGUMENT_ERROR, message, 1); - efree(message); - } zval_ptr_dtor_str(&local_zv_tz); return NULL; } timeZone = to->utimezone->clone(); if (UNEXPECTED(timeZone == NULL)) { - spprintf(&message, 0, "%s: could not clone TimeZone", func); - if (message) { - intl_errors_set(outside_error, U_MEMORY_ALLOCATION_ERROR, message, 1); - efree(message); - } + zend_throw_error(IntlException_ce_ptr, "%s: could not clone TimeZone", func); zval_ptr_dtor_str(&local_zv_tz); return NULL; } @@ -185,32 +177,20 @@ U_CFUNC TimeZone *timezone_process_timezone_argument(zval *zv_timezone, } if (intl_stringFromChar(id, Z_STRVAL_P(zv_timezone), Z_STRLEN_P(zv_timezone), &status) == FAILURE) { - spprintf(&message, 0, "%s: Time zone identifier given is not a " + zend_throw_error(IntlException_ce_ptr, "%s: Time zone identifier given is not a " "valid UTF-8 string", func); - if (message) { - intl_errors_set(outside_error, status, message, 1); - efree(message); - } zval_ptr_dtor_str(&local_zv_tz); return NULL; } timeZone = TimeZone::createTimeZone(id); if (UNEXPECTED(timeZone == NULL)) { - spprintf(&message, 0, "%s: Could not create time zone", func); - if (message) { - intl_errors_set(outside_error, U_MEMORY_ALLOCATION_ERROR, message, 1); - efree(message); - } + zend_throw_error(IntlException_ce_ptr, "%s: Could not create time zone", func); zval_ptr_dtor_str(&local_zv_tz); return NULL; } if (*timeZone == TimeZone::getUnknown()) { - spprintf(&message, 0, "%s: No such time zone: '%s'", + zend_throw_error(IntlException_ce_ptr, "%s: No such time zone: '%s'", func, Z_STRVAL_P(zv_timezone)); - if (message) { - intl_errors_set(outside_error, U_ILLEGAL_ARGUMENT_ERROR, message, 1); - efree(message); - } zval_ptr_dtor_str(&local_zv_tz); delete timeZone; return NULL; From df6db27580025e2089a27e09615389ec03ab8a3d Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Mon, 23 Dec 2024 18:51:08 +0100 Subject: [PATCH 2/3] Fix GH-17246: GC during SCCP causes segfault This bug happens because of a nested `SHM_UNPROTECT()` sequence. In particular: ``` unprotect memory at ext/opcache/ZendAccelerator.c:2127 protect memory at ext/opcache/ZendAccelerator.c:2160 unprotect memory at ext/opcache/ZendAccelerator.c:2164 unprotect memory at ext/opcache/jit/zend_jit_trace.c:7464 ^^^ Nested protect memory at ext/opcache/jit/zend_jit_trace.c:7591 ^^^ Problem is here: it should not protect again due to the nested unprotect protect memory at ext/opcache/ZendAccelerator.c:2191 ^^^ This one should actually protect, not the previous one ``` The reason this nesting happen is because: 1. We try to include the script, this eventually calls `cache_script_in_shared_memory` 2. `zend_optimize_script` will eventually run SCCP as part of the DFA pass. 3. SCCP will try to replace constants, but can also run destructors when a partial array is destructed here: https://github.com/php/php-src/blob/4e9cde758eadf30cc4d596d6398c2c34c64197b4/Zend/Optimizer/sccp.c#L2387-L2389 In this case, this destruction invokes the GC which invokes the tracing JIT, leading to the nested unprotects. This patch disables the GC to prevent invoking user code, as user code is not supposed to run during the optimizer pipeline. Closes GH-17249. Co-authored-by: Dmitry Stogov --- NEWS | 1 + ext/opcache/ZendAccelerator.c | 3 +++ ext/opcache/tests/jit/gh17246.inc | 8 ++++++ ext/opcache/tests/jit/gh17246.phpt | 39 ++++++++++++++++++++++++++++++ 4 files changed, 51 insertions(+) create mode 100644 ext/opcache/tests/jit/gh17246.inc create mode 100755 ext/opcache/tests/jit/gh17246.phpt diff --git a/NEWS b/NEWS index f2a0b0246d1fc..257623b283397 100644 --- a/NEWS +++ b/NEWS @@ -51,6 +51,7 @@ PHP NEWS - Opcache: . opcache_get_configuration() properly reports jit_prof_threshold. (cmb) + . Fixed bug GH-17246 (GC during SCCP causes segfault). (Dmitry) - PCNTL: . Fix memory leak in cleanup code of pcntl_exec() when a non stringable diff --git a/ext/opcache/ZendAccelerator.c b/ext/opcache/ZendAccelerator.c index fb886d39e60ac..66c10021442ad 100644 --- a/ext/opcache/ZendAccelerator.c +++ b/ext/opcache/ZendAccelerator.c @@ -2153,7 +2153,10 @@ zend_op_array *persistent_compile_file(zend_file_handle *file_handle, int type) */ from_shared_memory = false; if (persistent_script) { + /* See GH-17246: we disable GC so that user code cannot be executed during the optimizer run. */ + bool orig_gc_state = gc_enable(false); persistent_script = cache_script_in_shared_memory(persistent_script, key, &from_shared_memory); + gc_enable(orig_gc_state); } /* Caching is disabled, returning op_array; diff --git a/ext/opcache/tests/jit/gh17246.inc b/ext/opcache/tests/jit/gh17246.inc new file mode 100644 index 0000000000000..f9e7d78fc165a --- /dev/null +++ b/ext/opcache/tests/jit/gh17246.inc @@ -0,0 +1,8 @@ +field = function() {}; + } + + public function __destruct() + { + // Necessary because we need to invoke tracing JIT during destruction + } +} + +for ($i = 0; $i < 10000; ++$i) { + $obj = new Test(); +} + +require __DIR__.'/gh17246.inc'; + +?> +--EXPECTF-- +Fatal error: Uncaught Error: Class "NonExistentClass" not found in %s:%d +Stack trace: +#0 %s(%d): require() +#1 {main} + thrown in %s on line %d From a24eada99bd6a92a712d5c0eb9a192066adb3526 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Dunglas?= Date: Mon, 23 Dec 2024 15:01:57 +0100 Subject: [PATCH 3/3] [ci skip] Make build command for program using embed portable Closes GH-17247. --- NEWS | 3 +++ sapi/embed/README.md | 4 ++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/NEWS b/NEWS index 257623b283397..b89ef952f370d 100644 --- a/NEWS +++ b/NEWS @@ -22,6 +22,9 @@ PHP NEWS - DOM: . Fixed bug GH-17224 (UAF in importNode). (nielsdos) +- Embed: + . Make build command for program using embed portable. (dunglas) + - FFI: . Fixed bug #79075 (FFI header parser chokes on comments). (nielsdos) . Fix memory leak on ZEND_FFI_TYPE_CHAR conversion failure. (nielsdos) diff --git a/sapi/embed/README.md b/sapi/embed/README.md index c90ff354ab7d9..87b144b4b120d 100644 --- a/sapi/embed/README.md +++ b/sapi/embed/README.md @@ -36,12 +36,12 @@ To compile this, we must point the compiler to the PHP header files. The paths t We must also point the linker and the runtime loader to the `libphp.so` shared lib for linking PHP (`-lphp`) which is located at `$(php-config --prefix)/lib`. So the complete command to compile ends up being: ```bash -$ gcc \ +$ cc \ $(php-config --includes) \ -L$(php-config --prefix)/lib \ embed_sapi_basic_example.c \ -lphp \ - -Wl,-rpath=$(php-config --prefix)/lib + -Wl,-rpath,$(php-config --prefix)/lib ``` > :memo: The embed SAPI is disabled by default. In order for the above example to compile, PHP must be built with the embed SAPI enabled. To see what SAPIs are installed, run `php-config --php-sapis`. If you don't see `embed` in the list, you'll need to rebuild PHP with `./configure --enable-embed`. The PHP shared library `libphp.so` is built when the embed SAPI is enabled.