[FFI] Fix #80847 - Resolve struct type for internal struct fields#6762
[FFI] Fix #80847 - Resolve struct type for internal struct fields#6762nawarian wants to merge 5 commits intophp:PHP-7.4from
Conversation
nawarian
force-pushed
the
BUG-80847/FFI_NESTED_STRUCTS
branch
from
d5f1e3c to
7d7333d
Compare
See for instance 67f9b0b. Basically, you can add the required C code to the zend_test extension, and then use it with FFI. Adding a test first may help to indentify the memory leak issues. |
|
Thanks! This helped a lot with the test part! I'll keep checking the memory leaks part |
|
Most builds passed; One is breaking on APT step, not sure it relates to my changes... |
| t->elements[i] = &ffi_type_pointer; | ||
| break; | ||
| case ZEND_FFI_TYPE_STRUCT: | ||
| t->elements[i] = &ffi_type_pointer; |
There was a problem hiding this comment.
I'm not 100% sure why this works tbh. In my test's specific case this makes sure that the returned value won't be NULL. But I failed to trace back how ffi_type_pointer is actually used in the code.
I suppose ffi_type_* symbols come from libffi and given ffi_type_struct does not exist I just guessed that ffi_type_pointer would be the best way to go. Please let me know if this isn't a good idea.
There was a problem hiding this comment.
I doubt this is the right thing to do here. Probably only works because in your test case the struct happens to have the same size as a pointer.
There was a problem hiding this comment.
The fix is incorrect. We can't pass/return structures by value in general. libffi doesn't support this (e.g. see libffi/libffi#33). However, we may try to pass/return structures by value if they fit into 32/64-bit word. I'll try to do this...
There was a problem hiding this comment.
I was wrong. libffi seems support big structures. See my fix #6781
nawarian
changed the title
|
Closed in favour of #6781 |
4 participants
This is an incomplete fix for BUG #80847.
It uses recursion to resolve nested structs.
This Pull Request currently is incomplete:
This GIST generates a fatal error without this Pull Request with the following message: