com php-src: Disallow use of positional args after unpacking: Zend/tests/arg_unpack/basic.phpt Zend/tests/arg_unpack/ by_ref.phpt
Zend/tests/arg_unpack/dynamic.phpt Zend /tests/arg_unpack/invalid_type.phpt Zend/tests/arg_un pack/method.phpt Zend/tests/arg_unpack/new.phpt Zend/
tests/arg_unpack/positional_arg_after_unpack_error. phpt Zend/tests/arg_unpack/traversable_throwing_excepti on.phpt
Zend/tests/arg_unpack/traversable_with_by_r ef_parameters.phpt

Commit: d3b484df8268f7ab31c4ac39734d4b68ce2e6159 Author: Nikita Popov <nikic@php.net> Wed, 26 Feb 2014 16:08:58 +0100 Parents: 2eae18794945db6c8e2540525b10a59759e0ade9 Branches: master Link: http://git.php.net/?p=php-src.git;a=commitdiff;h=d3b484df8268f7ab31c4ac39734d4b68ce2e6159 Log: Disallow use of positional args after unpacking This commit disallows the use of trailing positional arguments after argument unpacking was used. The following calls are no longer valid: fn(...$array, $var); fn(...$array1, $var, ...$array2); However, all of the following continue to be valid: fn($var, ...$array); fn(...$array1, ...$array2); fn($var, ...$array1, ....$array2); The reason behind this change is a stack allocation issue pointed out by Dmitry: As of PHP 5.5 the stack necessary for pushing arguments is precomputed and preallocated, as such the individual SEND opcodes no longer verify that there is enough stack space. The unpacked arguments will occupy some of that preallocated space and as such following positional arguments could write past a stack page boundary. An alternative resolution for this issue is to ensure that there is enough space for the remaining arguments in the UNPACK opcode. However making this allocation precise (rather than using a conversative over-estimate) would require some effort. Given that this particular aspect of the feature wasn't very popular in the first place, it doesn't seem worth the effort. Changed paths: M Zend/tests/arg_unpack/basic.phpt M Zend/tests/arg_unpack/by_ref.phpt M Zend/tests/arg_unpack/dynamic.phpt M Zend/tests/arg_unpack/invalid_type.phpt M Zend/tests/arg_unpack/method.phpt M Zend/tests/arg_unpack/new.phpt A Zend/tests/arg_unpack/positional_arg_after_unpack_error.phpt M Zend/tests/arg_unpack/traversable_throwing_exception.phpt M Zend/tests/arg_unpack/traversable_with_by_ref_parameters.phpt M Zend/zend_compile.c M Zend/zend_compile.h

diff --git a/Zend/tests/arg_unpack/basic.phpt b/Zend/tests/arg_unpack/basic.phpt index 9c03655..f8bd019 100644 --- a/Zend/tests/arg_unpack/basic.phpt +++ b/Zend/tests/arg_unpack/basic.phpt @@ -30,12 +30,12 @@ test(...getArray([1, 2, 3])); test(...arrayGen([])); test(...arrayGen([1, 2, 3])); -test(1, ...[2, 3], ...[4, 5], 6); -test(1, ...getArray([2, 3]), ...arrayGen([4, 5]), 6); +test(1, ...[2, 3], ...[4, 5]); +test(1, ...getArray([2, 3]), ...arrayGen([4, 5])); test2(...[1, 2]); test2(...[1, 2, 3]); -test2(...[1], ...[], ...[], ...[2, 3], 4, ...[5, 6]); +test2(...[1], ...[], ...[], ...[2, 3], ...[4, 5]); ?> --EXPECT-- @@ -75,7 +75,7 @@ array(3) { [2]=> int(3) } -array(6) { +array(5) { [0]=> int(1) [1]=> @@ -86,10 +86,8 @@ array(6) { int(4) [4]=> int(5) - [5]=> - int(6) } -array(6) { +array(5) { [0]=> int(1) [1]=> @@ -100,8 +98,6 @@ array(6) { int(4) [4]=> int(5) - [5]=> - int(6) } int(1) int(2) diff --git a/Zend/tests/arg_unpack/by_ref.phpt b/Zend/tests/arg_unpack/by_ref.phpt index 0619a3b..7c8a86b 100644 --- a/Zend/tests/arg_unpack/by_ref.phpt +++ b/Zend/tests/arg_unpack/by_ref.phpt @@ -17,46 +17,37 @@ $array = [1, 2, 3]; test1(...$array); var_dump($array); -$array1 = [1, 2]; $val2 = 3; $array2 = [4, 5]; -test1(...$array1, $val2, ...$array2); -var_dump($array1, $val2, $array2); +$array1 = [1, 2]; $array2 = [3, 4]; +test1(...$array1, ...$array2); +var_dump($array1, $array2); function test2($val1, &$ref1, $val2, &$ref2) { $ref1++; $ref2++; } -$array = [1, 2, 3, 4]; +$array = [0, 0, 0, 0]; test2(...$array); var_dump($array); -$a = $b = $c = $d = 0; +$array1 = [1, 2]; $array2 = [4, 5]; +test1(...$array1, ...$array2); +var_dump($array1, $array2); -$array = []; -test2(...$array, $a, $b, $c, $d); -var_dump($array, $a, $b, $c, $d); +$a = $b = $c = $d = 0; +$array = [0, 0, 0, 0]; -$array = [1]; -test2(...$array, $a, $b, $c, $d); -var_dump($array, $a, $b, $c, $d); +test2($a, ...$array); +var_dump($a, $array); -$array = [1, 2]; -test2(...$array, $a, $b, $c, $d); -var_dump($array, $a, $b, $c, $d); +test2($a, $b, ...$array); +var_dump($a, $b, $array); -$array = [1, 2, 3]; -test2(...$array, $a, $b, $c, $d); -var_dump($array, $a, $b, $c, $d); +test2($a, $b, $c, ...$array); +var_dump($a, $b, $c, $array); -$vars = []; -$array = []; -test2(...$array, $vars['a'], $vars['b'], $vars['c'], $vars['d']); -var_dump($vars); - -$vars = []; -$array = [1]; -test2(...$array, $vars['a'], $vars['b'], $vars['c'], $vars['d']); -var_dump($vars); +test2($a, $b, $c, $d, ...$array); +var_dump($a, $b, $c, $d, $array); ?> --EXPECTF-- @@ -74,76 +65,81 @@ array(2) { [1]=> int(3) } -int(4) array(2) { [0]=> - int(5) + int(4) [1]=> - int(6) + int(5) } array(4) { [0]=> - int(1) + int(0) [1]=> - int(3) + int(1) [2]=> - int(3) + int(0) [3]=> - int(5) -} -array(0) { + int(1) } -int(0) -int(1) -int(0) -int(1) -array(1) { +array(2) { [0]=> - int(1) + int(2) + [1]=> + int(3) } -int(1) -int(1) -int(1) -int(1) array(2) { [0]=> + int(5) + [1]=> + int(6) +} +int(0) +array(4) { + [0]=> int(1) [1]=> - int(3) + int(0) + [2]=> + int(1) + [3]=> + int(0) } +int(0) int(1) -int(2) -int(1) -int(1) -array(3) { +array(4) { [0]=> int(1) [1]=> - int(3) + int(1) [2]=> - int(3) + int(1) + [3]=> + int(0) } +int(0) int(2) -int(2) -int(1) -int(1) - -Notice: Undefined index: a in %s on line %d - -Notice: Undefined index: c in %s on line %d -array(2) { - ["b"]=> +int(0) +array(4) { + [0]=> + int(2) + [1]=> int(1) - ["d"]=> + [2]=> int(1) + [3]=> + int(0) } - -Notice: Undefined index: b in %s on line %d - -Notice: Undefined index: d in %s on line %d -array(2) { - ["a"]=> +int(0) +int(3) +int(0) +int(1) +array(4) { + [0]=> + int(2) + [1]=> int(1) - ["c"]=> + [2]=> int(1) + [3]=> + int(0) } diff --git a/Zend/tests/arg_unpack/dynamic.phpt b/Zend/tests/arg_unpack/dynamic.phpt index efed84d..8f129f8 100644 --- a/Zend/tests/arg_unpack/dynamic.phpt +++ b/Zend/tests/arg_unpack/dynamic.phpt @@ -9,7 +9,7 @@ $fn = function(...$args) { $fn(...[]); $fn(...[1, 2, 3]); -$fn(1, ...[2, 3], ...[], 4, 5); +$fn(1, ...[2, 3], ...[], ...[4, 5]); ?> --EXPECT-- diff --git a/Zend/tests/arg_unpack/invalid_type.phpt b/Zend/tests/arg_unpack/invalid_type.phpt index 3efffeb..1ef5455 100644 --- a/Zend/tests/arg_unpack/invalid_type.phpt +++ b/Zend/tests/arg_unpack/invalid_type.phpt @@ -12,7 +12,7 @@ test(...42); test(...new stdClass); test(1, 2, 3, ..."foo", ...[4, 5]); -test(1, 2, ...new StdClass, 3, ...3.14, ...[4, 5]); +test(1, 2, 3, ...new StdClass, ...3.14, ...[4, 5]); ?> --EXPECTF-- diff --git a/Zend/tests/arg_unpack/method.phpt b/Zend/tests/arg_unpack/method.phpt index d6a6e47..fb9ace8 100644 --- a/Zend/tests/arg_unpack/method.phpt +++ b/Zend/tests/arg_unpack/method.phpt @@ -14,8 +14,7 @@ class Foo { } $foo = new Foo; -$foo->test(...[1, 2], 3, 4, ...[], 5); -Foo::test2(1, 2, ...[3, 4], ...[], 5); +Foo::test2(1, 2, ...[3, 4], ...[], ...[5]); ?> --EXPECT-- @@ -31,15 +30,3 @@ array(5) { [4]=> int(5) } -array(5) { - [0]=> - int(1) - [1]=> - int(2) - [2]=> - int(3) - [3]=> - int(4) - [4]=> - int(5) -} diff --git a/Zend/tests/arg_unpack/new.phpt b/Zend/tests/arg_unpack/new.phpt index 3cf224f..7a0968e 100644 --- a/Zend/tests/arg_unpack/new.phpt +++ b/Zend/tests/arg_unpack/new.phpt @@ -11,7 +11,7 @@ class Foo { new Foo(...[]); new Foo(...[1, 2, 3]); -new Foo(...[1], 2, ...[], ...[3, 4], 5); +new Foo(...[1], ...[], ...[2, 3]); ?> --EXPECT-- @@ -25,15 +25,11 @@ array(3) { [2]=> int(3) } -array(5) { +array(3) { [0]=> int(1) [1]=> int(2) [2]=> int(3) - [3]=> - int(4) - [4]=> - int(5) } diff --git a/Zend/tests/arg_unpack/positional_arg_after_unpack_error.phpt b/Zend/tests/arg_unpack/positional_arg_after_unpack_error.phpt new file mode 100644 index 0000000..30e13e3 --- /dev/null +++ b/Zend/tests/arg_unpack/positional_arg_after_unpack_error.phpt @@ -0,0 +1,10 @@ +--TEST-- +Positional arguments cannot be used after argument unpacking +--FILE-- +<?php + +var_dump(...[1, 2, 3], 4); + +?> +--EXPECTF-- +Fatal error: Cannot use positional argument after argument unpacking in %s on line %d diff --git a/Zend/tests/arg_unpack/traversable_throwing_exception.phpt b/Zend/tests/arg_unpack/traversable_throwing_exception.phpt index 8ddc24d..abbf537 100644 --- a/Zend/tests/arg_unpack/traversable_throwing_exception.phpt +++ b/Zend/tests/arg_unpack/traversable_throwing_exception.phpt @@ -20,11 +20,11 @@ function gen() { } try { - test(1, 2, ...new Foo, 3, 4); + test(1, 2, ...new Foo, ...[3, 4]); } catch (Exception $e) { var_dump($e->getMessage()); } try { - test(1, 2, ...gen(), 3, 4); + test(1, 2, ...gen(), ...[3, 4]); } catch (Exception $e) { var_dump($e->getMessage()); } ?> diff --git a/Zend/tests/arg_unpack/traversable_with_by_ref_parameters.phpt b/Zend/tests/arg_unpack/traversable_with_by_ref_parameters.phpt index e862341..711287e 100644 --- a/Zend/tests/arg_unpack/traversable_with_by_ref_parameters.phpt +++ b/Zend/tests/arg_unpack/traversable_with_by_ref_parameters.phpt @@ -13,8 +13,6 @@ function gen($array) { } } -test(...gen([1, 2, 3]), $a); -var_dump($a); test(1, 2, 3, $b, ...gen([4, 5, 6])); var_dump($b); @@ -25,7 +23,6 @@ test(...gen([1, 2]), ...gen([3, 4])); ?> --EXPECTF-- int(42) -int(42) Warning: Cannot pass by-reference argument 4 of test() by unpacking a Traversable, passing by-value instead in %s on line %d diff --git a/Zend/zend_compile.c b/Zend/zend_compile.c index d53cb96..1bcd430 100644 --- a/Zend/zend_compile.c +++ b/Zend/zend_compile.c @@ -2614,6 +2614,11 @@ void zend_do_pass_param(znode *param, zend_uchar op TSRMLS_DC) /* {{{ */ function_ptr = fcall->fbc; fcall->arg_num++; + if (fcall->uses_argument_unpacking) { + zend_error_noreturn(E_COMPILE_ERROR, + "Cannot use positional argument after argument unpacking"); + } + if (original_op == ZEND_SEND_REF) { if (function_ptr && function_ptr->common.function_name && @@ -2717,6 +2722,7 @@ void zend_do_unpack_params(znode *params TSRMLS_DC) /* {{{ */ zend_function_call_entry *fcall; zend_stack_top(&CG(function_call_stack), (void **) &fcall); + fcall->uses_argument_unpacking = 1; if (fcall->fbc) { /* If argument unpacking is used argument numbers and sending modes can no longer be diff --git a/Zend/zend_compile.h b/Zend/zend_compile.h index 270c085..5362058 100644 --- a/Zend/zend_compile.h +++ b/Zend/zend_compile.h @@ -366,6 +366,7 @@ typedef struct _zend_function_state { typedef struct _zend_function_call_entry { zend_function *fbc; zend_uint arg_num; + zend_bool uses_argument_unpacking; } zend_function_call_entry; typedef struct _zend_switch_entry {