Fix bug #76281

Make sure we keep the smart-branch inhibiting NOP even if there are multiple NOPs in sequence.
php · Apr 27, 2018 · 6738d19 · 6738d19
1 parent 279ba58
commit 6738d19
Show file tree

Hide file tree

Showing 3 changed files with 34 additions and 3 deletions.
diff --git a/NEWS b/NEWS
@@ -9,6 +9,8 @@ PHP                                                                        NEWS
 - Opcache:
   . Fixed bug #76275 (Assertion failure in file cache when unserializing empty
     try_catch_array). (Nikita)
+  . Fixed bug #76281 (Opcache causes incorrect "undefined variable" errors).
+    (Nikita)
 
 - Reflection:
   . Fixed arginfo for array_replace(_recursive) and array_merge(_recursive).

diff --git a/ext/opcache/Optimizer/dfa_pass.c b/ext/opcache/Optimizer/dfa_pass.c
@@ -144,12 +144,13 @@ static void zend_ssa_remove_nops(zend_op_array *op_array, zend_ssa *ssa)
 			while (i < end) {
 				shiftlist[i] = i - target;
 				if (EXPECTED(op_array->opcodes[i].opcode != ZEND_NOP) ||
-				   /*keep NOP to support ZEND_VM_SMART_BRANCH */
-				   (i > 0 &&
+				   /* Keep NOP to support ZEND_VM_SMART_BRANCH. Using "target-1" instead of
+				    * "i-1" here to check the last non-NOP instruction. */
+				   (target > 0 &&
 				    i + 1 < op_array->last &&
 				    (op_array->opcodes[i+1].opcode == ZEND_JMPZ ||
 				     op_array->opcodes[i+1].opcode == ZEND_JMPNZ) &&
-				    zend_is_smart_branch(op_array->opcodes + i - 1))) {
+				    zend_is_smart_branch(op_array->opcodes + target - 1))) {
 					if (i != target) {
 						op_array->opcodes[target] = op_array->opcodes[i];
 						ssa->ops[target] = ssa->ops[i];

diff --git a/ext/opcache/tests/bug76281.phpt b/ext/opcache/tests/bug76281.phpt
@@ -0,0 +1,28 @@
+--TEST--
+Bug #76281: Opcache causes incorrect "undefined variable" errors
+--FILE--
+<?php
+
+function test($r, $action) {
+    $user_sub_resource = in_array($action, array('get_securityquestions', 'get_status', 'get_groupstats'));
+
+    $user_id = null;
+    if ($user_sub_resource && isset($r['user_id'])) {
+        $user_id = $r['user_id'];
+    }
+    else if (isset($r['id']))  {
+        $user_id = $r['id'];
+    }
+
+    if ($user_sub_resource) {
+        return 'foo';
+    }
+
+    return 'bar';
+}
+
+var_dump(test(['user_id' => 1, 'id' => 2], 'foo'));
+
+?>
+--EXPECT--
+string(3) "bar"