From 3c872661c13a2543c28e1ce27ebd53876fc1327d Mon Sep 17 00:00:00 2001 From: nielsdos <7771979+nielsdos@users.noreply.github.com> Date: Thu, 22 Jun 2023 21:10:00 +0200 Subject: [PATCH] Fix GH-11507: String concatenation performance regression in 8.3 When the code was moved to solve the uaf for memory overflow, this caused the refcount to be higher than one in some self-concatenation scenarios. This in turn causes quadratic time performance problems when these concatenations happen in a loop. Closes GH-11508. --- NEWS | 4 ++++ Zend/zend_operators.c | 11 ++++++----- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/NEWS b/NEWS index deb75af657c..9171f2c5ae3 100644 --- a/NEWS +++ b/NEWS @@ -2,6 +2,10 @@ PHP NEWS ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||| ?? ??? ????, PHP 8.3.0alpha3 +- Core: + . Fixed bug GH-11507 (String concatenation performance regression in 8.3). + (nielsdos) + - MBString: . Implement mb_str_pad() RFC. (nielsdos) diff --git a/Zend/zend_operators.c b/Zend/zend_operators.c index 917557caf41..1afce6a1f89 100644 --- a/Zend/zend_operators.c +++ b/Zend/zend_operators.c @@ -2057,16 +2057,17 @@ has_op2_string:; } if (result == op1) { - /* special case, perform operations on result */ - result_str = zend_string_extend(op1_string, result_len, 0); - /* Free result after zend_string_extend(), as it may throw an out-of-memory error. If we - * free it before we would leave the released variable on the stack with shutdown trying - * to free it again. */ + /* Destroy the old result first to drop the refcount, such that $x .= ...; may happen in-place. */ if (free_op1_string) { /* op1_string will be used as the result, so we should not free it */ i_zval_ptr_dtor(result); + /* Set it to NULL in case that the extension will throw an out-of-memory error. + * Otherwise the shutdown sequence will try to free this again. */ + ZVAL_NULL(result); free_op1_string = false; } + /* special case, perform operations on result */ + result_str = zend_string_extend(op1_string, result_len, 0); /* account for the case where result_str == op1_string == op2_string and the realloc is done */ if (op1_string == op2_string) { if (free_op2_string) {