Opcache CFG jmp optimization with try-finally breaks the exception table

[SpencerMalone]

Description

The following code:

<?php

function readLinesFrom(string $filename): iterable
{
    if (!is_readable($filename)) {
        throw new Exception("File {$filename} does not exist or is not readable");
    }

    $file = fopen($filename, 'r');
    try {
        while (($line = fgets($file)) !== false) {
            yield trim($line);
        }
    } finally {
        fclose($file);
    }
}

$lines = iterator_to_array(readLinesFrom("/bad/file/path/1"));

when run multiple times with php -dopcache.enable_cli=on -dopcache.optimization_level=0x0010 -derror_log=/proc/self/fd/2 myfile.php

Resulted in this output:

PHP Warning:  Undefined variable $file in myfile.php on line 15

Warning: Undefined variable $file in myfile.php on line 15
PHP Fatal error:  Uncaught Exception: File /bad/file/path/1 does not exist or is not readable in myfile.php:7
Stack trace:
#0 [internal function]: readLinesFrom('/bad/file/path/...')
#1 myfile.php(19): iterator_to_array(Object(Generator))
#2 {main}

Next TypeError: fclose(): Argument #1 ($stream) must be of type resource, null given in myfile.php:15
Stack trace:
#0 myfile.php(15): fclose(NULL)
#1 [internal function]: readLinesFrom('/bad/file/path/...')
#2 myfile.php(19): iterator_to_array(Object(Generator))
#3 {main}
  thrown in myfile.php on line 15

Fatal error: Uncaught Exception: File /bad/file/path/1 does not exist or is not readable in myfile.php:7
Stack trace:
#0 [internal function]: readLinesFrom('/bad/file/path/...')
#1 myfile.php(19): iterator_to_array(Object(Generator))
#2 {main}

Next TypeError: fclose(): Argument #1 ($stream) must be of type resource, null given in myfile.php:15
Stack trace:
#0 myfile.php(15): fclose(NULL)
#1 [internal function]: readLinesFrom('/bad/file/path/...')
#2 myfile.php(19): iterator_to_array(Object(Generator))
#3 {main}
  thrown in myfile.php on line 15

But when run with php -dopcache.enable_cli=on -dopcache.optimization_level=0 -derror_log=/proc/self/fd/2 myfile.php I get this instead:

PHP Fatal error:  Uncaught Exception: File /bad/file/path/1 does not exist or is not readable in myfile.php:7
Stack trace:
#0 [internal function]: readLinesFrom('/bad/file/path/...')
#1 myfile.php(19): iterator_to_array(Object(Generator))
#2 {main}
  thrown in myfile.php on line 7

Fatal error: Uncaught Exception: File /bad/file/path/1 does not exist or is not readable in myfile.php:7
Stack trace:
#0 [internal function]: readLinesFrom('/bad/file/path/...')
#1 myfile.php(19): iterator_to_array(Object(Generator))
#2 {main}
  thrown in myfile.php on line 7

Slightly unsure if this is just another example of issue #8269 or not

PHP Version

PHP 8.3.16 and PHP 8.2.19

Operating System

Amazon Linux 2 and MacOS 15.2 (likely not relevant?)

[SpencerMalone]

Note that there is a workaround by doing an extra throw in the finally statement, but that shouldn't be needed as far as I know. Generally this doesn't impact a lot, other than making unit tests where you expect a specific exception fiddly.

[nielsdos]

The particular code you posted doesn't reproduce the issue for me, at least not on 8.3.16 nor 8.3-dev.
I also notice that it warns for the $file variable on line 15, but that variable is not on line 15. Is the code you posted actually the code you're running?

[SpencerMalone]

I apologize, I thought I had it narrowed down and I think I had a different bug surfacing when I didn't include $file = fopen($filename, 'r');.

Here's a reproducing file:

<?php declare(strict_types=1);

final class Owners
{
    public static function readLinesFrom(string $filename): iterable
    {
        if (!is_readable($filename)) {
            throw new Exception("File {$filename} does not exist or is not readable");
        }

        $file = fopen($filename, 'r');
        if ($file === false) {
            throw new Exception("Failed to open file: {$filename}");
        }
        try {
            while (($line = fgets($file)) !== false) {
                yield trim($line);
            }
        } finally {
            fclose($file);
        }
    }
}

$owner = new Owners();

$lines = iterator_to_array($owner->readLinesFrom("/bad/file/path/1"));

and here's a bash log:

opcache-test % php -v
PHP 8.3.16 (cli) (built: Jan 14 2025 18:25:29) (NTS)
Copyright (c) The PHP Group
Zend Engine v4.3.16, Copyright (c) Zend Technologies
    with Zend OPcache v8.3.16, Copyright (c), by Zend Technologies
opcache-test % cat myfile.php 
<?php declare(strict_types=1);

final class Owners
{
    public static function readLinesFrom(string $filename): iterable
    {
        if (!is_readable($filename)) {
            throw new Exception("File {$filename} does not exist or is not readable");
        }

        $file = fopen($filename, 'r');
        if ($file === false) {
            throw new Exception("Failed to open file: {$filename}");
        }
        try {
            while (($line = fgets($file)) !== false) {
                yield trim($line);
            }
        } finally {
            fclose($file);
        }
    }
}

$owner = new Owners();

$lines = iterator_to_array($owner->readLinesFrom("/bad/file/path/1"));
opcache-test % php -dopcache.enable_cli=on -dopcache.optimization_level=0x0010 -derror_log=/proc/self/fd/2 myfile.php
PHP Warning:  Undefined variable $file in /Volumes/CaseSensitive/opcache-test/myfile.php on line 20

Warning: Undefined variable $file in /Volumes/CaseSensitive/opcache-test/myfile.php on line 20
PHP Fatal error:  Uncaught Exception: File /bad/file/path/1 does not exist or is not readable in /Volumes/CaseSensitive/opcache-test/myfile.php:8
Stack trace:
#0 [internal function]: Owners::readLinesFrom('/bad/file/path/...')
#1 /Volumes/CaseSensitive/opcache-test/myfile.php(27): iterator_to_array(Object(Generator))
#2 {main}

Next TypeError: fclose(): Argument #1 ($stream) must be of type resource, null given in /Volumes/CaseSensitive/opcache-test/myfile.php:20
Stack trace:
#0 /Volumes/CaseSensitive/opcache-test/myfile.php(20): fclose(NULL)
#1 [internal function]: Owners::readLinesFrom('/bad/file/path/...')
#2 /Volumes/CaseSensitive/opcache-test/myfile.php(27): iterator_to_array(Object(Generator))
#3 {main}
  thrown in /Volumes/CaseSensitive/opcache-test/myfile.php on line 20

Fatal error: Uncaught Exception: File /bad/file/path/1 does not exist or is not readable in /Volumes/CaseSensitive/opcache-test/myfile.php:8
Stack trace:
#0 [internal function]: Owners::readLinesFrom('/bad/file/path/...')
#1 /Volumes/CaseSensitive/opcache-test/myfile.php(27): iterator_to_array(Object(Generator))
#2 {main}

Next TypeError: fclose(): Argument #1 ($stream) must be of type resource, null given in /Volumes/CaseSensitive/opcache-test/myfile.php:20
Stack trace:
#0 /Volumes/CaseSensitive/opcache-test/myfile.php(20): fclose(NULL)
#1 [internal function]: Owners::readLinesFrom('/bad/file/path/...')
#2 /Volumes/CaseSensitive/opcache-test/myfile.php(27): iterator_to_array(Object(Generator))
#3 {main}
  thrown in /Volumes/CaseSensitive/opcache-test/myfile.php on line 20
opcache-test % php -dopcache.enable_cli=on -dopcache.optimization_level=0x0000 -derror_log=/proc/self/fd/2 myfile.php
PHP Fatal error:  Uncaught Exception: File /bad/file/path/1 does not exist or is not readable in /Volumes/CaseSensitive/opcache-test/myfile.php:8
Stack trace:
#0 [internal function]: Owners::readLinesFrom('/bad/file/path/...')
#1 /Volumes/CaseSensitive/opcache-test/myfile.php(27): iterator_to_array(Object(Generator))
#2 {main}
  thrown in /Volumes/CaseSensitive/opcache-test/myfile.php on line 8

Fatal error: Uncaught Exception: File /bad/file/path/1 does not exist or is not readable in /Volumes/CaseSensitive/opcache-test/myfile.php:8
Stack trace:
#0 [internal function]: Owners::readLinesFrom('/bad/file/path/...')
#1 /Volumes/CaseSensitive/opcache-test/myfile.php(27): iterator_to_array(Object(Generator))
#2 {main}
  thrown in /Volumes/CaseSensitive/opcache-test/myfile.php on line 8

[SpencerMalone]

The static reference to an object doesn't matter, I'm gonna try to figure out the minimum script here, but you can do $lines = iterator_to_array(Owners::readLinesFrom("/bad/file/path/1")); as well and reproduce

[SpencerMalone]

Ah, sorry for the rapid updates, here's the minimum script I think:

<?php declare(strict_types=1);


function readLinesFrom(string $filename): iterable
{
    if (!is_readable($filename)) {
        throw new Exception("File {$filename} does not exist or is not readable");
    }

    $file = fopen($filename, 'r');
    if ($file === false) {
        throw new Exception("Failed to open file: {$filename}");
    }
    try {
        while (($line = fgets($file)) !== false) {
            yield trim($line);
        }
    } finally {
        fclose($file);
    }
}

$lines = iterator_to_array(readLinesFrom("/bad/file/path/1"));

the

    if ($file === false) {
        throw new Exception("Failed to open file: {$filename}");
    }

I guess is required to reproduce

[nielsdos]

Thanks a lot! This reproduces on 8.3-dev for me, reduced a bit further:

<?php

function readLinesFrom($filename)
{
    $file = fopen($filename, 'r');
    if ($file === false) {
        throw new Exception;
    }
    try {
        while (fgets($file)) {
            yield 1;
        }
    } finally {
        fclose($file);
    }
}

iterator_to_array(readLinesFrom("/bad/file/path/1"));

EDIT: the difference between the optimization before / after is a 0012 JMP 0014 getting eliminated which shouldn't be AFAICT (seemingly caused by zend_jmp_optimization). Oh in combination with 0008 JMPZ T4 0012 changing, so it's a JMPZ optimization that breaks this not a JMP...

[nielsdos]

After the jump optimization is run, the 0012 JMP 0014 block becomes unreachable, and so the exception table is updated here, more specifically try_op is updated. Since we don't have a catch_op (i.e. it is 0), try_op becomes 0 which seems wrong:

php-src/Zend/Optimizer/zend_cfg.c

Lines 141 to 154 in b450a9c

	if (!(b->flags & ZEND_BB_REACHABLE)) {
	if (op_array->try_catch_array[j].finally_op) {
	end = blocks + block_map[op_array->try_catch_array[j].finally_op];
	while (b != end) {
	if (b->flags & ZEND_BB_REACHABLE) {
	op_array->try_catch_array[j].try_op = op_array->try_catch_array[j].catch_op;
	changed = 1;
	zend_mark_reachable(op_array->opcodes, cfg, blocks + block_map[op_array->try_catch_array[j].try_op]);
	break;
	}
	b++;
	}
	}
	}

[SpencerMalone]

OOoooo you're right on the money, add an empty catch block and the issue goes away.

[nielsdos]

Yeah, I think the assignment of try_op should've been op_array->try_catch_array[j].try_op = b->start; which would make more sense and seems to solve your bug. I'm running the test suite right now.

[SpencerMalone]

Oh, so this isn't even yield or generator related, it's about the while loop.

function readLinesFrom($filename)
{
    $file = fopen($filename, 'r');
    if ($file === false) {
        throw new Exception;
    }
    try {
        while (true) { }
    } finally {
        fclose($file);
    }
}

seems to replicate as well

[SpencerMalone]

Also works thinned all the way to:

<?php

if (!isset($badvar)) {
    throw new Exception("Should happen");
}
try {
    while (true) { }
} finally {
    throw new Exception("Should not happen");
}

From there, thinning further seems tricky. Mostly noting this to help in documentation of what exactly your WIP fix is fixing.

[nielsdos]

It won't get simpler than that I think. That's a nice reproducer, thanks a lot for the effort; and for initially narrowing it down to the right optimization pass. I'll take that last reproducer as a test and I'm preparing a PR soon.