Skip to content

Fix #79096: FFI Struct Segfault #5079

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.

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

Closed
wants to merge 2 commits into from
Closed

Conversation

cmb69
Copy link
Member

@cmb69 cmb69 commented Jan 11, 2020

We must not assume that the size of a function's return value is at
most sizeof(ffi_arg), but rather have to use the size which already
has been determined for the return type if it is larger than
sizeof(ffi_arg).


I'm having trouble to come up with a portable regression test using some widely available API; maybe it's worthwhile to integrate building of supportive libs for tests into configure/make? Or maybe it's not really relevant to support oversized return types (and maybe not even fully supported by libffi?), and to disallow them altogether?

We must not assume that the size of a function's return value is at
most `sizeof(ffi_arg)`, but rather have to use the size which already
has been determined for the return type if it is larger than
`sizeof(ffi_arg)`.
@nikic
Copy link
Member

nikic commented Jan 11, 2020

Possibly export some APIs from the zend test extension?

To be able to have a regression test, we export the required test
function from the zend-test extension, and make sure that the test
can be run on different platforms regardless of whether zend-tests was
built statically or dynamically.
@cmb69 cmb69 requested a review from dstogov January 12, 2020 13:44
Copy link
Member

@nikic nikic left a comment

Choose a reason for hiding this comment

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

@cmb69
Copy link
Member Author

cmb69 commented Jan 14, 2020

Thanks! Applied as 05f3cd2.

@cmb69 cmb69 closed this Jan 14, 2020
@cmb69 cmb69 deleted the cmb/fix-79096 branch January 14, 2020 15:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants