From ce894b3dc4c1dbbfa6c967a9513c67676ae003bd Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Tue, 26 May 2020 18:26:32 +0200 Subject: [PATCH 1/5] Fix #73927: phpdbg fails with windows error prompt at "watch array" We expect zvals, so we should request zvals. --- sapi/phpdbg/phpdbg_utils.c | 2 +- sapi/phpdbg/tests/bug73927.phpt | 52 +++++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 1 deletion(-) create mode 100644 sapi/phpdbg/tests/bug73927.phpt diff --git a/sapi/phpdbg/phpdbg_utils.c b/sapi/phpdbg/phpdbg_utils.c index dca70deb8f3e1..90582c6edd96d 100644 --- a/sapi/phpdbg/phpdbg_utils.c +++ b/sapi/phpdbg/phpdbg_utils.c @@ -473,7 +473,7 @@ PHPDBG_API int phpdbg_parse_variable_with_arg(char *input, size_t len, HashTable if (new_index && index_len == 0) { zend_ulong numkey; zend_string *strkey; - ZEND_HASH_FOREACH_KEY_PTR(parent, numkey, strkey, zv) { + ZEND_HASH_FOREACH_KEY_VAL(parent, numkey, strkey, zv) { while (Z_TYPE_P(zv) == IS_INDIRECT) { zv = Z_INDIRECT_P(zv); } diff --git a/sapi/phpdbg/tests/bug73927.phpt b/sapi/phpdbg/tests/bug73927.phpt new file mode 100644 index 0000000000000..0d012ad6f68f2 --- /dev/null +++ b/sapi/phpdbg/tests/bug73927.phpt @@ -0,0 +1,52 @@ +--TEST-- +Bug #73927 (phpdbg fails with windows error prompt at "watch array") +--PHPDBG-- +b 19 +r +c +w $value +w $lower[] +q +--EXPECT-- +[Successful compilation of C:\php-sdk\phpdev\vc15\x64\php-src-pr5599\sapi\phpdbg\tests\bug73927.php] +prompt> [Breakpoint #0 added at C:\php-sdk\phpdev\vc15\x64\php-src-pr5599\sapi\phpdbg\tests\bug73927.php:19] +prompt> [Breakpoint #0 at C:\php-sdk\phpdev\vc15\x64\php-src-pr5599\sapi\phpdbg\tests\bug73927.php:19, hits: 1] +>00019: if ($value < 100) { + 00020: $lower[] = $value; + 00021: } else { +prompt> [Breakpoint #0 at C:\php-sdk\phpdev\vc15\x64\php-src-pr5599\sapi\phpdbg\tests\bug73927.php:19, hits: 2] +>00019: if ($value < 100) { + 00020: $lower[] = $value; + 00021: } else { +prompt> [Added watchpoint #0 for $value] +prompt> [Added watchpoint #1 for $lower[0]] +prompt> [$lower[0] has been removed, removing watchpoint] +[$lower[0] has been removed, removing watchpoint] +[$value has been removed, removing watchpoint] +--FILE-- + $value) { + if ($value < 100) { + $lower[] = $value; + } else { + doCoolStuff($value); + } +} + +?> From 407abf90f561b87dcb763cefb338ad48a2a00ecc Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Wed, 27 May 2020 09:08:42 +0200 Subject: [PATCH 2/5] Make test expectations portable --- sapi/phpdbg/tests/bug73927.phpt | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/sapi/phpdbg/tests/bug73927.phpt b/sapi/phpdbg/tests/bug73927.phpt index 0d012ad6f68f2..52d0861f12faf 100644 --- a/sapi/phpdbg/tests/bug73927.phpt +++ b/sapi/phpdbg/tests/bug73927.phpt @@ -7,14 +7,14 @@ c w $value w $lower[] q ---EXPECT-- -[Successful compilation of C:\php-sdk\phpdev\vc15\x64\php-src-pr5599\sapi\phpdbg\tests\bug73927.php] -prompt> [Breakpoint #0 added at C:\php-sdk\phpdev\vc15\x64\php-src-pr5599\sapi\phpdbg\tests\bug73927.php:19] -prompt> [Breakpoint #0 at C:\php-sdk\phpdev\vc15\x64\php-src-pr5599\sapi\phpdbg\tests\bug73927.php:19, hits: 1] +--EXPECTF-- +[Successful compilation of %s] +prompt> [Breakpoint #0 added at %s:%d] +prompt> [Breakpoint #0 at %s:%d, hits: 1] >00019: if ($value < 100) { 00020: $lower[] = $value; 00021: } else { -prompt> [Breakpoint #0 at C:\php-sdk\phpdev\vc15\x64\php-src-pr5599\sapi\phpdbg\tests\bug73927.php:19, hits: 2] +prompt> [Breakpoint #0 at %s:%d, hits: 2] >00019: if ($value < 100) { 00020: $lower[] = $value; 00021: } else { From 162db744fa53a1840b202a6e4c72c2ff6e2bab72 Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Wed, 27 May 2020 10:20:02 +0200 Subject: [PATCH 3/5] Simplify loop We can use `ZEND_HASH_FOREACH_KEY_VAL_IND()` in the first place. --- sapi/phpdbg/phpdbg_utils.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/sapi/phpdbg/phpdbg_utils.c b/sapi/phpdbg/phpdbg_utils.c index 90582c6edd96d..d32f2fb7f86bb 100644 --- a/sapi/phpdbg/phpdbg_utils.c +++ b/sapi/phpdbg/phpdbg_utils.c @@ -473,11 +473,7 @@ PHPDBG_API int phpdbg_parse_variable_with_arg(char *input, size_t len, HashTable if (new_index && index_len == 0) { zend_ulong numkey; zend_string *strkey; - ZEND_HASH_FOREACH_KEY_VAL(parent, numkey, strkey, zv) { - while (Z_TYPE_P(zv) == IS_INDIRECT) { - zv = Z_INDIRECT_P(zv); - } - + ZEND_HASH_FOREACH_KEY_VAL_IND(parent, numkey, strkey, zv) { if (i == len || (i == len - 1 && input[len - 1] == ']')) { char *key, *propkey; size_t namelen, keylen; From 274db0bd1752d40d10f5849dd206d4d65b6480bf Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Wed, 27 May 2020 17:48:44 +0200 Subject: [PATCH 4/5] Suppress spurious watchpoint removal notice If a watchpoint has already been removed from `watch_elements`, we must not report that it has been removed again. --- sapi/phpdbg/phpdbg_watch.c | 6 ++++-- sapi/phpdbg/tests/bug73927.phpt | 1 - 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/sapi/phpdbg/phpdbg_watch.c b/sapi/phpdbg/phpdbg_watch.c index 317aa9666a3c1..dcb6a5502315c 100644 --- a/sapi/phpdbg/phpdbg_watch.c +++ b/sapi/phpdbg/phpdbg_watch.c @@ -713,8 +713,10 @@ void phpdbg_automatic_dequeue_free(phpdbg_watch_element *element) { child = child->child; } PHPDBG_G(watchpoint_hit) = 1; - phpdbg_notice("watchdelete", "variable=\"%.*s\" recursive=\"%s\"", "%.*s has been removed, removing watchpoint%s", (int) ZSTR_LEN(child->str), ZSTR_VAL(child->str), (child->flags & PHPDBG_WATCH_RECURSIVE_ROOT) ? " recursively" : ""); - zend_hash_index_del(&PHPDBG_G(watch_elements), child->id); + if (zend_hash_index_find(&PHPDBG_G(watch_elements), child->id)) { + phpdbg_notice("watchdelete", "variable=\"%.*s\" recursive=\"%s\"", "%.*s has been removed, removing watchpoint%s", (int) ZSTR_LEN(child->str), ZSTR_VAL(child->str), (child->flags & PHPDBG_WATCH_RECURSIVE_ROOT) ? " recursively" : ""); + zend_hash_index_del(&PHPDBG_G(watch_elements), child->id); + } phpdbg_free_watch_element_tree(element); } diff --git a/sapi/phpdbg/tests/bug73927.phpt b/sapi/phpdbg/tests/bug73927.phpt index 52d0861f12faf..81a1d2fa47fe7 100644 --- a/sapi/phpdbg/tests/bug73927.phpt +++ b/sapi/phpdbg/tests/bug73927.phpt @@ -21,7 +21,6 @@ prompt> [Breakpoint #0 at %s:%d, hits: 2] prompt> [Added watchpoint #0 for $value] prompt> [Added watchpoint #1 for $lower[0]] prompt> [$lower[0] has been removed, removing watchpoint] -[$lower[0] has been removed, removing watchpoint] [$value has been removed, removing watchpoint] --FILE-- Date: Wed, 27 May 2020 18:26:32 +0200 Subject: [PATCH 5/5] Spare unnecessary HT lookup --- sapi/phpdbg/phpdbg_watch.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/sapi/phpdbg/phpdbg_watch.c b/sapi/phpdbg/phpdbg_watch.c index dcb6a5502315c..1dab7551719cb 100644 --- a/sapi/phpdbg/phpdbg_watch.c +++ b/sapi/phpdbg/phpdbg_watch.c @@ -713,9 +713,8 @@ void phpdbg_automatic_dequeue_free(phpdbg_watch_element *element) { child = child->child; } PHPDBG_G(watchpoint_hit) = 1; - if (zend_hash_index_find(&PHPDBG_G(watch_elements), child->id)) { + if (zend_hash_index_del(&PHPDBG_G(watch_elements), child->id) == SUCCESS) { phpdbg_notice("watchdelete", "variable=\"%.*s\" recursive=\"%s\"", "%.*s has been removed, removing watchpoint%s", (int) ZSTR_LEN(child->str), ZSTR_VAL(child->str), (child->flags & PHPDBG_WATCH_RECURSIVE_ROOT) ? " recursively" : ""); - zend_hash_index_del(&PHPDBG_G(watch_elements), child->id); } phpdbg_free_watch_element_tree(element); }