php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #51758 zend_object_handlers.h prescribes incorrect behavior of write property handler
Submitted: 2010-05-06 20:40 UTC Modified: 2010-05-12 20:38 UTC
Votes:1
Avg. Score:5.0 ± 0.0
Reproduced:1 of 1 (100.0%)
Same Version:1 (100.0%)
Same OS:1 (100.0%)
From: cataphract@php.net Assigned:
Status: Closed Package: Class/Object related
PHP Version: 5.3.2 OS: Not applicable
Private report: No CVE-ID: None
View Add Comment Developer Edit
Welcome! If you don't have a Git account, you can't do anything here.
You can add a comment by following this link or if you reported this bug, you can edit this bug over here.
(description)
Block user comment
Status: Assign to:
Package:
Bug Type:
Summary:
From: cataphract@php.net
New email:
PHP Version: OS:

 

 [2010-05-06 20:40 UTC] cataphract@php.net
Description:
------------
zend_object_handler.h reads (line 39):

/* The following rule applies to write_property() and write_dimension() implementations:
   If you receive a value zval in write_property/write_dimension, you may only modify it if
   its reference count is 1.  Otherwise, you must create a copy of that zval before making
   any changes.  You should NOT modify the reference count of the value passed to you. */

Perhaps I'm reading the last phrase out of context, but zend_std_write_property() changes the reference count of the passed value in multiple places.

Test script:
---------------
Not applicable.

Expected result:
----------------
Expected either no prescription for the refcount of the passed value not to be changed or the refcount of the passed value actually not be changed.

Actual result:
--------------
The implementation and the header prescription are inconsistent.

Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2010-05-06 20:47 UTC] cataphract@php.net
I also have some doubts about the accuracy of the first part (you may only modify it if its reference count is 1). Why 1? Since zend_std_write_property increments the refcount before storing the zval in the hash table, it would make more sense if it read "...its reference count is 0". Together with issue raised in body of the bug report, it makes me think perhaps this comment was written thinking the refcount would be incremented before the call to write_property.
 [2010-05-06 22:02 UTC] degeberg@php.net
-Type: Documentation Problem +Type: Bug
 [2010-05-12 09:39 UTC] mike@php.net
-Status: Open +Status: Feedback
 [2010-05-12 09:39 UTC] mike@php.net
Where's the value modified in zend_std_weite_property()?
 [2010-05-12 20:11 UTC] cataphract@php.net
Usually in zend_object.properties hash table. This is the code executed if the hash table lookup is successful (otherwise there's a fallback to __set) and the zval* stored is different from the one passed:

if (PZVAL_IS_REF(*variable_ptr)) {
	zval garbage = **variable_ptr; /* old value should be destroyed */

	/* To check: can't *variable_ptr be some system variable like error_zval here? */
	Z_TYPE_PP(variable_ptr) = Z_TYPE_P(value);
	(*variable_ptr)->value = value->value;
	if (Z_REFCOUNT_P(value) > 0) {
		zval_copy_ctor(*variable_ptr);
	}
	zval_dtor(&garbage);
} else {
	zval *garbage = *variable_ptr;

	/* if we assign referenced variable, we should separate it */
	Z_ADDREF_P(value);
	if (PZVAL_IS_REF(value)) {
		SEPARATE_ZVAL(&value);
	}
	*variable_ptr = value;
	zval_ptr_dtor(&garbage);
}

As you can see, the reference count is changed.
 [2010-05-12 20:31 UTC] cataphract@php.net
Well, re-reading I think I misread your question. So to be clear:

The comment reads:
«You should NOT modify the reference count of the value passed to you»
zend_std_write_property() changes the the reference count of the passed "value" zval if
1. the property already exists in the object properties hash table or it doesn't exist but there is no __set magic
2. the value stored (if any) is not the same as the "value" passed
3. the value passed is not a reference, or at least, if it is, its refcount is 1

The call to __set itself may modify the refcount of "value".
 [2010-05-12 20:38 UTC] cataphract@php.net
-Status: Feedback +Status: Open
 [2021-01-14 08:50 UTC] nikic@php.net
Automatic comment on behalf of shinji.igarashi@gmail.com
Revision: http://git.php.net/?p=php-src.git;a=commit;h=37b94ac38adf899930ffcc98df10634173b80204
Log: Fix #51758: delete an outdated comment from zend_object_handler.h [ci skip]
 [2021-01-14 08:50 UTC] nikic@php.net
-Status: Open +Status: Closed
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Tue Mar 19 05:01:29 2024 UTC