Home Original page

zend_call_function does not respect ZEND_SEND_PREFER_REF

Bug #52939 zend_call_function does not respect ZEND_SEND_PREFER_REF
Submitted: 2010-09-28 03:44 UTC Modified: 2010-10-14 08:51 UTC
From: cataphract@php.net Assigned: dmitry (profile)
Status: Closed Package: Scripting Engine problem
PHP Version: trunk-SVN-2010-09-28 (SVN) OS: Windows 7 x64
Private report: No CVE-ID: None

 [2010-09-28 03:44 UTC] cataphract@php.net

Description:
------------
The function zend_call_function interprets ZEND_SEND_PREFER_REF as if it were ZEND_SEND_BY_REF.

Test script:
---------------
<?php
$a = 'a';
$b = 7;
echo "direct call\n";
test_passing(1, $a, 7);
echo "call via zend_call_function\n";
//separate definition to put the refcount of the third parameter=2.
//If it were 1, zend_call_function would use its special "convert
//into reference" case and wouldn't error out
//i.e., this works: call_user_func_array('test_passing', array(1, &$a, 7));
$arguments = array(1, &$a, 7);
call_user_func_array('test_passing', $arguments);

------

PHP_FUNCTION(test_passing)
{
	zval ***params;
	unsigned param_count;
	int i;

	param_count = ZEND_NUM_ARGS();
    params = emalloc(param_count * sizeof *params);
    for (i = 0; i < (int) param_count; i++) {
        params[i] = (zval **) (zend_vm_stack_top(TSRMLS_C) - 1 - (param_count - i));
		php_printf("[%p, refcount=%u] ", *params[i], (*params[i])->refcount__gc);
		php_var_dump(params[i], 0 TSRMLS_CC);
    }
	efree(params);
}

ZEND_BEGIN_ARG_INFO_EX(arginfo_test_passing, 0, 0, 3)
	ZEND_ARG_INFO(0, req_pass_by_val)
	ZEND_ARG_INFO(1, req_pass_by_ref)
	ZEND_ARG_INFO(2, req_prefer_ pass_by_ref)
ZEND_END_ARG_INFO()

Expected result:
----------------
Both calls would be successful.

Actual result:
--------------
Only the direct call is successful:

direct call
[0x5ddbed0, refcount=1] int(1)
[0x5ddc530, refcount=2] &string(1) "a"
[0x5ddbf20, refcount=1] int(7)
call via zend_call_function
PHP Warning:  Parameter 3 to test_passing() expected to be a reference, value gi
ven in - on line 12

Patches

zend_call_user_function_prefer_ref (last revision 2010-09-28 01:57 UTC by cataphract@php.net)

Pull Requests

History

AllCommentsChangesGit/SVN commitsRelated reports

 [2010-10-12 09:39 UTC] dmitry@php.net

The bug is already fixed in SVN (see Zend/tests/bug52939.phpt)

 [2010-10-12 09:39 UTC] dmitry@php.net

-Status: Assigned +Status: Closed

 [2010-10-12 09:39 UTC] dmitry@php.net

This bug has been fixed in SVN.

Snapshots of the sources are packaged every three hours; this change
will be in the next snapshot. You can grab the snapshot at
http://snaps.php.net/.
 
Thank you for the report, and for helping us make PHP better.

 [2010-10-12 16:15 UTC] cataphract@php.net

The test you added doesn't test the issue.

The problem is not zend_call_function + ZEND_SEND_PREF_REF not accepting reference; it's not accepting values. This is the correct test:

<?php
	var_dump(array_multisort(array("row1" => 2, "row2" => 1)));
	
	$ar1 = array("row1" => 2, "row2" => 1);
	var_dump(call_user_func_array("array_multisort", array($ar1)));
	var_dump($ar1);

The "normal" call works, because even though it's a value, it's accepted because of ZEND_SEND_PREFER_REF. The call via call_user_func_array still fails because of this bug.

 [2010-10-12 20:48 UTC] dmitry@php.net

-Status: Assigned +Status: Feedback

 [2010-10-12 20:48 UTC] dmitry@php.net

Note that array($ar1) creates an array with a copy of $ar1 and its modification doesn't affect the original $ar1 value. Probably the proper test would be

<?php
$args = array(array("row1" => 2, "row2" => 1));
call_user_func_array("array_multisort", $args);
var_dump($args[0]);
?>

I'll check if it works tomorrow. But anyway it's not the thing you like.

 [2010-10-12 23:49 UTC] cataphract@php.net

This last test is exactly the same thing, and still gives an error. The only thing that would be different would be

call_user_func_array("array_multisort", array(array("row1" => 2, "row2" => 1)));

because now the inner array refcount would be 1 and in that case zend_call_function flips is_ref.

This code

$args = array(array("row1" => 2, "row2" => 1));
call_user_func_array("array_multisort", $args);

should still pass $args[0] by value. The fact that it can't be modified is irrelevant; array_multisort explicitly declares it can accept a value (probably because it can accept multiple arrays and one may want to modify only some).

 [2010-10-13 10:52 UTC] dmitry@php.net

-Status: Assigned +Status: Closed

 [2010-10-13 10:52 UTC] dmitry@php.net

This bug has been fixed in SVN.

Snapshots of the sources are packaged every three hours; this change
will be in the next snapshot. You can grab the snapshot at
http://snaps.php.net/.
 
Thank you for the report, and for helping us make PHP better.

 [2010-10-14 08:11 UTC] dmitry@php.net

I agree that separation might be omitted, but with your patch we just convert value into reference without separation. In case of no separation we also have to prevent Z_SET_ISREF_PP(fci->params[i]);

 [2010-10-14 08:28 UTC] cataphract@php.net

Ah right, the part after the if in line #854 is still executed.

You also seem to have missed an implication of in your commit, because with a value + SEND_PREFER_REF, the newly separated value will be have is_ref set to 1 by that part in #880 - #882.

The test script in the report gives this result:

direct call
[0x43436c0, refcount=1] int(1)
[0x4343d20, refcount=2] &string(1) "a"
[0x4343710, refcount=1] int(7)
call via zend_call_function
[0x43436c0, refcount=3] int(1)
[0x4343d20, refcount=4] &string(1) "a"
[0x43426d8, refcount=2] &int(7)

 [2010-10-14 08:51 UTC] dmitry@php.net

right, but it can't break something because the function can accept references.
It's just unnecessary copying, but I suppose that it's a very rare case.