From: Dmitry Stogov <dmitry@zend.com>
Date: Tue, 17 Oct 2017 10:25:19 +0000 (+0300)
Subject: Revert "Allow internal functions to return values by reference (this was disabled... 
X-Git-Tag: php-7.3.0alpha1~1257
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=61ef91bf0dc8a3b6e88e20c37511fd19f98db809;p=php

Revert "Allow internal functions to return values by reference (this was disabled in implementation of DO_ICALL, but enabled in DO_FCALL)."

This reverts commit da781a5ac2cb9d2f983ef9fe070900664db12c67.
---

diff --git a/Zend/zend_vm_def.h b/Zend/zend_vm_def.h
index c8194e8f5b..b8f0aa1f34 100644
--- a/Zend/zend_vm_def.h
+++ b/Zend/zend_vm_def.h
@@ -3480,12 +3480,11 @@ ZEND_VM_HOT_HANDLER(129, ZEND_DO_ICALL, ANY, ANY, SPEC(RETVAL))
 	fbc->internal_function.handler(call, ret);
 
 #if ZEND_DEBUG
-	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));
-	}
+	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));
 #endif
 
 	EG(current_execute_data) = execute_data;
@@ -3583,8 +3582,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));
+			ZEND_ASSERT((call->func->common.fn_flags & ZEND_ACC_RETURN_REFERENCE)
+				? Z_ISREF_P(ret) : !Z_ISREF_P(ret));
 		}
 #endif
 
@@ -3677,8 +3676,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));
+			ZEND_ASSERT((call->func->common.fn_flags & ZEND_ACC_RETURN_REFERENCE)
+				? Z_ISREF_P(ret) : !Z_ISREF_P(ret));
 		}
 #endif
 
@@ -7838,12 +7837,10 @@ ZEND_VM_HANDLER(158, ZEND_CALL_TRAMPOLINE, ANY, ANY)
 		}
 
 #if ZEND_DEBUG
-		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));
-		}
+		ZEND_ASSERT(
+			EG(exception) || !call->func ||
+			!(call->func->common.fn_flags & ZEND_ACC_HAS_RETURN_TYPE) ||
+			zend_verify_internal_return_type(call->func, 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 b290c5036e..9d99daf986 100644
--- a/Zend/zend_vm_execute.h
+++ b/Zend/zend_vm_execute.h
@@ -573,12 +573,11 @@ static ZEND_VM_HOT ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_DO_ICALL_SPEC_RETV
 	fbc->internal_function.handler(call, ret);
 
 #if ZEND_DEBUG
-	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));
-	}
+	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));
 #endif
 
 	EG(current_execute_data) = execute_data;
@@ -618,12 +617,11 @@ static ZEND_VM_HOT ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_DO_ICALL_SPEC_RETV
 	fbc->internal_function.handler(call, ret);
 
 #if ZEND_DEBUG
-	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));
-	}
+	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));
 #endif
 
 	EG(current_execute_data) = execute_data;
@@ -743,8 +741,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));
+			ZEND_ASSERT((call->func->common.fn_flags & ZEND_ACC_RETURN_REFERENCE)
+				? Z_ISREF_P(ret) : !Z_ISREF_P(ret));
 		}
 #endif
 
@@ -821,8 +819,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));
+			ZEND_ASSERT((call->func->common.fn_flags & ZEND_ACC_RETURN_REFERENCE)
+				? Z_ISREF_P(ret) : !Z_ISREF_P(ret));
 		}
 #endif
 
@@ -915,8 +913,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));
+			ZEND_ASSERT((call->func->common.fn_flags & ZEND_ACC_RETURN_REFERENCE)
+				? Z_ISREF_P(ret) : !Z_ISREF_P(ret));
 		}
 #endif
 
@@ -1040,8 +1038,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));
+			ZEND_ASSERT((call->func->common.fn_flags & ZEND_ACC_RETURN_REFERENCE)
+				? Z_ISREF_P(ret) : !Z_ISREF_P(ret));
 		}
 #endif
 
@@ -2034,12 +2032,10 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_CALL_TRAMPOLINE_SPEC_HANDLER(Z
 		}
 
 #if ZEND_DEBUG
-		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));
-		}
+		ZEND_ASSERT(
+			EG(exception) || !call->func ||
+			!(call->func->common.fn_flags & ZEND_ACC_HAS_RETURN_TYPE) ||
+			zend_verify_internal_return_type(call->func, 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 e1f1580703..b733430a38 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 5090363f82..957d53864e 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 | MAY_BE_REF),
+	FN("unserialize",                  UNKNOWN_INFO),
 	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 7bb579c692..93d469168b 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, 1, 1)
+ZEND_BEGIN_ARG_INFO_EX(arginfo_unserialize, 0, 0, 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 f8406d8988..e28afe9177 100644
--- a/ext/standard/var.c
+++ b/ext/standard/var.c
@@ -1124,6 +1124,13 @@ 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);
+	}
 }
 /* }}} */