From 0736b8a6c2d34ac54f3e056676758c80e3a716ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Vo=C5=99=C3=AD=C5=A1ek?= Date: Sat, 12 Nov 2022 14:20:29 +0100 Subject: [PATCH 1/6] dedup munmap call /w error handling --- Zend/zend_alloc.c | 40 ++++++++++++++++++---------------------- 1 file changed, 18 insertions(+), 22 deletions(-) diff --git a/Zend/zend_alloc.c b/Zend/zend_alloc.c index dfdc4e2bb4c14..9cde813784c61 100644 --- a/Zend/zend_alloc.c +++ b/Zend/zend_alloc.c @@ -416,6 +416,23 @@ stderr_last_error(char *msg) /* OS Allocation */ /*****************/ +static void zend_mm_munmap(void *addr, size_t size) +{ +#ifdef _WIN32 + if (VirtualFree(addr, 0, MEM_RELEASE) == 0) { +#if ZEND_MM_ERROR + stderr_last_error("VirtualFree() failed"); +#endif + } +#else + if (munmap(addr, size) != 0) { +#if ZEND_MM_ERROR + fprintf(stderr, "\nmunmap() failed: [%d] %s\n", errno, strerror(errno)); +#endif + } +#endif +} + #ifndef HAVE_MREMAP static void *zend_mm_mmap_fixed(void *addr, size_t size) { @@ -435,11 +452,7 @@ static void *zend_mm_mmap_fixed(void *addr, size_t size) #endif return NULL; } else if (ptr != addr) { - if (munmap(ptr, size) != 0) { -#if ZEND_MM_ERROR - fprintf(stderr, "\nmunmap() failed: [%d] %s\n", errno, strerror(errno)); -#endif - } + zend_mm_munmap(ptr, size); return NULL; } return ptr; @@ -483,23 +496,6 @@ static void *zend_mm_mmap(size_t size) #endif } -static void zend_mm_munmap(void *addr, size_t size) -{ -#ifdef _WIN32 - if (VirtualFree(addr, 0, MEM_RELEASE) == 0) { -#if ZEND_MM_ERROR - stderr_last_error("VirtualFree() failed"); -#endif - } -#else - if (munmap(addr, size) != 0) { -#if ZEND_MM_ERROR - fprintf(stderr, "\nmunmap() failed: [%d] %s\n", errno, strerror(errno)); -#endif - } -#endif -} - /***********/ /* Bitmask */ /***********/ From 8b92113b6bc4c8df2768222e67bc194b205a4383 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Vo=C5=99=C3=AD=C5=A1ek?= Date: Sat, 12 Nov 2022 14:31:19 +0100 Subject: [PATCH 2/6] add error handling for fixed Windows alloc --- Zend/zend_alloc.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/Zend/zend_alloc.c b/Zend/zend_alloc.c index 9cde813784c61..e27eb4f7ad9b5 100644 --- a/Zend/zend_alloc.c +++ b/Zend/zend_alloc.c @@ -437,7 +437,15 @@ static void zend_mm_munmap(void *addr, size_t size) static void *zend_mm_mmap_fixed(void *addr, size_t size) { #ifdef _WIN32 - return VirtualAlloc(addr, size, MEM_COMMIT | MEM_RESERVE, PAGE_READWRITE); + void *ptr = VirtualAlloc(addr, size, MEM_COMMIT | MEM_RESERVE, PAGE_READWRITE); + + if (ptr == NULL) { +#if ZEND_MM_ERROR + stderr_last_error("VirtualAlloc() fixed failed"); +#endif + return NULL; + } + return ptr; #else int flags = MAP_PRIVATE | MAP_ANON; #if defined(MAP_EXCL) @@ -448,7 +456,7 @@ static void *zend_mm_mmap_fixed(void *addr, size_t size) if (ptr == MAP_FAILED) { #if ZEND_MM_ERROR && !defined(MAP_EXCL) - fprintf(stderr, "\nmmap() failed: [%d] %s\n", errno, strerror(errno)); + fprintf(stderr, "\nmmap() fixed failed: [%d] %s\n", errno, strerror(errno)); #endif return NULL; } else if (ptr != addr) { From d8d8fd9035ee26f1e5da09ff359882ec02832067 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Vo=C5=99=C3=AD=C5=A1ek?= Date: Sat, 12 Nov 2022 14:27:27 +0100 Subject: [PATCH 3/6] fix log alloc errors only once --- Zend/zend_alloc.c | 18 +++--------------- 1 file changed, 3 insertions(+), 15 deletions(-) diff --git a/Zend/zend_alloc.c b/Zend/zend_alloc.c index e27eb4f7ad9b5..e0b32f296e7ab 100644 --- a/Zend/zend_alloc.c +++ b/Zend/zend_alloc.c @@ -1851,11 +1851,7 @@ static zend_mm_heap *zend_mm_init(void) if (UNEXPECTED(chunk == NULL)) { #if ZEND_MM_ERROR -#ifdef _WIN32 - stderr_last_error("Can't initialize heap"); -#else - fprintf(stderr, "\nCan't initialize heap: [%d] %s\n", errno, strerror(errno)); -#endif + fprintf(stderr, "Can't initialize heap\n"); #endif return NULL; } @@ -2982,11 +2978,7 @@ ZEND_API zend_mm_heap *zend_mm_startup_ex(const zend_mm_handlers *handlers, void chunk = (zend_mm_chunk*)handlers->chunk_alloc(&tmp_storage, ZEND_MM_CHUNK_SIZE, ZEND_MM_CHUNK_SIZE); if (UNEXPECTED(chunk == NULL)) { #if ZEND_MM_ERROR -#ifdef _WIN32 - stderr_last_error("Can't initialize heap"); -#else - fprintf(stderr, "\nCan't initialize heap: [%d] %s\n", errno, strerror(errno)); -#endif + fprintf(stderr, "Can't initialize heap\n"); #endif return NULL; } @@ -3029,11 +3021,7 @@ ZEND_API zend_mm_heap *zend_mm_startup_ex(const zend_mm_handlers *handlers, void if (!storage) { handlers->chunk_free(&tmp_storage, chunk, ZEND_MM_CHUNK_SIZE); #if ZEND_MM_ERROR -#ifdef _WIN32 - stderr_last_error("Can't initialize heap"); -#else - fprintf(stderr, "\nCan't initialize heap: [%d] %s\n", errno, strerror(errno)); -#endif + fprintf(stderr, "Can't initialize heap\n"); #endif return NULL; } From 93b71b8034af29b0a7dc1e1bb45f5794a2884f55 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Vo=C5=99=C3=AD=C5=A1ek?= Date: Sat, 12 Nov 2022 15:49:56 +0100 Subject: [PATCH 4/6] Fix fixed addr range alloc is not free for Windows --- Zend/zend_alloc.c | 39 +++++++++++++++++++++++++++++++++------ 1 file changed, 33 insertions(+), 6 deletions(-) diff --git a/Zend/zend_alloc.c b/Zend/zend_alloc.c index e0b32f296e7ab..3d209f38865b6 100644 --- a/Zend/zend_alloc.c +++ b/Zend/zend_alloc.c @@ -419,6 +419,14 @@ stderr_last_error(char *msg) static void zend_mm_munmap(void *addr, size_t size) { #ifdef _WIN32 + MEMORY_BASIC_INFORMATION mbi; + if (VirtualQuery(addr, &mbi, sizeof(mbi)) == 0) { +#if ZEND_MM_ERROR + stderr_last_error("VirtualQuery() failed"); +#endif + } + addr = mbi.AllocationBase; + if (VirtualFree(addr, 0, MEM_RELEASE) == 0) { #if ZEND_MM_ERROR stderr_last_error("VirtualFree() failed"); @@ -440,11 +448,16 @@ static void *zend_mm_mmap_fixed(void *addr, size_t size) void *ptr = VirtualAlloc(addr, size, MEM_COMMIT | MEM_RESERVE, PAGE_READWRITE); if (ptr == NULL) { + /** ERROR_INVALID_ADDRESS is expected when fixed addr range is not free */ + if (GetLastError() != ERROR_INVALID_ADDRESS) { #if ZEND_MM_ERROR - stderr_last_error("VirtualAlloc() fixed failed"); + stderr_last_error("VirtualAlloc() fixed failed"); #endif + } + SetLastError(0); return NULL; } + ZEND_ASSERT(ptr == addr); return ptr; #else int flags = MAP_PRIVATE | MAP_ANON; @@ -686,14 +699,28 @@ static void *zend_mm_chunk_alloc_int(size_t size, size_t alignment) zend_mm_munmap(ptr, size); ptr = zend_mm_mmap(size + alignment - REAL_PAGE_SIZE); #ifdef _WIN32 - offset = ZEND_MM_ALIGNED_OFFSET(ptr, alignment); - zend_mm_munmap(ptr, size + alignment - REAL_PAGE_SIZE); - ptr = zend_mm_mmap_fixed((void*)((char*)ptr + (alignment - offset)), size); offset = ZEND_MM_ALIGNED_OFFSET(ptr, alignment); if (offset != 0) { - zend_mm_munmap(ptr, size); - return NULL; + offset = alignment - offset; } + zend_mm_munmap(ptr, size + alignment - REAL_PAGE_SIZE); + ptr = zend_mm_mmap_fixed((void*)((char*)ptr + offset), size); + if (ptr == NULL) { // fix GH-9650, fixed addr range is not free + ptr = zend_mm_mmap(size + alignment - REAL_PAGE_SIZE); + if (ptr == NULL) { + return NULL; + } + offset = ZEND_MM_ALIGNED_OFFSET(ptr, alignment); + if (offset != 0) { + ptr = (void*)((char*)ptr + alignment - offset); + } + } else { + offset = ZEND_MM_ALIGNED_OFFSET(ptr, alignment); + if (offset != 0) { + zend_mm_munmap(ptr, size); + return NULL; + } + } return ptr; #else offset = ZEND_MM_ALIGNED_OFFSET(ptr, alignment); From e6ea3634d1edb652c18b92b626971a8fb32ae329 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Vo=C5=99=C3=AD=C5=A1ek?= Date: Mon, 14 Nov 2022 11:26:48 +0100 Subject: [PATCH 5/6] remove never reached code --- Zend/zend_alloc.c | 6 ------ 1 file changed, 6 deletions(-) diff --git a/Zend/zend_alloc.c b/Zend/zend_alloc.c index 3d209f38865b6..c10ef44380d8e 100644 --- a/Zend/zend_alloc.c +++ b/Zend/zend_alloc.c @@ -714,12 +714,6 @@ static void *zend_mm_chunk_alloc_int(size_t size, size_t alignment) if (offset != 0) { ptr = (void*)((char*)ptr + alignment - offset); } - } else { - offset = ZEND_MM_ALIGNED_OFFSET(ptr, alignment); - if (offset != 0) { - zend_mm_munmap(ptr, size); - return NULL; - } } return ptr; #else From 817fd193bdd0c3cf33fc41b5e4c7b686e3c1c077 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Vo=C5=99=C3=AD=C5=A1ek?= Date: Mon, 14 Nov 2022 15:16:31 +0100 Subject: [PATCH 6/6] call VirtualQuery only if VirtualFree failed first --- Zend/zend_alloc.c | 28 ++++++++++++++++++++-------- 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/Zend/zend_alloc.c b/Zend/zend_alloc.c index c10ef44380d8e..06ab26588fde2 100644 --- a/Zend/zend_alloc.c +++ b/Zend/zend_alloc.c @@ -419,18 +419,30 @@ stderr_last_error(char *msg) static void zend_mm_munmap(void *addr, size_t size) { #ifdef _WIN32 - MEMORY_BASIC_INFORMATION mbi; - if (VirtualQuery(addr, &mbi, sizeof(mbi)) == 0) { + if (VirtualFree(addr, 0, MEM_RELEASE) == 0) { + /** ERROR_INVALID_ADDRESS is expected when addr is not range start address */ + if (GetLastError() != ERROR_INVALID_ADDRESS) { #if ZEND_MM_ERROR - stderr_last_error("VirtualQuery() failed"); + stderr_last_error("VirtualFree() failed"); #endif - } - addr = mbi.AllocationBase; - - if (VirtualFree(addr, 0, MEM_RELEASE) == 0) { + return; + } + SetLastError(0); + + MEMORY_BASIC_INFORMATION mbi; + if (VirtualQuery(addr, &mbi, sizeof(mbi)) == 0) { +#if ZEND_MM_ERROR + stderr_last_error("VirtualQuery() failed"); +#endif + return; + } + addr = mbi.AllocationBase; + + if (VirtualFree(addr, 0, MEM_RELEASE) == 0) { #if ZEND_MM_ERROR - stderr_last_error("VirtualFree() failed"); + stderr_last_error("VirtualFree() failed"); #endif + } } #else if (munmap(addr, size) != 0) {