php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
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
Welcome back! If you're the original bug submitter, here's where you can edit the bug or add additional notes.
If this is not your bug, you can add a comment by following this link.
If this is your bug, but you forgot your password, you can retrieve your password here.
Password:
Status:
Package:
Bug Type:
Summary:
From: cataphract@php.net
New email:
PHP Version: OS:

 

 [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)

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2010-09-28 03:50 UTC] cataphract@php.net
The following patch has been added/updated:

Patch Name: zend_call_user_function_prefer_ref
Revision:   1285638612
URL:        http://bugs.php.net/patch-display.php?bug=52939&patch=zend_call_user_function_prefer_ref&revision=1285638612
 [2010-09-28 03:57 UTC] cataphract@php.net
The following patch has been added/updated:

Patch Name: zend_call_user_function_prefer_ref
Revision:   1285639039
URL:        http://bugs.php.net/patch-display.php?bug=52939&patch=zend_call_user_function_prefer_ref&revision=1285639039
 [2010-10-06 14:45 UTC] cataphract@php.net
-Assigned To: +Assigned To: dmitry
 [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
-Status: Closed +Status: Assigned
 [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
-Status: Feedback +Status: Assigned
 [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:51 UTC] dmitry@php.net
Automatic comment from SVN on behalf of dmitry
Revision: http://svn.php.net/viewvc/?view=revision&amp;revision=304364
Log: Fixed bug #52939 (zend_call_function does not respect ZEND_SEND_PREFER_REF)
 [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 03:11 UTC] cataphract@php.net
That does it.

On a minor note, the fix does an unnecessary zval separation. The need to separate the zval if !fci->no_separation, the zval is not a reference and the function expects a reference is because, presumably, if the target function expects a reference, it may assume it has one and then change the zval without separation. However, if it *prefers* a reference, it presumably doesn't rely on having one and it's (presumably...) safe not to separate the zval (see http://bugs.php.net/patch-display.php?bug=52939&patch=zend_call_user_function_prefer_ref&revision=1285639039 )
 [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.
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Tue Mar 19 03:01:29 2024 UTC