Fix [valid] valgrind warnings, add first watchpoints test

diff --git a/run-tests.php b/run-tests.php
index ba44ce54643c3e1a29c0441d9207095c50f8d25f..8222c7870973ee62253304b39888ed4b1b067715 100755 (executable)
--- a/run-tests.php
+++ b/run-tests.php
@@ -1876,11 +1876,12 @@ TEST $file
                $env['USE_ZEND_ALLOC'] = '0';
                $env['ZEND_DONT_UNLOAD_MODULES'] = 1;
 
+               /* --vex-iropt-register-updates=allregs-at-mem-access is necessary for phpdbg watchpoint tests */
                if (version_compare($valgrind_version, '3.3.0', '>=')) {
                        /* valgrind 3.3.0+ doesn't have --log-file-exactly option */
-                       $cmd = "valgrind -q --tool=memcheck --trace-children=yes --log-file=$memcheck_filename $cmd";
+                       $cmd = "valgrind -q --tool=memcheck --trace-children=yes --vex-iropt-register-updates=allregs-at-mem-access --log-file=$memcheck_filename $cmd";
                } else {
-                       $cmd = "valgrind -q --tool=memcheck --trace-children=yes --log-file-exactly=$memcheck_filename $cmd";
+                       $cmd = "valgrind -q --tool=memcheck --trace-children=yes --vex-iropt-register-updates=allregs-at-mem-access --log-file-exactly=$memcheck_filename $cmd";
                }
 
        } else {

diff --git a/sapi/phpdbg/phpdbg_watch.c b/sapi/phpdbg/phpdbg_watch.c
index c3fcb769a47388db0d901724a40c7778ea4473e0..7051b4202780eff5d10c0b0656ca871d43af3136 100644 (file)
--- a/sapi/phpdbg/phpdbg_watch.c
+++ b/sapi/phpdbg/phpdbg_watch.c
@@ -170,7 +170,7 @@ void phpdbg_watch_HashTable_dtor(zval *ptr);
 
 static void phpdbg_free_watch(phpdbg_watchpoint_t *watch) {
        zend_string_release(watch->str);
-       zend_string_release(watch->name_in_parent);     
+       zend_string_release(watch->name_in_parent);
 }
 
 static int phpdbg_delete_watchpoint(phpdbg_watchpoint_t *tmp_watch);
@@ -224,7 +224,7 @@ static void phpdbg_add_watch_collision(phpdbg_watchpoint_t *watch) {
                        if (flags & PHPDBG_WATCH_IMPLICIT) {
                                zend_hash_del(&cur->implicit_watches, watch->str);
                        }
-                       
+
                        old->flags = watch->flags;
                        phpdbg_free_watch(watch);
                        efree(watch);
@@ -332,8 +332,10 @@ static phpdbg_watchpoint_t *phpdbg_create_watchpoint(phpdbg_watchpoint_t *watch)
        if (ref) { \
                phpdbg_add_watch_collision(ref); \
        } \
-       phpdbg_free_watch(watch); \
-       efree(watch); \
+       if (watch != old_watch) { \
+               phpdbg_free_watch(watch); \
+               efree(watch); \
+       } \
        return (x); \
 }
                        if (watch->flags & PHPDBG_WATCH_RECURSIVE) {
@@ -751,12 +753,6 @@ void phpdbg_watch_HashTable_dtor(zval *zv) {
                        phpdbg_notice("watchdelete", "variable=\"%.*s\" recursive=\"%s\"", "%.*s was removed, removing watchpoint%s", (int) ZSTR_LEN(watch->str), ZSTR_VAL(watch->str), (watch->flags & PHPDBG_WATCH_RECURSIVE) ? " recursively" : "");
                }
 
-               if (watch->flags & PHPDBG_WATCH_RECURSIVE) {
-                       phpdbg_delete_watchpoint_recursive(watch, 0);
-               } else {
-                       zend_hash_del(&PHPDBG_G(watchpoints), watch->str);
-               }
-
                if ((result = phpdbg_btree_find(&PHPDBG_G(watch_HashTables), (zend_ulong) watch->parent_container))) {
                        phpdbg_watch_ht_info *hti = result->ptr;
                        hti->dtor(orig_zv);
@@ -764,11 +760,18 @@ void phpdbg_watch_HashTable_dtor(zval *zv) {
                        if (zend_hash_num_elements(&hti->watches) == 0) {
                                watch->parent_container->pDestructor = hti->dtor;
                                zend_hash_destroy(&hti->watches);
+                               phpdbg_btree_delete(&PHPDBG_G(watch_HashTables), (zend_ulong) watch->parent_container);
                                efree(hti);
                        }
                } else {
                        zval_ptr_dtor_wrapper(orig_zv);
                }
+
+               if (watch->flags & PHPDBG_WATCH_RECURSIVE) {
+                       phpdbg_delete_watchpoint_recursive(watch, 0);
+               } else {
+                       zend_hash_del(&PHPDBG_G(watchpoints), watch->str);
+               }
        }
 }
 
@@ -819,6 +822,7 @@ int phpdbg_watchpoint_segfault_handler(siginfo_t *info, void *context) {
        dump = malloc(MEMDUMP_SIZE(size));
        dump->page = page;
        dump->size = size;
+       dump->reenable_writing = 0;
 
        memcpy(&dump->data, page, size);
 
@@ -857,6 +861,7 @@ static void phpdbg_watch_dtor(zval *pDest) {
        phpdbg_remove_watchpoint(watch);
 
        phpdbg_free_watch(watch);
+       efree(watch);
 }
 
 static void phpdbg_watch_mem_dtor(void *llist_data) {

diff --git a/sapi/phpdbg/tests/watch_001.phpt b/sapi/phpdbg/tests/watch_001.phpt
new file mode 100644 (file)
index 0000000..1183bb0
--- /dev/null
+++ b/sapi/phpdbg/tests/watch_001.phpt
@@ -0,0 +1,46 @@
+--TEST--
+Test simple recursive watchpoint
+--PHPDBG--
+b 3
+r
+w r $b
+c
+
+
+
+q
+--EXPECTF--
+[Successful compilation of %s]
+prompt> [Breakpoint #0 added at %s:3]
+prompt> [Breakpoint #0 at %s:3, hits: 1]
+>00003: $a = 1;
+ 00004: $b = [$a];
+ 00005: 
+prompt> [Set recursive watchpoint on $b]
+prompt> [Breaking on watchpoint $b]
+Old value: 
+New value: Array ([0] => 1)
+>00006: unset($b);
+ 00007: $b = 2;
+ 00008: 
+prompt> [Breaking on watchpoint $b]
+Old value inaccessible or destroyed
+New value: 
+>00007: $b = 2;
+ 00008: 
+prompt> [Breaking on watchpoint $b]
+Old value: 
+New value: 2
+>00007: $b = 2;
+ 00008: 
+prompt> [$b was removed, removing watchpoint recursively]
+[Script ended normally]
+prompt> 
+--FILE--
+<?php
+
+$a = 1;
+$b = [$a];
+
+unset($b);
+$b = 2;