Skip to content

Commit

Permalink
Add observer temporary to dl'ed functions
Browse files Browse the repository at this point in the history
When observer is enabled, we normally add an extra temporary to all
functions, to store the previously observed frame. However, this is done in
zend_observer_post_startup() so it doesn't happen to dl'ed() functions.

One possible fix would be to move that from zend_observer_post_startup()
to zend_register_functions(), but this would be too early: Observer may
not be enabled when zend_register_functions() is called, and may still be
enabled later.

However, when zend_register_functions() is called at run-time (during dl()),
we know definitively whether observer is enabled.

Here I update zend_register_functions() to add a temporary to dl'ed()
functions when observer is enabled.

Fixes: GH-17211
Closes: GH-17220
  • Loading branch information
arnaud-lb committed Dec 20, 2024
1 parent fa64a1d commit 6f57993
Show file tree
Hide file tree
Showing 6 changed files with 95 additions and 2 deletions.
3 changes: 3 additions & 0 deletions NEWS
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@ PHP NEWS
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
?? ??? ????, PHP 8.2.28

- Core:
. Fixed bug GH-17211 (observer segfault on function loaded with dl()).
(Arnaud)

19 Dec 2024, PHP 8.2.27

Expand Down
9 changes: 8 additions & 1 deletion Zend/zend_API.c
Original file line number Diff line number Diff line change
Expand Up @@ -2718,7 +2718,14 @@ ZEND_API zend_result zend_register_functions(zend_class_entry *scope, const zend
}
internal_function->type = ZEND_INTERNAL_FUNCTION;
internal_function->module = EG(current_module);
internal_function->T = 0;
if (EG(active) && ZEND_OBSERVER_ENABLED) {
/* Add an observer temporary to store previous observed frames. This is
* normally handled by zend_observer_post_startup(), except for
* functions registered at runtime (EG(active)). */
internal_function->T = 1;
} else {
internal_function->T = 0;
}
memset(internal_function->reserved, 0, ZEND_MAX_RESERVED_RESOURCES * sizeof(void*));

while (ptr->fname) {
Expand Down
18 changes: 18 additions & 0 deletions ext/dl_test/dl_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -76,9 +76,27 @@ PHP_INI_BEGIN()
PHP_INI_END()
/* }}} */

PHP_METHOD(DlTest, test)
{
char *var = "World";
size_t var_len = sizeof("World") - 1;
zend_string *retval;

ZEND_PARSE_PARAMETERS_START(0, 1)
Z_PARAM_OPTIONAL
Z_PARAM_STRING(var, var_len)
ZEND_PARSE_PARAMETERS_END();

retval = strpprintf(0, "Hello %s", var);

RETURN_STR(retval);
}

/* {{{ PHP_MINIT_FUNCTION */
PHP_MINIT_FUNCTION(dl_test)
{
register_class_DlTest();

/* Test backwards compatibility */
if (getenv("PHP_DL_TEST_USE_OLD_REGISTER_INI_ENTRIES")) {
zend_register_ini_entries(ini_entries, module_number);
Expand Down
4 changes: 4 additions & 0 deletions ext/dl_test/dl_test.stub.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,7 @@
function dl_test_test1(): void {}

function dl_test_test2(string $str = ""): string {}

class DlTest {
public function test(string $str = ""): string {}
}
21 changes: 20 additions & 1 deletion ext/dl_test/dl_test_arginfo.h

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

42 changes: 42 additions & 0 deletions ext/standard/tests/general_functions/gh17211.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
--TEST--
dl() / observer segfault
--EXTENSIONS--
zend_test
--SKIPIF--
<?php include dirname(__DIR__, 3) . "/dl_test/tests/skip.inc"; ?>
--INI--
zend_test.observer.enabled=1
zend_test.observer.observe_functions=1
zend_test.observer.show_output=1
--FILE--
<?php

if (PHP_OS_FAMILY === 'Windows') {
$loaded = dl('php_dl_test.dll');
} else {
$loaded = dl('dl_test.so');
}

var_dump(dl_test_test2("World!"));

$test = new DlTest();
var_dump($test->test("World!"));
?>
--EXPECTF--
<!-- init '%sgh17211.php' -->
<!-- init dl() -->
<dl>
</dl>
<!-- init dl_test_test2() -->
<dl_test_test2>
</dl_test_test2>
<!-- init var_dump() -->
<var_dump>
string(12) "Hello World!"
</var_dump>
<!-- init DlTest::test() -->
<DlTest::test>
</DlTest::test>
<var_dump>
string(12) "Hello World!"
</var_dump>

0 comments on commit 6f57993

Please sign in to comment.