Skip to content

Comments

Fix preloaded constant propagated to file-cached script#21281

Closed
iluuu1994 wants to merge 3 commits intophp:PHP-8.4from
iluuu1994:gh-21052
Closed

Fix preloaded constant propagated to file-cached script#21281
iluuu1994 wants to merge 3 commits intophp:PHP-8.4from
iluuu1994:gh-21052

Conversation

@iluuu1994
Copy link
Member

Since GH-15021 preloaded constants are propagated to compiled scripts. This is problematic for file cache, which assumes all referenced zvals are either persistently allocated or local to the current script. However, preloaded constants live in shm as immutable, but not persistent.

To solve this, we'd need to duplicate propagated constants in the optimizer when file cache is used. This is error prone given it needs to happen in many places. It's debatable whether constant propagation is even correct in this case, as running the preloaded script on a restart isn't guaranteed to produce the same result.

Hence, avoid the issue for now by just not relying on preloaded symbols when file cache is used.

Fixes GH-21052

Since phpGH-15021 preloaded constants are propagated to compiled scripts. This is
problematic for file cache, which assumes all referenced zvals are either
persistently allocated or local to the current script. However, preloaded
constants live in shm as immutable, but not persistent.

To solve this, we'd need to duplicate propagated constants in the optimizer when
file cache is used. This is error prone given it needs to happen in many places.
It's debatable whether constant propagation is even correct in this case, as
running the preloaded script on a restart isn't guaranteed to produce the same
result.

Hence, avoid the issue for now by just not relying on preloaded symbols when
file cache is used.

Fixes phpGH-21052
Comment on lines +823 to +825
if (CG(compiler_options) & ZEND_COMPILE_WITH_FILE_CACHE) {
return true;
}
Copy link
Member

@dstogov dstogov Feb 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the reason for this change?
It's not covered by test(s) and it's not related to constants propagation.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm using the term constant propagation in the loose sense. It's function inlining in this case that causes the issue. I expanded the test to cover this case as well.

@iluuu1994 iluuu1994 closed this in ec5a1e0 Feb 24, 2026
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