Skip to content

Commit

Permalink
Improve error message for 'Cannot pass parameter by reference'
Browse files Browse the repository at this point in the history
  • Loading branch information
kocsismate committed Aug 16, 2020
1 parent 517c993 commit 9425533
Show file tree
Hide file tree
Showing 18 changed files with 101 additions and 54 deletions.
21 changes: 9 additions & 12 deletions Zend/tests/bug72038.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -6,28 +6,25 @@ Bug #72038 (Function calls with values to a by-ref parameter don't always throw
try {
test($foo = new stdClass);
var_dump($foo);
} catch (Throwable $e) {
echo "Exception: " . $e->getMessage() . "\n";
} catch (Error $e) {
echo $e->getMessage() . "\n";
}
try {
test($bar = 2);
var_dump($bar);
} catch (Throwable $e) {
echo "Exception: " . $e->getMessage() . "\n";
}
try {
test($baz = &$bar);
var_dump($baz);
} catch (Throwable $e) {
echo "Exception: " . $e->getMessage() . "\n";
} catch (Error $e) {
echo $e->getMessage() . "\n";
}

test($baz = &$bar);
var_dump($baz);

function test(&$param) {
$param = 1;
}

?>
--EXPECT--
Exception: Cannot pass parameter 1 by reference
Exception: Cannot pass parameter 1 by reference
test(): Argument #1 ($param) cannot be passed by reference
test(): Argument #1 ($param) cannot be passed by reference
int(1)
2 changes: 1 addition & 1 deletion Zend/tests/bug73663_2.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ change(list($val) = $array);
var_dump($array);
?>
--EXPECTF--
Fatal error: Uncaught Error: Cannot pass parameter 1 by reference in %s:%d
Fatal error: Uncaught Error: change(): Argument #1 ($ref) cannot be passed by reference in %s:%d
Stack trace:
#0 {main}
thrown in %s on line %d
4 changes: 2 additions & 2 deletions Zend/tests/bug78154.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -22,5 +22,5 @@ namespace Foo {

?>
--EXPECT--
Exception: Cannot pass parameter 3 by reference
Exception: Cannot pass parameter 3 by reference
Exception: similar_text(): Argument #3 ($percent) cannot be passed by reference
Exception: similar_text(): Argument #3 ($percent) cannot be passed by reference
2 changes: 1 addition & 1 deletion Zend/tests/bug79783.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ Bug #79783: Segfault in php_str_replace_common
str_replace("a", "b", "c", strlen("d"));
?>
--EXPECTF--
Fatal error: Uncaught Error: Cannot pass parameter 4 by reference in %s:%d
Fatal error: Uncaught Error: str_replace(): Argument #4 ($replace_count) cannot be passed by reference in %s:%d
Stack trace:
#0 {main}
thrown in %s on line %d
2 changes: 1 addition & 1 deletion Zend/tests/closure_019.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ int(9)
Notice: Only variable references should be returned by reference in %sclosure_019.php on line 4
int(81)

Fatal error: Uncaught Error: Cannot pass parameter 1 by reference in %s:%d
Fatal error: Uncaught Error: {closure}(): Argument #1 ($x) cannot be passed by reference in %s:%d
Stack trace:
#0 %s(%d): test()
#1 {main}
Expand Down
2 changes: 1 addition & 1 deletion Zend/tests/errmsg_022.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ foo(1);
echo "Done\n";
?>
--EXPECTF--
Fatal error: Uncaught Error: Cannot pass parameter 1 by reference in %s:%d
Fatal error: Uncaught Error: foo(): Argument #1 ($var) cannot be passed by reference in %s:%d
Stack trace:
#0 {main}
thrown in %s on line %d
6 changes: 3 additions & 3 deletions Zend/tests/match/027.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ main();
usesValue 0
i is 0

Fatal error: Uncaught Error: Cannot pass parameter 1 by reference in %s027.php:20
Fatal error: Uncaught Error: Test::usesRef(): Argument #1 ($x) cannot be passed by reference in %s:%d
Stack trace:
#0 %s027.php(23): main()
#0 %s(%d): main()
#1 {main}
thrown in %s027.php on line 20
thrown in %s on line %d
2 changes: 1 addition & 1 deletion Zend/tests/match/028.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -34,4 +34,4 @@ try {
usesValue 42
usesValue 42
int(42)
Caught Cannot pass parameter 1 by reference
Caught Test::usesRef(): Argument #1 ($x) cannot be passed by reference
8 changes: 4 additions & 4 deletions Zend/tests/nullsafe_operator/016.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ test(new Foo());

?>
--EXPECT--
Cannot pass parameter 1 by reference
Cannot pass parameter 1 by reference
Cannot pass parameter 1 by reference
Cannot pass parameter 1 by reference
set(): Argument #1 ($ref) cannot be passed by reference
set(): Argument #1 ($ref) cannot be passed by reference
set(): Argument #1 ($ref) cannot be passed by reference
set(): Argument #1 ($ref) cannot be passed by reference
2 changes: 1 addition & 1 deletion Zend/tests/variadic/by_ref_error.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ test(1);

?>
--EXPECTF--
Fatal error: Uncaught Error: Cannot pass parameter 1 by reference in %s:%d
Fatal error: Uncaught Error: test(): Argument #1 cannot be passed by reference in %s:%d
Stack trace:
#0 {main}
thrown in %s on line %d
10 changes: 5 additions & 5 deletions Zend/zend_API.c
Original file line number Diff line number Diff line change
Expand Up @@ -319,23 +319,23 @@ ZEND_API ZEND_COLD void ZEND_FASTCALL zend_unexpected_extra_named_error(void)

static ZEND_COLD void ZEND_FASTCALL zend_argument_error_variadic(zend_class_entry *error_ce, uint32_t arg_num, const char *format, va_list va) /* {{{ */
{
const char *space;
const char *class_name;
char *func_name;
const char *arg_name;
char *message = NULL;
if (EG(exception)) {
return;
}

class_name = get_active_class_name(&space);
func_name = get_active_function_or_method_name();
arg_name = get_active_function_arg_name(arg_num);

zend_vspprintf(&message, 0, format, va);
zend_throw_error(error_ce, "%s%s%s(): Argument #%d%s%s%s %s",
class_name, space, get_active_function_name(), arg_num,
zend_throw_error(error_ce, "%s(): Argument #%d%s%s%s %s",
func_name, arg_num,
arg_name ? " ($" : "", arg_name ? arg_name : "", arg_name ? ")" : "", message
);
efree(message);
efree(func_name);
}
/* }}} */

Expand Down
4 changes: 4 additions & 0 deletions Zend/zend_execute.h
Original file line number Diff line number Diff line change
Expand Up @@ -308,9 +308,13 @@ ZEND_API void ZEND_FASTCALL zend_free_extra_named_params(zend_array *extra_named

/* services */
ZEND_API const char *get_active_class_name(const char **space);
ZEND_API const char *get_class_name(const zend_function *func, const char **space);
ZEND_API const char *get_active_function_name(void);
ZEND_API const char *get_function_name(const zend_function *func);
ZEND_API const char *get_active_function_arg_name(uint32_t arg_num);
ZEND_API const char *get_function_arg_name(const zend_function *func, uint32_t arg_num);
ZEND_API char *get_active_function_or_method_name();
ZEND_API char *get_function_or_method_name(const zend_function *func);
ZEND_API const char *zend_get_executed_filename(void);
ZEND_API zend_string *zend_get_executed_filename_ex(void);
ZEND_API uint32_t zend_get_executed_lineno(void);
Expand Down
43 changes: 37 additions & 6 deletions Zend/zend_execute_API.c
Original file line number Diff line number Diff line change
Expand Up @@ -429,16 +429,20 @@ void shutdown_executor(void) /* {{{ */
/* return class name and "::" or "". */
ZEND_API const char *get_active_class_name(const char **space) /* {{{ */
{
zend_function *func;

if (!zend_is_executing()) {
if (space) {
*space = "";
}
return "";
}

func = EG(current_execute_data)->func;
return get_class_name(EG(current_execute_data)->func, space);
}
/* }}} */

/* return class name and "::" or "". */
ZEND_API const char *get_class_name(const zend_function *func, const char **space) /* {{{ */
{
switch (func->type) {
case ZEND_USER_FUNCTION:
case ZEND_INTERNAL_FUNCTION:
Expand All @@ -461,12 +465,16 @@ ZEND_API const char *get_active_class_name(const char **space) /* {{{ */

ZEND_API const char *get_active_function_name(void) /* {{{ */
{
zend_function *func;

if (!zend_is_executing()) {
return NULL;
}
func = EG(current_execute_data)->func;

return get_function_name(EG(current_execute_data)->func);
}
/* }}} */

ZEND_API const char *get_function_name(const zend_function *func) /* {{{ */
{
switch (func->type) {
case ZEND_USER_FUNCTION: {
zend_string *function_name = func->common.function_name;
Expand All @@ -487,6 +495,29 @@ ZEND_API const char *get_active_function_name(void) /* {{{ */
}
/* }}} */

ZEND_API char *get_active_function_or_method_name(void) /* {{{ */
{
if (!zend_is_executing()) {
return "";
}

return get_function_or_method_name(EG(current_execute_data)->func);
}
/* }}} */

ZEND_API char *get_function_or_method_name(const zend_function *func) /* {{{ */
{
char *name = NULL;
const char *class_name, *space;

class_name = get_class_name(func, &space);

zend_spprintf(&name, 0, "%s%s%s", class_name, space, get_function_name(func));

return name;
}
/* }}} */

ZEND_API const char *get_active_function_arg_name(uint32_t arg_num) /* {{{ */
{
zend_function *func;
Expand Down
9 changes: 8 additions & 1 deletion Zend/zend_vm_def.h
Original file line number Diff line number Diff line change
Expand Up @@ -4579,9 +4579,16 @@ ZEND_VM_COLD_HELPER(zend_cannot_pass_by_ref_helper, ANY, ANY)
USE_OPLINE
zval *arg;
uint32_t arg_num = opline->op2.num;
char *func_name = get_function_or_method_name(EX(call)->func);
const char *param_name = get_function_arg_name(EX(call)->func, arg_num);

SAVE_OPLINE();
zend_throw_error(NULL, "Cannot pass parameter %d by reference", arg_num);

zend_throw_error(NULL, "%s(): Argument #%d%s%s%s cannot be passed by reference",
func_name, arg_num, param_name ? " ($" : "",
param_name ? param_name : "", param_name ? ")" : ""
);
efree(func_name);
FREE_OP1();
arg = ZEND_CALL_VAR(EX(call), opline->result.var);
ZVAL_UNDEF(arg);
Expand Down
9 changes: 8 additions & 1 deletion Zend/zend_vm_execute.h
Original file line number Diff line number Diff line change
Expand Up @@ -1842,9 +1842,16 @@ static zend_never_inline ZEND_COLD ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL zend_ca
USE_OPLINE
zval *arg;
uint32_t arg_num = opline->op2.num;
char *func_name = get_function_or_method_name(EX(call)->func);
const char *param_name = get_function_arg_name(EX(call)->func, arg_num);

SAVE_OPLINE();
zend_throw_error(NULL, "Cannot pass parameter %d by reference", arg_num);

zend_throw_error(NULL, "%s(): Argument #%d%s%s%s cannot be passed by reference",
func_name, arg_num, param_name ? " ($" : "",
param_name ? param_name : "", param_name ? ")" : ""
);
efree(func_name);
FREE_OP(opline->op1_type, opline->op1.var);
arg = ZEND_CALL_VAR(EX(call), opline->result.var);
ZVAL_UNDEF(arg);
Expand Down
1 change: 1 addition & 0 deletions ext/opcache/jit/zend_jit_disasm_x86.c
Original file line number Diff line number Diff line change
Expand Up @@ -449,6 +449,7 @@ static int zend_jit_disasm_init(void)
REGISTER_HELPER(zend_jit_post_dec_typed_ref);
REGISTER_HELPER(zend_jit_assign_op_to_typed_ref);
REGISTER_HELPER(zend_jit_only_vars_by_reference);
REGISTER_HELPER(zend_jit_cannot_pass_by_reference);
REGISTER_HELPER(zend_jit_invalid_array_access);
REGISTER_HELPER(zend_jit_invalid_property_read);
REGISTER_HELPER(zend_jit_invalid_property_write);
Expand Down
12 changes: 12 additions & 0 deletions ext/opcache/jit/zend_jit_helpers.c
Original file line number Diff line number Diff line change
Expand Up @@ -1763,6 +1763,18 @@ static void ZEND_FASTCALL zend_jit_only_vars_by_reference(zval *arg)
zend_error(E_NOTICE, "Only variables should be passed by reference");
}

static void ZEND_FASTCALL zend_jit_cannot_pass_by_reference(uint32_t arg_num)
{
const zend_execute_data *execute_data = EG(current_execute_data);
char *func_name = get_function_or_method_name(EX(call)->func);
const char *param_name = get_function_arg_name(EX(call)->func, arg_num);

zend_throw_error(NULL, "%s(): Argument #%d%s%s%s cannot be passed by reference",
func_name, arg_num, param_name ? " ($" : "", param_name ? param_name : "", param_name ? ")" : ""
);
efree(func_name);
}

static void ZEND_FASTCALL zend_jit_invalid_array_access(zval *container)
{
zend_error(E_WARNING, "Trying to access array offset on value of type %s", zend_zval_type_name(container));
Expand Down
16 changes: 2 additions & 14 deletions ext/opcache/jit/zend_jit_x86.dasc
Original file line number Diff line number Diff line change
Expand Up @@ -1850,20 +1850,8 @@ static int zend_jit_throw_cannot_pass_by_ref_stub(dasm_State **Dst)
| mov EX->call, RX
|1:
| mov RX, r0
|.if X64
| xor CARG1, CARG1
| LOAD_ADDR CARG2, "Cannot pass parameter %d by reference"
| mov CARG3d, dword OP:r0->op2.num
| EXT_CALL zend_throw_error, r0
|.else
| mov r1, dword OP:r0->op2.num
| sub r4, 4
| push r1
| push "Cannot pass parameter %d by reference"
| push 0
| EXT_CALL zend_throw_error, r0
| add r4, 16
|.endif
| mov CARG1d, dword OP:r0->op2.num
| EXT_CALL zend_jit_cannot_pass_by_reference, r0
| cmp byte OP:RX->op1_type, IS_TMP_VAR
| jne >9
|.if X64
Expand Down

0 comments on commit 9425533

Please sign in to comment.