Skip to content

Commit

Permalink
Try memory leak workaround with zeroed mem (#1229)
Browse files Browse the repository at this point in the history
This PR attempts to fix the memory leak reported in #1128 and is an
iteration on PR #1189 which had problems with "memory out of bounds"
errors.

## What problem is it solving?

It stops PHP from leaking memory throughout the runtime of a script and
hopefully stops memory out of bounds errors by zeroing all memory given
to PHP.

## How is the problem addressed?

- By avoiding mmap()/munmap() which have incomplete implementations in
Emscripten
- By using posix_memalign() to allocate memory instead and manually
zeroing the memory before handing it to PHP

## Testing Instructions

- Observe CI test results
- Use `npm run dev` and exercise Playground locally in the browser
  • Loading branch information
brandonpayton authored Apr 11, 2024
1 parent d135caf commit e36e1b9
Show file tree
Hide file tree
Showing 63 changed files with 381 additions and 27 deletions.
41 changes: 41 additions & 0 deletions packages/php-wasm/compile/php-wasm-memory-storage/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
*.lo
*.la
.libs
acinclude.m4
aclocal.m4
autom4te.cache
build
config.guess
config.h
config.h.in
config.log
config.nice
config.status
config.sub
configure
configure.ac
configure.in
include
install-sh
libtool
ltmain.sh
Makefile
Makefile.fragments
Makefile.global
Makefile.objects
missing
mkinstalldirs
modules
php_test_results_*.txt
phpt.*
run-test-info.php
run-tests.php
tests/**/*.diff
tests/**/*.out
tests/**/*.php
tests/**/*.exp
tests/**/*.log
tests/**/*.sh
tests/**/*.db
tests/**/*.mem
tmp-php.ini
94 changes: 94 additions & 0 deletions packages/php-wasm/compile/php-wasm-memory-storage/config.m4
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
dnl config.m4 for extension wasm_memory_storage

dnl Comments in this file start with the string 'dnl'.
dnl Remove where necessary.

dnl If your extension references something external, use 'with':

dnl PHP_ARG_WITH([wasm_memory_storage],
dnl [for wasm_memory_storage support],
dnl [AS_HELP_STRING([--with-wasm_memory_storage],
dnl [Include wasm_memory_storage support])])

dnl Otherwise use 'enable':

PHP_ARG_ENABLE([wasm_memory_storage],
[whether to enable wasm_memory_storage support],
[AS_HELP_STRING([--enable-wasm_memory_storage],
[Enable wasm_memory_storage support])],
[no])

if test "$PHP_WASM_MEMORY_STORAGE" != "no"; then
dnl Write more examples of tests here...

dnl Remove this code block if the library does not support pkg-config.
dnl PKG_CHECK_MODULES([LIBFOO], [foo])
dnl PHP_EVAL_INCLINE($LIBFOO_CFLAGS)
dnl PHP_EVAL_LIBLINE($LIBFOO_LIBS, WASM_MEMORY_STORAGE_SHARED_LIBADD)

dnl If you need to check for a particular library version using PKG_CHECK_MODULES,
dnl you can use comparison operators. For example:
dnl PKG_CHECK_MODULES([LIBFOO], [foo >= 1.2.3])
dnl PKG_CHECK_MODULES([LIBFOO], [foo < 3.4])
dnl PKG_CHECK_MODULES([LIBFOO], [foo = 1.2.3])

dnl Remove this code block if the library supports pkg-config.
dnl --with-wasm_memory_storage -> check with-path
dnl SEARCH_PATH="/usr/local /usr" # you might want to change this
dnl SEARCH_FOR="/include/wasm_memory_storage.h" # you most likely want to change this
dnl if test -r $PHP_WASM_MEMORY_STORAGE/$SEARCH_FOR; then # path given as parameter
dnl WASM_MEMORY_STORAGE_DIR=$PHP_WASM_MEMORY_STORAGE
dnl else # search default path list
dnl AC_MSG_CHECKING([for wasm_memory_storage files in default path])
dnl for i in $SEARCH_PATH ; do
dnl if test -r $i/$SEARCH_FOR; then
dnl WASM_MEMORY_STORAGE_DIR=$i
dnl AC_MSG_RESULT(found in $i)
dnl fi
dnl done
dnl fi
dnl
dnl if test -z "$WASM_MEMORY_STORAGE_DIR"; then
dnl AC_MSG_RESULT([not found])
dnl AC_MSG_ERROR([Please reinstall the wasm_memory_storage distribution])
dnl fi

dnl Remove this code block if the library supports pkg-config.
dnl --with-wasm_memory_storage -> add include path
dnl PHP_ADD_INCLUDE($WASM_MEMORY_STORAGE_DIR/include)

dnl Remove this code block if the library supports pkg-config.
dnl --with-wasm_memory_storage -> check for lib and symbol presence
dnl LIBNAME=WASM_MEMORY_STORAGE # you may want to change this
dnl LIBSYMBOL=WASM_MEMORY_STORAGE # you most likely want to change this

dnl If you need to check for a particular library function (e.g. a conditional
dnl or version-dependent feature) and you are using pkg-config:
dnl PHP_CHECK_LIBRARY($LIBNAME, $LIBSYMBOL,
dnl [
dnl AC_DEFINE(HAVE_WASM_MEMORY_STORAGE_FEATURE, 1, [ ])
dnl ],[
dnl AC_MSG_ERROR([FEATURE not supported by your wasm_memory_storage library.])
dnl ], [
dnl $LIBFOO_LIBS
dnl ])

dnl If you need to check for a particular library function (e.g. a conditional
dnl or version-dependent feature) and you are not using pkg-config:
dnl PHP_CHECK_LIBRARY($LIBNAME, $LIBSYMBOL,
dnl [
dnl PHP_ADD_LIBRARY_WITH_PATH($LIBNAME, $WASM_MEMORY_STORAGE_DIR/$PHP_LIBDIR, WASM_MEMORY_STORAGE_SHARED_LIBADD)
dnl AC_DEFINE(HAVE_WASM_MEMORY_STORAGE_FEATURE, 1, [ ])
dnl ],[
dnl AC_MSG_ERROR([FEATURE not supported by your wasm_memory_storage library.])
dnl ],[
dnl -L$WASM_MEMORY_STORAGE_DIR/$PHP_LIBDIR -lm
dnl ])
dnl
dnl PHP_SUBST(WASM_MEMORY_STORAGE_SHARED_LIBADD)

dnl In case of no dependencies
AC_DEFINE(HAVE_WASM_MEMORY_STORAGE, 1, [ Have wasm_memory_storage support ])

PHP_NEW_EXTENSION(wasm_memory_storage, wasm_memory_storage.c, $ext_shared)
fi
7 changes: 7 additions & 0 deletions packages/php-wasm/compile/php-wasm-memory-storage/config.w32
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
ARG_ENABLE('wasm_memory_storage', 'wasm_memory_storage support', 'no');

if (PHP_WASM_MEMORY_STORAGE != 'no') {
AC_DEFINE('HAVE_WASM_MEMORY_STORAGE', 1, 'wasm_memory_storage support enabled');

EXTENSION('wasm_memory_storage', 'wasm_memory_storage.c', null, '/DZEND_ENABLE_STATIC_TSRMLS_CACHE=1');
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/* wasm_memory_storage extension for PHP */

#ifndef PHP_WASM_MEMORY_STORAGE_H
# define PHP_WASM_MEMORY_STORAGE_H

extern zend_module_entry wasm_memory_storage_module_entry;
# define phpext_wasm_memory_storage_ptr &wasm_memory_storage_module_entry

# define PHP_WASM_MEMORY_STORAGE_VERSION "0.0.1"

#endif /* PHP_WASM_MEMORY_STORAGE_H */
Original file line number Diff line number Diff line change
@@ -0,0 +1,137 @@
/**
* wasm_memory_storage extension for PHP.
*
* The purpose of this extension is to work around a memory leak caused by
* failing attempts to partially unmap memory allocated with mmap().
* By providing custom memory storage, we can avoid mmap()/munmap() calls and
* use posix_memalign()/free() instead.
*
* Background:
* Issue: https://github.com/WordPress/wordpress-playground/issues/1128
* PR: https://github.com/WordPress/wordpress-playground/pull/1189
*/

#ifdef HAVE_CONFIG_H
# include "config.h"
#endif

#include <stdlib.h>
#include <string.h>
#include "php.h"
#include "ext/standard/info.h"
#include "Zend/zend_alloc.h"
#include "php_wasm_memory_storage.h"

/**
* Allocate a chunk of memory.
*
* This function implements the PHP Zend custom memory storage operation "chunk_free()".
*
* Here is an example offered in the PHP source code:
* https://github.com/php/php-src/blob/dbaeb62ab1e34067057170ab50cf39d1bde584d8/Zend/zend_alloc.h#L325
*
* @param storage The zend_mm_storage struct for this allocation.
* @param size The number of bytes to allocate.
* @param alignment The byte alignment of the memory allocation.
* @returns A pointer to allocated memory or NULL on failure.
*/
void* wasm_memory_storage_chunk_alloc(zend_mm_storage* storage, size_t size, size_t alignment)
{
void* ptr = NULL;
if (posix_memalign(&ptr, alignment, size) == 0)
{
memset(ptr, 0, size);
return ptr;
} else {
return NULL;
}
}

/**
* Free a chunk of memory.
*
* This function implements the PHP Zend custom memory storage operation "chunk_free()".
*
* Here is an example offered in the PHP source code:
* https://github.com/php/php-src/blob/dbaeb62ab1e34067057170ab50cf39d1bde584d8/Zend/zend_alloc.h#L352
*
* @param storage The zend_mm_storage struct for this memory.
* @param ptr The pointer to the memory to free.
* @param size The size of the memory to free. Ignored.
* @returns void
*/
void wasm_memory_storage_chunk_free(zend_mm_storage* storage, void* ptr, size_t size)
{
free(ptr);
}

zend_mm_storage wasm_memory_storage_struct = {
.handlers = {
.chunk_alloc = &wasm_memory_storage_chunk_alloc,
.chunk_free = &wasm_memory_storage_chunk_free,
.chunk_truncate = NULL,
.chunk_extend = NULL,
},
.data = NULL,
};

// These definitions are mirrored from zend_alloc.c:
// https://github.com/php/php-src/blob/dbaeb62ab1e34067057170ab50cf39d1bde584d8/Zend/zend_alloc.c#L131-L137
#ifndef ZEND_MM_CUSTOM
# define ZEND_MM_CUSTOM 1 /* support for custom memory allocator */
#endif
#ifndef ZEND_MM_STORAGE
# define ZEND_MM_STORAGE 1 /* support for custom memory storage */
#endif

// This struct mirrors the heap structure, which is declared privately
// in zend_alloc.c. We use this to more easily and clearly assign our custom memory storage handler.
// https://github.com/php/php-src/blob/dbaeb62ab1e34067057170ab50cf39d1bde584d8/Zend/zend_alloc.c#L234-L240
typedef struct _wasm_memory_storage_heap {
#if ZEND_MM_CUSTOM
int custom;
#endif
#if ZEND_MM_STORAGE
zend_mm_storage *storage;
#endif
} wasm_memory_storage_heap;

PHP_MINIT_FUNCTION(wasm_memory_storage)
{
wasm_memory_storage_heap* heap = (wasm_memory_storage_heap*) zend_mm_get_heap();
heap->storage = &wasm_memory_storage_struct;

return SUCCESS;
}

PHP_MSHUTDOWN_FUNCTION(wasm_memory_storage)
{
wasm_memory_storage_heap* heap = (wasm_memory_storage_heap*) zend_mm_get_heap();
heap->storage = NULL;

return SUCCESS;
}

/* {{{ PHP_MINFO_FUNCTION */
PHP_MINFO_FUNCTION(wasm_memory_storage)
{
php_info_print_table_start();
php_info_print_table_row(2, "wasm_memory_storage support", "enabled");
php_info_print_table_end();
}
/* }}} */

/* {{{ wasm_memory_storage_module_entry */
zend_module_entry wasm_memory_storage_module_entry = {
STANDARD_MODULE_HEADER,
"wasm_memory_storage", /* Extension name */
NULL, /* zend_function_entry */
PHP_MINIT(wasm_memory_storage), /* PHP_MINIT - Module initialization */
PHP_MSHUTDOWN(wasm_memory_storage), /* PHP_MSHUTDOWN - Module shutdown */
NULL, /* PHP_RINIT - Request initialization */
NULL, /* PHP_RSHUTDOWN - Request shutdown */
PHP_MINFO(wasm_memory_storage), /* PHP_MINFO - Module info */
PHP_WASM_MEMORY_STORAGE_VERSION, /* Version */
STANDARD_MODULE_PROPERTIES
};
/* }}} */
8 changes: 8 additions & 0 deletions packages/php-wasm/compile/php/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ RUN git clone https://github.com/php/php-src.git php-src \
--single-branch \
--depth 1;

# Work around memory leak due to PHP using Emscripten's incomplete mmap/munmap support
COPY ./php-wasm-memory-storage /root/php-src/ext/wasm_memory_storage

RUN cd php-src && ./buildconf --force

# Bring in the libraries
Expand Down Expand Up @@ -214,6 +217,10 @@ RUN if [ "$WITH_MBREGEX" = "yes" ] && [ "${PHP_VERSION:0:3}" != "7.0" ]; \
COPY ./php/php*.patch /root/
RUN cd /root && \
git apply --no-index /root/php${PHP_VERSION:0:3}*.patch -v && \
( [[ "${PHP_VERSION:0:3}" == '8.3' ]] && \
git apply --no-index /root/php-chunk-alloc-zend-assert-8.3.patch -v || \
git apply --no-index /root/php-chunk-alloc-zend-assert.patch -v \
) && \
touch php-src/patched

# Add VRZNO if needed
Expand Down Expand Up @@ -270,6 +277,7 @@ RUN source /root/emsdk/emsdk_env.sh && \
--enable-bcmath \
--enable-ctype \
--enable-tokenizer \
--enable-wasm_memory_storage \
$(cat /root/.php-configure-flags)

# Silence the errors "munmap() failed: [28] Invalid argument"
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
diff --git a/php-src/Zend/zend_alloc.c b/php-src/Zend/zend_alloc.c
index bf2116fc91..bec65453d8 100644
--- a/php-src/Zend/zend_alloc.c
+++ b/php-src/Zend/zend_alloc.c
@@ -773,7 +773,7 @@ static void *zend_mm_chunk_alloc(zend_mm_heap *heap, size_t size, size_t alignme
#if ZEND_MM_STORAGE
if (UNEXPECTED(heap->storage)) {
void *ptr = heap->storage->handlers.chunk_alloc(heap->storage, size, alignment);
- ZEND_ASSERT(((uintptr_t)((char*)ptr + (alignment-1)) & (alignment-1)) == (uintptr_t)ptr);
+ ZEND_ASSERT(((uintptr_t)((char*)ptr + (alignment-1)) & ~(alignment-1)) == (uintptr_t)ptr);
return ptr;
}
#endif
13 changes: 13 additions & 0 deletions packages/php-wasm/compile/php/php-chunk-alloc-zend-assert.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
diff --git a/php-src/Zend/zend_alloc.c b/php-src/Zend/zend_alloc.c
index bf2116fc91..bec65453d8 100644
--- a/php-src/Zend/zend_alloc.c
+++ b/php-src/Zend/zend_alloc.c
@@ -773,7 +773,7 @@ static void *zend_mm_chunk_alloc(zend_mm_heap *heap, size_t size, size_t alignme
#if ZEND_MM_STORAGE
if (UNEXPECTED(heap->storage)) {
void *ptr = heap->storage->handlers.chunk_alloc(heap->storage, size, alignment);
- ZEND_ASSERT(((zend_uintptr_t)((char*)ptr + (alignment-1)) & (alignment-1)) == (zend_uintptr_t)ptr);
+ ZEND_ASSERT(((zend_uintptr_t)((char*)ptr + (alignment-1)) & ~(alignment-1)) == (zend_uintptr_t)ptr);
return ptr;
}
#endif
Binary file modified packages/php-wasm/node/public/7_0_33/php_7_0.wasm
Binary file not shown.
Binary file modified packages/php-wasm/node/public/7_1_30/php_7_1.wasm
Binary file not shown.
Binary file modified packages/php-wasm/node/public/7_2_34/php_7_2.wasm
Binary file not shown.
Binary file modified packages/php-wasm/node/public/7_3_33/php_7_3.wasm
Binary file not shown.
Binary file modified packages/php-wasm/node/public/7_4_33/php_7_4.wasm
Binary file not shown.
Binary file modified packages/php-wasm/node/public/8_0_30/php_8_0.wasm
Binary file not shown.
Binary file modified packages/php-wasm/node/public/8_1_23/php_8_1.wasm
Binary file not shown.
Binary file modified packages/php-wasm/node/public/8_2_10/php_8_2.wasm
Binary file not shown.
Binary file modified packages/php-wasm/node/public/8_3_0/php_8_3.wasm
Binary file not shown.
2 changes: 1 addition & 1 deletion packages/php-wasm/node/public/php_7_0.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
const dependencyFilename = __dirname + '/7_0_33/php_7_0.wasm';
export { dependencyFilename };
export const dependenciesTotalSize = 12778149;
export const dependenciesTotalSize = 12778840;
export function init(RuntimeName, PHPLoader) {
/**
* Overrides Emscripten's default ExitStatus object which gets
Expand Down
2 changes: 1 addition & 1 deletion packages/php-wasm/node/public/php_7_1.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
const dependencyFilename = __dirname + '/7_1_30/php_7_1.wasm';
export { dependencyFilename };
export const dependenciesTotalSize = 13300133;
export const dependenciesTotalSize = 13301175;
export function init(RuntimeName, PHPLoader) {
/**
* Overrides Emscripten's default ExitStatus object which gets
Expand Down
2 changes: 1 addition & 1 deletion packages/php-wasm/node/public/php_7_2.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
const dependencyFilename = __dirname + '/7_2_34/php_7_2.wasm';
export { dependencyFilename };
export const dependenciesTotalSize = 13991154;
export const dependenciesTotalSize = 13992133;
export function init(RuntimeName, PHPLoader) {
/**
* Overrides Emscripten's default ExitStatus object which gets
Expand Down
2 changes: 1 addition & 1 deletion packages/php-wasm/node/public/php_7_3.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
const dependencyFilename = __dirname + '/7_3_33/php_7_3.wasm';
export { dependencyFilename };
export const dependenciesTotalSize = 14097593;
export const dependenciesTotalSize = 14098611;
export function init(RuntimeName, PHPLoader) {
/**
* Overrides Emscripten's default ExitStatus object which gets
Expand Down
2 changes: 1 addition & 1 deletion packages/php-wasm/node/public/php_7_4.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
const dependencyFilename = __dirname + '/7_4_33/php_7_4.wasm';
export { dependencyFilename };
export const dependenciesTotalSize = 14330034;
export const dependenciesTotalSize = 14331012;
export function init(RuntimeName, PHPLoader) {
/**
* Overrides Emscripten's default ExitStatus object which gets
Expand Down
2 changes: 1 addition & 1 deletion packages/php-wasm/node/public/php_8_0.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
const dependencyFilename = __dirname + '/8_0_30/php_8_0.wasm';
export { dependencyFilename };
export const dependenciesTotalSize = 13597144;
export const dependenciesTotalSize = 13597816;
export function init(RuntimeName, PHPLoader) {
/**
* Overrides Emscripten's default ExitStatus object which gets
Expand Down
Loading

0 comments on commit e36e1b9

Please sign in to comment.