Skip to content

Commit 100aa6c

Browse files
ndosschendossche
committed
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.
1 parent df6db27 commit 100aa6c

File tree

3 files changed

+241
-23
lines changed

3 files changed

+241
-23
lines changed

‎ext/ffi/ffi.c‎

Lines changed: 69 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,22 @@ typedef struct _zend_ffi {
188188
#define ZEND_FFI_SIZEOF_ARG \
189189
MAX(FFI_SIZEOF_ARG, sizeof(double))
190190

191+
/* The FFI call return values follow widening rules.
192+
* We must widen to `ffi_arg` in the case we're handling a return value for types shorter than the machine width.
193+
* From http://www.chiark.greenend.org.uk/doc/libffi-dev/html/The-Closure-API.html:
194+
* > In most cases, ret points to an object of exactly the size of the type specified when cif was constructed.
195+
* > However, integral types narrower than the system register size are widened.
196+
* > In these cases your program may assume that ret points to an ffi_arg object.
197+
*/
198+
#define ZEND_FFI_READ_NARROW(ty, ptr, is_ret) (is_ret ? ((ty) *(ffi_arg *) ptr) : (*(ty *) ptr))
199+
#define ZEND_FFI_WRITE_NARROW(ty, ptr, val, is_ret) do { \
200+
if (is_ret) { \
201+
*(ffi_arg *) ptr = (ffi_arg) (ty) val; \
202+
} else { \
203+
*(ty *) ptr = val; \
204+
} \
205+
} while (0)
206+
191207
typedef struct _zend_ffi_cdata {
192208
zend_object std;
193209
zend_ffi_type *type;
@@ -560,22 +576,22 @@ static zend_always_inline void zend_ffi_cdata_to_zval(zend_ffi_cdata *cdata, voi
560576
return;
561577
#endif
562578
case ZEND_FFI_TYPE_UINT8:
563-
ZVAL_LONG(rv, *(uint8_t*)ptr);
579+
ZVAL_LONG(rv, ZEND_FFI_READ_NARROW(uint8_t, ptr, is_ret));
564580
return;
565581
case ZEND_FFI_TYPE_SINT8:
566-
ZVAL_LONG(rv, *(int8_t*)ptr);
582+
ZVAL_LONG(rv, ZEND_FFI_READ_NARROW(int8_t, ptr, is_ret));
567583
return;
568584
case ZEND_FFI_TYPE_UINT16:
569-
ZVAL_LONG(rv, *(uint16_t*)ptr);
585+
ZVAL_LONG(rv, ZEND_FFI_READ_NARROW(uint16_t, ptr, is_ret));
570586
return;
571587
case ZEND_FFI_TYPE_SINT16:
572-
ZVAL_LONG(rv, *(int16_t*)ptr);
588+
ZVAL_LONG(rv, ZEND_FFI_READ_NARROW(int16_t, ptr, is_ret));
573589
return;
574590
case ZEND_FFI_TYPE_UINT32:
575-
ZVAL_LONG(rv, *(uint32_t*)ptr);
591+
ZVAL_LONG(rv, ZEND_FFI_READ_NARROW(uint32_t, ptr, is_ret));
576592
return;
577593
case ZEND_FFI_TYPE_SINT32:
578-
ZVAL_LONG(rv, *(int32_t*)ptr);
594+
ZVAL_LONG(rv, ZEND_FFI_READ_NARROW(int32_t, ptr, is_ret));
579595
return;
580596
case ZEND_FFI_TYPE_UINT64:
581597
ZVAL_LONG(rv, *(uint64_t*)ptr);
@@ -584,10 +600,10 @@ static zend_always_inline void zend_ffi_cdata_to_zval(zend_ffi_cdata *cdata, voi
584600
ZVAL_LONG(rv, *(int64_t*)ptr);
585601
return;
586602
case ZEND_FFI_TYPE_BOOL:
587-
ZVAL_BOOL(rv, *(uint8_t*)ptr);
603+
ZVAL_BOOL(rv, ZEND_FFI_READ_NARROW(uint8_t, ptr, is_ret));
588604
return;
589605
case ZEND_FFI_TYPE_CHAR:
590-
ZVAL_CHAR(rv, *(char*)ptr);
606+
ZVAL_CHAR(rv, ZEND_FFI_READ_NARROW(char, ptr, is_ret));
591607
return;
592608
case ZEND_FFI_TYPE_ENUM:
593609
kind = type->enumeration.kind;
@@ -733,7 +749,7 @@ static void zend_ffi_zval_to_bit_field(void *ptr, zend_ffi_field *field, zval *v
733749
}
734750
/* }}} */
735751

736-
static zend_always_inline zend_result zend_ffi_zval_to_cdata(void *ptr, zend_ffi_type *type, zval *value) /* {{{ */
752+
static zend_always_inline zend_result zend_ffi_zval_to_cdata(void *ptr, zend_ffi_type *type, zval *value, bool is_ret) /* {{{ */
737753
{
738754
zend_long lval;
739755
double dval;
@@ -746,6 +762,36 @@ static zend_always_inline zend_result zend_ffi_zval_to_cdata(void *ptr, zend_ffi
746762
zend_ffi_cdata *cdata = (zend_ffi_cdata*)Z_OBJ_P(value);
747763
if (zend_ffi_is_compatible_type(type, ZEND_FFI_TYPE(cdata->type)) &&
748764
type->size == ZEND_FFI_TYPE(cdata->type)->size) {
765+
if (is_ret && type->size < sizeof(ffi_arg)) {
766+
again_narrowed:
767+
switch (kind) {
768+
case ZEND_FFI_TYPE_SINT8:
769+
*(ffi_arg *) ptr = (ffi_arg) *(int8_t *) cdata->ptr;
770+
return SUCCESS;
771+
case ZEND_FFI_TYPE_BOOL:
772+
case ZEND_FFI_TYPE_CHAR:
773+
case ZEND_FFI_TYPE_UINT8:
774+
*(ffi_arg *) ptr = (ffi_arg) *(uint8_t *) cdata->ptr;
775+
return SUCCESS;
776+
case ZEND_FFI_TYPE_SINT16:
777+
*(ffi_arg *) ptr = (ffi_arg) *(int16_t *) cdata->ptr;
778+
return SUCCESS;
779+
case ZEND_FFI_TYPE_UINT16:
780+
*(ffi_arg *) ptr = (ffi_arg) *(uint16_t *) cdata->ptr;
781+
return SUCCESS;
782+
case ZEND_FFI_TYPE_SINT32:
783+
*(ffi_arg *) ptr = (ffi_arg) *(int32_t *) cdata->ptr;
784+
return SUCCESS;
785+
case ZEND_FFI_TYPE_UINT32:
786+
*(ffi_arg *) ptr = (ffi_arg) *(uint32_t *) cdata->ptr;
787+
return SUCCESS;
788+
case ZEND_FFI_TYPE_ENUM:
789+
kind = type->enumeration.kind;
790+
goto again_narrowed;
791+
default:
792+
break;
793+
}
794+
}
749795
memcpy(ptr, cdata->ptr, type->size);
750796
return SUCCESS;
751797
}
@@ -769,27 +815,27 @@ static zend_always_inline zend_result zend_ffi_zval_to_cdata(void *ptr, zend_ffi
769815
#endif
770816
case ZEND_FFI_TYPE_UINT8:
771817
lval = zval_get_long(value);
772-
*(uint8_t*)ptr = lval;
818+
ZEND_FFI_WRITE_NARROW(uint8_t, ptr, lval, is_ret);
773819
break;
774820
case ZEND_FFI_TYPE_SINT8:
775821
lval = zval_get_long(value);
776-
*(int8_t*)ptr = lval;
822+
ZEND_FFI_WRITE_NARROW(int8_t, ptr, lval, is_ret);
777823
break;
778824
case ZEND_FFI_TYPE_UINT16:
779825
lval = zval_get_long(value);
780-
*(uint16_t*)ptr = lval;
826+
ZEND_FFI_WRITE_NARROW(uint16_t, ptr, lval, is_ret);
781827
break;
782828
case ZEND_FFI_TYPE_SINT16:
783829
lval = zval_get_long(value);
784-
*(int16_t*)ptr = lval;
830+
ZEND_FFI_WRITE_NARROW(int16_t, ptr, lval, is_ret);
785831
break;
786832
case ZEND_FFI_TYPE_UINT32:
787833
lval = zval_get_long(value);
788-
*(uint32_t*)ptr = lval;
834+
ZEND_FFI_WRITE_NARROW(uint32_t, ptr, lval, is_ret);
789835
break;
790836
case ZEND_FFI_TYPE_SINT32:
791837
lval = zval_get_long(value);
792-
*(int32_t*)ptr = lval;
838+
ZEND_FFI_WRITE_NARROW(int32_t, ptr, lval, is_ret);
793839
break;
794840
case ZEND_FFI_TYPE_UINT64:
795841
lval = zval_get_long(value);
@@ -800,12 +846,12 @@ static zend_always_inline zend_result zend_ffi_zval_to_cdata(void *ptr, zend_ffi
800846
*(int64_t*)ptr = lval;
801847
break;
802848
case ZEND_FFI_TYPE_BOOL:
803-
*(uint8_t*)ptr = zend_is_true(value);
849+
ZEND_FFI_WRITE_NARROW(uint8_t, ptr, zend_is_true(value), is_ret);
804850
break;
805851
case ZEND_FFI_TYPE_CHAR:
806852
str = zval_get_tmp_string(value, &tmp_str);
807853
if (ZSTR_LEN(str) == 1) {
808-
*(char*)ptr = ZSTR_VAL(str)[0];
854+
ZEND_FFI_WRITE_NARROW(char, ptr, ZSTR_VAL(str)[0], is_ret);
809855
} else {
810856
zend_tmp_string_release(tmp_str);
811857
zend_ffi_assign_incompatible(value, type);
@@ -984,7 +1030,7 @@ static void zend_ffi_callback_trampoline(ffi_cif* cif, void* ret, void** args, v
9841030

9851031
ret_type = ZEND_FFI_TYPE(callback_data->type->func.ret_type);
9861032
if (ret_type->kind != ZEND_FFI_TYPE_VOID) {
987-
zend_ffi_zval_to_cdata(ret, ret_type, &retval);
1033+
zend_ffi_zval_to_cdata(ret, ret_type, &retval, true);
9881034
}
9891035

9901036
zval_ptr_dtor(&retval);
@@ -1138,7 +1184,7 @@ static zval *zend_ffi_cdata_set(zend_object *obj, zend_string *member, zval *val
11381184
return &EG(uninitialized_zval);
11391185
}
11401186

1141-
zend_ffi_zval_to_cdata(cdata->ptr, type, value);
1187+
zend_ffi_zval_to_cdata(cdata->ptr, type, value, false);
11421188

11431189
return value;
11441190
}
@@ -1360,7 +1406,7 @@ static zval *zend_ffi_cdata_write_field(zend_object *obj, zend_string *field_nam
13601406

13611407
if (EXPECTED(!field->bits)) {
13621408
ptr = (void*)(((char*)ptr) + field->offset);
1363-
zend_ffi_zval_to_cdata(ptr, ZEND_FFI_TYPE(field->type), value);
1409+
zend_ffi_zval_to_cdata(ptr, ZEND_FFI_TYPE(field->type), value, false);
13641410
} else {
13651411
zend_ffi_zval_to_bit_field(ptr, field, value);
13661412
}
@@ -1474,7 +1520,7 @@ static void zend_ffi_cdata_write_dim(zend_object *obj, zval *offset, zval *value
14741520
return;
14751521
}
14761522

1477-
zend_ffi_zval_to_cdata(ptr, type, value);
1523+
zend_ffi_zval_to_cdata(ptr, type, value, false);
14781524
}
14791525
/* }}} */
14801526

@@ -2539,7 +2585,7 @@ static zval *zend_ffi_write_var(zend_object *obj, zend_string *var_name, zval *v
25392585
return value;
25402586
}
25412587

2542-
zend_ffi_zval_to_cdata(sym->addr, ZEND_FFI_TYPE(sym->type), value);
2588+
zend_ffi_zval_to_cdata(sym->addr, ZEND_FFI_TYPE(sym->type), value, false);
25432589
return value;
25442590
}
25452591
/* }}} */
@@ -4016,7 +4062,7 @@ ZEND_METHOD(FFI, cast) /* {{{ */
40164062
cdata->std.handlers = &zend_ffi_cdata_value_handlers;
40174063
cdata->type = type_ptr;
40184064
cdata->ptr = emalloc(type->size);
4019-
zend_ffi_zval_to_cdata(cdata->ptr, type, zv);
4065+
zend_ffi_zval_to_cdata(cdata->ptr, type, zv, false);
40204066
cdata->flags = ZEND_FFI_FLAG_OWNED;
40214067
if (is_const) {
40224068
cdata->flags |= ZEND_FFI_FLAG_CONST;

‎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)