Skip to content

Commit 7c98e65

Browse files
prateekbhujelprateekbhujel
authored
Fix GH-22118: Compare equivalent fake closures in FCCs (#22145)
The reason fake closures didn't compare as equal was that two different objects could represent the same fake closure. Therefore, we need to do a more detailed comparison when one or both operands are closures rather than just comparing pointers. A good follow-up would be to draft an RFC to fix the semantics of `==` with two different instances of the same fake closure.
1 parent a151551 commit 7c98e65

4 files changed

Lines changed: 188 additions & 2 deletions

File tree

‎Zend/zend_API.c‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4096,6 +4096,52 @@ ZEND_API zend_string *zend_get_callable_name_ex(const zval *callable, const zend
40964096
}
40974097
/* }}} */
40984098

4099+
static bool zend_fcc_function_handler_equals(const zend_function *func1, const zend_function *func2) /* {{{ */
4100+
{
4101+
if (func1 == func2) {
4102+
return true;
4103+
}
4104+
4105+
const bool fake_closure1 = (func1->common.fn_flags & ZEND_ACC_FAKE_CLOSURE) != 0;
4106+
const bool fake_closure2 = (func2->common.fn_flags & ZEND_ACC_FAKE_CLOSURE) != 0;
4107+
4108+
if (!fake_closure1 && !fake_closure2) {
4109+
return false;
4110+
}
4111+
if (((func1->common.fn_flags & ZEND_ACC_CLOSURE) && !fake_closure1) ||
4112+
((func2->common.fn_flags & ZEND_ACC_CLOSURE) && !fake_closure2)) {
4113+
return false;
4114+
}
4115+
if (func1->type != func2->type ||
4116+
func1->common.scope != func2->common.scope ||
4117+
!zend_string_equals(func1->common.function_name, func2->common.function_name)) {
4118+
return false;
4119+
}
4120+
4121+
if (func1->type == ZEND_USER_FUNCTION) {
4122+
return func1->op_array.opcodes == func2->op_array.opcodes;
4123+
}
4124+
4125+
return func1->internal_function.handler == func2->internal_function.handler;
4126+
}
4127+
/* }}} */
4128+
4129+
ZEND_API bool zend_fcc_closure_equals_ex(const zend_fcall_info_cache* a, const zend_fcall_info_cache* b) /* {{{ */
4130+
{
4131+
const zend_function *func1 = a->function_handler;
4132+
const zend_function *func2 = b->function_handler;
4133+
4134+
if (a->closure && a->closure->ce == zend_ce_closure) {
4135+
func1 = zend_get_closure_method_def(a->closure);
4136+
}
4137+
if (b->closure && b->closure->ce == zend_ce_closure) {
4138+
func2 = zend_get_closure_method_def(b->closure);
4139+
}
4140+
4141+
return zend_fcc_function_handler_equals(func1, func2);
4142+
}
4143+
/* }}} */
4144+
40994145
ZEND_API zend_string *zend_get_callable_name(const zval *callable) /* {{{ */
41004146
{
41014147
return zend_get_callable_name_ex(callable, NULL);

‎Zend/zend_API.h‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -758,20 +758,27 @@ ZEND_API void zend_fcall_info_argn(zend_fcall_info *fci, uint32_t argc, ...);
758758
ZEND_API zend_result zend_fcall_info_call(zend_fcall_info *fci, zend_fcall_info_cache *fcc, zval *retval, zval *args);
759759

760760
/* Zend FCC API to store and handle PHP userland functions */
761+
ZEND_API bool zend_fcc_closure_equals_ex(const zend_fcall_info_cache* a, const zend_fcall_info_cache* b);
762+
761763
static zend_always_inline bool zend_fcc_equals(const zend_fcall_info_cache* a, const zend_fcall_info_cache* b)
762764
{
765+
if (a->closure || b->closure) {
766+
return a->object == b->object
767+
&& a->calling_scope == b->calling_scope
768+
&& a->called_scope == b->called_scope
769+
&& (a->closure == b->closure || zend_fcc_closure_equals_ex(a, b))
770+
;
771+
}
763772
if (UNEXPECTED((a->function_handler->common.fn_flags & ZEND_ACC_CALL_VIA_TRAMPOLINE) &&
764773
(b->function_handler->common.fn_flags & ZEND_ACC_CALL_VIA_TRAMPOLINE))) {
765774
return a->object == b->object
766775
&& a->calling_scope == b->calling_scope
767-
&& a->closure == b->closure
768776
&& zend_string_equals(a->function_handler->common.function_name, b->function_handler->common.function_name)
769777
;
770778
}
771779
return a->function_handler == b->function_handler
772780
&& a->object == b->object
773781
&& a->calling_scope == b->calling_scope
774-
&& a->closure == b->closure
775782
;
776783
}
777784

‎ext/spl/tests/autoloading/gh22118.phpt‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
--TEST--
2+
GH-22118: spl_autoload_unregister() unregisters equivalent first-class method callables
3+
--FILE--
4+
<?php
5+
function autoload_string(string $class): void
6+
{
7+
echo "autoload $class\n";
8+
}
9+
10+
class AutoloadTest
11+
{
12+
public function load(string $class): void
13+
{
14+
echo "autoload $class\n";
15+
}
16+
17+
public function __call(string $name, array $arguments): void
18+
{
19+
echo "autoload {$arguments[0]}\n";
20+
}
21+
22+
public function __invoke(string $class): void
23+
{
24+
echo "autoload $class\n";
25+
}
26+
27+
public function run(): void
28+
{
29+
spl_autoload_register($this->load(...));
30+
var_dump(spl_autoload_unregister($this->load(...)));
31+
spl_autoload_call('MissingDirect');
32+
33+
spl_autoload_register($this->missing(...));
34+
var_dump(spl_autoload_unregister($this->missing(...)));
35+
spl_autoload_call('MissingTrampoline');
36+
37+
spl_autoload_register($this->load(...));
38+
var_dump(spl_autoload_unregister([$this, 'load']));
39+
spl_autoload_call('MissingArray');
40+
41+
spl_autoload_register($this(...));
42+
var_dump(spl_autoload_unregister($this));
43+
spl_autoload_call('MissingInvokable');
44+
}
45+
}
46+
47+
spl_autoload_register(autoload_string(...));
48+
var_dump(spl_autoload_unregister('autoload_string'));
49+
spl_autoload_call('MissingString');
50+
51+
(new AutoloadTest())->run();
52+
?>
53+
--EXPECT--
54+
bool(true)
55+
bool(true)
56+
bool(true)
57+
bool(true)
58+
bool(true)

‎ext/standard/tests/general_functions/gh22118.phpt‎

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
--TEST--
2+
GH-22118: unregister_tick_function() unregisters equivalent first-class method callables
3+
--FILE--
4+
<?php
5+
declare(ticks=1);
6+
7+
$string = 0;
8+
9+
function tick_string(): void
10+
{
11+
global $string;
12+
$string++;
13+
}
14+
15+
class TickTest
16+
{
17+
public int $direct = 0;
18+
public int $trampoline = 0;
19+
public int $array = 0;
20+
public int $invoke = 0;
21+
22+
public function kick(): void
23+
{
24+
$this->direct++;
25+
}
26+
27+
public function arrayCallable(): void
28+
{
29+
$this->array++;
30+
}
31+
32+
public function __invoke(): void
33+
{
34+
$this->invoke++;
35+
}
36+
37+
public function __call(string $name, array $arguments): void
38+
{
39+
$this->trampoline++;
40+
}
41+
42+
public function run(): void
43+
{
44+
register_tick_function($this->kick(...));
45+
unregister_tick_function($this->kick(...));
46+
echo "direct: {$this->direct}\n";
47+
48+
register_tick_function($this->missing(...));
49+
unregister_tick_function($this->missing(...));
50+
echo "trampoline: {$this->trampoline}\n";
51+
52+
register_tick_function($this->arrayCallable(...));
53+
unregister_tick_function([$this, 'arrayCallable']);
54+
echo "array: {$this->array}\n";
55+
56+
register_tick_function($this(...));
57+
unregister_tick_function($this);
58+
echo "invoke: {$this->invoke}\n";
59+
}
60+
}
61+
62+
register_tick_function(tick_string(...));
63+
unregister_tick_function('tick_string');
64+
echo "string: {$string}\n";
65+
66+
(new TickTest())->run();
67+
echo "done\n";
68+
?>
69+
--EXPECT--
70+
string: 1
71+
direct: 1
72+
trampoline: 1
73+
array: 1
74+
invoke: 1
75+
done

0 commit comments

Comments
 (0)