Fix JIT for overridden properties with added hooks

Zend/Optimizer/zend_inference.h

@@ -31,6 +31,7 @@
 #define MAY_BE_PACKED_GUARD (1<<27) /* needs packed array guard */
 #define MAY_BE_CLASS_GUARD (1<<27) /* needs class guard */
 #define MAY_BE_GUARD (1<<28) /* needs type guard */
+#define MAY_BE_NEW_HOOKS_GUARD (1<<29) /* object has new guards compared to comp-time class */
 
 #define MAY_HAVE_DTOR \
 	(MAY_BE_OBJECT|MAY_BE_RESOURCE \

Zend/tests/gh17376.phpt

@@ -0,0 +1,77 @@
+--TEST--
+GH-17376: Child classes may add hooks to plain properties
+--FILE--
+<?php
+
+class A {
+ public $prop = 1;
+}
+
+class B extends A {
+ public $prop = 1 {
+ get {
+ echo __METHOD__, "\n";
+ return $this->prop;
+ }
+ set {
+ echo __METHOD__, "\n";
+ $this->prop = $value;
+ }
+ }
+}
+
+function test(A $a) {
+ echo "read\n";
+ var_dump($a->prop);
+ echo "write\n";
+ $a->prop = 42;
+ echo "read-write\n";
+ $a->prop += 43;
+ echo "pre-inc\n";
+ ++$a->prop;
+ echo "pre-dec\n";
+ --$a->prop;
+ echo "post-inc\n";
+ $a->prop++;
+ echo "post-dec\n";
+ $a->prop--;
+}
+
+echo "A\n";
+test(new A);
+echo "\nB\n";
+test(new B);
+
+?>
+--EXPECT--
+A
+read
+int(1)
+write
+read-write
+pre-inc
+pre-dec
+post-inc
+post-dec
+
+B
+read
+B::$prop::get
+int(1)
+write
+B::$prop::set
+read-write
+B::$prop::get
+B::$prop::set
+pre-inc
+B::$prop::get
+B::$prop::set
+pre-dec
+B::$prop::get
+B::$prop::set
+post-inc
+B::$prop::get
+B::$prop::set
+post-dec
+B::$prop::get
+B::$prop::set

Zend/zend_inheritance.c

@@ -1653,11 +1653,6 @@ void zend_build_properties_info_table(zend_class_entry *ce)
  table, parent_table,
  sizeof(zend_property_info *) * ce->parent->default_properties_count
  );
-
- /* Child did not add any new properties, we are done */
- if (ce->default_properties_count == ce->parent->default_properties_count) {
- return;
- }
 	}
 
 	ZEND_HASH_MAP_FOREACH_PTR(&ce->properties_info, prop) {

ext/opcache/jit/zend_jit_ir.c

@@ -14024,6 +14024,52 @@ static int zend_jit_class_guard(zend_jit_ctx *jit, const zend_op *opline, ir_ref
 	return 1;
 }
 
+/* Child classes that add new hooks to existing properties will violate the
+ * assumption that properties may be accessed directly. This guard makes sure no
+ * new hooked properties have been created in the object, compared to the trace
+ * class. For function JIT, we just pick the slow path. */
+static bool zend_jit_new_hooks_guard(
+	zend_jit_ctx *jit,
+	const zend_op *opline,
+	zend_ssa *ssa,
+	const zend_ssa_op *ssa_op,
+	ir_ref obj_ref,
+	uint32_t op1_info,
+	zend_class_entry *ce,
+	bool ce_is_instanceof,
+	zend_property_info *prop_info,
+	ir_ref *slow_inputs
+) {
+	if ((JIT_G(trigger) != ZEND_JIT_ON_HOT_TRACE || !(op1_info & MAY_BE_NEW_HOOKS_GUARD))
+ && ce_is_instanceof
+ && !(prop_info->flags & ZEND_ACC_FINAL)) {
+ ir_ref num_hooked_props = ir_LOAD_A(ir_ADD_OFFSET(obj_ref, offsetof(zend_object, ce)));
+ num_hooked_props = ir_LOAD_U32(ir_ADD_OFFSET(num_hooked_props, offsetof(zend_class_entry, num_hooked_props)));
+ ir_ref has_same_num_hooked_props = ir_EQ(num_hooked_props, ir_CONST_U32(ce->num_hooked_props));
+ if (JIT_G(trigger) == ZEND_JIT_ON_HOT_TRACE) {
+ /* We use EXIT_TO_VM to avoid creating new side-traces that would
+ * just immediately fail. To make side-traces work properly, the
+ * side-trace needs to create a class guard for the given subclass. */
+ int32_t exit_point = zend_jit_trace_get_exit_point(opline, ZEND_JIT_EXIT_TO_VM);
+ const void *exit_addr = zend_jit_trace_get_exit_addr(exit_point);
+ if (!exit_addr) {
+ return false;
+ }
+ ir_GUARD(has_same_num_hooked_props, ir_CONST_ADDR(exit_addr));
+ if (ssa->var_info && ssa_op->op1_use >= 0) {
+ ssa->var_info[ssa_op->op1_use].type |= MAY_BE_NEW_HOOKS_GUARD;
+ }
+ } else {
+ ir_ref if_same_num_hooked_props = ir_IF(has_same_num_hooked_props);
+ ir_IF_FALSE_cold(if_same_num_hooked_props);
+ ir_END_list(*slow_inputs);
+ ir_IF_TRUE(if_same_num_hooked_props);
+ }
+	}
+
+	return true;
+}
+
 static int zend_jit_fetch_obj(zend_jit_ctx *jit,
  const zend_op *opline,
  const zend_op_array *op_array,
@@ -14291,6 +14337,10 @@ static int zend_jit_fetch_obj(zend_jit_ctx *jit,
  }
  }
 	} else {
+ if (!zend_jit_new_hooks_guard(jit, opline, ssa, ssa_op, obj_ref, op1_info, ce, ce_is_instanceof, prop_info, &slow_inputs)) {
+ return 0;
+ }
+
  prop_ref = ir_ADD_OFFSET(obj_ref, prop_info->offset);
  prop_addr = ZEND_ADDR_REF_ZVAL(prop_ref);
  if (JIT_G(trigger) == ZEND_JIT_ON_HOT_TRACE) {
@@ -14435,7 +14485,7 @@ static int zend_jit_fetch_obj(zend_jit_ctx *jit,
  SET_STACK_REG(JIT_G(current_frame)->stack, EX_VAR_TO_NUM(opline->op1.var), ZREG_NONE);
 	}
 
-	if (JIT_G(trigger) != ZEND_JIT_ON_HOT_TRACE || !prop_info) {
+	if (slow_inputs) {
  ir_MERGE_list(slow_inputs);
  jit_SET_EX_OPLINE(jit, opline);
 
@@ -14748,6 +14798,10 @@ static int zend_jit_assign_obj(zend_jit_ctx *jit,
  ir_IF_FALSE(if_has_prop_info);
  }
 	} else {
+ if (!zend_jit_new_hooks_guard(jit, opline, ssa, ssa_op, obj_ref, op1_info, ce, ce_is_instanceof, prop_info, &slow_inputs)) {
+ return 0;
+ }
+
  prop_ref = ir_ADD_OFFSET(obj_ref, prop_info->offset);
  prop_addr = ZEND_ADDR_REF_ZVAL(prop_ref);
  /* With the exception of __clone(), readonly assignment always happens on IS_UNDEF, doding
@@ -15072,6 +15126,10 @@ static int zend_jit_assign_obj_op(zend_jit_ctx *jit,
  }
  prop_addr = ZEND_ADDR_REF_ZVAL(prop_ref);
 	} else {
+ if (!zend_jit_new_hooks_guard(jit, opline, ssa, ssa_op, obj_ref, op1_info, ce, ce_is_instanceof, prop_info, &slow_inputs)) {
+ return 0;
+ }
+
  prop_ref = ir_ADD_OFFSET(obj_ref, prop_info->offset);
  prop_addr = ZEND_ADDR_REF_ZVAL(prop_ref);
 
@@ -15495,6 +15553,10 @@ static int zend_jit_incdec_obj(zend_jit_ctx *jit,
  }
  prop_addr = ZEND_ADDR_REF_ZVAL(prop_ref);
 	} else {
+ if (!zend_jit_new_hooks_guard(jit, opline, ssa, ssa_op, obj_ref, op1_info, ce, ce_is_instanceof, prop_info, &slow_inputs)) {
+ return 0;
+ }
+
  prop_ref = ir_ADD_OFFSET(obj_ref, prop_info->offset);
  prop_addr = ZEND_ADDR_REF_ZVAL(prop_ref);