Skip to content

Conversation

TimWolla
Copy link
Member

Fixes #19823 and makes the deprecation more reliable by triggering even when $_SERVER is not accessed.

/cc @nicolas-grekas

Fixes php#19823 and makes the deprecation more reliable by triggering
even when `$_SERVER` is not accessed.
@TimWolla TimWolla requested review from nielsdos and a team September 14, 2025 18:00
@TimWolla TimWolla requested a review from bukka as a code owner September 14, 2025 18:00
Copy link
Member

@nielsdos nielsdos left a comment

Choose a reason for hiding this comment

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

I'm not so convinced that changing the auto global JITting just for a deprecation is the right move.

@nielsdos
Copy link
Member

Although one could argue that register_argc_argv=1 is sufficiently rare for most applications that this doesn't matter in practice? I don't know.
I'd defer this to someone else who has more experience with this.

@TimWolla
Copy link
Member Author

Although one could argue that register_argc_argv=1 is sufficiently rare for most applications that this doesn't matter in practice?

Generally speaking, deprecations go through slower code paths (e.g. FCALL instead of ICALL / UCALL), since the expectation is that they'll become gradually less used (since they'll be removed).

In this specific case register_argc_argv was already disabled by default in the recommended INI files (and will be default-disabled with PHP 8.5 even without an INI file). So I would expect this to be sufficiently rare overall. And nowadays I also expect most applications to touch $_SERVER one way or another during the execution (e.g. to retrieve request headers), so it will be initialized anyways.

@nielsdos
Copy link
Member

Okay sure.
I can't double check whether the condition is right however, as I'm on vacation and don't have a development machine with me.

Copy link
Member

@edorian edorian left a comment

Choose a reason for hiding this comment

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

No RM objections 👍

@iluuu1994
Copy link
Member

Note that these tests only fail with a primed opcache file cache (opcache.file_cache and opcache.validate_timestamps=0). Maybe this is related to recording/replaying, with the deprecation being emitted an additional time? (It's emitted twice in the failed test with the given configuration). I haven't checked yet, but I can take a look.

@TimWolla
Copy link
Member Author

Maybe this is related to recording/replaying, with the deprecation being emitted an additional time?

Yes. The error message is recorded in OPcache, because the super-global JIT triggers the deprecation during compilation. With this change, the deprecation is triggered at SAPI start-up and will thus not be picked up by OPcache (and it's also arguably more correct to not trigger it at random stages during compilation, since it doesn't check what field of $_SERVER is accessed).

@TimWolla TimWolla merged commit c893591 into php:master Sep 17, 2025
9 checks passed
@TimWolla TimWolla deleted the server-jit-register-argc-argv branch September 17, 2025 17:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Nightly fails argc&argv test due to new deprecations
4 participants