From 99a14b805e4d4d0a39b0b81afc8602495f07f414 Mon Sep 17 00:00:00 2001
From: Niels Dossche <7771979+nielsdos@users.noreply.github.com>
Date: Wed, 25 Dec 2024 18:44:51 +0100
Subject: [PATCH] Fix GH-16013 and bug #80857: Big endian issues

The FFI call return values follow widening rules.
We must widen to `ffi_arg` in the case we're handling a return value for types shorter than the machine width.
From http://www.chiark.greenend.org.uk/doc/libffi-dev/html/The-Closure-API.html:
> In most cases, ret points to an object of exactly the size of the type specified when cif was constructed.
> However, integral types narrower than the system register size are widened.
> In these cases your program may assume that ret points to an ffi_arg object.

If we don't do this, we get wrong values when reading the return values.

Closes GH-17255.

Co-authored-by: Dmitry Stogov <dmitry@zend.com>
---
 NEWS                       |   1 +
 ext/ffi/ffi.c              |  47 ++++++++++++-
 ext/ffi/tests/gh16013.phpt | 137 +++++++++++++++++++++++++++++++++++++
 ext/zend_test/test.c       |  35 ++++++++++
 4 files changed, 219 insertions(+), 1 deletion(-)
 create mode 100644 ext/ffi/tests/gh16013.phpt

diff --git a/NEWS b/NEWS
index 730a38b648b60..beaee119d3b26 100644
--- a/NEWS
+++ b/NEWS
@@ -28,6 +28,7 @@ PHP                                                                        NEWS
 - FFI:
   . Fixed bug #79075 (FFI header parser chokes on comments). (nielsdos)
   . Fix memory leak on ZEND_FFI_TYPE_CHAR conversion failure. (nielsdos)
+  . Fixed bug GH-16013 and bug #80857 (Big endian issues). (Dmitry, nielsdos)
 
 - Filter:
   . Fixed bug GH-16944 (Fix filtering special IPv4 and IPv6 ranges, by using
diff --git a/ext/ffi/ffi.c b/ext/ffi/ffi.c
index d823f32127cfb..f9ad84a192649 100644
--- a/ext/ffi/ffi.c
+++ b/ext/ffi/ffi.c
@@ -985,6 +985,27 @@ static void zend_ffi_callback_trampoline(ffi_cif* cif, void* ret, void** args, v
 	ret_type = ZEND_FFI_TYPE(callback_data->type->func.ret_type);
 	if (ret_type->kind != ZEND_FFI_TYPE_VOID) {
 		zend_ffi_zval_to_cdata(ret, ret_type, &retval);
+
+#ifdef WORDS_BIGENDIAN
+		if (ret_type->size < sizeof(ffi_arg)
+		 && ret_type->kind >= ZEND_FFI_TYPE_UINT8
+		 && ret_type->kind < ZEND_FFI_TYPE_POINTER) {
+			/* We need to widen the value (zero extend) */
+			switch (ret_type->size) {
+				case 1:
+					*(ffi_arg*)ret = *(uint8_t*)ret;
+					break;
+				case 2:
+					*(ffi_arg*)ret = *(uint16_t*)ret;
+					break;
+				case 4:
+					*(ffi_arg*)ret = *(uint32_t*)ret;
+					break;
+				default:
+					break;
+			}
+		}
+#endif
 	}
 
 	zval_ptr_dtor(&retval);
@@ -2855,7 +2876,31 @@ static ZEND_FUNCTION(ffi_trampoline) /* {{{ */
 	}
 
 	if (ZEND_FFI_TYPE(type->func.ret_type)->kind != ZEND_FFI_TYPE_VOID) {
-		zend_ffi_cdata_to_zval(NULL, ret, ZEND_FFI_TYPE(type->func.ret_type), BP_VAR_R, return_value, 0, 1, 0);
+		zend_ffi_type *func_ret_type = ZEND_FFI_TYPE(type->func.ret_type);
+
+#ifdef WORDS_BIGENDIAN
+		if (func_ret_type->size < sizeof(ffi_arg)
+		 && func_ret_type->kind >= ZEND_FFI_TYPE_UINT8
+		 && func_ret_type->kind < ZEND_FFI_TYPE_POINTER) {
+
+			/* We need to narrow the value (truncate) */
+			switch (func_ret_type->size) {
+				case 1:
+					*(uint8_t*)ret = *(ffi_arg*)ret;
+					break;
+				case 2:
+					*(uint16_t*)ret = *(ffi_arg*)ret;
+					break;
+				case 4:
+					*(uint32_t*)ret = *(ffi_arg*)ret;
+					break;
+				default:
+					break;
+			}
+		}
+#endif
+
+		zend_ffi_cdata_to_zval(NULL, ret, func_ret_type, BP_VAR_R, return_value, 0, 1, 0);
 	} else {
 		ZVAL_NULL(return_value);
 	}
diff --git a/ext/ffi/tests/gh16013.phpt b/ext/ffi/tests/gh16013.phpt
new file mode 100644
index 0000000000000..be57846cc42bc
--- /dev/null
+++ b/ext/ffi/tests/gh16013.phpt
@@ -0,0 +1,137 @@
+--TEST--
+GH-16013 (endianness issue with FFI)
+--EXTENSIONS--
+ffi
+zend_test
+--FILE--
+<?php
+require_once('utils.inc');
+
+$header = <<<HEADER
+enum bug_gh16013_enum {
+	BUG_GH16013_A = 1,
+	BUG_GH16013_B = 2,
+};
+struct bug_gh16013_int_struct {
+	int field;
+};
+struct bug_gh16013_callback_struct {
+	int8_t (*return_int8)(int8_t);
+	uint8_t (*return_uint8)(uint8_t);
+	int16_t (*return_int16)(int16_t);
+	uint16_t (*return_uint16)(uint16_t);
+	int32_t (*return_int32)(int32_t);
+	uint32_t (*return_uint32)(uint32_t);
+	float (*return_float)(float);
+	struct bug_gh16013_int_struct (*return_struct)(struct bug_gh16013_int_struct);
+	enum bug_gh16013_enum (*return_enum)(enum bug_gh16013_enum);
+};
+
+char bug_gh16013_return_char();
+bool bug_gh16013_return_bool();
+short bug_gh16013_return_short();
+int bug_gh16013_return_int();
+enum bug_gh16013_enum bug_gh16013_return_enum();
+struct bug_gh16013_int_struct bug_gh16013_return_struct();
+HEADER;
+
+if (PHP_OS_FAMILY !== 'Windows') {
+    $ffi = FFI::cdef($header);
+} else {
+    try {
+        $ffi = FFI::cdef($header, 'php_zend_test.dll');
+    } catch (FFI\Exception $ex) {
+        $ffi = FFI::cdef($header, ffi_get_php_dll_name());
+    }
+}
+
+echo "--- Return values ---\n";
+var_dump($ffi->bug_gh16013_return_char());
+var_dump($ffi->bug_gh16013_return_bool());
+var_dump($ffi->bug_gh16013_return_short());
+var_dump($ffi->bug_gh16013_return_int());
+var_dump($ffi->bug_gh16013_return_enum());
+var_dump($ffi->bug_gh16013_return_struct());
+
+echo "--- Callback values ---\n";
+$bug_gh16013_callback_struct = $ffi->new('struct bug_gh16013_callback_struct');
+$bug_gh16013_callback_struct->return_int8 = function($val) use($ffi) {
+    $cdata = $ffi->new('int8_t');
+    $cdata->cdata = $val;
+    return $cdata;
+};
+$bug_gh16013_callback_struct->return_uint8 = function($val) use($ffi) {
+    $cdata = $ffi->new('uint8_t');
+    $cdata->cdata = $val;
+    return $cdata;
+};
+$bug_gh16013_callback_struct->return_int16 = function($val) use($ffi) {
+    $cdata = $ffi->new('int16_t');
+    $cdata->cdata = $val;
+    return $cdata;
+};
+$bug_gh16013_callback_struct->return_uint16 = function($val) use($ffi) {
+    $cdata = $ffi->new('uint16_t');
+    $cdata->cdata = $val;
+    return $cdata;
+};
+$bug_gh16013_callback_struct->return_int32 = function($val) use($ffi) {
+    $cdata = $ffi->new('int32_t');
+    $cdata->cdata = $val;
+    return $cdata;
+};
+$bug_gh16013_callback_struct->return_uint32 = function($val) use($ffi) {
+    $cdata = $ffi->new('uint32_t');
+    $cdata->cdata = $val;
+    return $cdata;
+};
+$bug_gh16013_callback_struct->return_float = function($val) use($ffi) {
+    $cdata = $ffi->new('float');
+    $cdata->cdata = $val;
+    return $cdata;
+};
+$bug_gh16013_callback_struct->return_struct = function($val) use($ffi) {
+    return $val;
+};
+$bug_gh16013_callback_struct->return_enum = function($val) use($ffi) {
+    $cdata = $ffi->new('enum bug_gh16013_enum');
+    $cdata->cdata = $val;
+    return $cdata;
+};
+
+var_dump(($bug_gh16013_callback_struct->return_int8)(-4));
+var_dump(($bug_gh16013_callback_struct->return_uint8)(4));
+var_dump(($bug_gh16013_callback_struct->return_int16)(-10000));
+var_dump(($bug_gh16013_callback_struct->return_uint16)(10000));
+var_dump(($bug_gh16013_callback_struct->return_int32)(-100000));
+var_dump(($bug_gh16013_callback_struct->return_uint32)(100000));
+var_dump(($bug_gh16013_callback_struct->return_float)(12.34));
+$struct = $ffi->new('struct bug_gh16013_int_struct');
+$struct->field = 10;
+var_dump(($bug_gh16013_callback_struct->return_struct)($struct));
+var_dump(($bug_gh16013_callback_struct->return_enum)($ffi->BUG_GH16013_B));
+?>
+--EXPECT--
+--- Return values ---
+string(1) "A"
+bool(true)
+int(12345)
+int(123456789)
+int(2)
+object(FFI\CData:struct bug_gh16013_int_struct)#2 (1) {
+  ["field"]=>
+  int(123456789)
+}
+--- Callback values ---
+int(-4)
+int(4)
+int(-10000)
+int(10000)
+int(-100000)
+int(100000)
+float(12.34000015258789)
+object(FFI\CData:struct bug_gh16013_int_struct)#13 (1) {
+  ["field"]=>
+  int(10)
+}
+int(2)
diff --git a/ext/zend_test/test.c b/ext/zend_test/test.c
index 6d349317384e6..b2e2756381fdc 100644
--- a/ext/zend_test/test.c
+++ b/ext/zend_test/test.c
@@ -1449,6 +1449,41 @@ PHP_ZEND_TEST_API void bug_gh9090_void_int_char_var(int i, char *fmt, ...) {
 
 PHP_ZEND_TEST_API int gh11934b_ffi_var_test_cdata;
 
+enum bug_gh16013_enum {
+	BUG_GH16013_A = 1,
+	BUG_GH16013_B = 2,
+};
+
+struct bug_gh16013_int_struct {
+	int field;
+};
+
+PHP_ZEND_TEST_API char bug_gh16013_return_char(void) {
+	return 'A';
+}
+
+PHP_ZEND_TEST_API bool bug_gh16013_return_bool(void) {
+	return true;
+}
+
+PHP_ZEND_TEST_API short bug_gh16013_return_short(void) {
+	return 12345;
+}
+
+PHP_ZEND_TEST_API int bug_gh16013_return_int(void) {
+	return 123456789;
+}
+
+PHP_ZEND_TEST_API enum bug_gh16013_enum bug_gh16013_return_enum(void) {
+	return BUG_GH16013_B;
+}
+
+PHP_ZEND_TEST_API struct bug_gh16013_int_struct bug_gh16013_return_struct(void) {
+	struct bug_gh16013_int_struct ret;
+	ret.field = 123456789;
+	return ret;
+}
+
 #ifdef HAVE_COPY_FILE_RANGE
 /**
  * This function allows us to simulate early return of copy_file_range by setting the limit_copy_file_range ini setting.