Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Merged
merged 2 commits into from
Mar 19, 2024
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0d88106
Compress interned string table offsets and increase maximum supported…
arnaud-lb Mar 11, 2024
c053f70
typo
arnaud-lb Mar 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 16 additions & 12 deletions ext/opcache/ZendAccelerator.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

5 here accounted for the null byte after ZSTR_LEN(s) + a slot for the collision chain.

#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,
Expand Down Expand Up @@ -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)) {
Expand Down Expand Up @@ -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)) {
Expand Down Expand Up @@ -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 */
Expand Down Expand Up @@ -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);
Expand All @@ -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),
Expand Down
5 changes: 5 additions & 0 deletions ext/opcache/ZendAccelerator.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion ext/opcache/tests/gh9259_001.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
12 changes: 11 additions & 1 deletion ext/opcache/zend_accelerator_module.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
6 changes: 1 addition & 5 deletions ext/opcache/zend_shared_alloc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
Loading