Skip to content

ext/ffi: Refactor methods to use ZPP parsing #13461

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

ext/ffi: Refactor methods to use ZPP parsing #13461

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b2457bc
Zend_API.h: Add support for deref with Z_PARAM_OBJ_OF_CLASS_OR_STR_EX…
Girgias Feb 21, 2024
73dcddf
ext/ffi: Refactor methods to use ZPP parsing
Girgias Feb 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions Zend/zend_API.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1761,8 +1761,8 @@ ZEND_API ZEND_COLD void zend_argument_value_error(uint32_t arg_num, const char *
#define Z_PARAM_OBJ_OR_STR_OR_NULL(destination_object, destination_string) \
Z_PARAM_OBJ_OR_STR_EX(destination_object, destination_string, 1);

#define Z_PARAM_OBJ_OF_CLASS_OR_STR_EX(destination_object, base_ce, destination_string, allow_null) \
Z_PARAM_PROLOGUE(0, 0); \
#define Z_PARAM_OBJ_OF_CLASS_OR_STR_EX(destination_object, base_ce, destination_string, allow_null, deref) \
Z_PARAM_PROLOGUE(deref, 0); \
Comment on lines -1764 to +1765
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change of prototype may affect other users.

if (UNEXPECTED(!zend_parse_arg_obj_or_str(_arg, &destination_object, base_ce, &destination_string, allow_null, _i))) { \
if (base_ce) { \
_error = ZSTR_VAL((base_ce)->name); \
Expand All @@ -1776,10 +1776,10 @@ ZEND_API ZEND_COLD void zend_argument_value_error(uint32_t arg_num, const char *
}

#define Z_PARAM_OBJ_OF_CLASS_OR_STR(destination_object, base_ce, destination_string) \
Z_PARAM_OBJ_OF_CLASS_OR_STR_EX(destination_object, base_ce, destination_string, 0);
Z_PARAM_OBJ_OF_CLASS_OR_STR_EX(destination_object, base_ce, destination_string, 0, /* deref */ false);

#define Z_PARAM_OBJ_OF_CLASS_OR_STR_OR_NULL(destination_object, base_ce, destination_string) \
Z_PARAM_OBJ_OF_CLASS_OR_STR_EX(destination_object, base_ce, destination_string, 1);
Z_PARAM_OBJ_OF_CLASS_OR_STR_EX(destination_object, base_ce, destination_string, 1, /* deref */ false);

/* old "d" */
#define Z_PARAM_DOUBLE_EX(dest, is_null, check_null, deref) \
Expand Down
178 changes: 80 additions & 98 deletions ext/ffi/ffi.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4382,54 +4382,73 @@ ZEND_METHOD(FFI, alignof) /* {{{ */
}
/* }}} */

static void *php_ffi_parse_cdata_to_ptr(zend_long size, zend_object *zobj, bool *is_error)
{
void *result_ptr = NULL;
*is_error = false;

zend_ffi_cdata *cdata = (zend_ffi_cdata*)zobj;
zend_ffi_type *type = ZEND_FFI_TYPE(cdata->type);

if (type->kind == ZEND_FFI_TYPE_POINTER) {
result_ptr = *(void**)cdata->ptr;
} else {
if (type->kind != ZEND_FFI_TYPE_POINTER && size > type->size) {
*is_error = true;
return NULL;
}
result_ptr = cdata->ptr;
}

return result_ptr;
}

static void *php_ffi_parse_cdata_or_string_to_ptr(zend_long size, zend_object *zobj, zend_string *zstr, bool *is_error)
{
void *result_ptr = NULL;
*is_error = false;

if (zstr) {
if (size > ZSTR_LEN(zstr)) {
*is_error = true;
return NULL;
}
result_ptr = ZSTR_VAL(zstr);
} else {
result_ptr = php_ffi_parse_cdata_to_ptr(size, zobj, is_error);
}

return result_ptr;
}

Comment on lines +4385 to +4423
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The separation of this functions doesn't make things more clear, because you delegated them to check for data/string size and this became unclear.

ZEND_METHOD(FFI, memcpy) /* {{{ */
{
zval *zv1, *zv2;
zend_ffi_cdata *cdata1, *cdata2;
zend_ffi_type *type1, *type2;
zval *zv1;
zend_object *op2_obj = NULL;
zend_string *op2_zstr = NULL;
void *ptr1, *ptr2;
zend_long size;

ZEND_FFI_VALIDATE_API_RESTRICTION();
ZEND_PARSE_PARAMETERS_START(3, 3)
Z_PARAM_OBJECT_OF_CLASS_EX(zv1, zend_ffi_cdata_ce, 0, 1);
Z_PARAM_ZVAL(zv2)
Z_PARAM_OBJECT_OF_CLASS_EX(zv1, zend_ffi_cdata_ce, /* allow null */ false, /* deref */ true);
Z_PARAM_OBJ_OF_CLASS_OR_STR_EX(op2_obj, zend_ffi_cdata_ce, op2_zstr, /* allow null */ false, /* deref */ true);
Z_PARAM_LONG(size)
ZEND_PARSE_PARAMETERS_END();

cdata1 = (zend_ffi_cdata*)Z_OBJ_P(zv1);
type1 = ZEND_FFI_TYPE(cdata1->type);
if (type1->kind == ZEND_FFI_TYPE_POINTER) {
ptr1 = *(void**)cdata1->ptr;
} else {
ptr1 = cdata1->ptr;
if (type1->kind != ZEND_FFI_TYPE_POINTER && size > type1->size) {
zend_throw_error(zend_ffi_exception_ce, "Attempt to write over data boundary");
RETURN_THROWS();
}
bool is_error = false;
ptr1 = php_ffi_parse_cdata_to_ptr(size, Z_OBJ_P(zv1), &is_error);
if (is_error) {
zend_throw_error(zend_ffi_exception_ce, "attempt to read over data boundary");
RETURN_THROWS();
}

ZVAL_DEREF(zv2);
if (Z_TYPE_P(zv2) == IS_STRING) {
ptr2 = Z_STRVAL_P(zv2);
if (size > Z_STRLEN_P(zv2)) {
zend_throw_error(zend_ffi_exception_ce, "Attempt to read over string boundary");
RETURN_THROWS();
}
} else if (Z_TYPE_P(zv2) == IS_OBJECT && Z_OBJCE_P(zv2) == zend_ffi_cdata_ce) {
cdata2 = (zend_ffi_cdata*)Z_OBJ_P(zv2);
type2 = ZEND_FFI_TYPE(cdata2->type);
if (type2->kind == ZEND_FFI_TYPE_POINTER) {
ptr2 = *(void**)cdata2->ptr;
ptr2 = php_ffi_parse_cdata_or_string_to_ptr(size, op2_obj, op2_zstr, &is_error);
if (is_error) {
if (op2_zstr) {
zend_throw_error(zend_ffi_exception_ce, "attempt to read over string boundary");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing RETURN_THROWS(); here and below

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I guess it's not missing, I just didn't notice the one at line 4532 (the diff is a bit clunky)

} else {
ptr2 = cdata2->ptr;
if (type2->kind != ZEND_FFI_TYPE_POINTER && size > type2->size) {
zend_throw_error(zend_ffi_exception_ce, "Attempt to read over data boundary");
RETURN_THROWS();
}
zend_throw_error(zend_ffi_exception_ce, "attempt to read over data boundary");
}
} else {
zend_wrong_parameter_class_error(2, "FFI\\CData or string", zv2);
RETURN_THROWS();
}

Expand All @@ -4439,65 +4458,38 @@ ZEND_METHOD(FFI, memcpy) /* {{{ */

ZEND_METHOD(FFI, memcmp) /* {{{ */
{
zval *zv1, *zv2;
zend_ffi_cdata *cdata1, *cdata2;
zend_ffi_type *type1, *type2;
zend_object *op1_obj = NULL;
zend_string *op1_zstr = NULL;
zend_object *op2_obj = NULL;
zend_string *op2_zstr = NULL;
void *ptr1, *ptr2;
zend_long size;
int ret;

ZEND_FFI_VALIDATE_API_RESTRICTION();
ZEND_PARSE_PARAMETERS_START(3, 3)
Z_PARAM_ZVAL(zv1);
Z_PARAM_ZVAL(zv2);
Z_PARAM_OBJ_OF_CLASS_OR_STR_EX(op1_obj, zend_ffi_cdata_ce, op1_zstr, /* allow null */ false, /* deref */ true);
Z_PARAM_OBJ_OF_CLASS_OR_STR_EX(op2_obj, zend_ffi_cdata_ce, op2_zstr, /* allow null */ false, /* deref */ true);
Z_PARAM_LONG(size)
ZEND_PARSE_PARAMETERS_END();

ZVAL_DEREF(zv1);
if (Z_TYPE_P(zv1) == IS_STRING) {
ptr1 = Z_STRVAL_P(zv1);
if (size > Z_STRLEN_P(zv1)) {
bool is_error = false;
ptr1 = php_ffi_parse_cdata_or_string_to_ptr(size, op1_obj, op1_zstr, &is_error);
if (is_error) {
if (op1_zstr) {
zend_throw_error(zend_ffi_exception_ce, "attempt to read over string boundary");
RETURN_THROWS();
}
} else if (Z_TYPE_P(zv1) == IS_OBJECT && Z_OBJCE_P(zv1) == zend_ffi_cdata_ce) {
cdata1 = (zend_ffi_cdata*)Z_OBJ_P(zv1);
type1 = ZEND_FFI_TYPE(cdata1->type);
if (type1->kind == ZEND_FFI_TYPE_POINTER) {
ptr1 = *(void**)cdata1->ptr;
} else {
ptr1 = cdata1->ptr;
if (type1->kind != ZEND_FFI_TYPE_POINTER && size > type1->size) {
zend_throw_error(zend_ffi_exception_ce, "attempt to read over data boundary");
RETURN_THROWS();
}
zend_throw_error(zend_ffi_exception_ce, "attempt to read over data boundary");
}
} else {
zend_wrong_parameter_class_error(1, "FFI\\CData or string", zv1);
RETURN_THROWS();
}

ZVAL_DEREF(zv2);
if (Z_TYPE_P(zv2) == IS_STRING) {
ptr2 = Z_STRVAL_P(zv2);
if (size > Z_STRLEN_P(zv2)) {
zend_throw_error(zend_ffi_exception_ce, "Attempt to read over string boundary");
RETURN_THROWS();
}
} else if (Z_TYPE_P(zv2) == IS_OBJECT && Z_OBJCE_P(zv2) == zend_ffi_cdata_ce) {
cdata2 = (zend_ffi_cdata*)Z_OBJ_P(zv2);
type2 = ZEND_FFI_TYPE(cdata2->type);
if (type2->kind == ZEND_FFI_TYPE_POINTER) {
ptr2 = *(void**)cdata2->ptr;
ptr2 = php_ffi_parse_cdata_or_string_to_ptr(size, op2_obj, op2_zstr, &is_error);
if (is_error) {
if (op2_zstr) {
zend_throw_error(zend_ffi_exception_ce, "attempt to read over string boundary");
} else {
ptr2 = cdata2->ptr;
if (type2->kind != ZEND_FFI_TYPE_POINTER && size > type2->size) {
zend_throw_error(zend_ffi_exception_ce, "Attempt to read over data boundary");
RETURN_THROWS();
}
zend_throw_error(zend_ffi_exception_ce, "attempt to read over data boundary");
}
} else {
zend_wrong_parameter_class_error(2, "FFI\\CData or string", zv2);
RETURN_THROWS();
}

Expand All @@ -4515,8 +4507,6 @@ ZEND_METHOD(FFI, memcmp) /* {{{ */
ZEND_METHOD(FFI, memset) /* {{{ */
{
zval *zv;
zend_ffi_cdata *cdata;
zend_ffi_type *type;
void *ptr;
zend_long ch, size;

Expand All @@ -4527,16 +4517,11 @@ ZEND_METHOD(FFI, memset) /* {{{ */
Z_PARAM_LONG(size)
ZEND_PARSE_PARAMETERS_END();

cdata = (zend_ffi_cdata*)Z_OBJ_P(zv);
type = ZEND_FFI_TYPE(cdata->type);
if (type->kind == ZEND_FFI_TYPE_POINTER) {
ptr = *(void**)cdata->ptr;
} else {
ptr = cdata->ptr;
if (type->kind != ZEND_FFI_TYPE_POINTER && size > type->size) {
zend_throw_error(zend_ffi_exception_ce, "attempt to write over data boundary");
RETURN_THROWS();
}
bool is_error = false;
ptr = php_ffi_parse_cdata_to_ptr(size, Z_OBJ_P(zv), &is_error);
if (is_error) {
zend_throw_error(zend_ffi_exception_ce, "attempt to write over data boundary");
RETURN_THROWS();
}

memset(ptr, ch, size);
Expand All @@ -4562,14 +4547,11 @@ ZEND_METHOD(FFI, string) /* {{{ */
cdata = (zend_ffi_cdata*)Z_OBJ_P(zv);
type = ZEND_FFI_TYPE(cdata->type);
if (!size_is_null) {
if (type->kind == ZEND_FFI_TYPE_POINTER) {
ptr = *(void**)cdata->ptr;
} else {
ptr = cdata->ptr;
if (type->kind != ZEND_FFI_TYPE_POINTER && size > type->size) {
zend_throw_error(zend_ffi_exception_ce, "attempt to read over data boundary");
RETURN_THROWS();
}
bool is_error = false;
ptr = php_ffi_parse_cdata_to_ptr(size, Z_OBJ_P(zv), &is_error);
if (is_error) {
zend_throw_error(zend_ffi_exception_ce, "attempt to read over data boundary");
RETURN_THROWS();
}
RETURN_STRINGL((char*)ptr, size);
} else {
Expand Down
7 changes: 2 additions & 5 deletions ext/ffi/ffi.stub.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,19 +46,16 @@ public static function sizeof(FFI\CData|FFI\CType $ptr): int {}
public static function alignof(FFI\CData|FFI\CType $ptr): int {}

/**
* @param FFI\CData|string $from
* @prefer-ref $to
* @prefer-ref $from
*/
public static function memcpy(FFI\CData $to, $from, int $size): void {}
public static function memcpy(FFI\CData $to, FFI\CData|string $from, int $size): void {}

/**
* @prefer-ref $ptr1
* @param string|FFI\CData $ptr1
* @prefer-ref $ptr2
* @param string|FFI\CData $ptr2
*/
public static function memcmp($ptr1, $ptr2, int $size): int {}
public static function memcmp(FFI\CData|string $ptr1, FFI\CData|string $ptr2, int $size): int {}

/** @prefer-ref $ptr */
public static function memset(FFI\CData $ptr, int $value, int $size): void {}
Expand Down
8 changes: 4 additions & 4 deletions ext/ffi/ffi_arginfo.h
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.