Skip to content

Commit e3c65db

Browse files
dstogovdstogov
committed
Merge branch 'master' into preload
* 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
2 parents 4f57c1e + 33e777a commit e3c65db

File tree

10 files changed

+86
-64
lines changed

10 files changed

+86
-64
lines changed

‎Zend/zend.c

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@ void (*zend_printf_to_smart_str)(smart_str *buf, const char *format, va_list ap)
7878
ZEND_API char *(*zend_getenv)(char *name, size_t name_len);
7979
ZEND_API zend_string *(*zend_resolve_path)(const char *filename, size_t filename_len);
8080
ZEND_API int (*zend_post_startup_cb)(void) = NULL;
81+
ZEND_API void (*zend_post_shutdown_cb)(void) = NULL;
8182

8283
void (*zend_on_timeout)(int seconds);
8384

@@ -954,7 +955,18 @@ int zend_post_startup(void) /* {{{ */
954955

955956
zend_compiler_globals *compiler_globals = ts_resource(compiler_globals_id);
956957
zend_executor_globals *executor_globals = ts_resource(executor_globals_id);
958+
#endif
959+
960+
if (zend_post_startup_cb) {
961+
int (*cb)(void) = zend_post_startup_cb;
962+
963+
zend_post_startup_cb = NULL;
964+
if (cb() != SUCCESS) {
965+
return FAILURE;
966+
}
967+
}
957968

969+
#ifdef ZTS
958970
*GLOBAL_FUNCTION_TABLE = *compiler_globals->function_table;
959971
*GLOBAL_CLASS_TABLE = *compiler_globals->class_table;
960972
*GLOBAL_CONSTANTS_TABLE = *executor_globals->zend_constants;
@@ -981,15 +993,6 @@ int zend_post_startup(void) /* {{{ */
981993
global_map_ptr_last = CG(map_ptr_last);
982994
#endif
983995

984-
if (zend_post_startup_cb) {
985-
int (*cb)(void) = zend_post_startup_cb;
986-
987-
zend_post_startup_cb = NULL;
988-
if (cb() != SUCCESS) {
989-
return FAILURE;
990-
}
991-
}
992-
993996
return SUCCESS;
994997
}
995998
/* }}} */
@@ -1025,6 +1028,8 @@ void zend_shutdown(void) /* {{{ */
10251028
GLOBAL_CLASS_TABLE = NULL;
10261029
GLOBAL_AUTO_GLOBALS_TABLE = NULL;
10271030
GLOBAL_CONSTANTS_TABLE = NULL;
1031+
ts_free_id(executor_globals_id);
1032+
ts_free_id(compiler_globals_id);
10281033
#else
10291034
if (CG(map_ptr_base)) {
10301035
free(CG(map_ptr_base));

‎Zend/zend.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -293,7 +293,10 @@ extern void (*zend_printf_to_smart_string)(smart_string *buf, const char *format
293293
extern void (*zend_printf_to_smart_str)(smart_str *buf, const char *format, va_list ap);
294294
extern ZEND_API char *(*zend_getenv)(char *name, size_t name_len);
295295
extern ZEND_API zend_string *(*zend_resolve_path)(const char *filename, size_t filename_len);
296+
297+
/* These two callbacks are especially for opcache */
296298
extern ZEND_API int (*zend_post_startup_cb)(void);
299+
extern ZEND_API void (*zend_post_shutdown_cb)(void);
297300

298301
ZEND_API ZEND_COLD void zend_error(int type, const char *format, ...) ZEND_ATTRIBUTE_FORMAT(printf, 2, 3);
299302
ZEND_API ZEND_COLD void zend_throw_error(zend_class_entry *exception_ce, const char *format, ...) ZEND_ATTRIBUTE_FORMAT(printf, 2, 3);

‎Zend/zend_string.c

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,6 @@ static HashTable interned_strings_permanent;
3939

4040
static zend_new_interned_string_func_t interned_string_request_handler = zend_new_interned_string_request;
4141
static zend_string_init_interned_func_t interned_string_init_request_handler = zend_string_init_interned_request;
42-
static zend_string_copy_storage_func_t interned_string_copy_storage = NULL;
43-
static zend_string_copy_storage_func_t interned_string_restore_storage = NULL;
4442

4543
ZEND_API zend_string *zend_empty_string = NULL;
4644
ZEND_API zend_string *zend_one_char_string[256];
@@ -83,8 +81,6 @@ ZEND_API void zend_interned_strings_init(void)
8381

8482
interned_string_request_handler = zend_new_interned_string_request;
8583
interned_string_init_request_handler = zend_string_init_interned_request;
86-
interned_string_copy_storage = NULL;
87-
interned_string_restore_storage = NULL;
8884

8985
zend_empty_string = NULL;
9086
zend_known_strings = NULL;
@@ -301,26 +297,14 @@ ZEND_API void zend_interned_strings_set_request_storage_handlers(zend_new_intern
301297
interned_string_init_request_handler = init_handler;
302298
}
303299

304-
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)
305-
{
306-
interned_string_copy_storage = copy_handler;
307-
interned_string_restore_storage = restore_handler;
308-
}
309-
310300
ZEND_API void zend_interned_strings_switch_storage(zend_bool request)
311301
{
312302
if (request) {
313-
if (interned_string_copy_storage) {
314-
interned_string_copy_storage();
315-
}
316303
zend_new_interned_string = interned_string_request_handler;
317304
zend_string_init_interned = interned_string_init_request_handler;
318305
} else {
319306
zend_new_interned_string = zend_new_interned_string_permanent;
320307
zend_string_init_interned = zend_string_init_interned_permanent;
321-
if (interned_string_restore_storage) {
322-
interned_string_restore_storage();
323-
}
324308
}
325309
}
326310

‎Zend/zend_string.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@ ZEND_API void zend_interned_strings_dtor(void);
3939
ZEND_API void zend_interned_strings_activate(void);
4040
ZEND_API void zend_interned_strings_deactivate(void);
4141
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);
42-
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);
4342
ZEND_API void zend_interned_strings_switch_storage(zend_bool request);
4443

4544
ZEND_API extern zend_string *zend_empty_string;

‎ext/opcache/Optimizer/zend_inference.c

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2019,7 +2019,10 @@ static void handle_type_narrowing(const zend_op_array *op_array, zend_ssa *ssa,
20192019
{
20202020
if (1) {
20212021
/* Right now, this is always a bug */
2022-
zend_error(E_WARNING, "Narrowing occurred during type inference. Please file a bug report on bugs.php.net");
2022+
int def_op_num = ssa->vars[var].definition;
2023+
const zend_op *def_opline = def_op_num >= 0 ? &op_array->opcodes[def_op_num] : NULL;
2024+
const char *def_op_name = def_opline ? zend_get_opcode_name(def_opline->opcode) : "PHI";
2025+
zend_error(E_WARNING, "Narrowing occurred during type inference of %s. Please file a bug report on bugs.php.net", def_op_name);
20232026
} else {
20242027
/* if new_type set resets some bits from old_type set
20252028
* 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,
25552558
tmp |= MAY_BE_RCN;
25562559
}
25572560
}
2558-
if ((t1 & MAY_BE_ANY) == MAY_BE_LONG) {
2561+
if ((t1 & (MAY_BE_ANY|MAY_BE_UNDEF)) == MAY_BE_LONG) {
25592562
if (!ssa_var_info[ssa_ops[i].op1_use].has_range ||
25602563
(opline->opcode == ZEND_PRE_DEC &&
25612564
(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,
26172620
if (t1 & (MAY_BE_RC1|MAY_BE_RCN)) {
26182621
tmp |= MAY_BE_RC1;
26192622
}
2620-
if ((t1 & MAY_BE_ANY) == MAY_BE_LONG) {
2623+
if ((t1 & (MAY_BE_ANY|MAY_BE_UNDEF)) == MAY_BE_LONG) {
26212624
if (!ssa_var_info[ssa_ops[i].op1_use].has_range ||
26222625
(opline->opcode == ZEND_PRE_DEC &&
26232626
(ssa_var_info[ssa_ops[i].op1_use].range.underflow ||

‎ext/opcache/Optimizer/zend_optimizer.c

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1481,8 +1481,11 @@ int zend_optimize_script(zend_script *script, zend_long optimization_level, zend
14811481
for (i = 0; i < call_graph.op_arrays_count; i++) {
14821482
func_info = ZEND_FUNC_INFO(call_graph.op_arrays[i]);
14831483
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+
}
14861489
}
14871490
}
14881491

‎ext/opcache/ZendAccelerator.c

Lines changed: 22 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -734,19 +734,6 @@ static zend_string* ZEND_FASTCALL accel_replace_string_by_shm_permanent(zend_str
734734
return str;
735735
}
736736

737-
static zend_string* ZEND_FASTCALL accel_replace_string_by_process_permanent(zend_string *str)
738-
{
739-
zend_string *ret = zend_interned_string_find_permanent(str);
740-
741-
if (ret) {
742-
zend_string_release(str);
743-
return ret;
744-
}
745-
ZEND_ASSERT(0);
746-
return str;
747-
}
748-
749-
750737
static void accel_use_shm_interned_strings(void)
751738
{
752739
HANDLE_BLOCK_INTERRUPTIONS();
@@ -768,15 +755,6 @@ static void accel_use_shm_interned_strings(void)
768755
HANDLE_UNBLOCK_INTERRUPTIONS();
769756
}
770757

771-
static void accel_use_permanent_interned_strings(void)
772-
{
773-
if (ZCSG(preload_script)) {
774-
preload_shutdown();
775-
}
776-
777-
accel_copy_permanent_strings(accel_replace_string_by_process_permanent);
778-
}
779-
780758
#ifndef ZEND_WIN32
781759
static inline void kill_all_lockers(struct flock *mem_usage_check)
782760
{
@@ -2578,8 +2556,6 @@ static int zend_accel_init_shm(void)
25782556
STRTAB_INVALID_POS,
25792557
(char*)ZCSG(interned_strings).start -
25802558
((char*)&ZCSG(interned_strings) + sizeof(zend_string_table)));
2581-
2582-
zend_interned_strings_set_permanent_storage_copy_handlers(accel_use_shm_interned_strings, accel_use_permanent_interned_strings);
25832559
}
25842560

25852561
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)
28022778
orig_post_startup_cb = zend_post_startup_cb;
28032779
zend_post_startup_cb = accel_post_startup;
28042780

2781+
/* Prevent unloadig */
2782+
extension->handle = 0;
2783+
28052784
return SUCCESS;
28062785
}
28072786

@@ -2842,9 +2821,6 @@ static int accel_post_startup(void)
28422821
case SUCCESSFULLY_REATTACHED:
28432822
zend_shared_alloc_lock();
28442823
accel_shared_globals = (zend_accel_shared_globals *) ZSMMG(app_shared_globals);
2845-
if (ZCG(accel_directives).interned_strings_buffer) {
2846-
zend_interned_strings_set_permanent_storage_copy_handlers(accel_use_shm_interned_strings, accel_use_permanent_interned_strings);
2847-
}
28482824
zend_interned_strings_set_request_storage_handlers(accel_new_interned_string_for_php, accel_init_interned_string_for_php);
28492825
zend_shared_alloc_unlock();
28502826
break;
@@ -2940,9 +2916,20 @@ static int accel_post_startup(void)
29402916

29412917
zend_optimizer_startup();
29422918

2919+
if (!file_cache_only && ZCG(accel_directives).interned_strings_buffer) {
2920+
accel_use_shm_interned_strings();
2921+
}
2922+
29432923
return accel_finish_startup();
29442924
}
29452925

2926+
static void (*orig_post_shutdown_cb)(void);
2927+
2928+
static void accel_post_shutdown(void)
2929+
{
2930+
zend_shared_alloc_shutdown();
2931+
}
2932+
29462933
void accel_shutdown(void)
29472934
{
29482935
zend_ini_entry *ini_entry;
@@ -2959,6 +2946,10 @@ void accel_shutdown(void)
29592946
return;
29602947
}
29612948

2949+
if (ZCSG(preload_script)) {
2950+
preload_shutdown();
2951+
}
2952+
29622953
#ifdef HAVE_OPCACHE_FILE_CACHE
29632954
_file_cache_only = file_cache_only;
29642955
#endif
@@ -2970,8 +2961,11 @@ void accel_shutdown(void)
29702961
#endif
29712962

29722963
if (!_file_cache_only) {
2973-
zend_shared_alloc_shutdown();
2964+
/* Delay SHM dettach */
2965+
orig_post_shutdown_cb = zend_post_shutdown_cb;
2966+
zend_post_shutdown_cb = accel_post_shutdown;
29742967
}
2968+
29752969
zend_compile_file = accelerator_orig_compile_file;
29762970

29772971
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)
38763870
sapi_module.getenv = NULL;
38773871

38783872
zend_interned_strings_switch_storage(1);
3879-
if (ZCG(accel_directives).interned_strings_buffer) {
3880-
zend_interned_strings_set_permanent_storage_copy_handlers(NULL, accel_use_permanent_interned_strings);
3881-
}
38823873

38833874
SIGG(reset) = 0;
38843875
if (php_request_startup() == SUCCESS) {

‎ext/opcache/tests/bug77058.phpt

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
--TEST--
2+
Bug #77058: Type inference in opcache causes side effects
3+
--FILE--
4+
<?php
5+
6+
function myfunc(){
7+
$Nr = 0;
8+
while(1){
9+
$x--;
10+
$x++;
11+
if( ++ $Nr >= 2 ) break;
12+
}
13+
echo "'$Nr' is expected to be 2", PHP_EOL;
14+
}
15+
myfunc();
16+
17+
?>
18+
--EXPECTF--
19+
Notice: Undefined variable: x in %s on line %d
20+
'2' is expected to be 2

‎ext/opcache/zend_shared_alloc.c

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -259,21 +259,28 @@ int zend_shared_alloc_startup(size_t requested_size)
259259
void zend_shared_alloc_shutdown(void)
260260
{
261261
zend_shared_segment **tmp_shared_segments;
262+
zend_shared_segment *shared_segments_buf[16];
262263
size_t shared_segments_array_size;
263264
zend_smm_shared_globals tmp_shared_globals;
264265
int i;
265266

266267
tmp_shared_globals = *smm_shared_globals;
267268
smm_shared_globals = &tmp_shared_globals;
268269
shared_segments_array_size = ZSMMG(shared_segments_count) * (S_H(segment_type_size)() + sizeof(void *));
269-
tmp_shared_segments = emalloc(shared_segments_array_size);
270+
if (shared_segments_array_size > 16) {
271+
tmp_shared_segments = malloc(shared_segments_array_size);
272+
} else {
273+
tmp_shared_segments = shared_segments_buf;
274+
}
270275
copy_shared_segments(tmp_shared_segments, ZSMMG(shared_segments)[0], ZSMMG(shared_segments_count), S_H(segment_type_size)());
271276
ZSMMG(shared_segments) = tmp_shared_segments;
272277

273278
for (i = 0; i < ZSMMG(shared_segments_count); i++) {
274279
S_H(detach_segment)(ZSMMG(shared_segments)[i]);
275280
}
276-
efree(ZSMMG(shared_segments));
281+
if (shared_segments_array_size > 16) {
282+
free(ZSMMG(shared_segments));
283+
}
277284
ZSMMG(shared_segments) = NULL;
278285
g_shared_alloc_handler = NULL;
279286
#ifndef ZEND_WIN32

‎main/main.c

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2520,6 +2520,13 @@ void php_module_shutdown(void)
25202520
zend_interned_strings_dtor();
25212521
#endif
25222522

2523+
if (zend_post_shutdown_cb) {
2524+
void (*cb)(void) = zend_post_shutdown_cb;
2525+
2526+
zend_post_shutdown_cb = NULL;
2527+
cb();
2528+
}
2529+
25232530
module_initialized = 0;
25242531

25252532
#ifndef ZTS

0 commit comments

Comments
 (0)