summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorXinchen Hui <laruence@gmail.com>2017-02-13 19:17:09 +0800
committerXinchen Hui <laruence@gmail.com>2017-02-13 19:17:09 +0800
commit9df7cc3e68a327ad6fb38cecb55ca151a033695d (patch)
tree5366926e4f9f5ceb09263a081013b7cfce43458a
parent134d0b33a3bc18044c44deb1b33755e69b0c5631 (diff)
parent391735053181f3d166e4ebb58cf04a8acf3d1724 (diff)
downloadphp-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.phpt28
-rw-r--r--Zend/zend_gc.c123
-rw-r--r--Zend/zend_gc.h13
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