Skip to content

Support neighbouring extension in the ZendMM custom handlers hook in zend_test #12792

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

Support neighbouring extension in the ZendMM custom handlers hook in zend_test #12792

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions ext/zend_test/php_test.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,11 @@ ZEND_BEGIN_MODULE_GLOBALS(zend_test)
bool print_stderr_mshutdown;
zend_long limit_copy_file_range;
int observe_opline_in_zendmm;
zend_mm_heap* zend_orig_heap;
zend_mm_heap* zend_test_heap;
// previous handlers that might have been installed
// `USE_ZEND_ALLOC=0` installs custom handlers
void* (*zendmm_orig_malloc)(size_t);
void (*zendmm_orig_free)(void*);
void* (*zendmm_orig_realloc)(void *, size_t);
zend_test_fiber *active_fiber;
zend_long quantity_value;
zend_string *str_test;
Expand Down
115 changes: 90 additions & 25 deletions ext/zend_test/test.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -517,28 +517,110 @@ static bool has_opline(zend_execute_data *execute_data)
;
}

// When a custom memory manager is installed, ZendMM alters its behaviour. For
// example `zend_mm_gc()` will not do anything anymore, or `zend_mm_shutdown()`
// won't cleanup chunks and leak memory. This might not be a problem in tests,
// but I fear folks might see and use this implementation as a boiler plate when
// trying to use this hook
int zend_test_prepare_zendmm_for_call(zend_mm_heap *heap)
{
int flag = 0;
memcpy(&flag, heap, sizeof(int));
int new_flag = 0;
memcpy(heap, &new_flag, sizeof(int));
Comment on lines +527 to +530
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, no.
It works, but we should not do it this way, especially not advertise that in php-src itself.

You probably should include a

#ifndef ZEND_ALLOC_HIDE_STRUCTS
struct _zend_mm_heap {
#if ZEND_MM_CUSTOM
    int use_custom_mm_heap;
#endif
}
#endif

to zend_alloc.h, with #define ZEND_ALLOC_HIDE_STRUCTS 1 in zend_alloc.c

and have it public API.

If we rely on such implementation details, it can well just be public.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I get the point and the zend_test_prepare_zendmm_for_call() is super ugly 😉
OTOH I would rather not make the use_custom_mm_heap flag public just for the sake of this test.

I am about to prepare a PR that would add a gc and shutdown custom handler (#13432) that might help solve this.

return flag;
}

void zend_test_restore_zendmm_after_call(zend_mm_heap *heap, int flag)
{
memcpy(heap, &flag, sizeof(int));
}

void * zend_test_custom_malloc(size_t len)
{
if (has_opline(EG(current_execute_data))) {
assert(EG(current_execute_data)->opline->lineno != (uint32_t)-1);
}
return _zend_mm_alloc(ZT_G(zend_orig_heap), len ZEND_FILE_LINE_EMPTY_CC ZEND_FILE_LINE_EMPTY_CC);
if (ZT_G(zendmm_orig_malloc)) {
return ZT_G(zendmm_orig_malloc)(len);
}
int flag = zend_test_prepare_zendmm_for_call(zend_mm_get_heap());
void * ptr = _zend_mm_alloc(zend_mm_get_heap(), len ZEND_FILE_LINE_EMPTY_CC ZEND_FILE_LINE_EMPTY_CC);
zend_test_restore_zendmm_after_call(zend_mm_get_heap(), flag);
return ptr;
}

void zend_test_custom_free(void *ptr)
{
if (has_opline(EG(current_execute_data))) {
assert(EG(current_execute_data)->opline->lineno != (uint32_t)-1);
}
_zend_mm_free(ZT_G(zend_orig_heap), ptr ZEND_FILE_LINE_EMPTY_CC ZEND_FILE_LINE_EMPTY_CC);
if (ZT_G(zendmm_orig_free)) {
ZT_G(zendmm_orig_free)(ptr);
return;
}
int flag = zend_test_prepare_zendmm_for_call(zend_mm_get_heap());
_zend_mm_free(zend_mm_get_heap(), ptr ZEND_FILE_LINE_EMPTY_CC ZEND_FILE_LINE_EMPTY_CC);
zend_test_restore_zendmm_after_call(zend_mm_get_heap(), flag);
}

void * zend_test_custom_realloc(void * ptr, size_t len)
{
if (has_opline(EG(current_execute_data))) {
assert(EG(current_execute_data)->opline->lineno != (uint32_t)-1);
}
return _zend_mm_realloc(ZT_G(zend_orig_heap), ptr, len ZEND_FILE_LINE_EMPTY_CC ZEND_FILE_LINE_EMPTY_CC);
if (ZT_G(zendmm_orig_realloc)) {
return ZT_G(zendmm_orig_realloc)(ptr, len);
}
int flag = zend_test_prepare_zendmm_for_call(zend_mm_get_heap());
void * new_ptr = _zend_mm_realloc(zend_mm_get_heap(), ptr, len ZEND_FILE_LINE_EMPTY_CC ZEND_FILE_LINE_EMPTY_CC);
zend_test_restore_zendmm_after_call(zend_mm_get_heap(), flag);
return new_ptr;
}

void zend_test_install_custom_mm_handler(void)
{
// fetch maybe installed previous handlers
if (!is_zend_mm()) {
zend_mm_get_custom_handlers(
zend_mm_get_heap(),
&ZT_G(zendmm_orig_malloc),
&ZT_G(zendmm_orig_free),
&ZT_G(zendmm_orig_realloc)
);
}
zend_mm_set_custom_handlers(
zend_mm_get_heap(),
zend_test_custom_malloc,
zend_test_custom_free,
zend_test_custom_realloc
);
}

void zend_test_uninstall_custom_mm_handler(void)
{
void* (*custom_malloc)(size_t);
void (*custom_free)(void*);
void* (*custom_realloc)(void *, size_t);
zend_mm_get_custom_handlers(
zend_mm_get_heap(),
&custom_malloc,
&custom_free,
&custom_realloc
);
if (custom_malloc == zend_test_custom_malloc ||
custom_free == zend_test_custom_free ||
custom_realloc == zend_test_custom_realloc) {
zend_mm_set_custom_handlers(
zend_mm_get_heap(),
ZT_G(zendmm_orig_malloc),
ZT_G(zendmm_orig_free),
ZT_G(zendmm_orig_realloc)
);
ZT_G(zendmm_orig_malloc) = NULL;
ZT_G(zendmm_orig_free) = NULL;
ZT_G(zendmm_orig_realloc) = NULL;
}
}

static PHP_INI_MH(OnUpdateZendTestObserveOplineInZendMM)
Expand All @@ -550,22 +632,9 @@ static PHP_INI_MH(OnUpdateZendTestObserveOplineInZendMM)
int int_value = zend_ini_parse_bool(new_value);

if (int_value == 1) {
// `zend_mm_heap` is a private struct, so we have not way to find the
// actual size, but 4096 bytes should be enough
ZT_G(zend_test_heap) = malloc(4096);
memset(ZT_G(zend_test_heap), 0, 4096);
zend_mm_set_custom_handlers(
ZT_G(zend_test_heap),
zend_test_custom_malloc,
zend_test_custom_free,
zend_test_custom_realloc
);
ZT_G(zend_orig_heap) = zend_mm_get_heap();
zend_mm_set_heap(ZT_G(zend_test_heap));
} else if (ZT_G(zend_test_heap)) {
free(ZT_G(zend_test_heap));
ZT_G(zend_test_heap) = NULL;
zend_mm_set_heap(ZT_G(zend_orig_heap));
zend_test_install_custom_mm_handler();
} else {
zend_test_uninstall_custom_mm_handler();
}
return OnUpdateBool(entry, new_value, mh_arg1, mh_arg2, mh_arg3, stage);
}
Expand Down Expand Up @@ -948,6 +1017,8 @@ PHP_MSHUTDOWN_FUNCTION(zend_test)

zend_test_observer_shutdown(SHUTDOWN_FUNC_ARGS_PASSTHRU);

zend_test_uninstall_custom_mm_handler();

if (ZT_G(print_stderr_mshutdown)) {
fprintf(stderr, "[zend-test] MSHUTDOWN\n");
}
Expand All @@ -970,12 +1041,6 @@ PHP_RSHUTDOWN_FUNCTION(zend_test)
} ZEND_HASH_FOREACH_END();
zend_hash_destroy(&ZT_G(global_weakmap));

if (ZT_G(zend_test_heap)) {
free(ZT_G(zend_test_heap));
ZT_G(zend_test_heap) = NULL;
zend_mm_set_heap(ZT_G(zend_orig_heap));
}

return SUCCESS;
}

Expand Down
2 changes: 0 additions & 2 deletions ext/zend_test/tests/opline_dangling.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@ possible segfault in `ZEND_BIND_STATIC`
https://github.com/php/php-src/pull/12758
--EXTENSIONS--
zend_test
--ENV--
USE_ZEND_ALLOC=1
--INI--
zend_test.observe_opline_in_zendmm=1
--FILE--
Expand Down
2 changes: 0 additions & 2 deletions ext/zend_test/tests/opline_dangling_02.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@
possible segfault in `ZEND_FUNC_GET_ARGS`
--EXTENSIONS--
zend_test
--ENV--
USE_ZEND_ALLOC=1
--INI--
zend_test.observe_opline_in_zendmm=1
--FILE--
Expand Down