From 72fcd94236c59c60656ce0e70d3320a35efbca26 Mon Sep 17 00:00:00 2001 From: Sam Clegg Date: Thu, 19 Sep 2024 16:10:32 -0700 Subject: [PATCH 1/2] Reapply "Prefer `calloc` over of malloc+zeroMemory. NFC" (#22568) This reverts commit 50e0b0785ad5f9adc9bd082451d5cdbf5c4c5229. --- src/library_dylink.js | 6 +++--- src/library_pthread.js | 2 +- src/library_sdl.js | 10 ++++------ test/other/codesize/test_codesize_hello_dylink.exports | 2 +- test/other/codesize/test_codesize_hello_dylink.funcs | 2 +- test/other/codesize/test_codesize_hello_dylink.gzsize | 2 +- test/other/codesize/test_codesize_hello_dylink.jssize | 2 +- test/other/codesize/test_codesize_hello_dylink.size | 2 +- test/test_core.py | 4 +--- test/test_other.py | 1 + tools/emscripten.py | 1 + 11 files changed, 16 insertions(+), 18 deletions(-) diff --git a/src/library_dylink.js b/src/library_dylink.js index a450db61cc888..f9f088d5e379a 100644 --- a/src/library_dylink.js +++ b/src/library_dylink.js @@ -363,7 +363,7 @@ var LibraryDylink = { // Allocate memory even if malloc isn't ready yet. The allocated memory here // must be zero initialized since its used for all static data, including bss. $getMemory__noleakcheck: true, - $getMemory__deps: ['$GOT', '__heap_base', '$zeroMemory', '$alignMemory', 'malloc'], + $getMemory__deps: ['$GOT', '__heap_base', '$alignMemory', 'calloc'], $getMemory: (size) => { // After the runtime is initialized, we must only use sbrk() normally. #if DYLINK_DEBUG @@ -373,7 +373,7 @@ var LibraryDylink = { // Currently we don't support freeing of static data when modules are // unloaded via dlclose. This function is tagged as `noleakcheck` to // avoid having this reported as leak. - return zeroMemory(_malloc(size), size); + return _calloc(size, 1); } var ret = ___heap_base; // Keep __heap_base stack aligned. @@ -599,7 +599,7 @@ var LibraryDylink = { $loadWebAssemblyModule__deps: [ '$loadDynamicLibrary', '$getMemory', '$relocateExports', '$resolveGlobalSymbol', '$GOTHandler', - '$getDylinkMetadata', '$alignMemory', '$zeroMemory', + '$getDylinkMetadata', '$alignMemory', '$currentModuleWeakSymbols', '$updateTableMap', '$wasmTable', diff --git a/src/library_pthread.js b/src/library_pthread.js index 5ccb829dbc44c..680a6ac9dd316 100644 --- a/src/library_pthread.js +++ b/src/library_pthread.js @@ -35,7 +35,7 @@ var LibraryPThread = { $PThread__postset: 'PThread.init();', $PThread__deps: ['_emscripten_thread_init', '$terminateWorker', - '$cleanupThread', '$zeroMemory', + '$cleanupThread', #if MAIN_MODULE '$markAsFinished', #endif diff --git a/src/library_sdl.js b/src/library_sdl.js index 160849a38bf17..0d54f422b3ad7 100644 --- a/src/library_sdl.js +++ b/src/library_sdl.js @@ -1357,7 +1357,7 @@ var LibrarySDL = { return SDL.version; }, - SDL_Init__deps: ['$zeroMemory', 'memcpy'], + SDL_Init__deps: ['calloc', 'memcpy'], SDL_Init__proxy: 'sync', SDL_Init__docs: '/** @param{number} initFlags */', SDL_Init: (initFlags) => { @@ -1376,8 +1376,7 @@ var LibrarySDL = { } window.addEventListener("unload", SDL.receiveEvent); - SDL.keyboardState = _malloc(0x10000); // Our SDL needs 512, but 64K is safe for older SDLs - zeroMemory(SDL.keyboardState, 0x10000); + SDL.keyboardState = _calloc(0x10000, 1); // Our SDL needs 512, but 64K is safe for older SDLs // Initialize this structure carefully for closure SDL.DOMEventToSDLEvent['keydown'] = 0x300 /* SDL_KEYDOWN */; SDL.DOMEventToSDLEvent['keyup'] = 0x301 /* SDL_KEYUP */; @@ -1413,11 +1412,10 @@ var LibrarySDL = { return 1; }, - SDL_GetVideoInfo__deps: ['$zeroMemory'], + SDL_GetVideoInfo__deps: ['calloc'], SDL_GetVideoInfo__proxy: 'sync', SDL_GetVideoInfo: () => { - var ret = _malloc({{{ C_STRUCTS.SDL_VideoInfo.__size__ }}}); - zeroMemory(ret, {{{ C_STRUCTS.SDL_version.__size__ }}}); + var ret = _calloc({{{ C_STRUCTS.SDL_VideoInfo.__size__ }}}, 1); {{{ makeSetValue('ret', C_STRUCTS.SDL_VideoInfo.current_w, 'Module["canvas"].width', 'i32') }}}; {{{ makeSetValue('ret', C_STRUCTS.SDL_VideoInfo.current_h, 'Module["canvas"].height', 'i32') }}}; return ret; diff --git a/test/other/codesize/test_codesize_hello_dylink.exports b/test/other/codesize/test_codesize_hello_dylink.exports index 9aa298858fd18..f1e9af5fcfbf2 100644 --- a/test/other/codesize/test_codesize_hello_dylink.exports +++ b/test/other/codesize/test_codesize_hello_dylink.exports @@ -2,8 +2,8 @@ __wasm_apply_data_relocs __wasm_call_ctors _emscripten_stack_alloc _emscripten_stack_restore +calloc dynCall_jiji emscripten_stack_get_current main -malloc setThrew diff --git a/test/other/codesize/test_codesize_hello_dylink.funcs b/test/other/codesize/test_codesize_hello_dylink.funcs index 4ec6c20d0ee6c..2c23b988a7191 100644 --- a/test/other/codesize/test_codesize_hello_dylink.funcs +++ b/test/other/codesize/test_codesize_hello_dylink.funcs @@ -8,7 +8,7 @@ $__wasm_apply_global_relocs $__wasm_call_ctors $_emscripten_stack_alloc $_emscripten_stack_restore -$dlmalloc +$dlcalloc $emscripten_stack_get_current $legalstub$dynCall_jiji $main diff --git a/test/other/codesize/test_codesize_hello_dylink.gzsize b/test/other/codesize/test_codesize_hello_dylink.gzsize index a38f2dcf3d560..190e05fb52d22 100644 --- a/test/other/codesize/test_codesize_hello_dylink.gzsize +++ b/test/other/codesize/test_codesize_hello_dylink.gzsize @@ -1 +1 @@ -6292 +6285 diff --git a/test/other/codesize/test_codesize_hello_dylink.jssize b/test/other/codesize/test_codesize_hello_dylink.jssize index b252c4d4631e2..61d1af505618a 100644 --- a/test/other/codesize/test_codesize_hello_dylink.jssize +++ b/test/other/codesize/test_codesize_hello_dylink.jssize @@ -1 +1 @@ -13833 +13820 diff --git a/test/other/codesize/test_codesize_hello_dylink.size b/test/other/codesize/test_codesize_hello_dylink.size index 01ddd66476899..1b0fcc83b67b3 100644 --- a/test/other/codesize/test_codesize_hello_dylink.size +++ b/test/other/codesize/test_codesize_hello_dylink.size @@ -1 +1 @@ -9350 +9763 diff --git a/test/test_core.py b/test/test_core.py index 838bd906c21ce..6c38e9fede6df 100644 --- a/test/test_core.py +++ b/test/test_core.py @@ -1943,9 +1943,7 @@ def test_em_asm_side_module(self): def test_em_js(self, args, force_c): if '-sMAIN_MODULE=2' in args: self.check_dylink() - else: - self.emcc_args += ['-sEXPORTED_FUNCTIONS=_main,_malloc'] - self.emcc_args += args + self.emcc_args += ['-sEXPORTED_FUNCTIONS=_main,_malloc'] + args if '-pthread' in args: self.setup_node_pthreads() diff --git a/test/test_other.py b/test/test_other.py index 00a6ee7386d53..4e01eb5fa2b66 100644 --- a/test/test_other.py +++ b/test/test_other.py @@ -2189,6 +2189,7 @@ def test_dylink_pthread_bigint_em_asm(self): def test_dylink_pthread_bigint_em_js(self): self.set_setting('MAIN_MODULE', 2) self.set_setting('WASM_BIGINT') + self.set_setting('EXPORTED_FUNCTIONS', '_malloc,_main') self.emcc_args += ['-Wno-experimental', '-pthread'] self.do_runf('core/test_em_js.cpp') diff --git a/tools/emscripten.py b/tools/emscripten.py index 8319bd9ac46dd..502cc2dca7d04 100644 --- a/tools/emscripten.py +++ b/tools/emscripten.py @@ -1023,6 +1023,7 @@ def create_pointer_conversion_wrappers(metadata): '_emscripten_stack_alloc': 'pp', 'emscripten_builtin_malloc': 'pp', 'malloc': 'pp', + 'calloc': 'ppp', 'webidl_malloc': 'pp', 'memalign': 'ppp', 'memcmp': '_ppp', From 1c445e7086fe08ebd5a73da1bd0aa5419c6c4a78 Mon Sep 17 00:00:00 2001 From: Sam Clegg Date: Thu, 19 Sep 2024 16:21:47 -0700 Subject: [PATCH 2/2] fix lsan issue --- .circleci/config.yml | 1 + src/library.js | 11 ++++++++--- system/include/emscripten/heap.h | 1 + system/lib/dlmalloc.c | 1 + system/lib/emmalloc.c | 5 +++-- system/lib/mimalloc/src/alloc-override.c | 1 + tools/emscripten.py | 1 + tools/link.py | 6 ------ 8 files changed, 16 insertions(+), 11 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 5830c4f8bb69e..013f1effe8f33 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -515,6 +515,7 @@ jobs: asan.test_externref_emjs_dynlink asan.test_asyncify_longjmp asan.test_pthread_run_on_main_thread + lsan.test_dylink_dso_needed lsan.test_stdio_locking lsan.test_dlfcn_basic lsan.test_pthread_create diff --git a/src/library.js b/src/library.js index 800162151c8e8..727c77534182f 100644 --- a/src/library.js +++ b/src/library.js @@ -1574,21 +1574,26 @@ addToLibrary({ #if USE_ASAN || USE_LSAN || UBSAN_RUNTIME // When lsan or asan is enabled withBuiltinMalloc temporarily replaces calls - // to malloc, free, and memalign. - $withBuiltinMalloc__deps: ['emscripten_builtin_malloc', 'emscripten_builtin_free', 'emscripten_builtin_memalign' - ], + // to malloc, calloc, free, and memalign. + $withBuiltinMalloc__deps: [ + 'malloc', 'calloc', 'free', 'memalign', + 'emscripten_builtin_malloc', 'emscripten_builtin_free', 'emscripten_builtin_memalign', 'emscripten_builtin_calloc' + ], $withBuiltinMalloc__docs: '/** @suppress{checkTypes} */', $withBuiltinMalloc: (func) => { var prev_malloc = typeof _malloc != 'undefined' ? _malloc : undefined; + var prev_calloc = typeof _calloc != 'undefined' ? _calloc : undefined; var prev_memalign = typeof _memalign != 'undefined' ? _memalign : undefined; var prev_free = typeof _free != 'undefined' ? _free : undefined; _malloc = _emscripten_builtin_malloc; + _calloc = _emscripten_builtin_calloc; _memalign = _emscripten_builtin_memalign; _free = _emscripten_builtin_free; try { return func(); } finally { _malloc = prev_malloc; + _calloc = prev_calloc; _memalign = prev_memalign; _free = prev_free; } diff --git a/system/include/emscripten/heap.h b/system/include/emscripten/heap.h index 3eb1a8722fad0..969287e7da364 100644 --- a/system/include/emscripten/heap.h +++ b/system/include/emscripten/heap.h @@ -42,6 +42,7 @@ size_t emscripten_get_heap_max(void); // dlmalloc and emmalloc. void *emscripten_builtin_memalign(size_t alignment, size_t size); void *emscripten_builtin_malloc(size_t size); +void *emscripten_builtin_calloc(size_t nmemb, size_t size); void emscripten_builtin_free(void *ptr); #ifdef __cplusplus diff --git a/system/lib/dlmalloc.c b/system/lib/dlmalloc.c index dd28625fc4f8c..68fbe7f332756 100644 --- a/system/lib/dlmalloc.c +++ b/system/lib/dlmalloc.c @@ -6083,6 +6083,7 @@ int mspace_mallopt(int param_number, int value) { // This allows an easy mechanism for hooking into memory allocation. #if defined(__EMSCRIPTEN__) && !ONLY_MSPACES extern __typeof(malloc) emscripten_builtin_malloc __attribute__((alias("dlmalloc"))); +extern __typeof(calloc) emscripten_builtin_calloc __attribute__((alias("dlcalloc"))); extern __typeof(free) emscripten_builtin_free __attribute__((alias("dlfree"))); extern __typeof(memalign) emscripten_builtin_memalign __attribute__((alias("dlmemalign"))); #endif diff --git a/system/lib/emmalloc.c b/system/lib/emmalloc.c index 1746a73e4b653..940335c37cf5e 100644 --- a/system/lib/emmalloc.c +++ b/system/lib/emmalloc.c @@ -1140,8 +1140,9 @@ void *emmalloc_calloc(size_t num, size_t size) { } return ptr; } -EMMALLOC_ALIAS(__libc_calloc, emmalloc_calloc); -EMMALLOC_ALIAS(calloc, emmalloc_calloc); +EMMALLOC_ALIAS(emscripten_builtin_calloc, emmalloc_calloc); +EMMALLOC_ALIAS(__libc_calloc, emmalloc_calloc); +EMMALLOC_ALIAS(calloc, emmalloc_calloc); static int count_linked_list_size(Region *list) { int size = 1; diff --git a/system/lib/mimalloc/src/alloc-override.c b/system/lib/mimalloc/src/alloc-override.c index d6ebe1dc165bc..8f15331ff5fcc 100644 --- a/system/lib/mimalloc/src/alloc-override.c +++ b/system/lib/mimalloc/src/alloc-override.c @@ -286,6 +286,7 @@ void* _aligned_malloc(size_t alignment, size_t size) { return mi_aligned_allo void* emscripten_builtin_malloc(size_t size) MI_FORWARD1(mi_malloc, size) void* emscripten_builtin_free(void* p) MI_FORWARD0(mi_free, p) void* emscripten_builtin_memalign(size_t alignment, size_t size) { return mi_memalign(alignment, size); } + void* emscripten_builtin_calloc(size_t nmemb, size_t size) MI_FORWARD2(mi_calloc, nmemb, size) #endif #elif defined(__GLIBC__) && defined(__linux__) diff --git a/tools/emscripten.py b/tools/emscripten.py index 502cc2dca7d04..1c09305228fae 100644 --- a/tools/emscripten.py +++ b/tools/emscripten.py @@ -1022,6 +1022,7 @@ def create_pointer_conversion_wrappers(metadata): 'sbrk': 'pP', '_emscripten_stack_alloc': 'pp', 'emscripten_builtin_malloc': 'pp', + 'emscripten_builtin_calloc': 'ppp', 'malloc': 'pp', 'calloc': 'ppp', 'webidl_malloc': 'pp', diff --git a/tools/link.py b/tools/link.py index 0892b9f15368b..9e0f9c55d9623 100644 --- a/tools/link.py +++ b/tools/link.py @@ -1518,12 +1518,6 @@ def phase_linker_setup(options, state, newargs): if sanitize: settings.USE_OFFSET_CONVERTER = 1 - settings.REQUIRED_EXPORTS += [ - 'memalign', - 'emscripten_builtin_memalign', - 'emscripten_builtin_malloc', - 'emscripten_builtin_free', - ] if ('leak' in sanitize or 'address' in sanitize) and not settings.ALLOW_MEMORY_GROWTH: # Increase the minimum memory requirements to account for extra memory