Fix [valid] valgrind warnings, add first watchpoints test

3 files changed, 64 insertions, 12 deletions

diff --git a/run-tests.php b/run-tests.php
index ba44ce5464..8222c78709 100755
--- 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 c3fcb769a4..7051b42027 100644
--- 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
index 0000000000..1183bb0e57
--- /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;