Fix inheritance of class constants if mutable data used

[nikic]

Class constants from parent should always be directly reused,
rather than re-evaluated as a separate copy. Previously this used
to happen automatically, as we'd just inherit the class constant
entry from the parent class. With mutable data there may now be
a separate copy of the constant, so we need to use that copy
when updating constants. Otherwise we may evaluate the same
constant multiple times.

[nikic]

I find this solution a bit ugly, maybe I missed some simpler way to do this.

[dstogov]

The following patch works. I think it's a bit better.

diff --git a/Zend/zend_API.c b/Zend/zend_API.c
index 2ecfc6db2d..f8706f8600 100644
--- a/Zend/zend_API.c
+++ b/Zend/zend_API.c
@@ -1326,9 +1326,14 @@ ZEND_API HashTable *zend_separate_class_constants_table(zend_class_entry *class_
 
 	ZEND_HASH_MAP_FOREACH_STR_KEY_PTR(&class_type->constants_table, key, c) {
 		if (Z_TYPE(c->value) == IS_CONSTANT_AST) {
-			new_c = zend_arena_alloc(&CG(arena), sizeof(zend_class_constant));
-			memcpy(new_c, c, sizeof(zend_class_constant));
-			c = new_c;
+			if (c->ce == class_type) {
+				new_c = zend_arena_alloc(&CG(arena), sizeof(zend_class_constant));
+				memcpy(new_c, c, sizeof(zend_class_constant));
+				c = new_c;
+			} else {
+				c = zend_hash_find_ptr(CE_CONSTANTS_TABLE(c->ce), key);
+				ZEND_ASSERT(c);
+			}
 		}
 		Z_TRY_ADDREF(c->value);
 		_zend_hash_append_ptr(constants_table, key, c);
@@ -1412,8 +1417,19 @@ ZEND_API zend_result zend_update_class_constants(zend_class_entry *class_type) /
 		} else {
 			constants_table = &class_type->constants_table;
 		}
-		ZEND_HASH_MAP_FOREACH_PTR(constants_table, c) {
+
+		zend_string *name;
+		ZEND_HASH_MAP_FOREACH_STR_KEY_VAL(constants_table, name, val) {
+			c = Z_PTR_P(val);
 			if (Z_TYPE(c->value) == IS_CONSTANT_AST) {
+				if (c->ce != class_type) {
+					c = zend_hash_find_ptr(CE_CONSTANTS_TABLE(c->ce), name);
+					ZEND_ASSERT(c);
+					Z_PTR_P(val) = c;
+					if (Z_TYPE(c->value) != IS_CONSTANT_AST) {
+						continue;
+					}
+				}
 				val = &c->value;
 				if (UNEXPECTED(zval_update_constant_ex(val, c->ce) != SUCCESS)) {
 					return FAILURE;

[nikic]

It's not directly related, but I see an odd inconsistency: We update constants on the parent first, so those will always be already resolved. But we don't update constants on implemented interfaces first, so constants inherited from interfaces will be evaluated in every class (unless accessed in the interface first). Would it make sense to first update constants on interfaces as well?

[dstogov]

It's not directly related, but I see an odd inconsistency: We update constants on the parent first, so those will always be already resolved. But we don't update constants on implemented interfaces first, so constants inherited from interfaces will be evaluated in every class (unless accessed in the interface first). Would it make sense to first update constants on interfaces as well?

The last patch should already fix this.