Skip to content

Commit 18183ff

Browse files
committed
Fix order of checks to throw exception with better message
This clarifies the "->cdata" meaning.
1 parent ab58e4a commit 18183ff

File tree

2 files changed

+71
-26
lines changed

2 files changed

+71
-26
lines changed

ext/ffi/ffi.c

Lines changed: 33 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1169,20 +1169,10 @@ static zval *zend_ffi_cdata_read_field(zend_object *obj, zend_string *field_name
11691169
if (cache_slot && *cache_slot == type) {
11701170
field = *(cache_slot + 1);
11711171
} else {
1172+
if (type->kind == ZEND_FFI_TYPE_POINTER) {
1173+
type = ZEND_FFI_TYPE(type->pointer.type);
1174+
}
11721175
if (UNEXPECTED(type->kind != ZEND_FFI_TYPE_STRUCT)) {
1173-
if (type->kind == ZEND_FFI_TYPE_POINTER) {
1174-
/* transparently dereference the pointer */
1175-
if (UNEXPECTED(!ptr)) {
1176-
zend_throw_error(zend_ffi_exception_ce, "NULL pointer dereference");
1177-
return &EG(uninitialized_zval);
1178-
}
1179-
ptr = (void*)(*(char**)ptr);
1180-
if (UNEXPECTED(!ptr)) {
1181-
zend_throw_error(zend_ffi_exception_ce, "NULL pointer dereference");
1182-
return &EG(uninitialized_zval);
1183-
}
1184-
type = ZEND_FFI_TYPE(type->pointer.type);
1185-
}
11861176
if (UNEXPECTED(type->kind != ZEND_FFI_TYPE_STRUCT)) {
11871177
zend_throw_error(zend_ffi_exception_ce, "Attempt to read field '%s' of non C struct/union", ZSTR_VAL(field_name));
11881178
return &EG(uninitialized_zval);
@@ -1201,6 +1191,20 @@ static zval *zend_ffi_cdata_read_field(zend_object *obj, zend_string *field_name
12011191
}
12021192
}
12031193

1194+
if (ZEND_FFI_TYPE(cdata->type)->kind == ZEND_FFI_TYPE_POINTER) {
1195+
/* transparently dereference the pointer */
1196+
if (UNEXPECTED(!ptr)) {
1197+
zend_throw_error(zend_ffi_exception_ce, "NULL pointer dereference");
1198+
return &EG(uninitialized_zval);
1199+
}
1200+
ptr = (void*)(*(char**)ptr);
1201+
if (UNEXPECTED(!ptr)) {
1202+
zend_throw_error(zend_ffi_exception_ce, "NULL pointer dereference");
1203+
return &EG(uninitialized_zval);
1204+
}
1205+
type = ZEND_FFI_TYPE(type->pointer.type);
1206+
}
1207+
12041208
#if 0
12051209
if (UNEXPECTED(!ptr)) {
12061210
zend_throw_error(zend_ffi_exception_ce, "NULL pointer dereference");
@@ -1238,20 +1242,10 @@ static zval *zend_ffi_cdata_write_field(zend_object *obj, zend_string *field_nam
12381242
if (cache_slot && *cache_slot == type) {
12391243
field = *(cache_slot + 1);
12401244
} else {
1245+
if (type->kind == ZEND_FFI_TYPE_POINTER) {
1246+
type = ZEND_FFI_TYPE(type->pointer.type);
1247+
}
12411248
if (UNEXPECTED(type->kind != ZEND_FFI_TYPE_STRUCT)) {
1242-
if (type->kind == ZEND_FFI_TYPE_POINTER) {
1243-
/* transparently dereference the pointer */
1244-
if (UNEXPECTED(!ptr)) {
1245-
zend_throw_error(zend_ffi_exception_ce, "NULL pointer dereference");
1246-
return value;
1247-
}
1248-
ptr = (void*)(*(char**)ptr);
1249-
if (UNEXPECTED(!ptr)) {
1250-
zend_throw_error(zend_ffi_exception_ce, "NULL pointer dereference");
1251-
return value;
1252-
}
1253-
type = ZEND_FFI_TYPE(type->pointer.type);
1254-
}
12551249
if (UNEXPECTED(type->kind != ZEND_FFI_TYPE_STRUCT)) {
12561250
zend_throw_error(zend_ffi_exception_ce, "Attempt to assign field '%s' of non C struct/union", ZSTR_VAL(field_name));
12571251
return value;
@@ -1270,6 +1264,19 @@ static zval *zend_ffi_cdata_write_field(zend_object *obj, zend_string *field_nam
12701264
}
12711265
}
12721266

1267+
if (ZEND_FFI_TYPE(cdata->type)->kind == ZEND_FFI_TYPE_POINTER) {
1268+
/* transparently dereference the pointer */
1269+
if (UNEXPECTED(!ptr)) {
1270+
zend_throw_error(zend_ffi_exception_ce, "NULL pointer dereference");
1271+
return value;
1272+
}
1273+
ptr = (void*)(*(char**)ptr);
1274+
if (UNEXPECTED(!ptr)) {
1275+
zend_throw_error(zend_ffi_exception_ce, "NULL pointer dereference");
1276+
return value;
1277+
}
1278+
}
1279+
12731280
#if 0
12741281
if (UNEXPECTED(!ptr)) {
12751282
zend_throw_error(zend_ffi_exception_ce, "NULL pointer dereference");

ext/ffi/tests/047.phpt

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
--TEST--
2+
FFI 047: FFI::CData->cdata meaning
3+
--EXTENSIONS--
4+
ffi
5+
--INI--
6+
ffi.enable=1
7+
--FILE--
8+
<?php
9+
$x = FFI::new("int");
10+
$x->cdata = 42;
11+
var_dump($x);
12+
13+
$x = FFI::new("int*");
14+
try {
15+
$x->cdata = 42;
16+
var_dump($x);
17+
} catch (Throwable $e) {
18+
echo $e->getMessage() . "\n";
19+
}
20+
21+
$x = FFI::new("struct {int cdata;}");
22+
try {
23+
$x->cdata = 42;
24+
var_dump($x);
25+
} catch (Throwable $e) {
26+
echo $e->getMessage() . "\n";
27+
}
28+
?>
29+
--EXPECTF--
30+
object(FFI\CData:int32_t)#%d (1) {
31+
["cdata"]=>
32+
int(42)
33+
}
34+
Attempt to assign field 'cdata' of non C struct/union
35+
object(FFI\CData:struct <anonymous>)#%d (1) {
36+
["cdata"]=>
37+
int(42)
38+
}

0 commit comments

Comments
 (0)