Skip to content

Commit 1fd53db

Browse files
committed
Fix removal of the directly accessed hook after binding in DDTrace\remove_hook
Signed-off-by: Bob Weinand <[email protected]>
1 parent afa2b3c commit 1fd53db

File tree

3 files changed

+87
-6
lines changed

3 files changed

+87
-6
lines changed
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
--TEST--
2+
remove_hook() with class argument
3+
--FILE--
4+
<?php
5+
6+
interface Elder {
7+
function foo();
8+
}
9+
10+
$id = DDTrace\install_hook("Elder::foo", function () { print "HOOKED: " . static::class . "\n"; });
11+
12+
if (time()) {
13+
abstract class Child implements Elder {}
14+
15+
class GrandChild extends Child {
16+
function foo() {
17+
print static::class . "\n";
18+
}
19+
}
20+
}
21+
22+
DDTrace\remove_hook($id, "GrandChild");
23+
24+
(new GrandChild)->foo(); // no hook
25+
26+
?>
27+
--EXPECT--
28+
GrandChild
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
--TEST--
2+
Remove hook
3+
--ENV--
4+
DD_TRACE_LOGS_ENABLED=false
5+
--FILE--
6+
<?php
7+
8+
namespace Psr\Log {
9+
interface LoggerInterface {
10+
public function log($level, $message, array $context = array());
11+
}
12+
13+
abstract class AbstractLogger implements LoggerInterface {
14+
public function log($level, $message, array $context = array()) {
15+
echo static::class . "\n";
16+
}
17+
}
18+
19+
class NullLogger extends AbstractLogger {
20+
public function log($level, $message, array $context = array()) {
21+
echo static::class . "\n";
22+
}
23+
}
24+
}
25+
26+
27+
namespace {
28+
$hook = \DDTrace\install_hook("Psr\Log\LoggerInterface::log", function () {
29+
echo "HOOKED: " . static::class . "\n";
30+
});
31+
\DDTrace\remove_hook($hook, \Psr\Log\NullLogger::class);
32+
33+
$logger = new \Psr\Log\NullLogger();
34+
for ($i = 0; $i < 3; $i++) {
35+
$logger->log("info", "Hello World!");
36+
}
37+
}
38+
39+
?>
40+
--EXPECTF--
41+
Psr\Log\NullLogger
42+
Psr\Log\NullLogger
43+
Psr\Log\NullLogger

zend_abstract_interface/hook/hook.c

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -472,7 +472,7 @@ static inline void zai_hook_resolved_install_shared_hook(zai_hook_t *hook, zend_
472472
static inline void zai_hook_resolved_install_abstract_recursive(zai_hook_t *hook, zend_ulong index, zend_class_entry *scope) {
473473
// find implementers by searching through all inheritors, recursively, stopping upon finding a non-abstract implementation
474474
zai_hook_inheritor_list *inheritors;
475-
zend_ulong ce_addr = ((zend_ulong)scope) << 3;
475+
zend_ulong ce_addr = ((zend_ulong)scope) >> 3;
476476
if ((inheritors = zend_hash_index_find_ptr(&zai_hook_tls->inheritors, ce_addr))) {
477477
for (size_t i = inheritors->size; i--;) {
478478
zend_class_entry *inheritor = inheritors->inheritor[i];
@@ -490,7 +490,7 @@ static inline void zai_hook_resolved_install_abstract_recursive(zai_hook_t *hook
490490
static inline void zai_hook_resolved_install_inherited_internal_function_recursive(zai_hook_t *hook, zend_ulong index, zend_class_entry *scope, zif_handler handler) {
491491
// find implementers by searching through all inheritors, recursively, stopping upon finding an explicit override
492492
zai_hook_inheritor_list *inheritors;
493-
zend_ulong ce_addr = ((zend_ulong)scope) << 3;
493+
zend_ulong ce_addr = ((zend_ulong)scope) >> 3;
494494
if ((inheritors = zend_hash_index_find_ptr(&zai_hook_tls->inheritors, ce_addr))) {
495495
for (size_t i = inheritors->size; i--;) {
496496
zend_class_entry *inheritor = inheritors->inheritor[i];
@@ -554,7 +554,7 @@ static zend_long zai_hook_request_install(zai_hook_t *hook) {
554554
static inline void zai_hook_register_inheritor(zend_class_entry *child, zend_class_entry *parent, bool persistent) {
555555
const size_t min_size = 7;
556556

557-
zend_ulong addr = ((zend_ulong)parent) << 3;
557+
zend_ulong addr = ((zend_ulong)parent) >> 3;
558558
zai_hook_inheritor_list *inheritors;
559559
zval *inheritors_zv;
560560
HashTable *ht = persistent ? &zai_hook_static_inheritors : &zai_hook_tls->inheritors;
@@ -842,7 +842,7 @@ static inline void zai_hook_remove_shared_hook(zend_function *func, zend_ulong h
842842
static void zai_hook_remove_abstract_recursive(zai_hooks_entry *base_hooks, zend_class_entry *scope, zend_string *function_name, zend_ulong hook_id) {
843843
// find implementers by searching through all inheritors, recursively, stopping upon finding a non-abstract implementation
844844
zai_hook_inheritor_list *inheritors;
845-
zend_ulong ce_addr = ((zend_ulong)scope) << 3;
845+
zend_ulong ce_addr = ((zend_ulong)scope) >> 3;
846846
if ((inheritors = zend_hash_index_find_ptr(&zai_hook_tls->inheritors, ce_addr))) {
847847
for (size_t i = inheritors->size; i--;) {
848848
zend_class_entry *inheritor = inheritors->inheritor[i];
@@ -860,7 +860,7 @@ static void zai_hook_remove_abstract_recursive(zai_hooks_entry *base_hooks, zend
860860
static void zai_hook_remove_internal_inherited_recursive(zend_class_entry *scope, zend_string *function_name, zend_ulong hook_id, zif_handler handler) {
861861
// find implementers by searching through all inheritors, recursively, stopping upon finding an explicit override
862862
zai_hook_inheritor_list *inheritors;
863-
zend_ulong ce_addr = ((zend_ulong)scope) << 3;
863+
zend_ulong ce_addr = ((zend_ulong)scope) >> 3;
864864
if ((inheritors = zend_hash_index_find_ptr(&zai_hook_tls->inheritors, ce_addr))) {
865865
for (size_t i = inheritors->size; i--;) {
866866
zend_class_entry *inheritor = inheritors->inheritor[i];
@@ -1276,12 +1276,12 @@ void zai_hook_exclude_class_resolved(zai_install_address function_address, zend_
12761276
if (!hooks) {
12771277
return;
12781278
}
1279-
zai_hook_add_exclusion(hooks, index, lc_classname);
12801279

12811280
zend_class_entry *ce = NULL;
12821281
zend_string *function_name = hooks->resolved->common.function_name;
12831282
zend_function *resolved = zai_hook_lookup_function(zai_str_from_zstr(lc_classname), zai_str_from_zstr(function_name), &ce);
12841283
if (!ce || !resolved) {
1284+
zai_hook_add_exclusion(hooks, index, lc_classname);
12851285
return;
12861286
}
12871287
zai_hooks_entry *excluded_hooks = zend_hash_index_find_ptr(&zai_hook_resolved, zai_hook_install_address(resolved));
@@ -1294,6 +1294,16 @@ void zai_hook_exclude_class_resolved(zai_install_address function_address, zend_
12941294
return;
12951295
}
12961296

1297+
zend_hash_index_del(&excluded_hooks->hooks, (zend_ulong) index);
1298+
if (zend_hash_num_elements(&excluded_hooks->hooks) == 0) {
1299+
#if PHP_VERSION_ID >= 80200
1300+
if (excluded_hooks->internal_duplicate_count == 0)
1301+
#endif
1302+
{
1303+
zai_hook_entries_remove_resolved((zend_ulong) function_address);
1304+
}
1305+
}
1306+
12971307
zai_hook_remove_abstract_recursive(hooks, ce, function_name, (zend_ulong) index);
12981308
}
12991309

0 commit comments

Comments
 (0)