Skip to content

Commit

Permalink
Fixed incorrect DCE for ADD_ARRAY_ELEMENT instruction
Browse files Browse the repository at this point in the history
DCE might remove INIT_ARRAY instruction but then keep the related
ADD_ARRAY_ELEMENT, becuse its both operands need to be freed.

Fixes oss-fuzz #41309
  • Loading branch information
dstogov committed Nov 25, 2021
1 parent f662103 commit f302430
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 1 deletion.
7 changes: 6 additions & 1 deletion ext/opcache/Optimizer/dce.c
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,6 @@ static inline zend_bool may_have_side_effects(
case ZEND_ROPE_INIT:
case ZEND_ROPE_ADD:
case ZEND_INIT_ARRAY:
case ZEND_ADD_ARRAY_ELEMENT:
case ZEND_SPACESHIP:
case ZEND_STRLEN:
case ZEND_COUNT:
Expand All @@ -128,6 +127,12 @@ static inline zend_bool may_have_side_effects(
case ZEND_ARRAY_KEY_EXISTS:
/* No side effects */
return 0;
case ZEND_ADD_ARRAY_ELEMENT:
/* TODO: We can't free two vars. Keep instruction alive. <?php [0, "$a" => "$b"]; */
if ((opline->op1_type & (IS_VAR|IS_TMP_VAR)) && (opline->op2_type & (IS_VAR|IS_TMP_VAR))) {
return 1;
}
return 0;
case ZEND_ROPE_END:
/* TODO: Rope dce optimization, see #76446 */
return 1;
Expand Down
12 changes: 12 additions & 0 deletions ext/opcache/tests/opt/dce_011.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
--TEST--
Incorrect DCE of ADD_ARRAY_ELEMENT
--FILE--
<?php
[0, "$a" => "$b"];
?>
DONE
--EXPECTF--
Warning: Undefined variable $a in %sdce_011.php on line 2

Warning: Undefined variable $b in %sdce_011.php on line 2
DONE

0 comments on commit f302430

Please sign in to comment.