Skip to content

Commit

Permalink
Merge branch 'PHP-7.1'
Browse files Browse the repository at this point in the history 
* PHP-7.1:
  Update NEWS
  Fixed bug  #73989 (PHP 7.1 Segfaults within Symfony test suite)

Conflicts:
	Zend/zend_gc.c
  • Loading branch information
@laruence
laruence committed Feb 13, 2017
2 parents 91159bd + 930ce02 commit 00e5ea7
Show file tree
Hide file tree
Showing 3 changed files with 112 additions and 52 deletions.
28 changes: 28 additions & 0 deletions Zend/tests/bug73989.phpt
View file
Original file line number Diff line number Diff line change
@@ -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
123 changes: 71 additions & 52 deletions Zend/zend_gc.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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, ...) \
Expand Down Expand Up @@ -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) {
Expand All @@ -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;
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -682,7 +696,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);

Expand All @@ -705,25 +719,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;
Expand All @@ -734,7 +746,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;
Expand Down Expand Up @@ -769,7 +781,7 @@ static int gc_collect_white(zend_refcounted *ref, uint32_t *flags, gc_additional
#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) ||
Expand All @@ -793,7 +805,7 @@ static int gc_collect_white(zend_refcounted *ref, uint32_t *flags, gc_additional
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++;
Expand All @@ -815,7 +827,7 @@ static int gc_collect_white(zend_refcounted *ref, uint32_t *flags, gc_additional
#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) {
Expand Down Expand Up @@ -855,7 +867,7 @@ static int gc_collect_white(zend_refcounted *ref, uint32_t *flags, gc_additional
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++;
Expand All @@ -873,7 +885,7 @@ static int gc_collect_white(zend_refcounted *ref, uint32_t *flags, gc_additional
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;
Expand All @@ -882,16 +894,20 @@ 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;
}

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;
}
Expand Down Expand Up @@ -927,12 +943,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);
Expand Down Expand Up @@ -1028,7 +1047,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
Expand All @@ -1052,8 +1071,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
Expand Down Expand Up @@ -1097,7 +1116,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]) &&
Expand Down Expand Up @@ -1133,7 +1152,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 (IS_OBJ_VALID(EG(objects_store).object_buckets[obj->handle])) {
Expand All @@ -1151,7 +1170,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;
Expand All @@ -1177,10 +1196,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");
Expand Down
13 changes: 13 additions & 0 deletions Zend/zend_gc.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand Down

0 comments on commit 00e5ea7

Please sign in to comment.