Fix tests/serialize/bug64146.phpt

The var hash now retains a reference to its elements, to ensure
that addresses are not reused.

Furthermore the var hash now only stores objects and references
and directly uses their pointer as key, thus making serialization
about two times faster.

diff --git a/ext/standard/basic_functions.h b/ext/standard/basic_functions.h
index 1b0325caa8ca2c6b2b7f7155335555a16da44eee..eaac7a1609afd3b7b5e111b4186ce53b9f6ff0e3 100644 (file)
--- a/ext/standard/basic_functions.h
+++ b/ext/standard/basic_functions.h
@@ -204,11 +204,11 @@ typedef struct _php_basic_globals {
        zend_class_entry *incomplete_class;
        unsigned serialize_lock; /* whether to use the locally supplied var_hash instead (__sleep/__wakeup) */
        struct {
-               void *var_hash;
+               struct php_serialize_data *data;
                unsigned level;
        } serialize;
        struct {
-               void *var_hash;
+               struct php_unserialize_data *data;
                unsigned level;
        } unserialize;
 

diff --git a/ext/standard/php_var.h b/ext/standard/php_var.h
index 12adc51ffaa859948658ede0e35e12dcc45bfb0e..23225cdc42a8dc2e21c961d943b3650bec56c3ad 100644 (file)
--- a/ext/standard/php_var.h
+++ b/ext/standard/php_var.h
@@ -38,7 +38,10 @@ PHPAPI void php_var_export_ex(zval *struc, int level, smart_str *buf TSRMLS_DC);
 
 PHPAPI void php_debug_zval_dump(zval *struc, int level TSRMLS_DC);
 
-typedef HashTable* php_serialize_data_t;
+struct php_serialize_data {
+       HashTable ht;
+       uint32_t n;
+};
 
 struct php_unserialize_data {
        void *first;
@@ -47,71 +50,67 @@ struct php_unserialize_data {
        void *last_dtor;
 };
 
-typedef struct php_unserialize_data* php_unserialize_data_t;
+typedef struct php_serialize_data *php_serialize_data_t;
+typedef struct php_unserialize_data *php_unserialize_data_t;
 
-PHPAPI void php_var_serialize(smart_str *buf, zval *struc, php_serialize_data_t *var_hash TSRMLS_DC);
+PHPAPI void php_var_serialize(smart_str *buf, zval *struc, php_serialize_data_t *data TSRMLS_DC);
 PHPAPI int php_var_unserialize(zval *rval, const unsigned char **p, const unsigned char *max, php_unserialize_data_t *var_hash TSRMLS_DC);
 PHPAPI int php_var_unserialize_ref(zval *rval, const unsigned char **p, const unsigned char *max, php_unserialize_data_t *var_hash TSRMLS_DC);
 PHPAPI int php_var_unserialize_intern(zval *rval, const unsigned char **p, const unsigned char *max, php_unserialize_data_t *var_hash TSRMLS_DC);
 
-#define PHP_VAR_SERIALIZE_INIT(var_hash_ptr) \
+#define PHP_VAR_SERIALIZE_INIT(d) \
 do  { \
        /* fprintf(stderr, "SERIALIZE_INIT      == lock: %u, level: %u\n", BG(serialize_lock), BG(serialize).level); */ \
        if (BG(serialize_lock) || !BG(serialize).level) { \
-               ALLOC_HASHTABLE(var_hash_ptr); \
-               zend_hash_init((var_hash_ptr), 16, NULL, NULL, 0); \
+               (d) = (php_serialize_data_t) emalloc(sizeof(struct php_serialize_data)); \
+               zend_hash_init(&(d)->ht, 16, NULL, ZVAL_PTR_DTOR, 0); \
+               (d)->n = 0; \
                if (!BG(serialize_lock)) { \
-                       BG(serialize).var_hash = (void *)(var_hash_ptr); \
+                       BG(serialize).data = d; \
                        BG(serialize).level = 1; \
                } \
        } else { \
-               (var_hash_ptr) = (php_serialize_data_t)BG(serialize).var_hash; \
+               (d) = BG(serialize).data; \
                ++BG(serialize).level; \
        } \
 } while(0)
 
-#define PHP_VAR_SERIALIZE_DESTROY(var_hash_ptr) \
+#define PHP_VAR_SERIALIZE_DESTROY(d) \
 do { \
        /* fprintf(stderr, "SERIALIZE_DESTROY   == lock: %u, level: %u\n", BG(serialize_lock), BG(serialize).level); */ \
-       if (BG(serialize_lock) || !BG(serialize).level) { \
-               zend_hash_destroy((var_hash_ptr)); \
-               FREE_HASHTABLE(var_hash_ptr); \
-       } else { \
-               if (!--BG(serialize).level) { \
-                       zend_hash_destroy((php_serialize_data_t)BG(serialize).var_hash); \
-                       FREE_HASHTABLE((php_serialize_data_t)BG(serialize).var_hash); \
-                       BG(serialize).var_hash = NULL; \
-               } \
+       if (BG(serialize_lock) || BG(serialize).level == 1) { \
+               zend_hash_destroy(&(d)->ht); \
+               efree((d)); \
+       } \
+       if (!BG(serialize_lock) && !--BG(serialize).level) { \
+               BG(serialize).data = NULL; \
        } \
 } while (0)
 
-#define PHP_VAR_UNSERIALIZE_INIT(var_hash_ptr) \
+#define PHP_VAR_UNSERIALIZE_INIT(d) \
 do { \
        /* fprintf(stderr, "UNSERIALIZE_INIT    == lock: %u, level: %u\n", BG(serialize_lock), BG(unserialize).level); */ \
        if (BG(serialize_lock) || !BG(unserialize).level) { \
-               (var_hash_ptr) = (php_unserialize_data_t)ecalloc(1, sizeof(struct php_unserialize_data)); \
+               (d) = (php_unserialize_data_t)ecalloc(1, sizeof(struct php_unserialize_data)); \
                if (!BG(serialize_lock)) { \
-                       BG(unserialize).var_hash = (void *)(var_hash_ptr); \
+                       BG(unserialize).data = (d); \
                        BG(unserialize).level = 1; \
                } \
        } else { \
-               (var_hash_ptr) = (php_unserialize_data_t)BG(unserialize).var_hash; \
+               (d) = BG(unserialize).data; \
                ++BG(unserialize).level; \
        } \
 } while (0)
 
-#define PHP_VAR_UNSERIALIZE_DESTROY(var_hash_ptr) \
+#define PHP_VAR_UNSERIALIZE_DESTROY(d) \
 do { \
        /* fprintf(stderr, "UNSERIALIZE_DESTROY == lock: %u, level: %u\n", BG(serialize_lock), BG(unserialize).level); */ \
-       if (BG(serialize_lock) || !BG(unserialize).level) { \
-               var_destroy(&(var_hash_ptr)); \
-               efree(var_hash_ptr); \
-       } else { \
-               if (!--BG(unserialize).level) { \
-                       var_destroy(&(var_hash_ptr)); \
-                       efree((var_hash_ptr)); \
-                       BG(unserialize).var_hash = NULL; \
-               } \
+       if (BG(serialize_lock) || BG(unserialize).level == 1) { \
+               var_destroy(&(d)); \
+               efree((d)); \
+       } \
+       if (!BG(serialize_lock) && !--BG(unserialize).level) { \
+               BG(unserialize).data = NULL; \
        } \
 } while (0)
 

diff --git a/ext/standard/var.c b/ext/standard/var.c
index f9d897a4ec5d3a8aef49b9955f172597a8bdd390..194715edf053692ad3725ad67e7a75ca48731852 100644 (file)
--- a/ext/standard/var.c
+++ b/ext/standard/var.c
@@ -598,54 +598,45 @@ PHP_FUNCTION(var_export)
 }
 /* }}} */
 
-static void php_var_serialize_intern(smart_str *buf, zval *struc, HashTable *var_hash TSRMLS_DC);
+static void php_var_serialize_intern(smart_str *buf, zval *struc, php_serialize_data_t var_hash TSRMLS_DC);
 
-static inline int php_add_var_hash(HashTable *var_hash, zval *var_ptr, zval *var_old TSRMLS_DC) /* {{{ */
+static inline uint32_t php_add_var_hash(php_serialize_data_t data, zval *var TSRMLS_DC) /* {{{ */
 {
-       zval var_no, *zv;
-       char id[32], *p;
-       register int len;
-       zval *var = var_ptr;
+       zval *zv;
+       zend_ulong key;
 
-       if (Z_ISREF_P(var)) {
-               var = Z_REFVAL_P(var);
-       }
-       if ((Z_TYPE_P(var) == IS_OBJECT) && Z_OBJ_HT_P(var)->get_class_entry) {
-               p = zend_print_long_to_buf(id + sizeof(id) - 1,
-                               (zend_long) Z_OBJ_P(var));
-               *(--p) = 'O';
-               len = id + sizeof(id) - 1 - p;
-       } else if (var_ptr != var) {
-               p = zend_print_long_to_buf(id + sizeof(id) - 1,
-                               (zend_long) Z_REF_P(var_ptr));
-               *(--p) = 'R';
-               len = id + sizeof(id) - 1 - p;
-       } else {
-               p = zend_print_long_to_buf(id + sizeof(id) - 1, (zend_long) var);
-               len = id + sizeof(id) - 1 - p;
+       data->n += 1;
+
+       if (Z_TYPE_P(var) != IS_OBJECT && Z_TYPE_P(var) != IS_REFERENCE) {
+               return 0;
        }
 
-       if ((zv = zend_hash_str_find(var_hash, p, len)) != NULL) {
-               ZVAL_COPY_VALUE(var_old, zv);
-               if (var == var_ptr) {
-                       /* we still need to bump up the counter, since non-refs will
-                        * be counted separately by unserializer */
-                       ZVAL_LONG(&var_no, -1);
-                       zend_hash_next_index_insert(var_hash, &var_no);
-               }
-#if 0
-               fprintf(stderr, "- had var (%d): %lu\n", Z_TYPE_P(var), **(zend_ulong**)var_old);
-#endif
-               return FAILURE;
+       /* References to objects are treated as if the reference didn't exist */
+       if (Z_TYPE_P(var) == IS_REFERENCE && Z_TYPE_P(Z_REFVAL_P(var)) == IS_OBJECT) {
+               var = Z_REFVAL_P(var);
        }
 
-       /* +1 because otherwise hash will think we are trying to store NULL pointer */
-       ZVAL_LONG(&var_no, zend_hash_num_elements(var_hash) + 1);
-       zend_hash_str_add(var_hash, p, len, &var_no);
-#if 0
-       fprintf(stderr, "+ add var (%d): %lu\n", Z_TYPE_P(var), Z_LVAL(var_no));
-#endif
-       return SUCCESS;
+       /* Index for the variable is stored using the numeric value of the pointer to
+        * the zend_refcounted struct */
+       key = (zend_ulong) (zend_uintptr_t) Z_COUNTED_P(var);
+       zv = zend_hash_index_find(&data->ht, key);
+
+       if (zv) {
+               return Z_LVAL_P(zv);
+       } else {
+               zval zv_n;
+               ZVAL_LONG(&zv_n, data->n);
+               zend_hash_index_add_new(&data->ht, key, &zv_n);
+
+               /* Additionally to the index, we also store the variable, to ensure that it is
+                * not destroyed during serialization and its pointer reused. The variable is
+                * stored at the numeric value of the pointer + 1, which cannot be the location
+                * of another zend_refcounted structure. */
+               zend_hash_index_add_new(&data->ht, key + 1, var);
+               Z_ADDREF_P(var);
+
+               return 0;
+       }
 }
 /* }}} */
 
@@ -682,7 +673,7 @@ static inline zend_bool php_var_serialize_class_name(smart_str *buf, zval *struc
 }
 /* }}} */
 
-static void php_var_serialize_class(smart_str *buf, zval *struc, zval *retval_ptr, HashTable *var_hash TSRMLS_DC) /* {{{ */
+static void php_var_serialize_class(smart_str *buf, zval *struc, zval *retval_ptr, php_serialize_data_t var_hash TSRMLS_DC) /* {{{ */
 {
        uint32_t count;
        zend_bool incomplete_class;
@@ -780,27 +771,24 @@ static void php_var_serialize_class(smart_str *buf, zval *struc, zval *retval_pt
 }
 /* }}} */
 
-static void php_var_serialize_intern(smart_str *buf, zval *struc, HashTable *var_hash TSRMLS_DC) /* {{{ */
+static void php_var_serialize_intern(smart_str *buf, zval *struc, php_serialize_data_t var_hash TSRMLS_DC) /* {{{ */
 {
-       zval var_already;
+       uint32_t var_already;
        HashTable *myht;
 
        if (EG(exception)) {
                return;
        }
 
-       ZVAL_UNDEF(&var_already);
-
-       if (var_hash &&
-               php_add_var_hash(var_hash, struc, &var_already TSRMLS_CC) == FAILURE) {
+       if (var_hash && (var_already = php_add_var_hash(var_hash, struc TSRMLS_CC))) {
                if (Z_ISREF_P(struc)) {
                        smart_str_appendl(buf, "R:", 2);
-                       smart_str_append_long(buf, Z_LVAL(var_already));
+                       smart_str_append_long(buf, var_already);
                        smart_str_appendc(buf, ';');
                        return;
                } else if (Z_TYPE_P(struc) == IS_OBJECT) {
                        smart_str_appendl(buf, "r:", 2);
-                       smart_str_append_long(buf, Z_LVAL(var_already));
+                       smart_str_append_long(buf, var_already);
                        smart_str_appendc(buf, ';');
                        return;
                }
@@ -971,9 +959,9 @@ again:
 }
 /* }}} */
 
-PHPAPI void php_var_serialize(smart_str *buf, zval *struc, php_serialize_data_t *var_hash TSRMLS_DC) /* {{{ */
+PHPAPI void php_var_serialize(smart_str *buf, zval *struc, php_serialize_data_t *data TSRMLS_DC) /* {{{ */
 {
-       php_var_serialize_intern(buf, struc, *var_hash TSRMLS_CC);
+       php_var_serialize_intern(buf, struc, *data TSRMLS_CC);
        smart_str_0(buf);
 }
 /* }}} */