Skip to content

PHPDBG ASAN failures under tracked alloc without zend alloc #11053

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
nielsdos opened this issue Apr 10, 2023 · 8 comments
Open

PHPDBG ASAN failures under tracked alloc without zend alloc #11053

nielsdos opened this issue Apr 10, 2023 · 8 comments

Comments

@nielsdos
Copy link
Member

Description

The following tests fail with USE_TRACKED_ALLOC=1 and USE_ZEND_ALLOC=0 under ASAN:

Bug #73927 (phpdbg fails with windows error prompt at "watch array") [sapi/phpdbg/tests/bug73927.phpt]
Test simple recursive watchpoint [sapi/phpdbg/tests/watch_001.phpt]
Test simple array watchpoint with replace [sapi/phpdbg/tests/watch_002.phpt]
Test simple watchpoint with replace [sapi/phpdbg/tests/watch_003.phpt]
Test detection of inline string manipulations on zval watch [sapi/phpdbg/tests/watch_004.phpt]
Test proper watch comparisons when having multiple levels of indirection from a zval to its value [sapi/phpdbg/tests/watch_005.phpt]
Test multiple watch elements pointing to the same watchpoint [sapi/phpdbg/tests/watch_006.phpt]

Some fail with:

Zend/zend_alloc.c:2806: tracked_get_size_zv: Assertion size_zv && "Trying to free pointer not allocated through ZendMM"' failed.`

and some fail with a use-after-free message

PHP Version

current master, stable versions not tested

Operating System

Linux 6.2.10

@Girgias
Copy link
Member

Girgias commented Apr 11, 2023

It also fails on 8.2, we've been aware about this in a while and have been monkey patching run-test.php, also this only happens when compiling with GCC, with Clang it works just fine (except if this is somehow a different issue)

@iluuu1994
Copy link
Member

iluuu1994 commented Apr 11, 2023

I don't think that's the same issue. I've fixed that issue here and it was PHP-8.2+ only. This here sounds like some efree call goes to zend_alloc even though USE_ZEND_ALLOC=0 is specified.

@Girgias
Copy link
Member

Girgias commented Apr 11, 2023

Oh I forgot, indeed!

@nielsdos
Copy link
Member Author

So the problem might actually be me after all.
Protip: don't make issue reports when you're half asleep.

At the top of the test file I see: die("skip intentionally causes segfaults");
oops 😬
Anyhow, it doesn't segfault though, it fails ASAN. And it does seem kinda weird that you can intentionally segfault your debugger tbh; I don't see how that is legal behaviour :/

In any case, feel free to close if these shouldn't be fixed

@iluuu1994
Copy link
Member

Well, it segfaults without ASAN. ASAN catches the memory errors before it gets a chance to crash. The --asan flag in run-tests.php would set the SKIP_ASAN env variable, maybe that flag is missing in your command?

@nielsdos
Copy link
Member Author

Yes it was missing.

@nielsdos nielsdos closed this as not planned Won't fix, can't repro, duplicate, stale Apr 11, 2023
@iluuu1994
Copy link
Member

Oh, I see what you mean now. There seem to be quite a few leaks that are not reported with ZMM for some reason. I managed to find a few but there are still more.

diff --git a/sapi/phpdbg/phpdbg_info.c b/sapi/phpdbg/phpdbg_info.c
index d6457ef805..1f19b76bde 100644
--- a/sapi/phpdbg/phpdbg_info.c
+++ b/sapi/phpdbg/phpdbg_info.c
@@ -157,6 +157,8 @@ PHPDBG_INFO(constants) /* {{{ */
 		} ZEND_HASH_FOREACH_END();
 	}
 
+	zend_hash_destroy(&consts);
+
 	return SUCCESS;
 } /* }}} */
 
diff --git a/sapi/phpdbg/phpdbg_prompt.c b/sapi/phpdbg/phpdbg_prompt.c
index f8041c660f..db50613dbc 100644
--- a/sapi/phpdbg/phpdbg_prompt.c
+++ b/sapi/phpdbg/phpdbg_prompt.c
@@ -880,6 +880,7 @@ PHPDBG_COMMAND(run) /* {{{ */
 			PHPDBG_G(flags) ^= PHPDBG_IS_INTERACTIVE;
 			PHPDBG_G(flags) |= PHPDBG_IS_RUNNING;
 			zend_execute(PHPDBG_G(ops), &PHPDBG_G(retval));
+			zval_ptr_dtor(&PHPDBG_G(retval));
 			PHPDBG_G(flags) ^= PHPDBG_IS_INTERACTIVE;
 		} zend_catch {
 			PHPDBG_G(in_execution) = 0;
@@ -1547,6 +1548,9 @@ int phpdbg_interactive(bool allow_async_unsafe, char *input) /* {{{ */
 				ret = phpdbg_stack_execute(&stack, allow_async_unsafe);
 			} zend_catch {
 				phpdbg_stack_free(&stack);
+				if (input) {
+					phpdbg_destroy_input(&input);
+				}
 				zend_bailout();
 			} zend_end_try();

@iluuu1994 iluuu1994 reopened this Apr 11, 2023
@iluuu1994
Copy link
Member

Oh, and tracked alloc doesn't seem to work at all, there's some issue with restoring the custom handlers, as phpdbg overrides them itself.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants