Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:106916
MIME-Version: 1.0
References: <CAGKNXQEaC1DajOogF98iq=PL-MvOom43kK_JPpREYrQQ_o_KYA@mail.gmail.com>
 <4af5ebe3-d88a-925d-66dc-9159912ece81@gmail.com>
In-Reply-To: <4af5ebe3-d88a-925d-66dc-9159912ece81@gmail.com>
Date: Mon, 9 Sep 2019 11:27:06 +0200
Message-ID: <CAGKNXQGKuTiEhv6EG_dvz-kEtSbtZwvJefVw9r73x05ya9xdsA@mail.gmail.com>
To: Stanislav Malyshev <smalyshev@gmail.com>
Cc: PHP internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="000000000000d3bb8f05921b645a"
Subject: Re: [PHP-DEV] FFI extension / NULL-Checks
From: phofstetter@sensational.ch (Philip Hofstetter)

--000000000000d3bb8f05921b645a
Content-Type: text/plain; charset="UTF-8"

On Mon, Sep 9, 2019 at 10:13 AM Stanislav Malyshev <smalyshev@gmail.com>
wrote:


> > isset($err)
> > $err[0] != null
> > -> is always true, regardless of whether func as assigned an error or not
> >
> > $err[0];
> > -> segfault
>
> This is a bit confusing - if $err[0] segfaults, how it can be != null?
> Does this behave differently in different contexts? Anyway, it shouldn't
> probably segfault in any case, so this looks like a bug.
>

I think there are a few disconnected issues. One is that checking something
for being NULL is a bit tricky with the given API and the other is that the
API around FFI\CData is a bit inconsistent and tricky to use.

Given

$err = $ffi->new("Error*");

what the library does is assume that $err is now a pointer to an array
(which is what C does I guess), but yet, there seems to be a bit of an
issue with actually accessing that array (if it's set to NULL) - and also,
don't use var_dump() to inspect it - that's deceptive because there's a
debug_info handler that knows ore than the actual field accessors (which
too makes this more complicated than needed).

So. Let's assume that in

$err = $ffi->new("Error*");
$ffi->func(FFI::addr($err));

func() sets that argument to NULL, then

$f = $err[0];
var_dump(gettype($f));

returns "object" - so the zeroth index exists (hence the isset being true).

However there's also
$f = $err->{0};

which normally would throw "Attempt to read undefined field '0' of C
struct/union" but because $err is seen as an array, accessing a property
named 0 is possible, but then a second null check will happen via
https://github.com/php/php-src/blob/09e9c4f3c11a875c67334578621084f156155307/ext/ffi/ffi.c#L1103-L1107
and
you get "NULL pointer dereference" instead.

I believe this difference between [0] and {0} shouldn't exist.

And even aside of that, I see no way in which user-code can ever check any
reference for NULL because any access of a Cdata property or index that's
happen to be set to NULL will either segfault, throw or fail a comparison
in PHP because Cdata instances, according to PHP's type conversion rules,
always convert to something that compares truthy when put in boolean
context.

The inconsistency between [] and {} aside, I believe there should be an
FFI::isNull() static method or a Cdata::isNull() instance method that allow
user-code to check things for being NULL. Or the throw in the code I liked
to above is removed, but that would leave the inconsistency between [] and
{}

Or maybe I'm just overlooking something and there's a simple way to check
something for being NULL, but if there is, it IMHO needs better docs.

Philip

--000000000000d3bb8f05921b645a--