From c446d68f7c9de6da8dc614ca4e065d81130d3956 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Mon, 17 May 2021 15:43:31 +0200 Subject: [PATCH] Fixed bug #81046 Literal compaction was incorrectly assuming that literals with the same base literal and the same number of related literals would be equal. Maybe that was the case historically, but at least it isn't true in PHP 8, where FETCH_CONSTANT and INIT_METHOD have distinct literals at the second position. Fix this by making the cache key a concatenation of all literals, rather than just the base literal. We still distinguish the number of related literals based on a bias added to the string hash. --- NEWS | 2 ++ ext/opcache/Optimizer/compact_literals.c | 46 ++++++++++++++++++------ ext/opcache/tests/bug81046.phpt | 19 ++++++++++ 3 files changed, 57 insertions(+), 10 deletions(-) create mode 100644 ext/opcache/tests/bug81046.phpt diff --git a/NEWS b/NEWS index 92d0d8ff05a..8c5c599a0c7 100644 --- a/NEWS +++ b/NEWS @@ -31,6 +31,8 @@ PHP NEWS (Nikita) . Fixed bug #81015 (Opcache optimization assumes wrong part of ternary operator in if-condition). (Nikita) + . Fixed bug #81046 (Literal compaction merges non-equal related literals). + (Nikita) - PDO_MySQL: . Fixed bug #81037 (PDO discards error message text from prepared diff --git a/ext/opcache/Optimizer/compact_literals.c b/ext/opcache/Optimizer/compact_literals.c index 0e1529d2bd1..9340f2b055b 100644 --- a/ext/opcache/Optimizer/compact_literals.c +++ b/ext/opcache/Optimizer/compact_literals.c @@ -113,6 +113,41 @@ static uint32_t add_static_slot(HashTable *hash, return ret; } +static zend_string *create_str_cache_key(zval *literal, uint32_t flags) +{ + ZEND_ASSERT(Z_TYPE_P(literal) == IS_STRING); + uint32_t num_related = LITERAL_NUM_RELATED(flags); + if (num_related == 1) { + return zend_string_copy(Z_STR_P(literal)); + } + if ((flags & LITERAL_KIND_MASK) == LITERAL_VALUE) { + /* Don't merge LITERAL_VALUE that has related literals */ + return NULL; + } + + /* Concatenate all the related literals for the cache key. */ + zend_string *key; + if (num_related == 2) { + ZEND_ASSERT(Z_TYPE_P(literal + 1) == IS_STRING); + key = zend_string_concat2( + Z_STRVAL_P(literal), Z_STRLEN_P(literal), + Z_STRVAL_P(literal + 1), Z_STRLEN_P(literal + 1)); + } else if (num_related == 3) { + ZEND_ASSERT(Z_TYPE_P(literal + 1) == IS_STRING && Z_TYPE_P(literal + 2) == IS_STRING); + key = zend_string_concat3( + Z_STRVAL_P(literal), Z_STRLEN_P(literal), + Z_STRVAL_P(literal + 1), Z_STRLEN_P(literal + 1), + Z_STRVAL_P(literal + 2), Z_STRLEN_P(literal + 2)); + } else { + ZEND_ASSERT(0 && "Currently not needed"); + } + + /* Add a bias to the hash so we can distinguish keys + * that would otherwise be the same after concatenation. */ + ZSTR_H(key) = zend_string_hash_val(key) + num_related - 1; + return key; +} + void zend_optimizer_compact_literals(zend_op_array *op_array, zend_optimizer_ctx *ctx) { zend_op *opline, *end; @@ -407,16 +442,7 @@ void zend_optimizer_compact_literals(zend_op_array *op_array, zend_optimizer_ctx } break; case IS_STRING: { - if (LITERAL_NUM_RELATED(info[i].flags) == 1) { - key = zend_string_copy(Z_STR(op_array->literals[i])); - } else if ((info[i].flags & LITERAL_KIND_MASK) != LITERAL_VALUE) { - key = zend_string_init(Z_STRVAL(op_array->literals[i]), Z_STRLEN(op_array->literals[i]), 0); - ZSTR_H(key) = ZSTR_HASH(Z_STR(op_array->literals[i])) + - LITERAL_NUM_RELATED(info[i].flags) - 1; - } else { - /* Don't merge LITERAL_VALUE that has related literals */ - key = NULL; - } + key = create_str_cache_key(&op_array->literals[i], info[i].flags); if (key && (pos = zend_hash_find(&hash, key)) != NULL && Z_TYPE(op_array->literals[Z_LVAL_P(pos)]) == IS_STRING && LITERAL_NUM_RELATED(info[i].flags) == LITERAL_NUM_RELATED(info[Z_LVAL_P(pos)].flags) && diff --git a/ext/opcache/tests/bug81046.phpt b/ext/opcache/tests/bug81046.phpt new file mode 100644 index 00000000000..f01890cb781 --- /dev/null +++ b/ext/opcache/tests/bug81046.phpt @@ -0,0 +1,19 @@ +--TEST-- +Bug #81046: Literal compaction merges non-equal related literals +--FILE-- + +--EXPECT-- +int(1) +Method called