[wp-trac] [WordPress Trac] #64538: memoize wp_normalize_path

[WordPress Trac]

#64538: memoize wp_normalize_path
--------------------------------------+--------------------------
 Reporter:  josephscott               |       Owner:  dmsnell
     Type:  defect (bug)              |      Status:  reopened
 Priority:  normal                    |   Milestone:  7.0
Component:  General                   |     Version:  3.9
 Severity:  normal                    |  Resolution:
 Keywords:  has-patch has-unit-tests  |     Focuses:  performance
--------------------------------------+--------------------------

Comment (by josephscott):

 I had Claude code go through the PHP source changes to see if something
 could be found to explain why this only failed on those two versions of
 PHP ( 8.1 and 8.2 ).  It looks like the way this test works was
 particularly unlucky.

 {{{
 PHP 8.1/8.2 Bug: ReflectionFunction::getStaticVariables() Returns Stale
 Values
   When OPcache Is Enabled

   ReflectionFunction::getStaticVariables() returns compile-time default
 values
   instead of current runtime values on PHP 8.1 and 8.2 when OPcache is
 enabled.

   Affected versions: PHP 8.1.0–8.1.x, 8.2.0–8.2.x (with OPcache enabled)
   Not affected: PHP 7.4, 8.0, 8.3+

   Cause

   Two changes combined to create this bug:

   1. PHP 8.1 changed how static variable pointers are stored. In PHP 8.0
 and
   earlier, the internal static_variables_ptr pointed directly at the
 function's
   own static_variables field, so reflection always read the live data. PHP
 8.1
   switched to a separate storage slot initialized to NULL, relying on the
   ZEND_BIND_STATIC opcode to populate it at runtime.
   2. OPcache's Dead Code Elimination (DCE) incorrectly treats
 ZEND_BIND_STATIC as
   side-effect-free for static variables with simple initializers (like
 static
   $cache = array()). This allows the optimizer to eliminate or weaken the
 opcode,
   so the storage slot is never populated. getStaticVariables() then falls
 back to
   returning the compile-time defaults.

   PHP 8.0 was unaffected because the self-referencing pointer masked the
   optimizer's behavior. PHP 8.3 fixed the issue via the
   https://wiki.php.net/rfc/arbitrary_static_variable_initializers
   (https://github.com/php/php-src/pull/9301), which introduced a new
   ZEND_BIND_INIT_STATIC_OR_JMP opcode that DCE always treats as side-
 effectful,
   and updated DCE to recognize that reference bindings with initializers
 are
   observable through reflection.

   Reproduction

   <?php
   // Run with: php -d opcache.enable_cli=1 repro.php

   function demo() {
       static $cache = array();
       if ( isset( $cache['key'] ) ) {
           return $cache['key'];
       }
       $cache['key'] = 'value';
       return $cache['key'];
   }

   demo();

   $statics = ( new ReflectionFunction( 'demo' ) )->getStaticVariables();

   // PHP 8.1/8.2 with OPcache: array() (empty — bug)
   // All other versions:        array( 'key' => 'value' )
   var_dump( $statics['cache'] );

   References

   - https://wiki.php.net/rfc/arbitrary_static_variable_initializers — the
 PHP 8.3
   RFC whose implementation fixed this as a side effect
   - https://github.com/php/php-src/pull/9301 — the implementation,
 including the
   DCE fix and the regression test optimize_static_002.phpt ("Keep
 BIND_STATIC when
    static variable has an initializer")
   - https://github.com/php/php-src/issues/9177 — related issue about
 static
   variables and reflection
   - https://www.npopov.com/2021/10/13/How-opcache-works.html — background
 on
   ZEND_MAP_PTR and immutable functions
 }}}

 I don't have a lot of experience with PHP internals, and given that this
 is already fixed I think it is more helpful to focus addressing the
 testing code.  What if we split this test into two -
 https://gist.github.com/josephscott/e1265dc63d878a53947ec6c4cc555390 - one
 for each group of PHP versions ( with and without this bug ).

 For `wp_normlize_path()` itself, I haven't found anything that needs to be
 addressed.

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/64538#comment:29>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform