diff options
author | Xinchen Hui <laruence@gmail.com> | 2017-02-13 19:17:09 +0800 |
---|---|---|
committer | Xinchen Hui <laruence@gmail.com> | 2017-02-13 19:17:09 +0800 |
commit | 9df7cc3e68a327ad6fb38cecb55ca151a033695d (patch) | |
tree | 5366926e4f9f5ceb09263a081013b7cfce43458a | |
parent | 134d0b33a3bc18044c44deb1b33755e69b0c5631 (diff) | |
parent | 391735053181f3d166e4ebb58cf04a8acf3d1724 (diff) | |
download | php-git-9df7cc3e68a327ad6fb38cecb55ca151a033695d.tar.gz |
Merge branch 'PHP-7.0' into PHP-7.1
* PHP-7.0:
Fixed bug #73989 (PHP 7.1 Segfaults within Symfony test suite)
-rw-r--r-- | Zend/tests/bug73989.phpt | 28 | ||||
-rw-r--r-- | Zend/zend_gc.c | 123 | ||||
-rw-r--r-- | Zend/zend_gc.h | 13 |
3 files changed, 112 insertions, 52 deletions
diff --git a/Zend/tests/bug73989.phpt b/Zend/tests/bug73989.phpt new file mode 100644 index 0000000000..48e5a7a734 --- /dev/null +++ b/Zend/tests/bug73989.phpt @@ -0,0 +1,28 @@ +--TEST-- +Bug #73989 (PHP 7.1 Segfaults within Symfony test suite) +--FILE-- +<?php +class Cycle +{ + private $thing; + + public function __construct() + { + $obj = $this; + $this->thing = function() use($obj) {}; + } + + public function __destruct() + { + ($this->thing)(); + } + +} + +for ($i = 0; $i < 10000; ++$i) { + $obj = new Cycle(); +} +echo "OK\n"; +?> +--EXPECT-- +OK diff --git a/Zend/zend_gc.c b/Zend/zend_gc.c index a125c44ae4..087f04bc3f 100644 --- a/Zend/zend_gc.c +++ b/Zend/zend_gc.c @@ -75,26 +75,12 @@ /* one (0) is reserved */ #define GC_ROOT_BUFFER_MAX_ENTRIES 10001 -#define GC_FAKE_BUFFER_FLAG 0x80 -#define GC_TYPE_MASK 0x7f - #define GC_HAS_DESTRUCTORS (1<<0) #ifndef ZEND_GC_DEBUG # define ZEND_GC_DEBUG 0 #endif -#define GC_NUM_ADDITIONAL_ENTRIES \ - ((4096 - ZEND_MM_OVERHEAD - sizeof(void*) * 2) / sizeof(gc_root_buffer)) - -typedef struct _gc_additional_bufer gc_additional_buffer; - -struct _gc_additional_bufer { - uint32_t used; - gc_additional_buffer *next; - gc_root_buffer buf[GC_NUM_ADDITIONAL_ENTRIES]; -}; - #ifdef ZTS ZEND_API int gc_globals_id; #else @@ -103,9 +89,6 @@ ZEND_API zend_gc_globals gc_globals; ZEND_API int (*gc_collect_cycles)(void); -#define GC_REMOVE_FROM_ROOTS(current) \ - gc_remove_from_roots((current)) - #if ZEND_GC_DEBUG > 1 # define GC_TRACE(format, ...) fprintf(stderr, format "\n", ##__VA_ARGS__); # define GC_TRACE_REF(ref, format, ...) \ @@ -173,6 +156,12 @@ static zend_always_inline void gc_remove_from_roots(gc_root_buffer *root) GC_BENCH_DEC(root_buf_length); } +static zend_always_inline void gc_remove_from_additional_roots(gc_root_buffer *root) +{ + root->next->prev = root->prev; + root->prev->next = root->next; +} + static void root_buffer_dtor(zend_gc_globals *gc_globals) { if (gc_globals->buf) { @@ -199,6 +188,8 @@ static void gc_globals_ctor_ex(zend_gc_globals *gc_globals) gc_globals->gc_runs = 0; gc_globals->collected = 0; + gc_globals->additional_buffer = NULL; + #if GC_BENCH gc_globals->root_buf_length = 0; gc_globals->root_buf_peak = 0; @@ -254,6 +245,8 @@ ZEND_API void gc_reset(void) GC_G(first_unused) = NULL; GC_G(last_unused) = NULL; } + + GC_G(additional_buffer) = NULL; } ZEND_API void gc_init(void) @@ -326,21 +319,42 @@ ZEND_API void ZEND_FASTCALL gc_possible_root(zend_refcounted *ref) GC_BENCH_PEAK(root_buf_peak, root_buf_length); } +static zend_always_inline gc_root_buffer* gc_find_additional_buffer(zend_refcounted *ref) +{ + gc_additional_buffer *additional_buffer = GC_G(additional_buffer); + + /* We have to check each additional_buffer to find which one holds the ref */ + while (additional_buffer) { + gc_root_buffer *root = additional_buffer->buf + (GC_ADDRESS(GC_INFO(ref)) - GC_ROOT_BUFFER_MAX_ENTRIES); + if (root->ref == ref) { + return root; + } + additional_buffer = additional_buffer->next; + } + + ZEND_ASSERT(0); + return NULL; +} + ZEND_API void ZEND_FASTCALL gc_remove_from_buffer(zend_refcounted *ref) { gc_root_buffer *root; ZEND_ASSERT(GC_ADDRESS(GC_INFO(ref))); - ZEND_ASSERT(GC_ADDRESS(GC_INFO(ref)) < GC_ROOT_BUFFER_MAX_ENTRIES); GC_BENCH_INC(zval_remove_from_buffer); - root = GC_G(buf) + GC_ADDRESS(GC_INFO(ref)); + if (EXPECTED(GC_ADDRESS(GC_INFO(ref)) < GC_ROOT_BUFFER_MAX_ENTRIES)) { + root = GC_G(buf) + GC_ADDRESS(GC_INFO(ref)); + gc_remove_from_roots(root); + } else { + root = gc_find_additional_buffer(ref); + gc_remove_from_additional_roots(root); + } if (GC_REF_GET_COLOR(ref) != GC_BLACK) { GC_TRACE_SET_COLOR(ref, GC_PURPLE); } GC_INFO(ref) = 0; - GC_REMOVE_FROM_ROOTS(root); /* updete next root that is going to be freed */ if (GC_G(next_to_free) == root) { @@ -691,7 +705,7 @@ static void gc_scan_roots(void) } } -static void gc_add_garbage(zend_refcounted *ref, gc_additional_buffer **additional_buffer) +static void gc_add_garbage(zend_refcounted *ref) { gc_root_buffer *buf = GC_G(unused); @@ -714,25 +728,23 @@ static void gc_add_garbage(zend_refcounted *ref, gc_additional_buffer **addition #endif } else { /* If we don't have free slots in the buffer, allocate a new one and - * set it's address to GC_ROOT_BUFFER_MAX_ENTRIES that have special + * set it's address above GC_ROOT_BUFFER_MAX_ENTRIES that have special * meaning. */ - if (!*additional_buffer || (*additional_buffer)->used == GC_NUM_ADDITIONAL_ENTRIES) { + if (!GC_G(additional_buffer) || GC_G(additional_buffer)->used == GC_NUM_ADDITIONAL_ENTRIES) { gc_additional_buffer *new_buffer = emalloc(sizeof(gc_additional_buffer)); new_buffer->used = 0; - new_buffer->next = *additional_buffer; - *additional_buffer = new_buffer; + new_buffer->next = GC_G(additional_buffer); + GC_G(additional_buffer) = new_buffer; } - buf = (*additional_buffer)->buf + (*additional_buffer)->used; - (*additional_buffer)->used++; + buf = GC_G(additional_buffer)->buf + GC_G(additional_buffer)->used; #if 1 /* optimization: color is already GC_BLACK (0) */ - GC_INFO(ref) = GC_ROOT_BUFFER_MAX_ENTRIES; + GC_INFO(ref) = GC_ROOT_BUFFER_MAX_ENTRIES + GC_G(additional_buffer)->used; #else - GC_REF_SET_ADDRESS(ref, GC_ROOT_BUFFER_MAX_ENTRIES); + GC_REF_SET_ADDRESS(ref, GC_ROOT_BUFFER_MAX_ENTRIES) + GC_G(additional_buffer)->used; #endif - /* modify type to prevent indirect destruction */ - GC_TYPE(ref) |= GC_FAKE_BUFFER_FLAG; + GC_G(additional_buffer)->used++; } if (buf) { buf->ref = ref; @@ -743,7 +755,7 @@ static void gc_add_garbage(zend_refcounted *ref, gc_additional_buffer **addition } } -static int gc_collect_white(zend_refcounted *ref, uint32_t *flags, gc_additional_buffer **additional_buffer) +static int gc_collect_white(zend_refcounted *ref, uint32_t *flags) { int count = 0; HashTable *ht; @@ -776,7 +788,7 @@ tail_call: #else if (!GC_ADDRESS(GC_INFO(ref))) { #endif - gc_add_garbage(ref, additional_buffer); + gc_add_garbage(ref); } if (obj->handlers->dtor_obj && ((obj->handlers->dtor_obj != zend_objects_destroy_object) || @@ -800,7 +812,7 @@ tail_call: if (Z_REFCOUNTED_P(zv)) { ref = Z_COUNTED_P(zv); GC_REFCOUNT(ref)++; - count += gc_collect_white(ref, flags, additional_buffer); + count += gc_collect_white(ref, flags); /* count non-refcounted for compatibility ??? */ } else if (Z_TYPE_P(zv) != IS_UNDEF) { count++; @@ -822,7 +834,7 @@ tail_call: #else if (!GC_ADDRESS(GC_INFO(ref))) { #endif - gc_add_garbage(ref, additional_buffer); + gc_add_garbage(ref); } ht = (zend_array*)ref; } else if (GC_TYPE(ref) == IS_REFERENCE) { @@ -862,7 +874,7 @@ tail_call: if (Z_REFCOUNTED_P(zv)) { ref = Z_COUNTED_P(zv); GC_REFCOUNT(ref)++; - count += gc_collect_white(ref, flags, additional_buffer); + count += gc_collect_white(ref, flags); /* count non-refcounted for compatibility ??? */ } else if (Z_TYPE_P(zv) != IS_UNDEF) { count++; @@ -880,7 +892,7 @@ tail_call: return count; } -static int gc_collect_roots(uint32_t *flags, gc_additional_buffer **additional_buffer) +static int gc_collect_roots(uint32_t *flags) { int count = 0; gc_root_buffer *current = GC_G(roots).next; @@ -889,8 +901,12 @@ static int gc_collect_roots(uint32_t *flags, gc_additional_buffer **additional_b while (current != &GC_G(roots)) { gc_root_buffer *next = current->next; if (GC_REF_GET_COLOR(current->ref) == GC_BLACK) { + if (EXPECTED(GC_ADDRESS(GC_INFO(current->ref)) < GC_ROOT_BUFFER_MAX_ENTRIES)) { + gc_remove_from_roots(current); + } else { + gc_remove_from_additional_roots(current); + } GC_INFO(current->ref) = 0; /* reset GC_ADDRESS() and keep GC_BLACK */ - GC_REMOVE_FROM_ROOTS(current); } current = next; } @@ -898,7 +914,7 @@ static int gc_collect_roots(uint32_t *flags, gc_additional_buffer **additional_b current = GC_G(roots).next; while (current != &GC_G(roots)) { if (GC_REF_GET_COLOR(current->ref) == GC_WHITE) { - count += gc_collect_white(current->ref, flags, additional_buffer); + count += gc_collect_white(current->ref, flags); } current = current->next; } @@ -934,12 +950,15 @@ static void gc_remove_nested_data_from_buffer(zend_refcounted *ref, gc_root_buff tail_call: if (root || (GC_ADDRESS(GC_INFO(ref)) != 0 && - GC_REF_GET_COLOR(ref) == GC_BLACK && - GC_ADDRESS(GC_INFO(ref)) != GC_ROOT_BUFFER_MAX_ENTRIES)) { + GC_REF_GET_COLOR(ref) == GC_BLACK)) { GC_TRACE_REF(ref, "removing from buffer"); if (root) { + if (EXPECTED(GC_ADDRESS(GC_INFO(root->ref)) < GC_ROOT_BUFFER_MAX_ENTRIES)) { + gc_remove_from_roots(root); + } else { + gc_remove_from_additional_roots(root); + } GC_INFO(ref) = 0; - GC_REMOVE_FROM_ROOTS(root); root = NULL; } else { GC_REMOVE_FROM_BUFFER(ref); @@ -1033,7 +1052,7 @@ ZEND_API int zend_gc_collect_cycles(void) zend_refcounted *p; gc_root_buffer to_free; uint32_t gc_flags = 0; - gc_additional_buffer *additional_buffer; + gc_additional_buffer *additional_buffer_snapshot; #if ZEND_GC_DEBUG zend_bool orig_gc_full; #endif @@ -1057,8 +1076,8 @@ ZEND_API int zend_gc_collect_cycles(void) #endif GC_TRACE("Collecting roots"); - additional_buffer = NULL; - count = gc_collect_roots(&gc_flags, &additional_buffer); + additional_buffer_snapshot = GC_G(additional_buffer); + count = gc_collect_roots(&gc_flags); #if ZEND_GC_DEBUG GC_G(gc_full) = orig_gc_full; #endif @@ -1102,7 +1121,7 @@ ZEND_API int zend_gc_collect_cycles(void) while (current != &to_free) { p = current->ref; GC_G(next_to_free) = current->next; - if ((GC_TYPE(p) & GC_TYPE_MASK) == IS_OBJECT) { + if (GC_TYPE(p) == IS_OBJECT) { zend_object *obj = (zend_object*)p; if (IS_OBJ_VALID(EG(objects_store).object_buckets[obj->handle]) && @@ -1139,7 +1158,7 @@ ZEND_API int zend_gc_collect_cycles(void) p = current->ref; GC_G(next_to_free) = current->next; GC_TRACE_REF(p, "destroying"); - if ((GC_TYPE(p) & GC_TYPE_MASK) == IS_OBJECT) { + if (GC_TYPE(p) == IS_OBJECT) { zend_object *obj = (zend_object*)p; if (EG(objects_store).object_buckets && @@ -1158,7 +1177,7 @@ ZEND_API int zend_gc_collect_cycles(void) EG(objects_store).free_list_head = obj->handle; p = current->ref = (zend_refcounted*)(((char*)obj) - obj->handlers->offset); } - } else if ((GC_TYPE(p) & GC_TYPE_MASK) == IS_ARRAY) { + } else if (GC_TYPE(p) == IS_ARRAY) { zend_array *arr = (zend_array*)p; GC_TYPE(arr) = IS_NULL; @@ -1180,10 +1199,10 @@ ZEND_API int zend_gc_collect_cycles(void) current = next; } - while (additional_buffer != NULL) { - gc_additional_buffer *next = additional_buffer->next; - efree(additional_buffer); - additional_buffer = next; + while (GC_G(additional_buffer) != additional_buffer_snapshot) { + gc_additional_buffer *next = GC_G(additional_buffer)->next; + efree(GC_G(additional_buffer)); + GC_G(additional_buffer) = next; } GC_TRACE("Collection finished"); diff --git a/Zend/zend_gc.h b/Zend/zend_gc.h index 8e3bc46ff0..db67104aa6 100644 --- a/Zend/zend_gc.h +++ b/Zend/zend_gc.h @@ -67,6 +67,17 @@ typedef struct _gc_root_buffer { uint32_t refcount; } gc_root_buffer; +#define GC_NUM_ADDITIONAL_ENTRIES \ + ((4096 - ZEND_MM_OVERHEAD - sizeof(void*) * 2) / sizeof(gc_root_buffer)) + +typedef struct _gc_additional_bufer gc_additional_buffer; + +struct _gc_additional_bufer { + uint32_t used; + gc_additional_buffer *next; + gc_root_buffer buf[GC_NUM_ADDITIONAL_ENTRIES]; +}; + typedef struct _zend_gc_globals { zend_bool gc_enabled; zend_bool gc_active; @@ -93,6 +104,8 @@ typedef struct _zend_gc_globals { uint32_t zval_marked_grey; #endif + gc_additional_buffer *additional_buffer; + } zend_gc_globals; #ifdef ZTS |