Fix unregistering ini entries of dynamically loaded extension (#8435)

arnaud-lb · web-flow · commit f07a08df5c67 · 2022-05-06T15:25:44.000+02:00
Fixes GH-8185
diff --git a/.cirrus.yml b/.cirrus.yml
@@ -14,7 +14,7 @@ task:
     - pkg install -y autoconf bison gmake re2c icu libiconv png freetype2 enchant2 bzip2 krb5 t1lib gmp tidyp libsodium libzip libxml2 libxslt openssl oniguruma pkgconf webp libavif
   script:
     - ./buildconf -f
-    - ./configure --prefix=/usr/local --enable-debug --enable-option-checking=fatal --enable-fpm --with-pdo-sqlite --without-pear --with-bz2 --with-avif --with-jpeg --with-webp --with-freetype --enable-gd --enable-exif --with-zip --with-zlib --enable-soap --enable-xmlreader --with-xsl --with-libxml --enable-shmop --enable-pcntl --enable-mbstring --with-curl --enable-sockets --with-openssl --with-iconv=/usr/local --enable-bcmath --enable-calendar --enable-ftp --with-kerberos --with-ffi --enable-zend-test --enable-intl --with-mhash --with-sodium --enable-werror --with-config-file-path=/etc --with-config-file-scan-dir=/etc/php.d
+    - ./configure --prefix=/usr/local --enable-debug --enable-option-checking=fatal --enable-fpm --with-pdo-sqlite --without-pear --with-bz2 --with-avif --with-jpeg --with-webp --with-freetype --enable-gd --enable-exif --with-zip --with-zlib --enable-soap --enable-xmlreader --with-xsl --with-libxml --enable-shmop --enable-pcntl --enable-mbstring --with-curl --enable-sockets --with-openssl --with-iconv=/usr/local --enable-bcmath --enable-calendar --enable-ftp --with-kerberos --with-ffi --enable-zend-test --enable-dl-test=shared --enable-intl --with-mhash --with-sodium --enable-werror --with-config-file-path=/etc --with-config-file-scan-dir=/etc/php.d
     - gmake -j2
     - mkdir /etc/php.d
     - gmake install
diff --git a/.github/actions/configure-macos/action.yml b/.github/actions/configure-macos/action.yml
@@ -61,6 +61,7 @@ runs:
           --enable-sysvmsg \
           --with-ffi \
           --enable-zend-test \
+          --enable-dl-test=shared \
           --enable-intl \
           --with-mhash \
           --with-sodium \
diff --git a/.github/actions/configure-x64/action.yml b/.github/actions/configure-x64/action.yml
@@ -56,6 +56,7 @@ runs:
           --enable-sysvmsg \
           --with-ffi \
           --enable-zend-test \
+          --enable-dl-test=shared \
           --with-ldap \
           --with-ldap-sasl \
           --with-password-argon2 \
diff --git a/Zend/zend.c b/Zend/zend.c
@@ -1033,9 +1033,7 @@ void zend_startup(zend_utility_functions *utility_functions) /* {{{ */
 
 void zend_register_standard_ini_entries(void) /* {{{ */
 {
-	int module_number = 0;
-
-	REGISTER_INI_ENTRIES();
+	zend_register_ini_entries_ex(ini_entries, 0, MODULE_PERSISTENT);
 }
 /* }}} */
 
diff --git a/Zend/zend_API.c b/Zend/zend_API.c
@@ -2965,7 +2965,7 @@ void module_destructor(zend_module_entry *module) /* {{{ */
 	if (module->module_started
 	 && !module->module_shutdown_func
 	 && module->type == MODULE_TEMPORARY) {
-		zend_unregister_ini_entries(module->module_number);
+		zend_unregister_ini_entries_ex(module->module_number, module->type);
 	}
 
 	/* Deinitialize module globals */
diff --git a/Zend/zend_ini.c b/Zend/zend_ini.c
@@ -23,6 +23,7 @@
 #include "zend_alloc.h"
 #include "zend_operators.h"
 #include "zend_strtod.h"
+#include "zend_modules.h"
 
 static HashTable *registered_zend_ini_directives;
 
@@ -194,7 +195,7 @@ ZEND_API void zend_ini_sort_entries(void) /* {{{ */
 /*
  * Registration / unregistration
  */
-ZEND_API zend_result zend_register_ini_entries(const zend_ini_entry_def *ini_entry, int module_number) /* {{{ */
+ZEND_API zend_result zend_register_ini_entries_ex(const zend_ini_entry_def *ini_entry, int module_number, int module_type) /* {{{ */
 {
 	zend_ini_entry *p;
 	zval *default_value;
@@ -210,7 +211,10 @@ ZEND_API zend_result zend_register_ini_entries(const zend_ini_entry_def *ini_ent
 	 * lead to death.
 	 */
 	if (directives != EG(ini_directives)) {
+		ZEND_ASSERT(module_type == MODULE_TEMPORARY);
 		directives = EG(ini_directives);
+	} else {
+		ZEND_ASSERT(module_type == MODULE_PERSISTENT);
 	}
 #endif
 
@@ -234,7 +238,7 @@ ZEND_API zend_result zend_register_ini_entries(const zend_ini_entry_def *ini_ent
 			if (p->name) {
 				zend_string_release_ex(p->name, 1);
 			}
-			zend_unregister_ini_entries(module_number);
+			zend_unregister_ini_entries_ex(module_number, module_type);
 			return FAILURE;
 		}
 		if (((default_value = zend_get_configuration_directive(p->name)) != NULL) &&
@@ -255,9 +259,46 @@ ZEND_API zend_result zend_register_ini_entries(const zend_ini_entry_def *ini_ent
 }
 /* }}} */
 
+ZEND_API zend_result zend_register_ini_entries(const zend_ini_entry_def *ini_entry, int module_number) /* {{{ */
+{
+	zend_module_entry *module;
+
+	/* Module is likely to be the last one in the list */
+	ZEND_HASH_REVERSE_FOREACH_PTR(&module_registry, module) {
+		if (module->module_number == module_number) {
+			return zend_register_ini_entries_ex(ini_entry, module_number, module->type);
+		}
+	} ZEND_HASH_FOREACH_END();
+
+	return FAILURE;
+}
+/* }}} */
+
+ZEND_API void zend_unregister_ini_entries_ex(int module_number, int module_type) /* {{{ */
+{
+	static HashTable *ini_directives;
+
+	if (module_type == MODULE_TEMPORARY) {
+		ini_directives = EG(ini_directives);
+	} else {
+		ini_directives = registered_zend_ini_directives;
+	}
+
+	zend_hash_apply_with_argument(ini_directives, zend_remove_ini_entries, (void *) &module_number);
+}
+/* }}} */
+
 ZEND_API void zend_unregister_ini_entries(int module_number) /* {{{ */
 {
-	zend_hash_apply_with_argument(registered_zend_ini_directives, zend_remove_ini_entries, (void *) &module_number);
+	zend_module_entry *module;
+
+	/* Module is likely to be the last one in the list */
+	ZEND_HASH_REVERSE_FOREACH_PTR(&module_registry, module) {
+		if (module->module_number == module_number) {
+			zend_unregister_ini_entries_ex(module_number, module->type);
+			return;
+		}
+	} ZEND_HASH_FOREACH_END();
 }
 /* }}} */
 
diff --git a/Zend/zend_ini.h b/Zend/zend_ini.h
@@ -72,7 +72,9 @@ ZEND_API void zend_copy_ini_directives(void);
 ZEND_API void zend_ini_sort_entries(void);
 
 ZEND_API zend_result zend_register_ini_entries(const zend_ini_entry_def *ini_entry, int module_number);
+ZEND_API zend_result zend_register_ini_entries_ex(const zend_ini_entry_def *ini_entry, int module_number, int module_type);
 ZEND_API void zend_unregister_ini_entries(int module_number);
+ZEND_API void zend_unregister_ini_entries_ex(int module_number, int module_type);
 ZEND_API void zend_ini_refresh_caches(int stage);
 ZEND_API zend_result zend_alter_ini_entry(zend_string *name, zend_string *new_value, int modify_type, int stage);
 ZEND_API zend_result zend_alter_ini_entry_ex(zend_string *name, zend_string *new_value, int modify_type, int stage, bool force_change);
diff --git a/azure/configure.yml b/azure/configure.yml
@@ -50,6 +50,7 @@ steps:
         --enable-sysvmsg \
         --with-ffi \
         --enable-zend-test \
+        --enable-dl-test=shared \
         --with-ldap \
         --with-ldap-sasl \
         --with-password-argon2 \
diff --git a/azure/i386/job.yml b/azure/i386/job.yml
@@ -62,6 +62,7 @@ jobs:
             --enable-sysvmsg \
             --with-ffi \
             --enable-zend-test \
+            --enable-dl-test=shared \
             --with-mhash \
             --with-sodium \
             --enable-dba \
diff --git a/azure/msan_job.yml b/azure/msan_job.yml
@@ -49,6 +49,7 @@ jobs:
             --enable-calendar \
             --enable-ftp \
             --enable-zend-test \
+            --enable-dl-test=shared \
             --enable-werror \
             --enable-memory-sanitizer \
             --with-config-file-path=/etc \
diff --git a/build/Makefile.global b/build/Makefile.global
@@ -75,7 +75,10 @@ PHP_TEST_SETTINGS = -d 'open_basedir=' -d 'output_buffering=0' -d 'memory_limit=
 PHP_TEST_SHARED_EXTENSIONS =  ` \
 	if test "x$(PHP_MODULES)" != "x"; then \
 		for i in $(PHP_MODULES)""; do \
-			. $$i; $(top_srcdir)/build/shtool echo -n -- " -d extension=$$dlname"; \
+			. $$i; \
+			if test "x$$dlname" != "xdl_test.so"; then \
+				$(top_srcdir)/build/shtool echo -n -- " -d extension=$$dlname"; \
+			fi; \
 		done; \
 	fi; \
 	if test "x$(PHP_ZEND_EX)" != "x"; then \
diff --git a/ext/dl_test/config.m4 b/ext/dl_test/config.m4
@@ -0,0 +1,8 @@
+PHP_ARG_ENABLE([dl-test],
+  [whether to enable dl-test extension],
+  [AS_HELP_STRING([--enable-dl-test],
+    [Enable dl_test extension])])
+
+if test "$PHP_DL_TEST" != "no"; then
+  PHP_NEW_EXTENSION(dl_test, dl_test.c, [shared],, -DZEND_ENABLE_STATIC_TSRMLS_CACHE=1)
+fi
diff --git a/ext/dl_test/config.w32 b/ext/dl_test/config.w32
@@ -0,0 +1,8 @@
+// vim:ft=javascript
+
+ARG_ENABLE("dl-test", "enable dl_test extension", "no");
+
+if (PHP_DL_TEST != "no") {
+	EXTENSION("dl_test", "dl_test.c", true, "/DZEND_ENABLE_STATIC_TSRMLS_CACHE=1");
+	ADD_FLAG("CFLAGS_DL_TEST", "/D PHP_DL_TEST_EXPORTS ");
+}
diff --git a/ext/dl_test/dl_test.c b/ext/dl_test/dl_test.c
@@ -0,0 +1,146 @@
+/*
+  +----------------------------------------------------------------------+
+  | Copyright (c) The PHP Group                                          |
+  +----------------------------------------------------------------------+
+  | This source file is subject to version 3.01 of the PHP license,      |
+  | that is bundled with this package in the file LICENSE, and is        |
+  | available through the world-wide-web at the following url:           |
+  | https://www.php.net/license/3_01.txt                                 |
+  | If you did not receive a copy of the PHP license and are unable to   |
+  | obtain it through the world-wide-web, please send a note to          |
+  | license@php.net so we can mail you a copy immediately.               |
+  +----------------------------------------------------------------------+
+  | Author: Arnaud Le Blanc <arnaud.lb@gmail.com>                        |
+  +----------------------------------------------------------------------+
+*/
+
+#ifdef HAVE_CONFIG_H
+# include "config.h"
+#endif
+
+#include "php.h"
+#include "ext/standard/info.h"
+#include "php_dl_test.h"
+#include "dl_test_arginfo.h"
+
+ZEND_DECLARE_MODULE_GLOBALS(dl_test)
+
+/* {{{ void dl_test_test1() */
+PHP_FUNCTION(dl_test_test1)
+{
+	ZEND_PARSE_PARAMETERS_NONE();
+
+	php_printf("The extension %s is loaded and working!\r\n", "dl_test");
+}
+/* }}} */
+
+/* {{{ string dl_test_test2( [ string $var ] ) */
+PHP_FUNCTION(dl_test_test2)
+{
+	char *var = "World";
+	size_t var_len = sizeof("World") - 1;
+	zend_string *retval;
+
+	ZEND_PARSE_PARAMETERS_START(0, 1)
+		Z_PARAM_OPTIONAL
+		Z_PARAM_STRING(var, var_len)
+	ZEND_PARSE_PARAMETERS_END();
+
+	retval = strpprintf(0, "Hello %s", var);
+
+	RETURN_STR(retval);
+}
+/* }}}*/
+
+/* {{{ INI */
+PHP_INI_BEGIN()
+	STD_PHP_INI_BOOLEAN("dl_test.long",      "0", PHP_INI_ALL, OnUpdateLong,       long_value,       zend_dl_test_globals, dl_test_globals)
+	STD_PHP_INI_ENTRY("dl_test.string", "hello", PHP_INI_ALL, OnUpdateString,                           string_value, zend_dl_test_globals, dl_test_globals)
+PHP_INI_END()
+/* }}} */
+
+/* {{{ PHP_MINIT_FUNCTION */
+PHP_MINIT_FUNCTION(dl_test)
+{
+	/* Test backwards compatibility */
+	if (getenv("PHP_DL_TEST_USE_OLD_REGISTER_INI_ENTRIES")) {
+		zend_register_ini_entries(ini_entries, module_number);
+	} else {
+		REGISTER_INI_ENTRIES();
+	}
+
+	return SUCCESS;
+}
+/* }}} */
+
+/* {{{ PHP_MSHUTDOWN_FUNCTION */
+static PHP_MSHUTDOWN_FUNCTION(dl_test)
+{
+	/* Test backwards compatibility */
+	if (getenv("PHP_DL_TEST_USE_OLD_REGISTER_INI_ENTRIES")) {
+		zend_unregister_ini_entries(module_number);
+	} else {
+		UNREGISTER_INI_ENTRIES();
+	}
+
+	return SUCCESS;
+}
+/* }}} */
+
+/* {{{ PHP_RINIT_FUNCTION */
+PHP_RINIT_FUNCTION(dl_test)
+{
+#if defined(ZTS) && defined(COMPILE_DL_DL_TEST)
+	ZEND_TSRMLS_CACHE_UPDATE();
+#endif
+
+	return SUCCESS;
+}
+/* }}} */
+
+/* {{{ PHP_MINFO_FUNCTION */
+PHP_MINFO_FUNCTION(dl_test)
+{
+	php_info_print_table_start();
+	php_info_print_table_header(2, "dl_test support", "enabled");
+	php_info_print_table_end();
+
+	DISPLAY_INI_ENTRIES();
+}
+/* }}} */
+
+/* {{{ PHP_GINIT_FUNCTION */
+static PHP_GINIT_FUNCTION(dl_test)
+{
+#if defined(COMPILE_DL_DL_TEST) && defined(ZTS)
+	ZEND_TSRMLS_CACHE_UPDATE();
+#endif
+	memset(dl_test_globals, 0, sizeof(*dl_test_globals));
+}
+/* }}} */
+
+/* {{{ dl_test_module_entry */
+zend_module_entry dl_test_module_entry = {
+	STANDARD_MODULE_HEADER,
+	"dl_test",
+	ext_functions,
+	PHP_MINIT(dl_test),
+	PHP_MSHUTDOWN(dl_test),
+	PHP_RINIT(dl_test),
+	NULL,
+	PHP_MINFO(dl_test),
+	PHP_DL_TEST_VERSION,
+	PHP_MODULE_GLOBALS(dl_test),
+	PHP_GINIT(dl_test),
+	NULL,
+	NULL,
+	STANDARD_MODULE_PROPERTIES_EX
+};
+/* }}} */
+
+#ifdef COMPILE_DL_DL_TEST
+# ifdef ZTS
+ZEND_TSRMLS_CACHE_DEFINE()
+# endif
+ZEND_GET_MODULE(dl_test)
+#endif
diff --git a/ext/dl_test/dl_test.stub.php b/ext/dl_test/dl_test.stub.php
@@ -0,0 +1,7 @@
+<?php
+
+/** @generate-class-entries */
+
+function dl_test_test1(): void {}
+
+function dl_test_test2(string $str = ""): string {}
diff --git a/ext/dl_test/dl_test_arginfo.h b/ext/dl_test/dl_test_arginfo.h
@@ -0,0 +1,20 @@
+/* This is a generated file, edit the .stub.php file instead.
+ * Stub hash: 39ccf8141b0eb785cafd490b420f2e99d6a7a66d */
+
+ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_dl_test_test1, 0, 0, IS_VOID, 0)
+ZEND_END_ARG_INFO()
+
+ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_dl_test_test2, 0, 0, IS_STRING, 0)
+	ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, str, IS_STRING, 0, "\"\"")
+ZEND_END_ARG_INFO()
+
+
+ZEND_FUNCTION(dl_test_test1);
+ZEND_FUNCTION(dl_test_test2);
+
+
+static const zend_function_entry ext_functions[] = {
+	ZEND_FE(dl_test_test1, arginfo_dl_test_test1)
+	ZEND_FE(dl_test_test2, arginfo_dl_test_test2)
+	ZEND_FE_END
+};
diff --git a/ext/dl_test/php_dl_test.h b/ext/dl_test/php_dl_test.h
diff --git a/ext/dl_test/tests/skip.inc b/ext/dl_test/tests/skip.inc
diff --git a/ext/standard/tests/general_functions/dl-001.phpt b/ext/standard/tests/general_functions/dl-001.phpt
diff --git a/ext/standard/tests/general_functions/dl-002.phpt b/ext/standard/tests/general_functions/dl-002.phpt
diff --git a/ext/standard/tests/general_functions/dl-003.phpt b/ext/standard/tests/general_functions/dl-003.phpt
diff --git a/main/main.c b/main/main.c
diff --git a/travis/compile.sh b/travis/compile.sh

Original file line number	Diff line number	Diff line change
`@@ -1033,9 +1033,7 @@ void zend_startup(zend_utility_functions utility_functions) / {{{ */`
`1033`	`1033`
`1034`	`1034`	`void zend_register_standard_ini_entries(void) /* {{{ */`
`1035`	`1035`	`{`
`1036`		`- int module_number = 0;`
`1037`		`-`
`1038`		`- REGISTER_INI_ENTRIES();`
	`1036`	`+ zend_register_ini_entries_ex(ini_entries, 0, MODULE_PERSISTENT);`
`1039`	`1037`	`}`
`1040`	`1038`	`/* }}} */`
`1041`	`1039`
Original file line number	Diff line number	Diff line change
`@@ -2965,7 +2965,7 @@ void module_destructor(zend_module_entry module) / {{{ */`
`2965`	`2965`	`if (module->module_started`
`2966`	`2966`	`&& !module->module_shutdown_func`
`2967`	`2967`	`&& module->type == MODULE_TEMPORARY) {`
`2968`		`- zend_unregister_ini_entries(module->module_number);`
	`2968`	`+ zend_unregister_ini_entries_ex(module->module_number, module->type);`
`2969`	`2969`	`}`
`2970`	`2970`
`2971`	`2971`	`/* Deinitialize module globals */`