Fix GH-16957: Assertion failure in array_shift with self-referencing array

We have an RC1 violation because we're immediately dereferencing and
copying the resulting array in the test case. Instead, transfer the
lifetime using RETVAL_COPY_VALUE and unwrap only after the internal
iterator is reset.

Closes GH-16970.

Author: nielsdos

NEWS

@@ -76,6 +76,8 @@ PHP                                                                        NEWS
 - Standard:
   . Fixed bug GH-16905 (Internal iterator functions can't handle UNDEF
     properties). (nielsdos)
+  . Fixed bug GH-16957 (Assertion failure in array_shift with
+    self-referencing array). (nielsdos)
 
 - Streams:
   . Fixed network connect poll interuption handling. (Jakub Zelenka)

ext/standard/array.c

@@ -3511,7 +3511,8 @@ PHP_FUNCTION(array_shift)
 			}
 			idx++;
 		}
-		RETVAL_COPY_DEREF(val);
+		RETVAL_COPY_VALUE(val);
+		ZVAL_UNDEF(val);
 
 		/* Delete the first value */
 		zend_hash_packed_del_val(Z_ARRVAL_P(stack), val);
@@ -3565,7 +3566,8 @@ PHP_FUNCTION(array_shift)
 			}
 			idx++;
 		}
-		RETVAL_COPY_DEREF(val);
+		RETVAL_COPY_VALUE(val);
+		ZVAL_UNDEF(val);
 
 		/* Delete the first value */
 		zend_hash_del_bucket(Z_ARRVAL_P(stack), p);
@@ -3589,6 +3591,10 @@ PHP_FUNCTION(array_shift)
 	}
 
 	zend_hash_internal_pointer_reset(Z_ARRVAL_P(stack));
+
+	if (Z_ISREF_P(return_value)) {
+		zend_unwrap_reference(return_value);
+	}
 }
 /* }}} */
 

ext/standard/tests/array/gh16957.phpt

@@ -0,0 +1,41 @@
+--TEST--
+GH-16957 (Assertion failure in array_shift with self-referencing array)
+--FILE--
+<?php
+$new_array = array(&$new_array, 1, 'two');
+var_dump($shifted = array_shift($new_array));
+var_dump($new_array);
+var_dump($new_array === $shifted);
+
+$new_array2 = array(&$new_array2, 2 => 1, 300 => 'two');
+var_dump($shifted = array_shift($new_array2));
+var_dump($new_array2);
+var_dump($new_array2 === $shifted);
+?>
+--EXPECT--
+array(2) {
+  [0]=>
+  int(1)
+  [1]=>
+  string(3) "two"
+}
+array(2) {
+  [0]=>
+  int(1)
+  [1]=>
+  string(3) "two"
+}
+bool(true)
+array(2) {
+  [0]=>
+  int(1)
+  [1]=>
+  string(3) "two"
+}
+array(2) {
+  [0]=>
+  int(1)
+  [1]=>
+  string(3) "two"
+}
+bool(true)