Skip to content

Commit c446d68

Browse files
committed
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.
1 parent f073663 commit c446d68

File tree

3 files changed

+57
-10
lines changed

3 files changed

+57
-10
lines changed

NEWS

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@ PHP NEWS
3131
(Nikita)
3232
. Fixed bug #81015 (Opcache optimization assumes wrong part of ternary
3333
operator in if-condition). (Nikita)
34+
. Fixed bug #81046 (Literal compaction merges non-equal related literals).
35+
(Nikita)
3436

3537
- PDO_MySQL:
3638
. Fixed bug #81037 (PDO discards error message text from prepared

ext/opcache/Optimizer/compact_literals.c

Lines changed: 36 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,41 @@ static uint32_t add_static_slot(HashTable *hash,
113113
return ret;
114114
}
115115

116+
static zend_string *create_str_cache_key(zval *literal, uint32_t flags)
117+
{
118+
ZEND_ASSERT(Z_TYPE_P(literal) == IS_STRING);
119+
uint32_t num_related = LITERAL_NUM_RELATED(flags);
120+
if (num_related == 1) {
121+
return zend_string_copy(Z_STR_P(literal));
122+
}
123+
if ((flags & LITERAL_KIND_MASK) == LITERAL_VALUE) {
124+
/* Don't merge LITERAL_VALUE that has related literals */
125+
return NULL;
126+
}
127+
128+
/* Concatenate all the related literals for the cache key. */
129+
zend_string *key;
130+
if (num_related == 2) {
131+
ZEND_ASSERT(Z_TYPE_P(literal + 1) == IS_STRING);
132+
key = zend_string_concat2(
133+
Z_STRVAL_P(literal), Z_STRLEN_P(literal),
134+
Z_STRVAL_P(literal + 1), Z_STRLEN_P(literal + 1));
135+
} else if (num_related == 3) {
136+
ZEND_ASSERT(Z_TYPE_P(literal + 1) == IS_STRING && Z_TYPE_P(literal + 2) == IS_STRING);
137+
key = zend_string_concat3(
138+
Z_STRVAL_P(literal), Z_STRLEN_P(literal),
139+
Z_STRVAL_P(literal + 1), Z_STRLEN_P(literal + 1),
140+
Z_STRVAL_P(literal + 2), Z_STRLEN_P(literal + 2));
141+
} else {
142+
ZEND_ASSERT(0 && "Currently not needed");
143+
}
144+
145+
/* Add a bias to the hash so we can distinguish keys
146+
* that would otherwise be the same after concatenation. */
147+
ZSTR_H(key) = zend_string_hash_val(key) + num_related - 1;
148+
return key;
149+
}
150+
116151
void zend_optimizer_compact_literals(zend_op_array *op_array, zend_optimizer_ctx *ctx)
117152
{
118153
zend_op *opline, *end;
@@ -407,16 +442,7 @@ void zend_optimizer_compact_literals(zend_op_array *op_array, zend_optimizer_ctx
407442
}
408443
break;
409444
case IS_STRING: {
410-
if (LITERAL_NUM_RELATED(info[i].flags) == 1) {
411-
key = zend_string_copy(Z_STR(op_array->literals[i]));
412-
} else if ((info[i].flags & LITERAL_KIND_MASK) != LITERAL_VALUE) {
413-
key = zend_string_init(Z_STRVAL(op_array->literals[i]), Z_STRLEN(op_array->literals[i]), 0);
414-
ZSTR_H(key) = ZSTR_HASH(Z_STR(op_array->literals[i])) +
415-
LITERAL_NUM_RELATED(info[i].flags) - 1;
416-
} else {
417-
/* Don't merge LITERAL_VALUE that has related literals */
418-
key = NULL;
419-
}
445+
key = create_str_cache_key(&op_array->literals[i], info[i].flags);
420446
if (key && (pos = zend_hash_find(&hash, key)) != NULL &&
421447
Z_TYPE(op_array->literals[Z_LVAL_P(pos)]) == IS_STRING &&
422448
LITERAL_NUM_RELATED(info[i].flags) == LITERAL_NUM_RELATED(info[Z_LVAL_P(pos)].flags) &&

ext/opcache/tests/bug81046.phpt

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
--TEST--
2+
Bug #81046: Literal compaction merges non-equal related literals
3+
--FILE--
4+
<?php
5+
6+
class Test {
7+
static function methoD() {
8+
echo "Method called\n";
9+
}
10+
}
11+
12+
const methoD = 1;
13+
var_dump(methoD);
14+
test::methoD();
15+
16+
?>
17+
--EXPECT--
18+
int(1)
19+
Method called

0 commit comments

Comments
 (0)