Merge branch 'master' into preload

dstogov · dstogov · commit e3c65db09951 · 2018-10-25T20:52:26.000+03:00
* master:
  Improved shared interned strings handling. The previous implementation worked incorrectly in ZTS build. It changed strings only in function/class tables of one thread. Now all threads gets the same shared interned strings. Also, on shutdown, we don't try to replace SHM interned strings back to process strings, but delay dettachment of SHM instead.
  Don't use request heap at shutdown
  Don't optimize function if inference failed
  Fixed bug #77058
  Improve "narrowing" error message
  bump versions
diff --git a/Zend/zend.c b/Zend/zend.c
@@ -78,6 +78,7 @@ void (*zend_printf_to_smart_str)(smart_str *buf, const char *format, va_list ap)
 ZEND_API char *(*zend_getenv)(char *name, size_t name_len);
 ZEND_API zend_string *(*zend_resolve_path)(const char *filename, size_t filename_len);
 ZEND_API int (*zend_post_startup_cb)(void) = NULL;
+ZEND_API void (*zend_post_shutdown_cb)(void) = NULL;
 
 void (*zend_on_timeout)(int seconds);
 
@@ -954,7 +955,18 @@ int zend_post_startup(void) /* {{{ */
 
 	zend_compiler_globals *compiler_globals = ts_resource(compiler_globals_id);
 	zend_executor_globals *executor_globals = ts_resource(executor_globals_id);
+#endif
+
+	if (zend_post_startup_cb) {
+		int (*cb)(void) = zend_post_startup_cb;
+
+		zend_post_startup_cb = NULL;
+		if (cb() != SUCCESS) {
+			return FAILURE;
+		}
+	}
 
+#ifdef ZTS
 	*GLOBAL_FUNCTION_TABLE = *compiler_globals->function_table;
 	*GLOBAL_CLASS_TABLE = *compiler_globals->class_table;
 	*GLOBAL_CONSTANTS_TABLE = *executor_globals->zend_constants;
@@ -981,15 +993,6 @@ int zend_post_startup(void) /* {{{ */
 	global_map_ptr_last = CG(map_ptr_last);
 #endif
 
-	if (zend_post_startup_cb) {
-		int (*cb)(void) = zend_post_startup_cb;
-
-		zend_post_startup_cb = NULL;
-		if (cb() != SUCCESS) {
-			return FAILURE;
-		}
-	}
-
 	return SUCCESS;
 }
 /* }}} */
@@ -1025,6 +1028,8 @@ void zend_shutdown(void) /* {{{ */
 	GLOBAL_CLASS_TABLE = NULL;
 	GLOBAL_AUTO_GLOBALS_TABLE = NULL;
 	GLOBAL_CONSTANTS_TABLE = NULL;
+	ts_free_id(executor_globals_id);
+	ts_free_id(compiler_globals_id);
 #else
 	if (CG(map_ptr_base)) {
 		free(CG(map_ptr_base));
diff --git a/Zend/zend.h b/Zend/zend.h
@@ -293,7 +293,10 @@ extern void (*zend_printf_to_smart_string)(smart_string *buf, const char *format
 extern void (*zend_printf_to_smart_str)(smart_str *buf, const char *format, va_list ap);
 extern ZEND_API char *(*zend_getenv)(char *name, size_t name_len);
 extern ZEND_API zend_string *(*zend_resolve_path)(const char *filename, size_t filename_len);
+
+/* These two callbacks are especially for opcache */
 extern ZEND_API int (*zend_post_startup_cb)(void);
+extern ZEND_API void (*zend_post_shutdown_cb)(void);
 
 ZEND_API ZEND_COLD void zend_error(int type, const char *format, ...) ZEND_ATTRIBUTE_FORMAT(printf, 2, 3);
 ZEND_API ZEND_COLD void zend_throw_error(zend_class_entry *exception_ce, const char *format, ...) ZEND_ATTRIBUTE_FORMAT(printf, 2, 3);
diff --git a/Zend/zend_string.c b/Zend/zend_string.c
@@ -39,8 +39,6 @@ static HashTable interned_strings_permanent;
 
 static zend_new_interned_string_func_t interned_string_request_handler = zend_new_interned_string_request;
 static zend_string_init_interned_func_t interned_string_init_request_handler = zend_string_init_interned_request;
-static zend_string_copy_storage_func_t interned_string_copy_storage = NULL;
-static zend_string_copy_storage_func_t interned_string_restore_storage = NULL;
 
 ZEND_API zend_string  *zend_empty_string = NULL;
 ZEND_API zend_string  *zend_one_char_string[256];
@@ -83,8 +81,6 @@ ZEND_API void zend_interned_strings_init(void)
 
 	interned_string_request_handler = zend_new_interned_string_request;
 	interned_string_init_request_handler = zend_string_init_interned_request;
-	interned_string_copy_storage = NULL;
-	interned_string_restore_storage = NULL;
 
 	zend_empty_string = NULL;
 	zend_known_strings = NULL;
@@ -301,26 +297,14 @@ ZEND_API void zend_interned_strings_set_request_storage_handlers(zend_new_intern
 	interned_string_init_request_handler = init_handler;
 }
 
-ZEND_API void zend_interned_strings_set_permanent_storage_copy_handlers(zend_string_copy_storage_func_t copy_handler, zend_string_copy_storage_func_t restore_handler)
-{
-	interned_string_copy_storage = copy_handler;
-	interned_string_restore_storage = restore_handler;
-}
-
 ZEND_API void zend_interned_strings_switch_storage(zend_bool request)
 {
 	if (request) {
-		if (interned_string_copy_storage) {
-			interned_string_copy_storage();
-		}
 		zend_new_interned_string = interned_string_request_handler;
 		zend_string_init_interned = interned_string_init_request_handler;
 	} else {
 		zend_new_interned_string = zend_new_interned_string_permanent;
 		zend_string_init_interned = zend_string_init_interned_permanent;
-		if (interned_string_restore_storage) {
-			interned_string_restore_storage();
-		}
 	}
 }
 
diff --git a/Zend/zend_string.h b/Zend/zend_string.h
@@ -39,7 +39,6 @@ ZEND_API void zend_interned_strings_dtor(void);
 ZEND_API void zend_interned_strings_activate(void);
 ZEND_API void zend_interned_strings_deactivate(void);
 ZEND_API void zend_interned_strings_set_request_storage_handlers(zend_new_interned_string_func_t handler, zend_string_init_interned_func_t init_handler);
-ZEND_API void zend_interned_strings_set_permanent_storage_copy_handlers(zend_string_copy_storage_func_t copy_handler, zend_string_copy_storage_func_t restore_handler);
 ZEND_API void zend_interned_strings_switch_storage(zend_bool request);
 
 ZEND_API extern zend_string  *zend_empty_string;
diff --git a/ext/opcache/Optimizer/zend_inference.c b/ext/opcache/Optimizer/zend_inference.c
@@ -2019,7 +2019,10 @@ static void handle_type_narrowing(const zend_op_array *op_array, zend_ssa *ssa,
 {
 	if (1) {
 		/* Right now, this is always a bug */
-		zend_error(E_WARNING, "Narrowing occurred during type inference. Please file a bug report on bugs.php.net");
+		int def_op_num = ssa->vars[var].definition;
+		const zend_op *def_opline = def_op_num >= 0 ? &op_array->opcodes[def_op_num] : NULL;
+		const char *def_op_name = def_opline ? zend_get_opcode_name(def_opline->opcode) : "PHI";
+		zend_error(E_WARNING, "Narrowing occurred during type inference of %s. Please file a bug report on bugs.php.net", def_op_name);
 	} else {
 		/* if new_type set resets some bits from old_type set
 		 * We have completely recalculate types of some dependent SSA variables
@@ -2555,7 +2558,7 @@ static int zend_update_type_info(const zend_op_array *op_array,
 					tmp |= MAY_BE_RCN;
 				}
 			}
-			if ((t1 & MAY_BE_ANY) == MAY_BE_LONG) {
+			if ((t1 & (MAY_BE_ANY|MAY_BE_UNDEF)) == MAY_BE_LONG) {
 				if (!ssa_var_info[ssa_ops[i].op1_use].has_range ||
 				    (opline->opcode == ZEND_PRE_DEC &&
 				     (ssa_var_info[ssa_ops[i].op1_use].range.underflow ||
@@ -2617,7 +2620,7 @@ static int zend_update_type_info(const zend_op_array *op_array,
 			if (t1 & (MAY_BE_RC1|MAY_BE_RCN)) {
 				tmp |= MAY_BE_RC1;
 			}
-			if ((t1 & MAY_BE_ANY) == MAY_BE_LONG) {
+			if ((t1 & (MAY_BE_ANY|MAY_BE_UNDEF)) == MAY_BE_LONG) {
 				if (!ssa_var_info[ssa_ops[i].op1_use].has_range ||
 				     (opline->opcode == ZEND_PRE_DEC &&
 				      (ssa_var_info[ssa_ops[i].op1_use].range.underflow ||
diff --git a/ext/opcache/Optimizer/zend_optimizer.c b/ext/opcache/Optimizer/zend_optimizer.c
@@ -1481,8 +1481,11 @@ int zend_optimize_script(zend_script *script, zend_long optimization_level, zend
 		for (i = 0; i < call_graph.op_arrays_count; i++) {
 			func_info = ZEND_FUNC_INFO(call_graph.op_arrays[i]);
 			if (func_info) {
-				zend_dfa_analyze_op_array(call_graph.op_arrays[i], &ctx, &func_info->ssa);
-				func_info->flags = func_info->ssa.cfg.flags;
+				if (zend_dfa_analyze_op_array(call_graph.op_arrays[i], &ctx, &func_info->ssa) == SUCCESS) {
+					func_info->flags = func_info->ssa.cfg.flags;
+				} else {
+					ZEND_SET_FUNC_INFO(call_graph.op_arrays[i], NULL);
+				}
 			}
 		}
 
diff --git a/ext/opcache/ZendAccelerator.c b/ext/opcache/ZendAccelerator.c
@@ -734,19 +734,6 @@ static zend_string* ZEND_FASTCALL accel_replace_string_by_shm_permanent(zend_str
 	return str;
 }
 
-static zend_string* ZEND_FASTCALL accel_replace_string_by_process_permanent(zend_string *str)
-{
-	zend_string *ret = zend_interned_string_find_permanent(str);
-
-	if (ret) {
-		zend_string_release(str);
-		return ret;
-	}
-	ZEND_ASSERT(0);
-	return str;
-}
-
-
 static void accel_use_shm_interned_strings(void)
 {
 	HANDLE_BLOCK_INTERRUPTIONS();
@@ -768,15 +755,6 @@ static void accel_use_shm_interned_strings(void)
 	HANDLE_UNBLOCK_INTERRUPTIONS();
 }
 
-static void accel_use_permanent_interned_strings(void)
-{
-	if (ZCSG(preload_script)) {
-		preload_shutdown();
-	}
-
-	accel_copy_permanent_strings(accel_replace_string_by_process_permanent);
-}
-
 #ifndef ZEND_WIN32
 static inline void kill_all_lockers(struct flock *mem_usage_check)
 {
@@ -2578,8 +2556,6 @@ static int zend_accel_init_shm(void)
 			STRTAB_INVALID_POS,
 			(char*)ZCSG(interned_strings).start -
 				((char*)&ZCSG(interned_strings) + sizeof(zend_string_table)));
-
-		zend_interned_strings_set_permanent_storage_copy_handlers(accel_use_shm_interned_strings, accel_use_permanent_interned_strings);
 	}
 
 	zend_interned_strings_set_request_storage_handlers(accel_new_interned_string_for_php, accel_init_interned_string_for_php);
@@ -2802,6 +2778,9 @@ static int accel_startup(zend_extension *extension)
 	orig_post_startup_cb = zend_post_startup_cb;
 	zend_post_startup_cb = accel_post_startup;
 
+	/* Prevent unloadig */
+	extension->handle = 0;
+
 	return SUCCESS;
 }
 
@@ -2842,9 +2821,6 @@ static int accel_post_startup(void)
 			case SUCCESSFULLY_REATTACHED:
 				zend_shared_alloc_lock();
 				accel_shared_globals = (zend_accel_shared_globals *) ZSMMG(app_shared_globals);
-				if (ZCG(accel_directives).interned_strings_buffer) {
-					zend_interned_strings_set_permanent_storage_copy_handlers(accel_use_shm_interned_strings, accel_use_permanent_interned_strings);
-				}
 				zend_interned_strings_set_request_storage_handlers(accel_new_interned_string_for_php, accel_init_interned_string_for_php);
 				zend_shared_alloc_unlock();
 				break;
@@ -2940,9 +2916,20 @@ static int accel_post_startup(void)
 
 	zend_optimizer_startup();
 
+	if (!file_cache_only && ZCG(accel_directives).interned_strings_buffer) {
+		accel_use_shm_interned_strings();
+	}
+
 	return accel_finish_startup();
 }
 
+static void (*orig_post_shutdown_cb)(void);
+
+static void accel_post_shutdown(void)
+{
+	zend_shared_alloc_shutdown();
+}
+
 void accel_shutdown(void)
 {
 	zend_ini_entry *ini_entry;
@@ -2959,6 +2946,10 @@ void accel_shutdown(void)
 		return;
 	}
 
+	if (ZCSG(preload_script)) {
+		preload_shutdown();
+	}
+
 #ifdef HAVE_OPCACHE_FILE_CACHE
 	_file_cache_only = file_cache_only;
 #endif
@@ -2970,8 +2961,11 @@ void accel_shutdown(void)
 #endif
 
 	if (!_file_cache_only) {
-		zend_shared_alloc_shutdown();
+		/* Delay SHM dettach */
+		orig_post_shutdown_cb = zend_post_shutdown_cb;
+		zend_post_shutdown_cb = accel_post_shutdown;
 	}
+
 	zend_compile_file = accelerator_orig_compile_file;
 
 	if ((ini_entry = zend_hash_str_find_ptr(EG(ini_directives), "include_path", sizeof("include_path")-1)) != NULL) {
@@ -3876,9 +3870,6 @@ static int accel_finish_startup(void)
 		sapi_module.getenv = NULL;
 
 		zend_interned_strings_switch_storage(1);
-		if (ZCG(accel_directives).interned_strings_buffer) {
-			zend_interned_strings_set_permanent_storage_copy_handlers(NULL, accel_use_permanent_interned_strings);
-		}
 
 		SIGG(reset) = 0;
 		if (php_request_startup() == SUCCESS) {
diff --git a/ext/opcache/tests/bug77058.phpt b/ext/opcache/tests/bug77058.phpt
@@ -0,0 +1,20 @@
+--TEST--
+Bug #77058: Type inference in opcache causes side effects
+--FILE--
+<?php 
+
+function myfunc(){
+  $Nr = 0;
+  while(1){
+    $x--;
+    $x++;
+    if( ++ $Nr >= 2 ) break;
+  }
+  echo "'$Nr' is expected to be 2", PHP_EOL;
+}
+myfunc();
+
+?>
+--EXPECTF--
+Notice: Undefined variable: x in %s on line %d
+'2' is expected to be 2
diff --git a/ext/opcache/zend_shared_alloc.c b/ext/opcache/zend_shared_alloc.c
@@ -259,21 +259,28 @@ int zend_shared_alloc_startup(size_t requested_size)
 void zend_shared_alloc_shutdown(void)
 {
 	zend_shared_segment **tmp_shared_segments;
+	zend_shared_segment *shared_segments_buf[16];
 	size_t shared_segments_array_size;
 	zend_smm_shared_globals tmp_shared_globals;
 	int i;
 
 	tmp_shared_globals = *smm_shared_globals;
 	smm_shared_globals = &tmp_shared_globals;
 	shared_segments_array_size = ZSMMG(shared_segments_count) * (S_H(segment_type_size)() + sizeof(void *));
-	tmp_shared_segments = emalloc(shared_segments_array_size);
+	if (shared_segments_array_size > 16) {
+		tmp_shared_segments = malloc(shared_segments_array_size);
+	} else {
+		tmp_shared_segments = shared_segments_buf;
+	}
 	copy_shared_segments(tmp_shared_segments, ZSMMG(shared_segments)[0], ZSMMG(shared_segments_count), S_H(segment_type_size)());
 	ZSMMG(shared_segments) = tmp_shared_segments;
 
 	for (i = 0; i < ZSMMG(shared_segments_count); i++) {
 		S_H(detach_segment)(ZSMMG(shared_segments)[i]);
 	}
-	efree(ZSMMG(shared_segments));
+	if (shared_segments_array_size > 16) {
+		free(ZSMMG(shared_segments));
+	}
 	ZSMMG(shared_segments) = NULL;
 	g_shared_alloc_handler = NULL;
 #ifndef ZEND_WIN32
diff --git a/main/main.c b/main/main.c
@@ -2520,6 +2520,13 @@ void php_module_shutdown(void)
 	zend_interned_strings_dtor();
 #endif
 
+	if (zend_post_shutdown_cb) {
+		void (*cb)(void) = zend_post_shutdown_cb;
+
+		zend_post_shutdown_cb = NULL;
+		cb();
+	}
+
 	module_initialized = 0;
 
 #ifndef ZTS

Original file line number	Diff line number	Diff line change
`@@ -1481,8 +1481,11 @@ int zend_optimize_script(zend_script *script, zend_long optimization_level, zend`
`1481`	`1481`	`for (i = 0; i < call_graph.op_arrays_count; i++) {`
`1482`	`1482`	`func_info = ZEND_FUNC_INFO(call_graph.op_arrays[i]);`
`1483`	`1483`	`if (func_info) {`
`1484`		`- zend_dfa_analyze_op_array(call_graph.op_arrays[i], &ctx, &func_info->ssa);`
`1485`		`- func_info->flags = func_info->ssa.cfg.flags;`
	`1484`	`+ if (zend_dfa_analyze_op_array(call_graph.op_arrays[i], &ctx, &func_info->ssa) == SUCCESS) {`
	`1485`	`+ func_info->flags = func_info->ssa.cfg.flags;`
	`1486`	`+ } else {`
	`1487`	`+ ZEND_SET_FUNC_INFO(call_graph.op_arrays[i], NULL);`
	`1488`	`+ }`
`1486`	`1489`	`}`
`1487`	`1490`	`}`
`1488`	`1491`