Compress interned string table offsets and increase maximum supported buffer size (#13676)

Compress interned string table offsets and increase maximum supported buffer size

The interned string buffer is organized as a header + a hash table + a
zend_string arena. Hash slots point to the arena, but are represented as 32bit
offsets from the buffer, which limits the maximum buffer size to about 4GiB.
However zend_strings are 8-byte aligned in the buffer, so we can compress the
3 lower bits. This allows to increase the maximum supported interned string
buffer size from 4095 MiB to 32767 MiB.

Author: arnaud-lb

ext/opcache/ZendAccelerator.c

@@ -404,22 +404,22 @@ static inline void accel_unlock_all(void)
 #define STRTAB_INVALID_POS 0
 
 #define STRTAB_HASH_TO_SLOT(tab, h) \
-	((uint32_t*)((char*)(tab) + sizeof(*(tab)) + ((h) & (tab)->nTableMask)))
+	((zend_string_table_pos_t*)((char*)(tab) + sizeof(*(tab)) + ((h) & (tab)->nTableMask)))
 #define STRTAB_STR_TO_POS(tab, s) \
-	((uint32_t)((char*)s - (char*)(tab)))
+	((zend_string_table_pos_t)(((char*)s - (char*)(tab)) / ZEND_STRING_TABLE_POS_ALIGNMENT))
 #define STRTAB_POS_TO_STR(tab, pos) \
-	((zend_string*)((char*)(tab) + (pos)))
+	((zend_string*)((char*)(tab) + ((uintptr_t)(pos) * ZEND_STRING_TABLE_POS_ALIGNMENT)))
 #define STRTAB_COLLISION(s) \
-	(*((uint32_t*)((char*)s - sizeof(uint32_t))))
+	(*((zend_string_table_pos_t*)((char*)s - sizeof(zend_string_table_pos_t))))
 #define STRTAB_STR_SIZE(s) \
-	ZEND_MM_ALIGNED_SIZE_EX(_ZSTR_HEADER_SIZE + ZSTR_LEN(s) + 5, 8)
+	ZEND_MM_ALIGNED_SIZE_EX(_ZSTR_STRUCT_SIZE(ZSTR_LEN(s)) + sizeof(zend_string_table_pos_t), ZEND_STRING_TABLE_POS_ALIGNMENT)
 #define STRTAB_NEXT(s) \
 	((zend_string*)((char*)(s) + STRTAB_STR_SIZE(s)))
 
 static void accel_interned_strings_restore_state(void)
 {
 	zend_string *s, *top;
-	uint32_t *hash_slot, n;
+	zend_string_table_pos_t *hash_slot, n;
 
 	/* clear removed content */
 	memset(ZCSG(interned_strings).saved_top,
@@ -465,7 +465,7 @@ static void accel_interned_strings_save_state(void)
 static zend_always_inline zend_string *accel_find_interned_string(zend_string *str)
 {
 	zend_ulong   h;
-	uint32_t     pos;
+	zend_string_table_pos_t pos;
 	zend_string *s;
 
 	if (IS_ACCEL_INTERNED(str)) {
@@ -500,7 +500,7 @@ static zend_always_inline zend_string *accel_find_interned_string(zend_string *s
 zend_string* ZEND_FASTCALL accel_new_interned_string(zend_string *str)
 {
 	zend_ulong   h;
-	uint32_t     pos, *hash_slot;
+	zend_string_table_pos_t pos, *hash_slot;
 	zend_string *s;
 
 	if (UNEXPECTED(file_cache_only)) {
@@ -575,7 +575,7 @@ static zend_string* ZEND_FASTCALL accel_new_interned_string_for_php(zend_string
 
 static zend_always_inline zend_string *accel_find_interned_string_ex(zend_ulong h, const char *str, size_t size)
 {
-	uint32_t     pos;
+	zend_string_table_pos_t pos;
 	zend_string *s;
 
 	/* check for existing interned string */
@@ -2842,7 +2842,7 @@ static zend_result zend_accel_init_shm(void)
 	} else {
 		/* Make sure there is always at least one interned string hash slot,
 		 * so the table can be queried unconditionally. */
-		accel_shared_globals_size = sizeof(zend_accel_shared_globals) + sizeof(uint32_t);
+		accel_shared_globals_size = sizeof(zend_accel_shared_globals) + sizeof(zend_string_table_pos_t);
 	}
 
 	accel_shared_globals = zend_shared_alloc(accel_shared_globals_size);
@@ -2869,18 +2869,22 @@ static zend_result zend_accel_init_shm(void)
 		hash_size |= (hash_size >> 8);
 		hash_size |= (hash_size >> 16);
 
-		ZCSG(interned_strings).nTableMask = hash_size << 2;
+		ZCSG(interned_strings).nTableMask =
+			hash_size * sizeof(zend_string_table_pos_t);
 		ZCSG(interned_strings).nNumOfElements = 0;
 		ZCSG(interned_strings).start =
 			(zend_string*)((char*)&ZCSG(interned_strings) +
 				sizeof(zend_string_table) +
-				((hash_size + 1) * sizeof(uint32_t))) +
+				((hash_size + 1) * sizeof(zend_string_table_pos_t))) +
 				8;
+		ZEND_ASSERT(((uintptr_t)ZCSG(interned_strings).start & 0x7) == 0); /* should be 8 byte aligned */
+
 		ZCSG(interned_strings).top =
 			ZCSG(interned_strings).start;
 		ZCSG(interned_strings).end =
 			(zend_string*)((char*)(accel_shared_globals + 1) + /* table data is stored after accel_shared_globals */
 				ZCG(accel_directives).interned_strings_buffer * 1024 * 1024);
+		ZEND_ASSERT(((uintptr_t)ZCSG(interned_strings).end - (uintptr_t)&ZCSG(interned_strings)) / ZEND_STRING_TABLE_POS_ALIGNMENT < ZEND_STRING_TABLE_POS_MAX);
 		ZCSG(interned_strings).saved_top = NULL;
 
 		memset((char*)&ZCSG(interned_strings) + sizeof(zend_string_table),

ext/opcache/ZendAccelerator.h

@@ -236,6 +236,11 @@ typedef struct _zend_string_table {
 	zend_string *saved_top;
 } zend_string_table;
 
+typedef uint32_t zend_string_table_pos_t;
+
+#define ZEND_STRING_TABLE_POS_MAX UINT32_MAX
+#define ZEND_STRING_TABLE_POS_ALIGNMENT 8
+
 typedef struct _zend_accel_shared_globals {
 	/* Cache Data Structures */
 	zend_ulong   hits;

ext/opcache/tests/gh9259_001.phpt

@@ -13,6 +13,6 @@ echo 'OK';
 
 ?>
 --EXPECTF--
-%sWarning opcache.interned_strings_buffer must be less than or equal to 4095, 131072 given%s
+%sWarning opcache.interned_strings_buffer must be less than or equal to 32767, 131072 given%s
 
 OK

ext/opcache/zend_accelerator_module.c

@@ -41,7 +41,17 @@
 #define STRING_NOT_NULL(s) (NULL == (s)?"":s)
 #define MIN_ACCEL_FILES 200
 #define MAX_ACCEL_FILES 1000000
-#define MAX_INTERNED_STRINGS_BUFFER_SIZE ((zend_long)((UINT32_MAX-PLATFORM_ALIGNMENT-sizeof(zend_accel_shared_globals))/(1024*1024)))
+/* Max value of opcache.interned_strings_buffer */
+#define MAX_INTERNED_STRINGS_BUFFER_SIZE ((zend_long)MIN( \
+	MIN( \
+		/* STRTAB_STR_TO_POS() must not overflow (zend_string_table_pos_t) */ \
+		(ZEND_STRING_TABLE_POS_MAX - sizeof(zend_string_table)) / (1024 * 1024 / ZEND_STRING_TABLE_POS_ALIGNMENT), \
+		/* nTableMask must not overflow (uint32_t) */ \
+		UINT32_MAX / (32 * 1024 * sizeof(zend_string_table_pos_t)) \
+	), \
+	/* SHM allocation must not overflow (size_t) */ \
+	(SIZE_MAX - sizeof(zend_accel_shared_globals)) / (1024 * 1024) \
+))
 #define TOKENTOSTR(X) #X
 
 static zif_handler orig_file_exists = NULL;

ext/opcache/zend_shared_alloc.c

@@ -369,11 +369,7 @@ void *zend_shared_alloc(size_t size)
 	ZEND_ASSERT(ZCG(locked));
 
 	int i;
-	unsigned int block_size = ZEND_ALIGNED_SIZE(size);
-
-	if (UNEXPECTED(block_size < size)) {
-		zend_accel_error_noreturn(ACCEL_LOG_ERROR, "Possible integer overflow in shared memory allocation (%zu + %zu)", size, PLATFORM_ALIGNMENT);
-	}
+	size_t block_size = ZEND_ALIGNED_SIZE(size);
 
 	if (block_size > ZSMMG(shared_free)) { /* No hope to find a big-enough block */
 		SHARED_ALLOC_FAILED();