Stabilize (simple) watchpoints with IS_INDIRECT/IS_REFERENCE situations

diff --git a/sapi/phpdbg/phpdbg_utils.c b/sapi/phpdbg/phpdbg_utils.c
index 8f1d46aac002d50eb6786f46310c70d0652f85d4..7a7b70ceaf70bc1b330f979f4a1bd3149f1b020b 100644 (file)
--- a/sapi/phpdbg/phpdbg_utils.c
+++ b/sapi/phpdbg/phpdbg_utils.c
@@ -502,7 +502,7 @@ PHPDBG_API int phpdbg_parse_variable_with_arg(char *input, size_t len, HashTable
                        } else if (Z_TYPE_P(zv) == IS_ARRAY) {
                                parent = Z_ARRVAL_P(zv);
                        } else {
-                               phpdbg_error("variable", "type=\"notiterable\" variable=\"%.*s\"", "%.*s is nor an array nor an object", (int) i, input);
+                               phpdbg_error("variable", "type=\"notiterable\" variable=\"%.*s\"", "%.*s is nor an array nor an object", (int) (input[i] == '>' ? i - 1 : i), input);
                                return FAILURE;
                        }
                        index_len = 0;

diff --git a/sapi/phpdbg/phpdbg_watch.c b/sapi/phpdbg/phpdbg_watch.c
index 97cd5224af2695b09ca4ecf8f844d1038dda438d..3c78314c08753968688856b28bfee87a5b26ffb5 100644 (file)
--- a/sapi/phpdbg/phpdbg_watch.c
+++ b/sapi/phpdbg/phpdbg_watch.c
@@ -143,7 +143,7 @@ static void phpdbg_add_watch_collision(phpdbg_watchpoint_t *watch) {
                phpdbg_activate_watchpoint(&cur->watch);
        }
 
-       zend_hash_str_add_ptr(&cur->watches, watch->parent->str, watch->parent->str_len, watch->parent);
+       zend_hash_add_ptr(&cur->watches, watch->parent->str, watch->parent);
 }
 
 static void phpdbg_remove_watch_collision(zend_refcounted *ref) {
@@ -155,7 +155,7 @@ static void phpdbg_remove_watch_collision(zend_refcounted *ref) {
                        phpdbg_delete_watchpoints_recursive(watch);
                }
 
-               zend_hash_str_del(&cur->watches, watch->str, watch->str_len);
+               zend_hash_del(&cur->watches, watch->str);
 
                if (!--cur->num) {
                        phpdbg_deactivate_watchpoint(&cur->watch);
@@ -168,11 +168,25 @@ static void phpdbg_remove_watch_collision(zend_refcounted *ref) {
        }
 }
 
+static void phpdbg_create_reference_watch(phpdbg_watchpoint_t *watch) {
+       phpdbg_watchpoint_t *ref = emalloc(sizeof(phpdbg_watchpoint_t));
+       watch->reference = ref;
+       ref->flags = watch->flags;
+       ref->str = watch->str;
+       ++GC_REFCOUNT(ref->str);
+       ref->parent = watch;
+       ref->parent_container = NULL;
+       phpdbg_create_zval_watchpoint(Z_REFVAL_P(watch->addr.zv), ref);
+
+       phpdbg_store_watchpoint(ref);
+       phpdbg_activate_watchpoint(ref);
+}
+
 static int phpdbg_create_watchpoint(phpdbg_watchpoint_t *watch) {
        watch->flags |= PHPDBG_WATCH_SIMPLE;
 
        phpdbg_store_watchpoint(watch);
-       zend_hash_str_add_ptr(&PHPDBG_G(watchpoints), watch->str, watch->str_len, watch);
+       zend_hash_add_ptr(&PHPDBG_G(watchpoints), watch->str, watch);
 
        if (watch->parent && watch->parent->type == WATCH_ON_ZVAL && Z_REFCOUNTED_P(watch->parent->addr.zv)) {
                phpdbg_add_watch_collision(phpdbg_create_refcounted_watchpoint(watch, Z_COUNTED_P(watch->parent->addr.zv)));
@@ -185,15 +199,7 @@ static int phpdbg_create_watchpoint(phpdbg_watchpoint_t *watch) {
                }
 
                if (Z_ISREF_P(watch->addr.zv)) {
-                       phpdbg_watchpoint_t *ref = emalloc(sizeof(phpdbg_watchpoint_t));
-                       ref->flags = watch->flags;
-                       ref->str = watch->str;
-                       ref->str_len = watch->str_len;
-                       ref->parent = watch;
-                       ref->parent_container = NULL;
-                       phpdbg_create_zval_watchpoint(Z_REFVAL_P(watch->addr.zv), ref);
-
-                       phpdbg_create_watchpoint(ref);
+                       phpdbg_create_reference_watch(watch);
                }
        }
 
@@ -229,6 +235,7 @@ static int phpdbg_create_array_watchpoint(phpdbg_watchpoint_t *zv_watch) {
 }
 
 static int phpdbg_create_recursive_watchpoint(phpdbg_watchpoint_t *watch) {
+       size_t str_len;
        HashTable *ht;
        zval *zvp = watch->addr.zv;
 
@@ -246,26 +253,30 @@ static int phpdbg_create_recursive_watchpoint(phpdbg_watchpoint_t *watch) {
        }
 
        {
-               HashPosition position;
                zval *zv;
-               zval key;
+               zend_string *key;
+               zend_long h;
 
-               ZEND_HASH_FOREACH_VAL(ht, zv) {
+               ZEND_HASH_FOREACH_KEY_VAL(ht, h, key, zv) {
+                       char *str = NULL;
                        phpdbg_watchpoint_t *new_watch = emalloc(sizeof(phpdbg_watchpoint_t));
 
                        new_watch->flags = PHPDBG_WATCH_RECURSIVE;
                        new_watch->parent = watch;
                        new_watch->parent_container = ht;
 
-                       zend_hash_get_current_key_zval_ex(ht, &key, &position);
-                       if (Z_TYPE(key) == IS_STRING) {
-                               new_watch->name_in_parent = estrndup(Z_STRVAL(key), Z_STRLEN(key));
-                               new_watch->name_in_parent_len = Z_STRLEN(key);
+                       if (key) {
+                               new_watch->name_in_parent = key;
+                               ++GC_REFCOUNT(key);
                        } else {
-                               new_watch->name_in_parent_len = spprintf(&new_watch->name_in_parent, 0, "%lld", Z_LVAL(key));
+                               str_len = spprintf(&str, 0, "%lld", h);
+                               new_watch->name_in_parent = zend_string_init(str, str_len, 0);
+                               efree(str);
                        }
 
-                       new_watch->str_len = spprintf(&new_watch->str, 0, "%.*s%s%s%s", (int) watch->str_len, watch->str, Z_TYPE_P(zvp) == IS_ARRAY ? "[" : "->", phpdbg_get_property_key(new_watch->name_in_parent), Z_TYPE_P(zvp) == IS_ARRAY ? "]" : "");
+                       str_len = spprintf(&str, 0, "%.*s%s%s%s", (int) watch->str->len, watch->str->val, Z_TYPE_P(zvp) == IS_ARRAY ? "[" : "->", phpdbg_get_property_key(new_watch->name_in_parent->val), Z_TYPE_P(zvp) == IS_ARRAY ? "]" : "");
+                       new_watch->str = zend_string_init(str, str_len, 0);
+                       efree(str);
 
                        while (Z_TYPE_P(zv) == IS_INDIRECT) {
                                zv = Z_INDIRECT_P(zv);
@@ -277,15 +288,17 @@ static int phpdbg_create_recursive_watchpoint(phpdbg_watchpoint_t *watch) {
        }
 
        {
+               char *str = NULL;
                phpdbg_watchpoint_t *new_watch = emalloc(sizeof(phpdbg_watchpoint_t));
 
+               new_watch->flags = PHPDBG_WATCH_RECURSIVE;
                new_watch->parent = watch;
                new_watch->parent_container = watch->parent_container;
-               new_watch->name_in_parent = estrndup(watch->name_in_parent, watch->name_in_parent_len);
-               new_watch->name_in_parent_len = watch->name_in_parent_len;
-               new_watch->str = NULL;
-               new_watch->str_len = spprintf(&new_watch->str, 0, "%.*s[]", (int) watch->str_len, watch->str);
-               new_watch->flags = PHPDBG_WATCH_RECURSIVE;
+               new_watch->name_in_parent = watch->name_in_parent;
+               ++GC_REFCOUNT(new_watch->name_in_parent);
+               str_len = spprintf(&str, 0, "%.*s[]", (int) watch->str->len, watch->str->val);
+               new_watch->str = zend_string_init(str, str_len, 0);
+               efree(str);
 
                if (Z_TYPE_P(zvp) == IS_ARRAY) {
                        new_watch->flags |= PHPDBG_WATCH_ARRAY;
@@ -318,26 +331,28 @@ static int phpdbg_delete_watchpoint_recursive(phpdbg_watchpoint_t *watch, zend_b
                phpdbg_delete_zval_watchpoints_recursive(watch);
        }
 
-       return zend_hash_str_del(&PHPDBG_G(watchpoints), watch->str, watch->str_len);
+       return zend_hash_del(&PHPDBG_G(watchpoints), watch->str);
 }
 
 static void phpdbg_delete_ht_watchpoints_recursive(phpdbg_watchpoint_t *watch) {
        zend_string *strkey;
        zend_long numkey;
        char *str;
-       int str_len;
+       size_t str_len;
        phpdbg_watchpoint_t *watchpoint;
 
        ZEND_HASH_FOREACH_KEY(watch->addr.ht, numkey, strkey) {
                if (strkey) {
-                       str_len = asprintf(&str, "%.*s%s%s%s", (int) watch->str_len, watch->str, (watch->flags & PHPDBG_WATCH_ARRAY) ? "[" : "->", phpdbg_get_property_key(strkey->val), (watch->flags & PHPDBG_WATCH_ARRAY) ? "]" : "");
+                       str_len = spprintf(&str, 0, "%.*s%s%s%s", (int) watch->str->len, watch->str->val, (watch->flags & PHPDBG_WATCH_ARRAY) ? "[" : "->", phpdbg_get_property_key(strkey->val), (watch->flags & PHPDBG_WATCH_ARRAY) ? "]" : "");
                } else {
-                       str_len = asprintf(&str, "%.*s%s" ZEND_LONG_FMT "%s", (int) watch->str_len, watch->str, (watch->flags & PHPDBG_WATCH_ARRAY) ? "[" : "->", numkey, (watch->flags & PHPDBG_WATCH_ARRAY) ? "]" : "");
+                       str_len = spprintf(&str, 0, "%.*s%s" ZEND_LONG_FMT "%s", (int) watch->str->len, watch->str->val, (watch->flags & PHPDBG_WATCH_ARRAY) ? "[" : "->", numkey, (watch->flags & PHPDBG_WATCH_ARRAY) ? "]" : "");
                }
 
                if ((watchpoint = zend_hash_str_find_ptr(&PHPDBG_G(watchpoints), str, str_len))) {
                        phpdbg_delete_watchpoint_recursive(watchpoint, 1);
                }
+
+               efree(str);
        } ZEND_HASH_FOREACH_END();
 }
 
@@ -369,33 +384,32 @@ static int phpdbg_delete_watchpoint(phpdbg_watchpoint_t *tmp_watch) {
        if (watch->flags & PHPDBG_WATCH_RECURSIVE) {
                ret = phpdbg_delete_watchpoint_recursive(watch, 1);
        } else {
-               ret = zend_hash_str_del(&PHPDBG_G(watchpoints), watch->str, watch->str_len);
+               ret = zend_hash_del(&PHPDBG_G(watchpoints), watch->str);
        }
 
-       efree(tmp_watch->str);
-       efree(tmp_watch->name_in_parent);
+       zend_string_release(tmp_watch->str);
+       zend_string_release(tmp_watch->name_in_parent);
        efree(tmp_watch);
 
        return ret;
 }
 
-static int phpdbg_watchpoint_parse_wrapper(char *name, size_t len, char *keyname, size_t keylen, HashTable *parent, zval *zv, int (*callback)(phpdbg_watchpoint_t *)) {
+static int phpdbg_watchpoint_parse_wrapper(char *name, size_t namelen, char *key, size_t keylen, HashTable *parent, zval *zv, int (*callback)(phpdbg_watchpoint_t *)) {
        int ret;
-       phpdbg_watchpoint_t *watch = emalloc(sizeof(phpdbg_watchpoint_t));
+       phpdbg_watchpoint_t *watch = ecalloc(1, sizeof(phpdbg_watchpoint_t));
        watch->flags = 0;
-       watch->str = name;
-       watch->str_len = len;
-       watch->name_in_parent = keyname;
-       watch->name_in_parent_len = keylen;
+       watch->str = zend_string_init(name, namelen, 0);
+       watch->name_in_parent = zend_string_init(key, keylen, 0);
        watch->parent_container = parent;
        phpdbg_create_zval_watchpoint(zv, watch);
 
        ret = callback(watch);
 
+       efree(name);
+       efree(key);
+
        if (ret != SUCCESS) {
                efree(watch);
-               efree(name);
-               efree(keyname);
        }
 
        return ret;
@@ -477,17 +491,21 @@ void phpdbg_watch_HashTable_dtor(zval *zv) {
 
        zval_ptr_dtor_wrapper(zv);
 
+       while (Z_TYPE_P(zv) == IS_INDIRECT) {
+               zv = Z_INDIRECT_P(zv);
+       }
+
        if ((result = phpdbg_btree_find(&PHPDBG_G(watchpoint_tree), (zend_ulong) zv))) {
                phpdbg_watchpoint_t *watch = result->ptr;
 
                PHPDBG_G(watchpoint_hit) = 1;
 
-               phpdbg_notice("watchdelete", "variable=\"%.*s\" recursive=\"%s\"", "%.*s was removed, removing watchpoint%s", (int) watch->str_len, watch->str, (watch->flags & PHPDBG_WATCH_RECURSIVE) ? " recursively" : "");
+               phpdbg_notice("watchdelete", "variable=\"%.*s\" recursive=\"%s\"", "%.*s was removed, removing watchpoint%s", (int) watch->str->len, watch->str->val, (watch->flags & PHPDBG_WATCH_RECURSIVE) ? " recursively" : "");
 
                if (watch->flags & PHPDBG_WATCH_RECURSIVE) {
                        phpdbg_delete_watchpoint_recursive(watch, 0);
                } else {
-                       zend_hash_str_del(&PHPDBG_G(watchpoints), watch->str, watch->str_len);
+                       zend_hash_del(&PHPDBG_G(watchpoints), watch->str);
                }
        }
 }
@@ -558,8 +576,8 @@ static void phpdbg_watch_dtor(zval *pDest) {
        phpdbg_deactivate_watchpoint(watch);
        phpdbg_remove_watchpoint(watch);
 
-       efree(watch->str);
-       efree(watch->name_in_parent);
+       zend_string_release(watch->str);
+       zend_string_release(watch->name_in_parent);
 }
 
 static void phpdbg_watch_mem_dtor(void *llist_data) {
@@ -570,7 +588,7 @@ static void phpdbg_watch_mem_dtor(void *llist_data) {
                mprotect(dump->page, dump->size, PROT_READ);
        }
 
-       free(*(void **) llist_data);
+       free(dump);
 }
 
 void phpdbg_setup_watchpoints(void) {
@@ -610,9 +628,9 @@ static void phpdbg_print_changed_zval(phpdbg_watch_memdump *dump) {
                        continue;
                }
 
-               /* Test if the zval was separated and if necessary move the watchpoint */
+               /* Test if the zval was separated or replaced and if necessary move the watchpoint */
                if ((watch->type == WATCH_ON_HASHTABLE || watch->type == WATCH_ON_ZVAL) && watch->parent_container) {
-                       if ((curTest = zend_hash_str_find(watch->parent_container, watch->name_in_parent, watch->name_in_parent_len))) {
+                       if ((curTest = zend_symtable_find(watch->parent_container, watch->name_in_parent))) {
                                while (Z_TYPE_P((zval *) curTest) == IS_INDIRECT) {
                                        curTest = Z_INDIRECT_P((zval *) curTest);
                                }
@@ -664,13 +682,13 @@ static void phpdbg_print_changed_zval(phpdbg_watch_memdump *dump) {
                        if (do_break) {
                                PHPDBG_G(watchpoint_hit) = 1;
 
-                               phpdbg_notice("watchhit", "variable=\"%s\"", "Breaking on watchpoint %.*s", (int) watch->str_len, watch->str);
+                               phpdbg_notice("watchhit", "variable=\"%s\"", "Breaking on watchpoint %.*s", (int) watch->str->len, watch->str->val);
                                phpdbg_xml("<watchdata %r>");
                        }
 
                        switch (watch->type) {
                                case WATCH_ON_ZVAL: {
-                                       int show_value = memcmp(oldPtr, watch->addr.zv, sizeof(zval) - sizeof(uint32_t) /* no metadata info */);
+                                       int show_value = memcmp(oldPtr, watch->addr.zv, sizeof(zend_value) + sizeof(uint32_t) /* no metadata info */);
 
                                        if (removed || show_value) {
                                                if (removed && (Z_TYPE_P((zval *) oldPtr) == IS_ARRAY || Z_TYPE_P((zval *) oldPtr) == IS_OBJECT)) {
@@ -686,8 +704,8 @@ static void phpdbg_print_changed_zval(phpdbg_watch_memdump *dump) {
 
                                        /* check if zval was removed */
                                        if (removed) {
-                                               phpdbg_notice("watchdelete", "variable=\"%.*s\"", "Watchpoint %.*s was unset, removing watchpoint", (int) watch->str_len, watch->str);
-                                               zend_hash_str_del(&PHPDBG_G(watchpoints), watch->str, watch->str_len);
+                                               phpdbg_notice("watchdelete", "variable=\"%.*s\"", "Watchpoint %.*s was unset, removing watchpoint", (int) watch->str->len, watch->str->val);
+                                               zend_hash_del(&PHPDBG_G(watchpoints), watch->str);
 
                                                reenable = 0;
 
@@ -698,15 +716,15 @@ static void phpdbg_print_changed_zval(phpdbg_watch_memdump *dump) {
                                        }
 
                                        if (show_value) {
-                                               phpdbg_out("New value: ");
-                                               phpdbg_xml("<watchvalue %r type=\"new\">");
+                                               phpdbg_out("New value%s: ", Z_ISREF_P(watch->addr.zv) ? " (reference)" : "");
+                                               phpdbg_xml("<watchvalue %r%s type=\"new\">", Z_ISREF_P(watch->addr.zv) ? " reference=\"reference\"" : "");
                                                zend_print_flat_zval_r(watch->addr.zv);
                                                phpdbg_xml("</watchvalue>");
                                                phpdbg_out("\n");
                                        }
 
                                        /* add new watchpoints if necessary */
-                                       if (Z_PTR_P(watch->addr.zv) != Z_PTR_P((zval *) oldPtr)) {
+                                       if (Z_PTR_P(watch->addr.zv) != Z_PTR_P((zval *) oldPtr) || Z_TYPE_P(watch->addr.zv) != Z_TYPE_P((zval *) oldPtr)) {
                                                if (Z_REFCOUNTED_P((zval *) oldPtr)) {
                                                        phpdbg_remove_watch_collision(Z_COUNTED_P((zval *) oldPtr));
                                                }
@@ -716,6 +734,8 @@ static void phpdbg_print_changed_zval(phpdbg_watch_memdump *dump) {
                                                        }
                                                        if (watch->flags & PHPDBG_WATCH_RECURSIVE) {
                                                                phpdbg_create_recursive_watchpoint(watch);
+                                                       } else if (Z_ISREF_P(watch->addr.zv)) {
+                                                               phpdbg_create_reference_watch(watch);
                                                        }
                                                }
                                        }
@@ -725,8 +745,8 @@ static void phpdbg_print_changed_zval(phpdbg_watch_memdump *dump) {
                                case WATCH_ON_HASHTABLE:
 #if 0 && ZEND_DEBUG
                                        if (watch->addr.arr->ht->inconsistent) {
-                                               phpdbg_notice("watchdelete", "variable=\"%.*s\"", "Watchpoint %.*s was unset, removing watchpoint", (int) watch->str_len, watch->str);
-                                               zend_hash_del(&PHPDBG_G(watchpoints), watch->str, watch->str_len);
+                                               phpdbg_notice("watchdelete", "variable=\"%.*s\"", "Watchpoint %.*s was unset, removing watchpoint", (int) watch->str->len, watch->str->val);
+                                               zend_hash_del(&PHPDBG_G(watchpoints), watch->str);
 
                                                reenable = 0;
 
@@ -801,7 +821,7 @@ void phpdbg_list_watchpoints(void) {
        phpdbg_xml("<watchlist %r>");
 
        ZEND_HASH_FOREACH_PTR(&PHPDBG_G(watchpoints), watch) {
-               phpdbg_writeln("watchvariable", "variable=\"%.*s\" on=\"%s\" type=\"%s\"", "%.*s (%s, %s)", (int) watch->str_len, watch->str, watch->type == WATCH_ON_HASHTABLE ? "array" : watch->type == WATCH_ON_REFCOUNTED ? "refcount" : "variable", watch->flags == PHPDBG_WATCH_RECURSIVE ? "recursive" : "simple");
+               phpdbg_writeln("watchvariable", "variable=\"%.*s\" on=\"%s\" type=\"%s\"", "%.*s (%s, %s)", (int) watch->str->len, watch->str->val, watch->type == WATCH_ON_HASHTABLE ? "array" : watch->type == WATCH_ON_REFCOUNTED ? "refcount" : "variable", watch->flags == PHPDBG_WATCH_RECURSIVE ? "recursive" : "simple");
        } ZEND_HASH_FOREACH_END();
 
        phpdbg_xml("</watchlist>");
@@ -819,7 +839,9 @@ void phpdbg_watch_efree(void *ptr) {
                        if (watch->type == WATCH_ON_ZVAL) {
                                phpdbg_remove_watch_collision(Z_COUNTED_P(watch->addr.zv));
                        }
-                       zend_hash_str_del(&PHPDBG_G(watchpoints), watch->str, watch->str_len);
+                       if (watch->parent == NULL || watch->parent->reference != watch) {
+                               zend_hash_del(&PHPDBG_G(watchpoints), watch->str);
+                       }
                }
        }
 

diff --git a/sapi/phpdbg/phpdbg_watch.h b/sapi/phpdbg/phpdbg_watch.h
index 2736b1df64e1e0b3d753d7dcf918147eadb5b0db..d101ab116d1c8bc8c77e7687d997b21bacc686dc 100644 (file)
--- a/sapi/phpdbg/phpdbg_watch.h
+++ b/sapi/phpdbg/phpdbg_watch.h
@@ -21,7 +21,6 @@
 #ifndef PHPDBG_WATCH_H
 #define PHPDBG_WATCH_H
 
-#include "TSRM.h"
 #include "phpdbg_cmd.h"
 
 #ifdef _WIN32
@@ -41,6 +40,7 @@ extern const phpdbg_command_t phpdbg_watch_commands[];
 
 /* Watchpoint functions/typedefs */
 
+/* References are managed through their parent zval *, being a simple WATCH_ON_ZVAL and eventually WATCH_ON_REFCOUNTED */
 typedef enum {
        WATCH_ON_ZVAL,
        WATCH_ON_HASHTABLE,
@@ -66,11 +66,10 @@ struct _phpdbg_watchpoint_t {
        phpdbg_watchtype type;
        char flags;
        phpdbg_watchpoint_t *parent;
+       phpdbg_watchpoint_t *reference;
        HashTable *parent_container;
-       char *name_in_parent;
-       size_t name_in_parent_len;
-       char *str;
-       size_t str_len;
+       zend_string *name_in_parent;
+       zend_string *str;
 };
 
 typedef struct {