Address review: clarify ordering comment + add unset variant test

Author: lacatoire

Zend/tests/gh20482_assign_dim_uaf_output_handler.phpt

@@ -1,5 +1,5 @@
 --TEST--
-GH-20482: Heap use-after-free in ZEND_ASSIGN_DIM when an output handler clobbers the array during the undefined-variable warning
+GH-20482: heap use-after-free in ZEND_ASSIGN_DIM when an output handler clobbers the array during the undefined-variable warning
 --FILE--
 <?php
 $a = [1, 2, 3];
@@ -21,7 +21,6 @@ ob_end_clean();
 var_dump($a);
 echo "ok\n";
 ?>
---EXPECTF--
-%AWarning: Undefined variable $undef in %s on line %d
+--EXPECT--
 string(5) "freed"
 ok

Zend/tests/gh20482_assign_dim_uaf_output_handler_unset.phpt

@@ -0,0 +1,21 @@
+--TEST--
+GH-20482: heap use-after-free in ZEND_ASSIGN_DIM when the output handler unsets the array during the undefined-variable warning
+--FILE--
+<?php
+$a = [1, 2, 3];
+
+ob_start(function () use (&$a) {
+    // Drop the last reference via unset, freeing the original array.
+    unset($a);
+    return '';
+}, 1);
+
+$a[0] = $undef;
+
+ob_end_clean();
+var_dump($a ?? null);
+echo "ok\n";
+?>
+--EXPECT--
+NULL
+ok

Zend/zend_vm_def.h

@@ -2732,14 +2732,18 @@ ZEND_VM_C_LABEL(try_assign_dim_array):
 		} else {
 			HashTable *ht = Z_ARRVAL_P(object_ptr);
 			dim = GET_OP2_ZVAL_PTR_UNDEF(BP_VAR_R);
+			/* Order is critical: fetch the data value (which may emit an
+			 * "Undefined variable" warning that can recurse into user code
+			 * via a user output handler) before resolving variable_ptr.
+			 * Resolving variable_ptr only afterwards guarantees a fresh
+			 * bucket pointer that cannot have been invalidated by user
+			 * code freeing the array, rehashing it, or unsetting keys. */
 			value = GET_OP_DATA_ZVAL_PTR_UNDEF(BP_VAR_R);
 			if (OP_DATA_TYPE == IS_CV && UNEXPECTED(Z_TYPE_P(value) == IS_UNDEF)) {
-				/* The undefined-variable warning may recurse into user code
-				 * (e.g. a user output handler) and clobber or destroy the
-				 * array we are about to write to. Temporarily addref the
-				 * array to detect destruction, and refetch the dim address
-				 * after the warning since the previous variable_ptr may have
-				 * been invalidated by hash table mutations. */
+				/* Temporarily addref the array around the warning so we
+				 * can detect a full destruction (last ref dropped by
+				 * reentrant user code) and bail out cleanly. Mirrors the
+				 * IS_UNUSED branch above. */
 				if (!(GC_FLAGS(ht) & IS_ARRAY_IMMUTABLE)) {
 					GC_ADDREF(ht);
 				}