Bug->Doc #79319 [Opn]: FFI::cast() from void* to intptr_t crashes PHP

Edit report at https://bugs.php.net/bug.php?id=79319&edit=1

 ID:                 79319
 Updated by:         [email protected]
 Reported by:        jprofic at yandex dot ru
 Summary:            FFI::cast() from void* to intptr_t crashes PHP
 Status:             Open
-Type:               Bug
+Type:               Documentation Problem
 Package:            FFI
 Operating System:   Windows
 PHP Version:        7.4.3
 Block user comment: N
 Private report:     N

 New Comment:

> What I mean is when ffi\CData is of a pointer type, then   maybe
> it's alright to dereference it in var_dump() and friends if it's
> of a known type to help with debugging.

The type is irrelevant here; the problem is dereferencing of a
pointer which points to an address not allocated by the process.
-1 is wrapped to 0xfffffffffffffff (on 64bit architectures), and
it is "unlikely" that this address is allocated by the process.
In the general case, we would need to set up handlers to catch the
SIGSEGV (or equivalents depending on the system), what appears to
be overkill.

> The type is irrelevant here; […]

Well, not quite.  There is special handling for void*, which is
not derefenced on output (var_dump()), but instead the address is
shown.

> Also I don't think that having a special case for intptr_t will
> help […]

Then there is nothing else to do than to fix the documentation[1]
regarding:

| Any C data can be visualized using var_dump(), print_r(), etc.

Obviously, that is not quite true.

[1] <https://www.php.net/manual/en/class.ffi-cdata.php>


Previous Comments:
------------------------------------------------------------------------
[2020-03-03 10:00:44] jprofic at yandex dot ru

Am I reading C wrong then? Doesn't “*(void**)ptr” mean cast “ptr” to “(void**)” and then dereference it to “void*”? Or is the storage method in ffi slightly more complicated than it looks at the first glance?

Hm, var_dump()'ing ffi::cast('void*', -1) produces strange and unintuitive result:
object(FFI\CData:void*)#1 (1) {
  [0]=>
  int(-1)
}
And var_dump()'ing ffi::cast('void*', -1)[0] is even funnier:
object(FFI\CData:void)#2 (1) {
  ["cdata"]=>
  *RECURSION*
}
Although I think I'm starting to get why it is so.

Yes, I'm currently comparing two ffi\CData:void* objects and it works. Still I would prefer to have the same behaviour as present in C in regard of casting and other pointer usage to prevent any possible misunderstanding, just keeping in mind that ffi\CData object itself is a kind of pointer. Also I don't think that having a special case for intptr_t will help as at first I was trying all types of integers (and was surprised that wchar_t wasn't supported).

To sum it up, I still don't think PHP should crash in my test script, and maybe there should be less magic involved in handling pointers. What I mean is when ffi\CData is of a pointer type, then   maybe it's alright to dereference it in var_dump() and friends if it's of a known type to help with debugging. But properties-wise, I think it should have the only property like “cdata” but for pointers, named for example “intptr” (also maybe readonly).

------------------------------------------------------------------------
[2020-03-03 09:08:59] [email protected]

> Moreover I thinks it's kinda dangerous to automatically
> dereference any pointer, especially the void* ones: […]

Yes, it is; that's why it is not done at the place you've linked
to.  The supplied test script actually crashes when var_dump()ing
the intptr_t, because -1 is dereferenced[1].  It may make sense to
add special support for intptr_t (currently this is just treated
as int32_t/int64_t[2]).  On the other hand, just sticking with
void* should also work for you (at least regarding equality
comparison; pointer arithmetic on void* is indeed another story).

[1] <https://github.com/php/php-src/blob/php-7.4.3/ext/ffi/ffi.c#L497>
[2] <https://github.com/php/php-src/blob/php-7.4.3/ext/ffi/ffi.c#L5205-L5217>

------------------------------------------------------------------------
[2020-03-02 08:28:20] jprofic at yandex dot ru

I can't declare HANDLE as intptr_t because there are Win32 API calls that treat HANDLE as real pointers (LocalFree() comes to mind).
If you meant my example, then the "ffi::cast('void*', -1)" part is emulating what I'm getting from for example CreateFileW() on error. And the rest is the culprit in the reason of this bug report.
Moreover I thinks it's kinda dangerous to automatically dereference any pointer, especially the void* ones: http://git.php.net/?p=php-src.git;a=blob;f=ext/ffi/ffi.c;h=0696512342ea2ad28ca0c7b3cbdb12c87baded6e;hb=HEAD#l3861

------------------------------------------------------------------------
[2020-03-02 08:02:08] [email protected]

Is there any particular reason why you don't use an intprt_t
in the first place?

    ffi::cast('intptr_t', -1)

------------------------------------------------------------------------
[2020-02-28 21:52:54] jprofic at yandex dot ru

Description:
------------
There is no way to FFI::cast() an opaque void* pointer to any scalar type (like intptr_t) so that its value can be compared to some constant (e.g. HANDLE returned by Win32 CreateFileW() call and INVALID_POINTER_VALUE = -1). Everything I tried just crashed PHP.
There are workarounds. Doing FFI::cast('void*', -1) and comparing them both as void* seems to work (“seems” because I haven't managed to subtract two pointers, but it's another tale), however it can't be a class constant. Also using FFI::memcpy(FFI:new('intptr_t'), FFI::addr(FFICData:void*))) seems to work, but the additional FFI::addr() call is required to prevent PHP from crashing as well.

Test script:
---------------
php -r "var_dump(ffi::cast('intptr_t', ffi::cast('void*', -1)));"

Expected result:
----------------
No crash and output similar to:
object(FFICData:int64_t)#1 (1) {
  ["cdata"]=>
  int(-1)
}

Actual result:
--------------
Crash and no output.


------------------------------------------------------------------------



--
Edit this bug report at https://bugs.php.net/bug.php?id=79319&edit=1