Bug #81096 [Csd]: Inconsistent opcache behavior with variables passed by reference to mysqli

From: Date: Thu, 10 Jun 2021 08:59:19 +0000
Subject: Bug #81096 [Csd]: Inconsistent opcache behavior with variables passed by reference to mysqli
References: 1  Groups: php.bugs 
Request: Send a blank email to php-bugs+get-234325@lists.php.net to get a copy of this message
Edit report at https://bugs.php.net/bug.php?id=81096&edit=1

 ID:                 81096
 Updated by:         nikic@php.net
 Reported by:        nickdnk at hotmail dot com
 Summary:            Inconsistent opcache behavior with variables passed
                     by reference to mysqli
 Status:             Closed
 Type:               Bug
 Package:            opcache
 Operating System:   Linux/docker
 PHP Version:        8.0.6
-Assigned To:        
+Assigned To:        nikic
 Block user comment: N
 Private report:     N

 New Comment:

For release branches, I've applied https://github.com/php/php-src/commit/3f4bc94b0092aa1699be89a88b8a4e62507d1843
as a workaround to address the originally reported issue.


Previous Comments:
------------------------------------------------------------------------
[2021-06-10 08:43:52] git@php.net

Automatic comment on behalf of dstogov
Revision: https://github.com/php/php-src/commit/7368d0c4185b4e65dfb342dc7523727b52a79114
Log: Fixed bug #81096: Inconsistent range inferece for variables passed by reference

------------------------------------------------------------------------
[2021-06-07 15:01:12] nikic@php.net

The following pull request has been associated:

Patch Name: Fix bug #81096: Re-infer ranges if ref type is inferred
On GitHub:  https://github.com/php/php-src/pull/7114
Patch:      https://github.com/php/php-src/pull/7114.patch

------------------------------------------------------------------------
[2021-06-07 13:30:26] nikic@php.net

This is a tricky issue. Before SCCP we have:

0000 INIT_FCALL 1 144 string("escape_x")
0001 SEND_REF #0.CV0($x) [undef] RANGE[0..0] -> #1.CV0($x) NOVAL [ref, any] 1
0002 DO_UCALL
0003 ASSIGN #1.CV0($x) NOVAL [ref, any] -> #2.CV0($x) [ref, any] RANGE[0..0] int(0)
0004 INIT_FCALL 0 128 string("modify_x")
0005 DO_UCALL
0006 #3.T1 [long] RANGE[0..0] = CAST (long) #2.CV0($x) [ref, any] RANGE[0..0]
0007 RETURN #3.T1 [long] RANGE[0..0]

SCCP sees that #3.T1 has RANGE[0..0] and replaces it with a zero literal.

Presumably the reason why it appeared in PHP 8.0 is https://github.com/php/php-src/commit/e0a8c7a8d0b312aae45ef46d27686771fc9297e9.
Previously the substitution wouldn't have happened because T1 is a temporary. But it's
easy to come up with a variant that fails on PHP 7.4 as well:

function test() {
    escape_x($x);
    $x = 0;
    modify_x();
    $y = (int) $x;
    return $y; 
}

We could mitigate this problem in SCCP by simply not using single-element range information.
However, the more fundamental problem here is that the range information is simply wrong, and other
incorrect optimizations may be done based on that.

For example, after disabling the SCCP replacement, if we consider this example:

function test() {
    escape_x($x);
    $x = 0; 
    modify_x();
    return PHP_INT_MAX + (int) $x;
}

Then the result will change from float(9.223372036854776E+18) to int(-9223372036854775808) with
opcache, because a no-overflow assumption has been introduced.

I think the right fix here is going to be something along the lines of: If we infer a ref type
during type inference, we also need to clear out the range on that variable, and propagate that to
all dependent ranges. This is a non-trivial change.

------------------------------------------------------------------------
[2021-06-07 13:10:20] nikic@php.net

Reduced:

<?php
  
function test() {
    escape_x($x);
    $x = 0;
    modify_x();
    return (int) $x;
}

function escape_x(&$x) {
    $GLOBALS['x'] =& $x;
}

function modify_x() {
    $GLOBALS['x']++;
}

var_dump(test());

------------------------------------------------------------------------
[2021-06-06 12:25:03] nickdnk at hotmail dot com

I would like to add that I am well aware that this code might not make a lot of sense, and that it
can be "fixed" quite easily by changing a few lines, but that's not the point. The
point is that there is a difference between PHP 7 and 8 which does not appear to be documented
anywhere, and I don't know what other issues this might cause.

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


The remainder of the comments for this report are too long. To view
the rest of the comments, please view the bug report online at

    https://bugs.php.net/bug.php?id=81096


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


Thread (8 messages)

« previous php.bugs (#234325) next »