Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:107491
MIME-Version: 1.0
References: <CAF+90c_Ds+UNZB4hAi1989SRafb=d1JkP2q4pdwkUUXfcHrLCw@mail.gmail.com>
In-Reply-To: <CAF+90c_Ds+UNZB4hAi1989SRafb=d1JkP2q4pdwkUUXfcHrLCw@mail.gmail.com>
Date: Fri, 11 Oct 2019 10:18:08 +0200
Message-ID: <CAF+90c_RCYvqpZkxWGyXiM1M+ZzwCh98JKtmed22gbA-OBUqaw@mail.gmail.com>
To: PHP internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="00000000000076bf7505949e2965"
Subject: Re: [VOTE] Reclassifying engine warnings
From: nikita.ppv@gmail.com (Nikita Popov)

--00000000000076bf7505949e2965
Content-Type: text/plain; charset="UTF-8"

On Thu, Sep 12, 2019 at 2:17 PM Nikita Popov <nikita.ppv@gmail.com> wrote:

> Hi internals,
>
> I've opened the vote on //wiki.php.net/rfc/engine_warnings.
>
> There are 4 votes, all of them independent. The first 3 are for specific
> cases that were controversial during the discussion, the last one is for
> the remainder of the proposal.
>
> Voting closes 2019-09-26.
>
> Regards,
> Nikita
>

As people have expressed interest in hearing about direct technical
benefits that these kinds of changes have ... let me give you an example
that came up yesterday.

Opcache performs a bunch of optimizations, and one class of optimizations
it does are subsequent jumps on the same operand. For example:

if ($x) { A; }
if ($x) { B; }

Currently, opcache will optimize the first if($x) condition to jump
directly until after the second if($x) if the value is false, on the
expectation that it is redundant to check the same condition twice in a
row: The result is going to be the same. Basically the result is something
like this:

if ($x) { A; } else { goto end; }
if ($x) { B; }
end:

Now, it turns out that this entire class of optimizations is technically
illegal. Why? Because $x might be an undefined variable! That means that
this optimization at the least loses an "undefined variable" notice, and at
worse changes control flow:

set_error_handler(function() {
    $GLOBALS['x'] = true;
});
if ($x) echo "foo\n";
if ($x) echo "bar\n";

Because it's been around for years and doesn't seem to have caused any
active issues, we're likely going to keep this, but nonetheless, it
illustrates the kind of issue we see with these notices. Either an
exception or nothing at all are fine, but notices caused problems.

Of course there are also other problems, such as
https://bugs.php.net/bug.php?id=78598, which is one of multiple
use-after-free issues related to notices thrown during write operations.
The root cause is that under the PHP memory model, it is not legal to run
arbitrary user code while holding writable references into structures -- an
invariant that is violated by some notices, such as the undefined array key
one, because those notices may invoke error handlers. Again, either
throwing nothing or throwing an exception would be unproblematic.

Generally notices thrown by the engine are a pretty big pain to deal with,
as well as something of a correctness and safety hazard. We have quite a
few bugs in this area, though most of them are thankfully not likely to be
hit by accident.

Nikita

--00000000000076bf7505949e2965--