Skip to content

Fix Nightly workflow Symfony assertion (ir_ra.c:326: ir_fix_live_range: Assertion `ival && p->start == old_start' failed) #19458

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 12, 2025

Conversation

dstogov
Copy link
Member

@dstogov dstogov commented Aug 11, 2025

The problem occurred because the usage of SSA variable wasn't dominated by its definition.
PHP-JIT decided to use register for variable #5, but loaded the value to register (d_66) too late.

---- TRACE 3662 TSSA start (side trace 3479/8) {closure:Symfony\Component\Cache\Adapter\AbstractTagAwareAdapter::__construct():66}() /home/dmitry/php/community_tests/symfony/src/Symfony/Component/Cache/Adapter/AbstractTagAwareAdapter.php:76
     ;#1.CV1($value) [rc1, rcn, array of [any, ref]]
     ;#3.CV3($item) [rc1, rcn, object]
     ;#5.X5 [!long]
0018 ASSIGN_OBJ #3.CV3($item) [rc1, rcn, object] -> #9.CV3($item) [rc1, rcn, object] string("value") ; op1(object of class Symfony\Component\Cache\CacheItem) op3(int) val(null)
0019 ;OP_DATA #5.T5 [!long]
	 ...
---- TRACE 3662 TSSA stop (return)
---- TRACE 3662 Live Ranges "{closure:Symfony\Component\Cache\Adapter\AbstractTagAwareAdapter::__construct():66}"
#5.X5 last_use load

TRACE-3662${closure:Symfony\Component\Cache\Adapter\AbstractTagAwareAdapter::__construct():66}$76: ; after folding
{
    ...
	l_62 = IF(l_61, d_61);
	l_63 = IF_TRUE(l_62, 1);
	uintptr_t d_64, l_64 = RLOAD(l_63, 14);
	uintptr_t d_65 = ADD(d_64, c_9);
	int64_t d_66, l_66 = LOAD(l_64, d_65);                     # LOAD(T5)
	l_67 = STORE(l_66, d_64, c_21);                            # EX(opline) = ?
	l_68 = CALL/4(l_67, c_22, d_53, d_61, d_65, c_1);          # zend_jit_assign_to_typed_prop()
	l_70 = END(l_68);
	l_71 = IF_FALSE(l_62);
	bool d_72 = EQ(d_55, c_23);
	l_73 = IF(l_71, d_72);
	l_74 = IF_TRUE(l_73);
	uintptr_t d_75, l_75 = LOAD(l_74, d_53);
	uintptr_t d_76 = ADD(d_75, c_24);
	uintptr_t d_77, l_77 = LOAD(l_75, d_76);
	l_78 = IF(l_77, d_77);
	l_79 = IF_TRUE(l_78, 1);
	uintptr_t d_80, l_80 = RLOAD(l_79, 14);
	l_81 = STORE(l_80, d_80, c_21);                           
	uintptr_t d_82 = ADD(d_80, c_9);
	l_83 = STORE(l_81, d_82, d_66);                            # SPILL_STORE(T5)
	uintptr_t d_84, l_84 = CALL/2(l_83, c_25, d_75, d_82);     # zend_jit_assign_tmp_to_typed_ref()
	...

Fix just moves the load at start of jit code for ASSIGN_OBJ.

It seems like SPILL_STORE code is useless, because we perform STORE into the same location as previous LOAD.

…e: Assertion `ival && p->start == old_start' failed)
@DanielEScherzer
Copy link
Member

Is there a minimal reproduction from Symfony? If not, I can try to identify one

@dstogov dstogov requested a review from iluuu1994 August 11, 2025 20:05
@dstogov
Copy link
Member Author

dstogov commented Aug 11, 2025

Is there a minimal reproduction from Symfony? If not, I can try to identify one

I didn't try to separate the test case.
The bug was visible in Nightly work-flow (https://github.com/php/php-src/actions/runs/16711963280/job/47298397595).
It's also reproducible locally with the following command (php with tracing JIT)

php ./phpunit src/Symfony/Component/Cache/Tests/Adapter

If you can provide a small test case, it would be great.
JIT log and explanation above may be helpful.

@DanielEScherzer
Copy link
Member

DanielEScherzer commented Aug 12, 2025

Here is the smallest I could make (I spent way too much time on this)

<?php

class CacheItem {
    public string $key;
    protected mixed $value = null;

    public function get(): mixed {
        return $this->value;
    }

    public function set($value): void {
        $this->value = $value;
    }
}

class FilesystemTagAwareAdapter {

    public function __construct() {
        $directory = sys_get_temp_dir() . \DIRECTORY_SEPARATOR . 'symfony-cache';
        $directory .= \DIRECTORY_SEPARATOR . '@';
        if (!is_dir($directory)) {
            @mkdir($directory, 0777, true);
        }
        $directory .= \DIRECTORY_SEPARATOR;

        $this->directory = $directory;
        $this->tmpSuffix = str_replace('/', '-', base64_encode(random_bytes(6)));
    }

    private string $directory;
    private string $tmpSuffix;

    private function getFile(string $id): string {
        return $this->directory . str_replace('/', '-', base64_encode(hash('xxh128', $id, true)));
    }

    public function __destruct() {
        if (is_file($this->directory . $this->tmpSuffix)) {
            unlink($this->directory . $this->tmpSuffix);
        }
    }

    public function getItem(string $key): CacheItem {
        $createCacheItem = \Closure::bind(
            static function ($key, $value) {
                $item = new CacheItem();
                $item->key = $key;
                if (\is_array($value) && \array_key_exists('value', $value)) {
                    $item->value = $value['value'];
                }
                return $item;
            },
            null,
            CacheItem::class
        );

        $file = $this->getFile($key);
        if (!is_file($file) || !$h = @fopen($file, 'r')) {
            return ($createCacheItem)($key, null);
        }
        fgets($h);
        $i = rawurldecode(rtrim(fgets($h)));
        $value = stream_get_contents($h);
        fclose($h);

        if ($i === $key) {
            return ($createCacheItem)($key, unserialize($value));
        }

        return ($createCacheItem)($key, null);
    }

    public function save(CacheItem $item) {
        foreach ([$item] as $item) {
            $tmp = $this->directory . $this->tmpSuffix;
            $h = fopen($tmp, 'x');
            fwrite($h, (0)."\n".rawurlencode($item->key)."\n".serialize(['value' => $item->get()]));
            fclose($h);
            rename($tmp, $this->getFile($item->key));
        }
    }

}

$cache = new FilesystemTagAwareAdapter();
$item = $cache->getItem('key');
$item->set('5');
$cache->save($item);

$item = $cache->getItem('key');
$item->get();

$cache = new FilesystemTagAwareAdapter();
$item = $cache->getItem('key');
$item->set(5);
$cache->save($item);

$item = $cache->getItem('key');
$item->get();

triggered with
php -d memory_limit=-1 -d opcache.enable_cli=1 -d opcache.enable=1 -d opcache.jit=tracing -d opcache.jit_buffer_size=1G -d opcache.jit_max_root_traces=100000 -d opcache.jit_max_side_traces=100000 -d opcache.jit_max_exit_counters=100000 -d opcache.jit_hot_loop=1 -d opcache.jit_hot_func=1 -d opcache.jit_hot_return=1 -d opcache.jit_hot_side_exit=1 test.php
note that the failure is a bit flaky, try running it a few times if you don't hit it at first

But I was running this with master rather than 8.4

@dstogov dstogov merged commit 47f9f3a into php:PHP-8.4 Aug 12, 2025
9 checks passed
dstogov added a commit that referenced this pull request Aug 12, 2025
* PHP-8.4:
  Fix Nightly workflow Symfony assertion (ir_ra.c:326: ir_fix_live_range: Assertion `ival && p->start == old_start' failed) (#19458)
@dstogov
Copy link
Member Author

dstogov commented Aug 13, 2025

@DanielEScherzer thaks! The test case definitely reproduces the problem (if I revert the fix).

dstogov added a commit that referenced this pull request Aug 13, 2025
dstogov added a commit that referenced this pull request Aug 13, 2025
* PHP-8.4:
  Added test for PR #19458
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants