From c9631123493c699e9decc1ca7e1232797ec314aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sa=C3=BAl=20Ibarra=20Corretg=C3=A9?= Date: Tue, 24 Sep 2024 09:50:55 +0200 Subject: [PATCH] Fix use of js_malloc_usable_size Make sure the one set in the malloc functions is used rather than the default one, since it will likely use a different allocator. For some reason, this didn't cause a problem on macOS, but it does in Linux. Opsie! Added some CI to prevent these kinds of bugs. --- .github/workflows/ci.yml | 34 ++++++++++++++++++++++++++++++++++ CMakeLists.txt | 11 ++++++++--- quickjs.c | 12 ++++++------ 3 files changed, 48 insertions(+), 9 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index fb3d86c..40dd383 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -484,3 +484,37 @@ jobs: run: | cmake --build build --config Release --target qjs ls -lh build + + mimalloc-linux: + runs-on: ubuntu-24.04 + env: + BUILD_CLI_WITH_MIMALLOC: ON + MIMALLOC_SHOW_STATS: 1 + steps: + - uses: actions/checkout@v4 + - name: install dependencies + run: | + sudo apt update && sudo apt -y install libmimalloc-dev + - name: build + run: | + make + - name: test + run: | + make test + + mimalloc-macos: + runs-on: macos-latest + env: + BUILD_CLI_WITH_STATIC_MIMALLOC: ON + MIMALLOC_SHOW_STATS: 1 + steps: + - uses: actions/checkout@v4 + - name: install dependencies + run: | + brew install mimalloc + - name: build + run: | + make + - name: test + run: | + make test diff --git a/CMakeLists.txt b/CMakeLists.txt index dd98a58..1f6d377 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -112,6 +112,7 @@ endif() xoption(BUILD_EXAMPLES "Build examples" OFF) xoption(BUILD_STATIC_QJS_EXE "Build a static qjs executable" OFF) xoption(BUILD_CLI_WITH_MIMALLOC "Build the qjs executable with mimalloc" OFF) +xoption(BUILD_CLI_WITH_STATIC_MIMALLOC "Build the qjs executable with mimalloc (statically linked)" OFF) xoption(CONFIG_ASAN "Enable AddressSanitizer (ASan)" OFF) xoption(CONFIG_MSAN "Enable MemorySanitizer (MSan)" OFF) xoption(CONFIG_UBSAN "Enable UndefinedBehaviorSanitizer (UBSan)" OFF) @@ -255,10 +256,14 @@ endif() if(NOT WIN32) set_target_properties(qjs_exe PROPERTIES ENABLE_EXPORTS TRUE) endif() -if(BUILD_CLI_WITH_MIMALLOC) +if(BUILD_CLI_WITH_MIMALLOC OR BUILD_CLI_WITH_STATIC_MIMALLOC) find_package(mimalloc REQUIRED) - target_compile_definitions(qjs_exe PRIVATE QJS_USE_MIMALLOC) - target_link_libraries(qjs_exe mimalloc-static) + # Upstream mimalloc doesn't provide a way to know if both libraries are supported. + if(BUILD_CLI_WITH_STATIC_MIMALLOC) + target_link_libraries(qjs_exe mimalloc-static) + else() + target_link_libraries(qjs_exe mimalloc) + endif() endif() # Test262 runner diff --git a/quickjs.c b/quickjs.c index 5708678..872d7d4 100644 --- a/quickjs.c +++ b/quickjs.c @@ -1409,7 +1409,7 @@ void *js_calloc_rt(JSRuntime *rt, size_t count, size_t size) return NULL; s->malloc_count++; - s->malloc_size += js__malloc_usable_size(ptr) + MALLOC_OVERHEAD; + s->malloc_size += rt->mf.js_malloc_usable_size(ptr) + MALLOC_OVERHEAD; return ptr; } @@ -1431,7 +1431,7 @@ void *js_malloc_rt(JSRuntime *rt, size_t size) return NULL; s->malloc_count++; - s->malloc_size += js__malloc_usable_size(ptr) + MALLOC_OVERHEAD; + s->malloc_size += rt->mf.js_malloc_usable_size(ptr) + MALLOC_OVERHEAD; return ptr; } @@ -1444,7 +1444,7 @@ void js_free_rt(JSRuntime *rt, void *ptr) s = &rt->malloc_state; s->malloc_count--; - s->malloc_size -= js__malloc_usable_size(ptr) + MALLOC_OVERHEAD; + s->malloc_size -= rt->mf.js_malloc_usable_size(ptr) + MALLOC_OVERHEAD; rt->mf.js_free(s->opaque, ptr); } @@ -1462,7 +1462,7 @@ void *js_realloc_rt(JSRuntime *rt, void *ptr, size_t size) js_free_rt(rt, ptr); return NULL; } - old_size = js__malloc_usable_size(ptr); + old_size = rt->mf.js_malloc_usable_size(ptr); s = &rt->malloc_state; /* When malloc_limit is 0 (unlimited), malloc_limit - 1 will be SIZE_MAX. */ if (s->malloc_size + size - old_size > s->malloc_limit - 1) @@ -1472,7 +1472,7 @@ void *js_realloc_rt(JSRuntime *rt, void *ptr, size_t size) if (!ptr) return NULL; - s->malloc_size += js__malloc_usable_size(ptr) - old_size; + s->malloc_size += rt->mf.js_malloc_usable_size(ptr) - old_size; return ptr; } @@ -1734,7 +1734,7 @@ JSRuntime *JS_NewRuntime2(const JSMallocFunctions *mf, void *opaque) return NULL; /* Inline what js_malloc_rt does since we cannot use it here. */ ms.malloc_count++; - ms.malloc_size += js__malloc_usable_size(rt) + MALLOC_OVERHEAD; + ms.malloc_size += mf->js_malloc_usable_size(rt) + MALLOC_OVERHEAD; rt->mf = *mf; if (!rt->mf.js_malloc_usable_size) { /* use dummy function if none provided */