php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #62991 Segfault with generator and closure.
Submitted: 2012-09-02 01:58 UTC Modified: 2012-09-05 05:52 UTC
From: softwareelves at gmail dot com Assigned: dmitry (profile)
Status: Closed Package: Reproducible crash
PHP Version: master-Git-2012-09-02 (Git) OS: Mac OSx 10.8.1
Private report: No CVE-ID: None
 [2012-09-02 01:58 UTC] softwareelves at gmail dot com
Description:
------------
If you create a generator-closure inside of a function and call that function 
before returning it, it'll cause memory corruption causing a segfault.

Test script:
---------------
<?php

function test( array $array )
{
	$closure = function() use ( $array ) {
		var_dump( $array );
		yield "hi";
	};
	return $closure(); // if you return the $closure and call it outside this function it works.
}

$generator = test( array( 1, 2, 3 ) );
foreach( $generator as $something ) {
	// Segmentation fault: 11
}

Expected result:
----------------
array(3) { [0]=> int(1) [1]=> int(2) [2]=> int(3) }

Actual result:
--------------
Segmentation fault: 11

Patches

bug62991.patch (last revision 2012-09-04 06:56 UTC by dmitry at zend dot com)
bug62991.phpt (last revision 2012-09-02 11:50 UTC by laruence@php.net)

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2012-09-02 02:05 UTC] stas@php.net
-Assigned To: +Assigned To: nikic
 [2012-09-02 08:25 UTC] laruence@php.net
seems the closure has been released after it was executed  while destruct the 
outter scope..
 [2012-09-02 09:54 UTC] laruence@php.net
The following patch has been added/updated:

Patch Name: bug62991.patch
Revision:   1346579695
URL:        https://bugs.php.net/patch-display.php?bug=62991&patch=bug62991.patch&revision=1346579695
 [2012-09-02 09:55 UTC] laruence@php.net
I got a fix for this. nikic, could you please review this? thanks
 [2012-09-02 09:58 UTC] laruence@php.net
The following patch has been added/updated:

Patch Name: bug62991.patch
Revision:   1346579896
URL:        https://bugs.php.net/patch-display.php?bug=62991&patch=bug62991.patch&revision=1346579896
 [2012-09-02 09:58 UTC] laruence@php.net
update patch, fix tabs
 [2012-09-02 10:23 UTC] nikic@php.net
@laruence: The patch looks fine for me.

The only thing that looks strange are these whitespace changes:

-ZEND_BEGIN_ARG_INFO_EX(arginfo_closure_bindto, 0, 0, 1)
+	ZEND_BEGIN_ARG_INFO_EX(arginfo_closure_bindto, 0, 0, 1)
 	ZEND_ARG_INFO(0, newthis)
 	ZEND_ARG_INFO(0, newscope)
 ZEND_END_ARG_INFO()
 
-ZEND_BEGIN_ARG_INFO_EX(arginfo_closure_bind, 0, 0, 2)
+	ZEND_BEGIN_ARG_INFO_EX(arginfo_closure_bind, 0, 0, 2)
 	ZEND_ARG_INFO(0, closure)
 	ZEND_ARG_INFO(0, newthis)
 	ZEND_ARG_INFO(0, newscope)
 ZEND_END_ARG_INFO()
 
-static const zend_function_entry closure_functions[] = {
-	ZEND_ME(Closure, __construct, NULL, ZEND_ACC_PRIVATE)
-	ZEND_ME(Closure, bind, arginfo_closure_bind, ZEND_ACC_PUBLIC|ZEND_ACC_STATIC)
-	ZEND_MALIAS(Closure, bindTo, bind, arginfo_closure_bindto, ZEND_ACC_PUBLIC)
-	{NULL, NULL, NULL}
-};
+	static const zend_function_entry closure_functions[] = {
+		ZEND_ME(Closure, __construct, NULL, ZEND_ACC_PRIVATE)
+			ZEND_ME(Closure, bind, arginfo_closure_bind, ZEND_ACC_PUBLIC|ZEND_ACC_STATIC)
+			ZEND_MALIAS(Closure, bindTo, bind, arginfo_closure_bindto, ZEND_ACC_PUBLIC)
+			{NULL, NULL, NULL}
+	};

Looks like the indentation is slightly off there :)
 [2012-09-02 10:26 UTC] nikic@php.net
Oh, and also, I think it would be a little bit nicer if this:

+		if (execute_data->op_array->fn_flags & ZEND_ACC_CLOSURE) {
+			destroy_op_array(execute_data->op_array);
+			efree(execute_data->op_array);
+		}

would be written as:

+		if (op_array->fn_flags & ZEND_ACC_CLOSURE) {
+			destroy_op_array(op_array);
+			efree(op_array);
+		}

There already is a local op_array variable for execute_data->op_array, so it's a bit shorter to use ;)
 [2012-09-02 11:24 UTC] laruence@php.net
okey, but is there a way to find out that whether a generator has been run once?

leaks reporting if the closure didn't run.
 [2012-09-02 11:45 UTC] laruence@php.net
The following patch has been added/updated:

Patch Name: bug62991.patch
Revision:   1346586306
URL:        https://bugs.php.net/patch-display.php?bug=62991&patch=bug62991.patch&revision=1346586306
 [2012-09-02 11:46 UTC] laruence@php.net
a new patch has been attached, fixed the memleak issue, but the way is a little 
tricky, used the op_array->reserved fields.

so I attached it here instead of ci it, wait for if we can find a better way
 [2012-09-02 11:50 UTC] laruence@php.net
The following patch has been added/updated:

Patch Name: bug62991.phpt
Revision:   1346586639
URL:        https://bugs.php.net/patch-display.php?bug=62991&patch=bug62991.phpt&revision=1346586639
 [2012-09-04 06:57 UTC] dmitry@php.net
I've added a much simpler patch. Please take a look.
 [2012-09-04 07:09 UTC] laruence@php.net
the static variable table should also be copied, and this func will be copied once 
/ per closure called (create a new genartor).

maybe add a new ACC flag is much more simple... which we have discussed before( I 
also discussed that with nikic :))
 [2012-09-05 05:52 UTC] dmitry@php.net
-Status: Assigned +Status: Closed -Assigned To: nikic +Assigned To: dmitry
 [2012-09-05 05: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/.

 For Windows:

http://windows.php.net/snapshots/
 
Thank you for the report, and for helping us make PHP better.


 [2012-09-05 05:55 UTC] dmitry@php.net
Automatic comment on behalf of dmitry@zend.com
Revision: http://git.php.net/?p=php-src.git;a=commit;h=72473962a9722ee0a8107909f32c145d3b44f906
Log: Fixed bug #62991 (Segfault with generator and closure)
 [2013-11-17 09:32 UTC] laruence@php.net
Automatic comment on behalf of dmitry@zend.com
Revision: http://git.php.net/?p=php-src.git;a=commit;h=72473962a9722ee0a8107909f32c145d3b44f906
Log: Fixed bug #62991 (Segfault with generator and closure)
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Fri Mar 29 11:01:29 2024 UTC