Don't reverse class order during preloading

We don't guarantee any particular order, but this reduces test failures under --preload that are sensitive to class order. Add some ZEND_HASH_FOREACH_*_FROM macros to allow skipping the persistent classes while iterating in forward direction.
php · Jul 30, 2021 · 67b5d8f · 67b5d8f
1 parent d836046
commit 67b5d8f
Show file tree

Hide file tree

Showing 2 changed files with 34 additions and 27 deletions.
diff --git a/Zend/zend_hash.h b/Zend/zend_hash.h
@@ -958,17 +958,19 @@ static zend_always_inline void *zend_hash_get_current_data_ptr_ex(HashTable *ht,
 #define zend_hash_get_current_data_ptr(ht) \
 	zend_hash_get_current_data_ptr_ex(ht, &(ht)->nInternalPointer)
 
-#define ZEND_HASH_FOREACH(_ht, indirect) do { \
+#define ZEND_HASH_FOREACH_FROM(_ht, indirect, _from) do { \
 		HashTable *__ht = (_ht); \
-		Bucket *_p = __ht->arData; \
-		Bucket *_end = _p + __ht->nNumUsed; \
+		Bucket *_p = __ht->arData + (_from); \
+		Bucket *_end = __ht->arData + __ht->nNumUsed; \
 		for (; _p != _end; _p++) { \
 			zval *_z = &_p->val; \
 			if (indirect && Z_TYPE_P(_z) == IS_INDIRECT) { \
 				_z = Z_INDIRECT_P(_z); \
 			} \
 			if (UNEXPECTED(Z_TYPE_P(_z) == IS_UNDEF)) continue;
 
+#define ZEND_HASH_FOREACH(_ht, indirect) ZEND_HASH_FOREACH_FROM(_ht, indirect, 0)
+
 #define ZEND_HASH_REVERSE_FOREACH(_ht, indirect) do { \
 		HashTable *__ht = (_ht); \
 		uint32_t _idx = __ht->nNumUsed; \
@@ -1011,6 +1013,10 @@ static zend_always_inline void *zend_hash_get_current_data_ptr_ex(HashTable *ht,
 	ZEND_HASH_FOREACH(ht, 0); \
 	_bucket = _p;
 
+#define ZEND_HASH_FOREACH_BUCKET_FROM(ht, _bucket, _from) \
+	ZEND_HASH_FOREACH_FROM(ht, 0, _from); \
+	_bucket = _p;
+
 #define ZEND_HASH_REVERSE_FOREACH_BUCKET(ht, _bucket) \
 	ZEND_HASH_REVERSE_FOREACH(ht, 0); \
 	_bucket = _p;
@@ -1080,6 +1086,11 @@ static zend_always_inline void *zend_hash_get_current_data_ptr_ex(HashTable *ht,
 	_key = _p->key; \
 	_val = _z;
 
+#define ZEND_HASH_FOREACH_STR_KEY_VAL_FROM(ht, _key, _val, _from) \
+	ZEND_HASH_FOREACH_FROM(ht, 0, _from); \
+	_key = _p->key; \
+	_val = _z;
+
 #define ZEND_HASH_REVERSE_FOREACH_STR_KEY_VAL(ht, _key, _val) \
 	ZEND_HASH_REVERSE_FOREACH(ht, 0); \
 	_key = _p->key; \

diff --git a/ext/opcache/ZendAccelerator.c b/ext/opcache/ZendAccelerator.c
@@ -3564,32 +3564,28 @@ static void preload_move_user_classes(HashTable *src, HashTable *dst)
 
 	src->pDestructor = NULL;
 	zend_hash_extend(dst, dst->nNumUsed + src->nNumUsed, 0);
-	ZEND_HASH_REVERSE_FOREACH_BUCKET(src, p) {
+	ZEND_HASH_FOREACH_BUCKET_FROM(src, p, EG(persistent_classes_count)) {
 		zend_class_entry *ce = Z_PTR(p->val);
-
-		if (EXPECTED(ce->type == ZEND_USER_CLASS)) {
-			if (ce->info.user.filename != filename) {
-				filename = ce->info.user.filename;
-				if (filename) {
-					if (!(copy = zend_hash_exists(preload_scripts, filename))) {
-						size_t eval_len = preload_try_strip_filename(filename);
-						if (eval_len) {
-							copy = zend_hash_str_exists(preload_scripts, ZSTR_VAL(filename), eval_len);
-						}
+		ZEND_ASSERT(ce->type == ZEND_USER_CLASS);
+		if (ce->info.user.filename != filename) {
+			filename = ce->info.user.filename;
+			if (filename) {
+				if (!(copy = zend_hash_exists(preload_scripts, filename))) {
+					size_t eval_len = preload_try_strip_filename(filename);
+					if (eval_len) {
+						copy = zend_hash_str_exists(preload_scripts, ZSTR_VAL(filename), eval_len);
 					}
-				} else {
-					copy = 0;
 				}
-			}
-			if (copy) {
-				_zend_hash_append(dst, p->key, &p->val);
 			} else {
-				orig_dtor(&p->val);
+				copy = 0;
 			}
-			zend_hash_del_bucket(src, p);
+		}
+		if (copy) {
+			_zend_hash_append(dst, p->key, &p->val);
 		} else {
-			break;
+			orig_dtor(&p->val);
 		}
+		zend_hash_del_bucket(src, p);
 	} ZEND_HASH_FOREACH_END();
 	src->pDestructor = orig_dtor;
 }
@@ -3844,11 +3840,10 @@ static void preload_link(void)
 	do {
 		changed = 0;
 
-		ZEND_HASH_REVERSE_FOREACH_STR_KEY_VAL(EG(class_table), key, zv) {
+		ZEND_HASH_FOREACH_STR_KEY_VAL_FROM(EG(class_table), key, zv, EG(persistent_classes_count)) {
 			ce = Z_PTR_P(zv);
-			if (ce->type == ZEND_INTERNAL_CLASS) {
-				break;
-			}
+			ZEND_ASSERT(ce->type != ZEND_INTERNAL_CLASS);
+
 			if ((ce->ce_flags & (ZEND_ACC_TOP_LEVEL|ZEND_ACC_ANON_CLASS))
 			 && !(ce->ce_flags & ZEND_ACC_LINKED)) {
 				zend_string *lcname = zend_string_tolower(ce->name);
@@ -3967,7 +3962,8 @@ static void preload_link(void)
 	} while (changed);
 
 	/* Warn for classes that could not be linked. */
-	ZEND_HASH_REVERSE_FOREACH_STR_KEY_VAL(EG(class_table), key, zv) {
+	ZEND_HASH_FOREACH_STR_KEY_VAL_FROM(
+			EG(class_table), key, zv, EG(persistent_classes_count)) {
 		ce = Z_PTR_P(zv);
 		if (ce->type == ZEND_INTERNAL_CLASS) {
 			break;