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

From: Date: Mon, 06 Dec 2021 15:27:10 +0000
Subject: Bug->Doc #79319 [Opn]: FFI::cast() from void* to intptr_t crashes PHP
References: 1  Groups: php.doc.bugs 
Request: Send a blank email to doc-bugs+get-19360@lists.php.net to get a copy of this message
Edit report at https://bugs.php.net/bug.php?id=79319&edit=1

 ID:                 79319
 Updated by:         cmb@php.net
 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] cmb@php.net

> 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] cmb@php.net

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


Thread (1 message)

  • cmb@php.net
  • Unknown Message
    • cmb@php.net
« previous php.doc.bugs (#19360) next »