Skip to content

Commit 99a14b8

Browse files
ndosschedstogov
ndossche
and
dstogov
committed
Fix phpGH-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 phpGH-17255. Co-authored-by: Dmitry Stogov <[email protected]>
1 parent 643a77d commit 99a14b8

File tree

4 files changed

+219
-1
lines changed

4 files changed

+219
-1
lines changed

‎NEWS‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ PHP NEWS
2828
- FFI:
2929
. Fixed bug #79075 (FFI header parser chokes on comments). (nielsdos)
3030
. Fix memory leak on ZEND_FFI_TYPE_CHAR conversion failure. (nielsdos)
31+
. Fixed bug GH-16013 and bug #80857 (Big endian issues). (Dmitry, nielsdos)
3132

3233
- Filter:
3334
. Fixed bug GH-16944 (Fix filtering special IPv4 and IPv6 ranges, by using

‎ext/ffi/ffi.c‎

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -985,6 +985,27 @@ static void zend_ffi_callback_trampoline(ffi_cif* cif, void* ret, void** args, v
985985
ret_type = ZEND_FFI_TYPE(callback_data->type->func.ret_type);
986986
if (ret_type->kind != ZEND_FFI_TYPE_VOID) {
987987
zend_ffi_zval_to_cdata(ret, ret_type, &retval);
988+
989+
#ifdef WORDS_BIGENDIAN
990+
if (ret_type->size < sizeof(ffi_arg)
991+
&& ret_type->kind >= ZEND_FFI_TYPE_UINT8
992+
&& ret_type->kind < ZEND_FFI_TYPE_POINTER) {
993+
/* We need to widen the value (zero extend) */
994+
switch (ret_type->size) {
995+
case 1:
996+
*(ffi_arg*)ret = *(uint8_t*)ret;
997+
break;
998+
case 2:
999+
*(ffi_arg*)ret = *(uint16_t*)ret;
1000+
break;
1001+
case 4:
1002+
*(ffi_arg*)ret = *(uint32_t*)ret;
1003+
break;
1004+
default:
1005+
break;
1006+
}
1007+
}
1008+
#endif
9881009
}
9891010

9901011
zval_ptr_dtor(&retval);
@@ -2855,7 +2876,31 @@ static ZEND_FUNCTION(ffi_trampoline) /* {{{ */
28552876
}
28562877

28572878
if (ZEND_FFI_TYPE(type->func.ret_type)->kind != ZEND_FFI_TYPE_VOID) {
2858-
zend_ffi_cdata_to_zval(NULL, ret, ZEND_FFI_TYPE(type->func.ret_type), BP_VAR_R, return_value, 0, 1, 0);
2879+
zend_ffi_type *func_ret_type = ZEND_FFI_TYPE(type->func.ret_type);
2880+
2881+
#ifdef WORDS_BIGENDIAN
2882+
if (func_ret_type->size < sizeof(ffi_arg)
2883+
&& func_ret_type->kind >= ZEND_FFI_TYPE_UINT8
2884+
&& func_ret_type->kind < ZEND_FFI_TYPE_POINTER) {
2885+
2886+
/* We need to narrow the value (truncate) */
2887+
switch (func_ret_type->size) {
2888+
case 1:
2889+
*(uint8_t*)ret = *(ffi_arg*)ret;
2890+
break;
2891+
case 2:
2892+
*(uint16_t*)ret = *(ffi_arg*)ret;
2893+
break;
2894+
case 4:
2895+
*(uint32_t*)ret = *(ffi_arg*)ret;
2896+
break;
2897+
default:
2898+
break;
2899+
}
2900+
}
2901+
#endif
2902+
2903+
zend_ffi_cdata_to_zval(NULL, ret, func_ret_type, BP_VAR_R, return_value, 0, 1, 0);
28592904
} else {
28602905
ZVAL_NULL(return_value);
28612906
}

‎ext/ffi/tests/gh16013.phpt‎

Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,137 @@
1+
--TEST--
2+
GH-16013 (endianness issue with FFI)
3+
--EXTENSIONS--
4+
ffi
5+
zend_test
6+
--FILE--
7+
<?php
8+
require_once('utils.inc');
9+
10+
$header = <<<HEADER
11+
enum bug_gh16013_enum {
12+
BUG_GH16013_A = 1,
13+
BUG_GH16013_B = 2,
14+
};
15+
struct bug_gh16013_int_struct {
16+
int field;
17+
};
18+
struct bug_gh16013_callback_struct {
19+
int8_t (*return_int8)(int8_t);
20+
uint8_t (*return_uint8)(uint8_t);
21+
int16_t (*return_int16)(int16_t);
22+
uint16_t (*return_uint16)(uint16_t);
23+
int32_t (*return_int32)(int32_t);
24+
uint32_t (*return_uint32)(uint32_t);
25+
float (*return_float)(float);
26+
struct bug_gh16013_int_struct (*return_struct)(struct bug_gh16013_int_struct);
27+
enum bug_gh16013_enum (*return_enum)(enum bug_gh16013_enum);
28+
};
29+
30+
char bug_gh16013_return_char();
31+
bool bug_gh16013_return_bool();
32+
short bug_gh16013_return_short();
33+
int bug_gh16013_return_int();
34+
enum bug_gh16013_enum bug_gh16013_return_enum();
35+
struct bug_gh16013_int_struct bug_gh16013_return_struct();
36+
HEADER;
37+
38+
if (PHP_OS_FAMILY !== 'Windows') {
39+
$ffi = FFI::cdef($header);
40+
} else {
41+
try {
42+
$ffi = FFI::cdef($header, 'php_zend_test.dll');
43+
} catch (FFI\Exception $ex) {
44+
$ffi = FFI::cdef($header, ffi_get_php_dll_name());
45+
}
46+
}
47+
48+
echo "--- Return values ---\n";
49+
var_dump($ffi->bug_gh16013_return_char());
50+
var_dump($ffi->bug_gh16013_return_bool());
51+
var_dump($ffi->bug_gh16013_return_short());
52+
var_dump($ffi->bug_gh16013_return_int());
53+
var_dump($ffi->bug_gh16013_return_enum());
54+
var_dump($ffi->bug_gh16013_return_struct());
55+
56+
echo "--- Callback values ---\n";
57+
$bug_gh16013_callback_struct = $ffi->new('struct bug_gh16013_callback_struct');
58+
$bug_gh16013_callback_struct->return_int8 = function($val) use($ffi) {
59+
$cdata = $ffi->new('int8_t');
60+
$cdata->cdata = $val;
61+
return $cdata;
62+
};
63+
$bug_gh16013_callback_struct->return_uint8 = function($val) use($ffi) {
64+
$cdata = $ffi->new('uint8_t');
65+
$cdata->cdata = $val;
66+
return $cdata;
67+
};
68+
$bug_gh16013_callback_struct->return_int16 = function($val) use($ffi) {
69+
$cdata = $ffi->new('int16_t');
70+
$cdata->cdata = $val;
71+
return $cdata;
72+
};
73+
$bug_gh16013_callback_struct->return_uint16 = function($val) use($ffi) {
74+
$cdata = $ffi->new('uint16_t');
75+
$cdata->cdata = $val;
76+
return $cdata;
77+
};
78+
$bug_gh16013_callback_struct->return_int32 = function($val) use($ffi) {
79+
$cdata = $ffi->new('int32_t');
80+
$cdata->cdata = $val;
81+
return $cdata;
82+
};
83+
$bug_gh16013_callback_struct->return_uint32 = function($val) use($ffi) {
84+
$cdata = $ffi->new('uint32_t');
85+
$cdata->cdata = $val;
86+
return $cdata;
87+
};
88+
$bug_gh16013_callback_struct->return_float = function($val) use($ffi) {
89+
$cdata = $ffi->new('float');
90+
$cdata->cdata = $val;
91+
return $cdata;
92+
};
93+
$bug_gh16013_callback_struct->return_struct = function($val) use($ffi) {
94+
return $val;
95+
};
96+
$bug_gh16013_callback_struct->return_enum = function($val) use($ffi) {
97+
$cdata = $ffi->new('enum bug_gh16013_enum');
98+
$cdata->cdata = $val;
99+
return $cdata;
100+
};
101+
102+
var_dump(($bug_gh16013_callback_struct->return_int8)(-4));
103+
var_dump(($bug_gh16013_callback_struct->return_uint8)(4));
104+
var_dump(($bug_gh16013_callback_struct->return_int16)(-10000));
105+
var_dump(($bug_gh16013_callback_struct->return_uint16)(10000));
106+
var_dump(($bug_gh16013_callback_struct->return_int32)(-100000));
107+
var_dump(($bug_gh16013_callback_struct->return_uint32)(100000));
108+
var_dump(($bug_gh16013_callback_struct->return_float)(12.34));
109+
$struct = $ffi->new('struct bug_gh16013_int_struct');
110+
$struct->field = 10;
111+
var_dump(($bug_gh16013_callback_struct->return_struct)($struct));
112+
var_dump(($bug_gh16013_callback_struct->return_enum)($ffi->BUG_GH16013_B));
113+
?>
114+
--EXPECT--
115+
--- Return values ---
116+
string(1) "A"
117+
bool(true)
118+
int(12345)
119+
int(123456789)
120+
int(2)
121+
object(FFI\CData:struct bug_gh16013_int_struct)#2 (1) {
122+
["field"]=>
123+
int(123456789)
124+
}
125+
--- Callback values ---
126+
int(-4)
127+
int(4)
128+
int(-10000)
129+
int(10000)
130+
int(-100000)
131+
int(100000)
132+
float(12.34000015258789)
133+
object(FFI\CData:struct bug_gh16013_int_struct)#13 (1) {
134+
["field"]=>
135+
int(10)
136+
}
137+
int(2)

‎ext/zend_test/test.c‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1449,6 +1449,41 @@ PHP_ZEND_TEST_API void bug_gh9090_void_int_char_var(int i, char *fmt, ...) {
14491449

14501450
PHP_ZEND_TEST_API int gh11934b_ffi_var_test_cdata;
14511451

1452+
enum bug_gh16013_enum {
1453+
BUG_GH16013_A = 1,
1454+
BUG_GH16013_B = 2,
1455+
};
1456+
1457+
struct bug_gh16013_int_struct {
1458+
int field;
1459+
};
1460+
1461+
PHP_ZEND_TEST_API char bug_gh16013_return_char(void) {
1462+
return 'A';
1463+
}
1464+
1465+
PHP_ZEND_TEST_API bool bug_gh16013_return_bool(void) {
1466+
return true;
1467+
}
1468+
1469+
PHP_ZEND_TEST_API short bug_gh16013_return_short(void) {
1470+
return 12345;
1471+
}
1472+
1473+
PHP_ZEND_TEST_API int bug_gh16013_return_int(void) {
1474+
return 123456789;
1475+
}
1476+
1477+
PHP_ZEND_TEST_API enum bug_gh16013_enum bug_gh16013_return_enum(void) {
1478+
return BUG_GH16013_B;
1479+
}
1480+
1481+
PHP_ZEND_TEST_API struct bug_gh16013_int_struct bug_gh16013_return_struct(void) {
1482+
struct bug_gh16013_int_struct ret;
1483+
ret.field = 123456789;
1484+
return ret;
1485+
}
1486+
14521487
#ifdef HAVE_COPY_FILE_RANGE
14531488
/**
14541489
* This function allows us to simulate early return of copy_file_range by setting the limit_copy_file_range ini setting.

0 commit comments

Comments
 (0)