Skip to content

Commit 1f19401

Browse files
committed
Handle operand replacement in JMP_NULL
In this case it's not sufficient to replace the JMP_NULL operand, as it keeps the temporary alive and there may be more uses later. Fix this by generalizing existing handling for other similar opcodes like CASE/SWITCH and LIST_R. Fixes oss-fuzz 5820123475214336.
1 parent 22b6aac commit 1f19401

File tree

2 files changed

+49
-62
lines changed

2 files changed

+49
-62
lines changed

Zend/Optimizer/zend_optimizer.c

Lines changed: 33 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -281,6 +281,14 @@ int zend_optimizer_update_op1_const(zend_op_array *op_array,
281281
opline->opcode = ZEND_SEND_VAL;
282282
opline->op1.constant = zend_optimizer_add_literal(op_array, val);
283283
break;
284+
case ZEND_CASE:
285+
opline->opcode = ZEND_IS_EQUAL;
286+
opline->op1.constant = zend_optimizer_add_literal(op_array, val);
287+
break;
288+
case ZEND_CASE_STRICT:
289+
opline->opcode = ZEND_IS_IDENTICAL;
290+
opline->op1.constant = zend_optimizer_add_literal(op_array, val);
291+
break;
284292
case ZEND_SEPARATE:
285293
case ZEND_SEND_VAR_NO_REF:
286294
case ZEND_SEND_VAR_NO_REF_EX:
@@ -289,9 +297,6 @@ int zend_optimizer_update_op1_const(zend_op_array *op_array,
289297
/* This would require a non-local change.
290298
* zend_optimizer_replace_by_const() supports this. */
291299
return 0;
292-
case ZEND_CASE:
293-
case ZEND_CASE_STRICT:
294-
case ZEND_FETCH_LIST_R:
295300
case ZEND_COPY_TMP:
296301
case ZEND_FETCH_CLASS_NAME:
297302
return 0;
@@ -562,72 +567,38 @@ int zend_optimizer_replace_by_const(zend_op_array *op_array,
562567
break;
563568
/* In most cases IS_TMP_VAR operand may be used only once.
564569
* The operands are usually destroyed by the opcode handler.
565-
* ZEND_CASE[_STRICT] and ZEND_FETCH_LIST_R are exceptions, they keeps operand
566-
* unchanged, and allows its reuse. these instructions
567-
* usually terminated by ZEND_FREE that finally kills the value.
570+
* However, there are some exception which keep the operand alive. In that case
571+
* we want to try to replace all uses of the temporary.
568572
*/
569-
case ZEND_FETCH_LIST_R: {
570-
zend_op *m = opline;
571-
572-
do {
573-
if (m->opcode == ZEND_FETCH_LIST_R &&
574-
m->op1_type == type &&
575-
m->op1.var == var) {
576-
zval v;
577-
ZVAL_COPY(&v, val);
578-
if (Z_TYPE(v) == IS_STRING) {
579-
zend_string_hash_val(Z_STR(v));
580-
}
581-
m->op1.constant = zend_optimizer_add_literal(op_array, &v);
582-
m->op1_type = IS_CONST;
583-
}
584-
m++;
585-
} while (m->opcode != ZEND_FREE || m->op1_type != type || m->op1.var != var);
586-
587-
ZEND_ASSERT(m->opcode == ZEND_FREE && m->op1_type == type && m->op1.var == var);
588-
MAKE_NOP(m);
589-
zval_ptr_dtor_nogc(val);
590-
return 1;
591-
}
573+
case ZEND_FETCH_LIST_R:
574+
case ZEND_CASE:
575+
case ZEND_CASE_STRICT:
592576
case ZEND_SWITCH_LONG:
593577
case ZEND_SWITCH_STRING:
594578
case ZEND_MATCH:
595-
case ZEND_CASE:
596-
case ZEND_CASE_STRICT: {
579+
case ZEND_JMP_NULL: {
597580
zend_op *end = op_array->opcodes + op_array->last;
598581
while (opline < end) {
599582
if (opline->op1_type == type && opline->op1.var == var) {
600-
if (
601-
opline->opcode == ZEND_CASE
602-
|| opline->opcode == ZEND_CASE_STRICT
603-
|| opline->opcode == ZEND_SWITCH_LONG
604-
|| opline->opcode == ZEND_SWITCH_STRING
605-
|| opline->opcode == ZEND_MATCH
606-
) {
607-
zval v;
608-
609-
if (opline->opcode == ZEND_CASE) {
610-
opline->opcode = ZEND_IS_EQUAL;
611-
} else if (opline->opcode == ZEND_CASE_STRICT) {
612-
opline->opcode = ZEND_IS_IDENTICAL;
613-
}
614-
ZVAL_COPY(&v, val);
615-
if (Z_TYPE(v) == IS_STRING) {
616-
zend_string_hash_val(Z_STR(v));
617-
}
618-
opline->op1.constant = zend_optimizer_add_literal(op_array, &v);
619-
opline->op1_type = IS_CONST;
620-
} else if (opline->opcode == ZEND_FREE) {
621-
if (opline->extended_value == ZEND_FREE_SWITCH) {
622-
/* We found the end of the switch. */
623-
MAKE_NOP(opline);
624-
break;
625-
}
626-
627-
ZEND_ASSERT(opline->extended_value == ZEND_FREE_ON_RETURN);
628-
MAKE_NOP(opline);
629-
} else {
630-
ZEND_UNREACHABLE();
583+
/* If this opcode doesn't keep the operand alive, we're done. Check
584+
* this early, because op replacement may modify the opline. */
585+
bool is_last = opline->opcode != ZEND_FETCH_LIST_R
586+
&& opline->opcode != ZEND_CASE
587+
&& opline->opcode != ZEND_CASE_STRICT
588+
&& opline->opcode != ZEND_SWITCH_LONG
589+
&& opline->opcode != ZEND_SWITCH_STRING
590+
&& opline->opcode != ZEND_MATCH
591+
&& opline->opcode != ZEND_JMP_NULL
592+
&& (opline->opcode != ZEND_FREE
593+
|| opline->extended_value != ZEND_FREE_ON_RETURN);
594+
595+
Z_TRY_ADDREF_P(val);
596+
if (!zend_optimizer_update_op1_const(op_array, opline, val)) {
597+
zval_ptr_dtor(val);
598+
return 0;
599+
}
600+
if (is_last) {
601+
break;
631602
}
632603
}
633604
opline++;
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
--TEST--
2+
Constant propagation with nullsafe operator
3+
--FILE--
4+
<?php
5+
6+
class Bar { const FOO = "foo"; }
7+
8+
try {
9+
Bar::FOO?->length();
10+
} catch (Error $e) {
11+
echo $e->getMessage(), "\n";
12+
}
13+
14+
?>
15+
--EXPECT--
16+
Call to a member function length() on string

0 commit comments

Comments
 (0)