Fix #79571: FFI: var_dumping unions may segfault #5544
Conversation
We must not attempt to access arbitrary union members when retrieving debug info, because that may not be valid. Therefore we do no longer dereference pointer types inside of unions, but report their address as integer instead.
ext/ffi/tests/bug79571.phpt
Outdated
| ["num"]=> | ||
| int(42) | ||
| ["str"]=> | ||
| int(42) |
There was a problem hiding this comment.
I think maybe a pointer format here is better, str => 0x2a?
There was a problem hiding this comment.
I'm not happy treating this as IS_LONG, but I'm not sure that IS_STRING would be better. Actually, I think I'd like to have this as IS_PTR, but that would require adjustments to php_var_dump() etc.
There was a problem hiding this comment.
yeah, IS_PTR seems better.. anyway, I think the current state is also okey, thanks
ext/ffi/ffi.c
Outdated
| goto again; | ||
| case ZEND_FFI_TYPE_POINTER: | ||
| if (debug_union) { | ||
| ZVAL_LONG(rv, (uintptr_t)*(void**)ptr); |
There was a problem hiding this comment.
I mean here, ZVAL_STR(rv, zend_strpprintf(“%p”, ptr))?
|
When access pointer value inside union, like Is this solved? |
|
@PeratX, no this issue is not solved, and likely will not be. It is the programmer's responsibility to not dereference uninitialized pointers. This fix just patches the case where some potentially large structure is output for debugging purposes (e.g. |
|
Yes, I agree with you. |
|
Looks good to me generally. I agree that printing this as something like |
|
Thanks! Applied as d530087. |
We must not attempt to access arbitrary union members when retrieving
debug info, because that may not be valid. Therefore we do no longer
dereference pointer types inside of unions, but report their address as
integer instead.
This is an alternative solution to PR #5538; we show all union members, but do not dereference pointers. This is not quite in line with the behavior of void pointers, which are normally shown as own objects with a single int member, but might be more suitable for unions for brevity.