From: Dmitry Stogov <dmitry@zend.com>
Date: Tue, 17 Oct 2017 08:50:34 +0000 (+0300)
Subject: Allow internal functions to return values by reference (this was disabled in implemen... 
X-Git-Tag: php-7.3.0alpha1~1258
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=da781a5ac2cb9d2f983ef9fe070900664db12c67;p=php

Allow internal functions to return values by reference (this was disabled in implementation of DO_ICALL, but enabled in DO_FCALL).
However, don't require internal functions returning by reference to return a reference.
Mark unserialize() as returning by reference and remove unwrap_reference hack, to allow proper returning of self referenced arrays using a reference.
Currently unserialize() is the only internal function that may return a reference.
---

diff --git a/Zend/zend_vm_def.h b/Zend/zend_vm_def.h
index b8f0aa1f34..c8194e8f5b 100644
--- a/Zend/zend_vm_def.h
+++ b/Zend/zend_vm_def.h
@@ -3480,11 +3480,12 @@ ZEND_VM_HOT_HANDLER(129, ZEND_DO_ICALL, ANY, ANY, SPEC(RETVAL))
 	fbc->internal_function.handler(call, ret);
 
 #if ZEND_DEBUG
-	ZEND_ASSERT(
-		EG(exception) || !call->func ||
-		!(call->func->common.fn_flags & ZEND_ACC_HAS_RETURN_TYPE) ||
-		zend_verify_internal_return_type(call->func, ret));
-	ZEND_ASSERT(!Z_ISREF_P(ret));
+	if (!EG(exception) && call->func) {
+		ZEND_ASSERT(!(call->func->common.fn_flags & ZEND_ACC_HAS_RETURN_TYPE) ||
+			zend_verify_internal_return_type(call->func, ret));
+		ZEND_ASSERT((call->func->common.fn_flags & ZEND_ACC_RETURN_REFERENCE) ||
+			!Z_ISREF_P(ret));
+	}
 #endif
 
 	EG(current_execute_data) = execute_data;
@@ -3582,8 +3583,8 @@ ZEND_VM_HOT_HANDLER(131, ZEND_DO_FCALL_BY_NAME, ANY, ANY, SPEC(RETVAL))
 		if (!EG(exception) && call->func) {
 			ZEND_ASSERT(!(call->func->common.fn_flags & ZEND_ACC_HAS_RETURN_TYPE) ||
 				zend_verify_internal_return_type(call->func, ret));
-			ZEND_ASSERT((call->func->common.fn_flags & ZEND_ACC_RETURN_REFERENCE)
-				? Z_ISREF_P(ret) : !Z_ISREF_P(ret));
+			ZEND_ASSERT((call->func->common.fn_flags & ZEND_ACC_RETURN_REFERENCE) ||
+				!Z_ISREF_P(ret));
 		}
 #endif
 
@@ -3676,8 +3677,8 @@ ZEND_VM_HOT_HANDLER(60, ZEND_DO_FCALL, ANY, ANY, SPEC(RETVAL))
 		if (!EG(exception) && call->func) {
 			ZEND_ASSERT(!(call->func->common.fn_flags & ZEND_ACC_HAS_RETURN_TYPE) ||
 				zend_verify_internal_return_type(call->func, ret));
-			ZEND_ASSERT((call->func->common.fn_flags & ZEND_ACC_RETURN_REFERENCE)
-				? Z_ISREF_P(ret) : !Z_ISREF_P(ret));
+			ZEND_ASSERT((call->func->common.fn_flags & ZEND_ACC_RETURN_REFERENCE) ||
+				!Z_ISREF_P(ret));
 		}
 #endif
 
@@ -7837,10 +7838,12 @@ ZEND_VM_HANDLER(158, ZEND_CALL_TRAMPOLINE, ANY, ANY)
 		}
 
 #if ZEND_DEBUG
-		ZEND_ASSERT(
-			EG(exception) || !call->func ||
-			!(call->func->common.fn_flags & ZEND_ACC_HAS_RETURN_TYPE) ||
-			zend_verify_internal_return_type(call->func, ret));
+		if (!EG(exception) && call->func) {
+			ZEND_ASSERT(!(call->func->common.fn_flags & ZEND_ACC_HAS_RETURN_TYPE) ||
+				zend_verify_internal_return_type(call->func, ret));
+			ZEND_ASSERT((call->func->common.fn_flags & ZEND_ACC_RETURN_REFERENCE) ||
+				!Z_ISREF_P(ret));
+		}
 #endif
 
 		EG(current_execute_data) = call->prev_execute_data;
diff --git a/Zend/zend_vm_execute.h b/Zend/zend_vm_execute.h
index 9d99daf986..b290c5036e 100644
--- a/Zend/zend_vm_execute.h
+++ b/Zend/zend_vm_execute.h
@@ -573,11 +573,12 @@ static ZEND_VM_HOT ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_DO_ICALL_SPEC_RETV
 	fbc->internal_function.handler(call, ret);
 
 #if ZEND_DEBUG
-	ZEND_ASSERT(
-		EG(exception) || !call->func ||
-		!(call->func->common.fn_flags & ZEND_ACC_HAS_RETURN_TYPE) ||
-		zend_verify_internal_return_type(call->func, ret));
-	ZEND_ASSERT(!Z_ISREF_P(ret));
+	if (!EG(exception) && call->func) {
+		ZEND_ASSERT(!(call->func->common.fn_flags & ZEND_ACC_HAS_RETURN_TYPE) ||
+			zend_verify_internal_return_type(call->func, ret));
+		ZEND_ASSERT((call->func->common.fn_flags & ZEND_ACC_RETURN_REFERENCE) ||
+			!Z_ISREF_P(ret));
+	}
 #endif
 
 	EG(current_execute_data) = execute_data;
@@ -617,11 +618,12 @@ static ZEND_VM_HOT ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_DO_ICALL_SPEC_RETV
 	fbc->internal_function.handler(call, ret);
 
 #if ZEND_DEBUG
-	ZEND_ASSERT(
-		EG(exception) || !call->func ||
-		!(call->func->common.fn_flags & ZEND_ACC_HAS_RETURN_TYPE) ||
-		zend_verify_internal_return_type(call->func, ret));
-	ZEND_ASSERT(!Z_ISREF_P(ret));
+	if (!EG(exception) && call->func) {
+		ZEND_ASSERT(!(call->func->common.fn_flags & ZEND_ACC_HAS_RETURN_TYPE) ||
+			zend_verify_internal_return_type(call->func, ret));
+		ZEND_ASSERT((call->func->common.fn_flags & ZEND_ACC_RETURN_REFERENCE) ||
+			!Z_ISREF_P(ret));
+	}
 #endif
 
 	EG(current_execute_data) = execute_data;
@@ -741,8 +743,8 @@ static ZEND_VM_HOT ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_DO_FCALL_BY_NAME_S
 		if (!EG(exception) && call->func) {
 			ZEND_ASSERT(!(call->func->common.fn_flags & ZEND_ACC_HAS_RETURN_TYPE) ||
 				zend_verify_internal_return_type(call->func, ret));
-			ZEND_ASSERT((call->func->common.fn_flags & ZEND_ACC_RETURN_REFERENCE)
-				? Z_ISREF_P(ret) : !Z_ISREF_P(ret));
+			ZEND_ASSERT((call->func->common.fn_flags & ZEND_ACC_RETURN_REFERENCE) ||
+				!Z_ISREF_P(ret));
 		}
 #endif
 
@@ -819,8 +821,8 @@ static ZEND_VM_HOT ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_DO_FCALL_BY_NAME_S
 		if (!EG(exception) && call->func) {
 			ZEND_ASSERT(!(call->func->common.fn_flags & ZEND_ACC_HAS_RETURN_TYPE) ||
 				zend_verify_internal_return_type(call->func, ret));
-			ZEND_ASSERT((call->func->common.fn_flags & ZEND_ACC_RETURN_REFERENCE)
-				? Z_ISREF_P(ret) : !Z_ISREF_P(ret));
+			ZEND_ASSERT((call->func->common.fn_flags & ZEND_ACC_RETURN_REFERENCE) ||
+				!Z_ISREF_P(ret));
 		}
 #endif
 
@@ -913,8 +915,8 @@ static ZEND_VM_HOT ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_DO_FCALL_SPEC_RETV
 		if (!EG(exception) && call->func) {
 			ZEND_ASSERT(!(call->func->common.fn_flags & ZEND_ACC_HAS_RETURN_TYPE) ||
 				zend_verify_internal_return_type(call->func, ret));
-			ZEND_ASSERT((call->func->common.fn_flags & ZEND_ACC_RETURN_REFERENCE)
-				? Z_ISREF_P(ret) : !Z_ISREF_P(ret));
+			ZEND_ASSERT((call->func->common.fn_flags & ZEND_ACC_RETURN_REFERENCE) ||
+				!Z_ISREF_P(ret));
 		}
 #endif
 
@@ -1038,8 +1040,8 @@ static ZEND_VM_HOT ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_DO_FCALL_SPEC_RETV
 		if (!EG(exception) && call->func) {
 			ZEND_ASSERT(!(call->func->common.fn_flags & ZEND_ACC_HAS_RETURN_TYPE) ||
 				zend_verify_internal_return_type(call->func, ret));
-			ZEND_ASSERT((call->func->common.fn_flags & ZEND_ACC_RETURN_REFERENCE)
-				? Z_ISREF_P(ret) : !Z_ISREF_P(ret));
+			ZEND_ASSERT((call->func->common.fn_flags & ZEND_ACC_RETURN_REFERENCE) ||
+				!Z_ISREF_P(ret));
 		}
 #endif
 
@@ -2032,10 +2034,12 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_CALL_TRAMPOLINE_SPEC_HANDLER(Z
 		}
 
 #if ZEND_DEBUG
-		ZEND_ASSERT(
-			EG(exception) || !call->func ||
-			!(call->func->common.fn_flags & ZEND_ACC_HAS_RETURN_TYPE) ||
-			zend_verify_internal_return_type(call->func, ret));
+		if (!EG(exception) && call->func) {
+			ZEND_ASSERT(!(call->func->common.fn_flags & ZEND_ACC_HAS_RETURN_TYPE) ||
+				zend_verify_internal_return_type(call->func, ret));
+			ZEND_ASSERT((call->func->common.fn_flags & ZEND_ACC_RETURN_REFERENCE) ||
+				!Z_ISREF_P(ret));
+		}
 #endif
 
 		EG(current_execute_data) = call->prev_execute_data;
diff --git a/ext/gmp/tests/unserialize_with_reference.phpt b/ext/gmp/tests/unserialize_with_reference.phpt
index b733430a38..e1f1580703 100644
--- a/ext/gmp/tests/unserialize_with_reference.phpt
+++ b/ext/gmp/tests/unserialize_with_reference.phpt
@@ -2,7 +2,7 @@
 Unserialize GMP instance with internal reference to itself
 --FILE--
 <?php
-$s = 'C:3:"GMP":23:{s:1:"2";a:1:{i:46;R:1;}}';
+$s = 'C:3:"GMP":23:{s:1:"2";a:1:{i:46;r:1;}}';
 var_dump(unserialize($s));
 ?>
 --EXPECT--
diff --git a/ext/opcache/Optimizer/zend_func_info.c b/ext/opcache/Optimizer/zend_func_info.c
index 957d53864e..5090363f82 100644
--- a/ext/opcache/Optimizer/zend_func_info.c
+++ b/ext/opcache/Optimizer/zend_func_info.c
@@ -531,7 +531,7 @@ static const func_info_t func_infos[] = {
 	FN("forward_static_call",          UNKNOWN_INFO),
 	FN("forward_static_call_array",    UNKNOWN_INFO),
 	F1("serialize",                    MAY_BE_NULL | MAY_BE_FALSE | MAY_BE_STRING),
-	FN("unserialize",                  UNKNOWN_INFO),
+	FN("unserialize",                  UNKNOWN_INFO | MAY_BE_REF),
 	F0("var_dump",                     MAY_BE_NULL),
 	F1("var_export",                   MAY_BE_NULL | MAY_BE_STRING),
 	F0("debug_zval_dump",              MAY_BE_NULL),
diff --git a/ext/standard/basic_functions.c b/ext/standard/basic_functions.c
index 93d469168b..7bb579c692 100644
--- a/ext/standard/basic_functions.c
+++ b/ext/standard/basic_functions.c
@@ -2669,7 +2669,7 @@ ZEND_BEGIN_ARG_INFO(arginfo_serialize, 0)
 	ZEND_ARG_INFO(0, var)
 ZEND_END_ARG_INFO()
 
-ZEND_BEGIN_ARG_INFO_EX(arginfo_unserialize, 0, 0, 1)
+ZEND_BEGIN_ARG_INFO_EX(arginfo_unserialize, 0, 1, 1)
 	ZEND_ARG_INFO(0, variable_representation)
 	ZEND_ARG_INFO(0, allowed_classes)
 ZEND_END_ARG_INFO()
diff --git a/ext/standard/var.c b/ext/standard/var.c
index e28afe9177..f8406d8988 100644
--- a/ext/standard/var.c
+++ b/ext/standard/var.c
@@ -1124,13 +1124,6 @@ PHP_FUNCTION(unserialize)
 	/* Reset to previous allowed_classes in case this is a nested call */
 	php_var_unserialize_set_allowed_classes(var_hash, prev_class_hash);
 	PHP_VAR_UNSERIALIZE_DESTROY(var_hash);
-
-	/* Per calling convention we must not return a reference here, so unwrap. We're doing this at
-	 * the very end, because __wakeup() calls performed during UNSERIALIZE_DESTROY might affect
-	 * the value we unwrap here. This is compatible with behavior in PHP <=7.0. */
-	if (Z_ISREF_P(return_value)) {
-		zend_unwrap_reference(return_value);
-	}
 }
 /* }}} */