Assertion failure for TRACK_VARS_SERVER #15905

Activity

1. YuanchengJiang

   added

   Bug

   Status: Needs Triage

   on Sep 16, 2024
2. cmb69

   added

   Category: Engine

   Status: Verified

   and removed

   Status: Needs Triage

   on Sep 16, 2024
3. cmb69

   changed the title [-]Core dumped with `-d "variables_order=E" -d "auto_globals_jit=0"`[/-] [+]Assertion failure with `-d "variables_order=E" -d "auto_globals_jit=0"`[/+] on Sep 16, 2024
4. cmb69 commented on Sep 16, 2024

   

   cmb69

   on Sep 16, 2024

   Member

   More actions

   Good finding, @YuanchengJiang!

   Relevant:

   php-src/main/php_variables.c

   Lines 902 to 905 in c65e042

   	/* TODO: TRACK_VARS_SERVER is modified in a number of places (e.g. phar) past this point,
   	* where rc>1 due to the $_SERVER global. Ideally this shouldn't happen, but for now we
   	* ignore this issue, as it would probably require larger changes. */
   	HT_ALLOW_COW_VIOLATION(Z_ARRVAL(PG(http_globals)[TRACK_VARS_SERVER]));

   However, with -d variables_order=E -d auto_globals_jit=0, PG(http_globals)[TRACK_VARS_SERVER]) is not properly initialized, so it looses the HASH_FLAG_ALLOW_COW_VIOLATION flag. A possible fix could be to initialize the hash table right away for this code path, i.e.:

    main/php_variables.c | 1 +
    1 file changed, 1 insertion(+)
   
   diff --git a/main/php_variables.c b/main/php_variables.c
   index c3b773516e..7569fd43e9 100644
   --- a/main/php_variables.c
   +++ b/main/php_variables.c
   @@ -893,6 +893,7 @@ static bool php_auto_globals_create_server(zend_string *name)
    	} else {
    		zval_ptr_dtor_nogc(&PG(http_globals)[TRACK_VARS_SERVER]);
    		array_init(&PG(http_globals)[TRACK_VARS_SERVER]);
   +		zend_hash_real_init_mixed(Z_ARRVAL(PG(http_globals)[TRACK_VARS_SERVER]));
    	}
    
    	check_http_proxy(Z_ARRVAL(PG(http_globals)[TRACK_VARS_SERVER]));

   👍React with 👍1Reacted by Jiang Yuancheng
5. ndossche commented on Sep 16, 2024

   

   ndossche

   on Sep 16, 2024

   Member

   More actions

   Reminds me of #13013 ...
   Anyway, I think your patch is fine @cmb69
6. cmb69

   self-assigned this

   on Sep 16, 2024
7. cmb69

   added a commit that references this issue on Sep 16, 2024

   Fix phpGH-15905: Assertion failure for TRACK_VARS_SERVER

   ...

   This issue will close once commit 3c6fb57 is merged into the 'master' branch.

   3c6fb57
8. cmb69

   changed the title [-]Assertion failure with `-d "variables_order=E" -d "auto_globals_jit=0"`[/-] [+]Assertion failure for TRACK_VARS_SERVER[/+] on Sep 16, 2024
9. cmb69

   linked a pull request that will close this issueFix GH-15905: Assertion failure for TRACK_VARS_SERVER #15930on Sep 16, 2024
10. cmb69 commented on Sep 16, 2024

    

    cmb69

    on Sep 16, 2024

    Member

    More actions

    Reminds me of #13013 ...

    Fascinating!
11. cmb69

    closed this as completedin 87d59d7on Sep 26, 2024
12. cmb69

    added 4 commits that reference this issue on Sep 26, 2024

    Merge branch 'PHP-8.2' into PHP-8.3

    ...

    This issue will close once commit bf8c01d is merged into the 'master' branch.

    bf8c01d

    Merge branch 'PHP-8.3' into PHP-8.4

    ...

    This issue will close once commit 05fce50 is merged into the 'master' branch.

    05fce50

    Merge branch 'PHP-8.4'

    ...

    This issue will close once commit d7030c2 is merged into the 'master' branch.

    d7030c2

    Fix GH-15905: Assertion failure for TRACK_VARS_SERVER

    ...

    This issue will close once commit 87d59d7 is merged into the 'master' branch.

    87d59d7