Skip to content

Commit 8fc9e7c

Browse files
bwoebibwoebi
committed
Fix assertion in VM when taking ref-returning fcall by-value
zend_return_unwrap_ref() is expected to be called now, but internal functions returning by reference were failing the debug assertion, expecting that they would unconditionally return by reference. Adjusted the assertion. Signed-off-by: Bob Weinand <bobwei9@hotmail.com>
1 parent 91dc887 commit 8fc9e7c

10 files changed

Lines changed: 79 additions & 54 deletions

File tree

‎Zend/zend_execute.c‎

Lines changed: 2 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -6004,26 +6004,7 @@ ZEND_API zval *zend_get_zval_ptr(const zend_op *opline, int op_type, const znode
60046004

60056005
ZEND_API void zend_return_unwrap_ref(zend_execute_data *execute_data, zval *return_value)
60066006
{
6007-
if (!return_value || !Z_ISREF_P(return_value)) {
6008-
return;
6009-
}
6010-
6011-
zend_execute_data *prev_ex = EX(prev_execute_data);
6012-
if (!prev_ex || !prev_ex->func || !ZEND_USER_CODE(prev_ex->func->type)) {
6013-
return;
6007+
if (return_value && Z_ISREF_P(return_value) && zend_return_should_unwrap_ref(execute_data)) {
6008+
zend_unwrap_reference(return_value);
60146009
}
6015-
6016-
const zend_op *do_opline = prev_ex->opline;
6017-
if (do_opline->result_type != IS_TMP_VAR) {
6018-
return;
6019-
}
6020-
6021-
if (do_opline->opcode != ZEND_DO_FCALL
6022-
&& do_opline->opcode != ZEND_DO_FCALL_BY_NAME
6023-
&& do_opline->opcode != ZEND_DO_ICALL
6024-
&& do_opline->opcode != ZEND_DO_UCALL) {
6025-
return;
6026-
}
6027-
6028-
zend_unwrap_reference(return_value);
60296010
}

‎Zend/zend_execute.h‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -676,6 +676,24 @@ static zend_always_inline void *zend_get_bad_ptr(void)
676676
return NULL;
677677
}
678678

679+
static zend_always_inline bool zend_return_should_unwrap_ref(const zend_execute_data *call)
680+
{
681+
const zend_execute_data *prev_ex = call->prev_execute_data;
682+
if (!prev_ex || !prev_ex->func || !ZEND_USER_CODE(prev_ex->func->type)) {
683+
return false;
684+
}
685+
686+
const zend_op *do_opline = prev_ex->opline;
687+
if (do_opline->result_type != IS_TMP_VAR) {
688+
return false;
689+
}
690+
691+
return do_opline->opcode == ZEND_DO_FCALL
692+
|| do_opline->opcode == ZEND_DO_FCALL_BY_NAME
693+
|| do_opline->opcode == ZEND_DO_ICALL
694+
|| do_opline->opcode == ZEND_DO_UCALL;
695+
}
696+
679697
ZEND_API void zend_return_unwrap_ref(zend_execute_data *call, zval *return_value);
680698

681699
END_EXTERN_C()

‎Zend/zend_execute_API.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1106,7 +1106,7 @@ zend_result zend_call_function(zend_fcall_info *fci, zend_fcall_info_cache *fci_
11061106
bool result = zend_verify_internal_return_type(call->func, fci->retval);
11071107
ZEND_ASSERT(result);
11081108
}
1109-
ZEND_ASSERT((call->func->common.fn_flags & ZEND_ACC_RETURN_REFERENCE)
1109+
ZEND_ASSERT(((call->func->common.fn_flags & ZEND_ACC_RETURN_REFERENCE) && !zend_return_should_unwrap_ref(call))
11101110
? Z_ISREF_P(fci->retval) : !Z_ISREF_P(fci->retval));
11111111
ZEND_ASSERT(!(call->func->common.fn_flags2 & ZEND_ACC2_FORBID_DYN_CALLS));
11121112
}

‎Zend/zend_vm_def.h‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4158,7 +4158,7 @@ ZEND_VM_HOT_HANDLER(129, ZEND_DO_ICALL, ANY, ANY, SPEC(RETVAL,OBSERVER))
41584158
bool result = zend_verify_internal_return_type(call->func, ret);
41594159
ZEND_ASSERT(result);
41604160
}
4161-
ZEND_ASSERT((call->func->common.fn_flags & ZEND_ACC_RETURN_REFERENCE)
4161+
ZEND_ASSERT(((call->func->common.fn_flags & ZEND_ACC_RETURN_REFERENCE) && !zend_return_should_unwrap_ref(call))
41624162
? Z_ISREF_P(ret) : !Z_ISREF_P(ret));
41634163
zend_verify_internal_func_info(call->func, ret);
41644164
ZEND_ASSERT(!(ZEND_CALL_INFO(call) & ZEND_CALL_DYNAMIC)
@@ -4291,7 +4291,7 @@ ZEND_VM_HOT_HANDLER(131, ZEND_DO_FCALL_BY_NAME, ANY, ANY, SPEC(RETVAL,OBSERVER))
42914291
bool result = zend_verify_internal_return_type(call->func, ret);
42924292
ZEND_ASSERT(result);
42934293
}
4294-
ZEND_ASSERT((call->func->common.fn_flags & ZEND_ACC_RETURN_REFERENCE)
4294+
ZEND_ASSERT(((call->func->common.fn_flags & ZEND_ACC_RETURN_REFERENCE) && !zend_return_should_unwrap_ref(call))
42954295
? Z_ISREF_P(ret) : !Z_ISREF_P(ret));
42964296
zend_verify_internal_func_info(call->func, ret);
42974297
ZEND_ASSERT(!(ZEND_CALL_INFO(call) & ZEND_CALL_DYNAMIC)
@@ -4424,7 +4424,7 @@ ZEND_VM_HOT_HANDLER(60, ZEND_DO_FCALL, ANY, ANY, SPEC(RETVAL,OBSERVER))
44244424
bool result = zend_verify_internal_return_type(call->func, ret);
44254425
ZEND_ASSERT(result);
44264426
}
4427-
ZEND_ASSERT((call->func->common.fn_flags & ZEND_ACC_RETURN_REFERENCE)
4427+
ZEND_ASSERT(((call->func->common.fn_flags & ZEND_ACC_RETURN_REFERENCE) && !zend_return_should_unwrap_ref(call))
44284428
? Z_ISREF_P(ret) : !Z_ISREF_P(ret));
44294429
zend_verify_internal_func_info(call->func, ret);
44304430
ZEND_ASSERT(!(ZEND_CALL_INFO(call) & ZEND_CALL_DYNAMIC)
@@ -9182,7 +9182,7 @@ ZEND_VM_HANDLER(158, ZEND_CALL_TRAMPOLINE, ANY, ANY, SPEC(OBSERVER))
91829182
bool result = zend_verify_internal_return_type(call->func, ret);
91839183
ZEND_ASSERT(result);
91849184
}
9185-
ZEND_ASSERT((call->func->common.fn_flags & ZEND_ACC_RETURN_REFERENCE)
9185+
ZEND_ASSERT(((call->func->common.fn_flags & ZEND_ACC_RETURN_REFERENCE) && !zend_return_should_unwrap_ref(call))
91869186
? Z_ISREF_P(ret) : !Z_ISREF_P(ret));
91879187
zend_verify_internal_func_info(call->func, ret);
91889188
ZEND_ASSERT(!(ZEND_CALL_INFO(call) & ZEND_CALL_DYNAMIC)

‎Zend/zend_vm_execute.h‎

Lines changed: 22 additions & 22 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎ext/zend_test/test.c‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,18 @@ static void pass2(zend_script *script, void *context)
132132
php_printf("pass2\n");
133133
}
134134

135+
static ZEND_FUNCTION(zend_test_forward_reference)
136+
{
137+
zval *value;
138+
139+
ZEND_PARSE_PARAMETERS_START(1, 1)
140+
Z_PARAM_ZVAL(value)
141+
ZEND_PARSE_PARAMETERS_END();
142+
143+
ZVAL_COPY(return_value, value);
144+
zend_return_unwrap_ref(execute_data, return_value);
145+
}
146+
135147
static ZEND_FUNCTION(zend_test_deprecated)
136148
{
137149
zval *arg1;

‎ext/zend_test/test.stub.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -265,6 +265,8 @@ function zend_test_nullable_array_return(): null|array {}
265265

266266
function zend_test_void_return(): void {}
267267

268+
function &zend_test_forward_reference(mixed &$value): mixed {}
269+
268270
function zend_test_compile_string(string $source_string, string $filename, int $position): void {}
269271

270272
/** @deprecated */

‎ext/zend_test/test_arginfo.h‎

Lines changed: 7 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎ext/zend_test/test_decl.h‎

Lines changed: 4 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)