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.

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

Reapply "Prefer calloc over of malloc+zeroMemory. NFC" #22596

Merged
merged 2 commits into from
Sep 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
11 changes: 8 additions & 3 deletions src/library.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
6 changes: 3 additions & 3 deletions src/library_dylink.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.
Expand Down Expand Up @@ -599,7 +599,7 @@ var LibraryDylink = {
$loadWebAssemblyModule__deps: [
'$loadDynamicLibrary', '$getMemory',
'$relocateExports', '$resolveGlobalSymbol', '$GOTHandler',
'$getDylinkMetadata', '$alignMemory', '$zeroMemory',
'$getDylinkMetadata', '$alignMemory',
'$currentModuleWeakSymbols',
'$updateTableMap',
'$wasmTable',
Expand Down
2 changes: 1 addition & 1 deletion src/library_pthread.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ var LibraryPThread = {
$PThread__postset: 'PThread.init();',
$PThread__deps: ['_emscripten_thread_init',
'$terminateWorker',
'$cleanupThread', '$zeroMemory',
'$cleanupThread',
#if MAIN_MODULE
'$markAsFinished',
#endif
Expand Down
10 changes: 4 additions & 6 deletions src/library_sdl.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand All @@ -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 */;
Expand Down Expand Up @@ -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;
Expand Down
1 change: 1 addition & 0 deletions system/include/emscripten/heap.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions system/lib/dlmalloc.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 3 additions & 2 deletions system/lib/emmalloc.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
1 change: 1 addition & 0 deletions system/lib/mimalloc/src/alloc-override.c
Original file line number Diff line number Diff line change
Expand Up @@ -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__)
Expand Down
2 changes: 1 addition & 1 deletion test/other/codesize/test_codesize_hello_dylink.exports
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 1 addition & 1 deletion test/other/codesize/test_codesize_hello_dylink.funcs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion test/other/codesize/test_codesize_hello_dylink.gzsize
Original file line number Diff line number Diff line change
@@ -1 +1 @@
6292
6285
2 changes: 1 addition & 1 deletion test/other/codesize/test_codesize_hello_dylink.jssize
Original file line number Diff line number Diff line change
@@ -1 +1 @@
13833
13820
2 changes: 1 addition & 1 deletion test/other/codesize/test_codesize_hello_dylink.size
Original file line number Diff line number Diff line change
@@ -1 +1 @@
9350
9763
4 changes: 1 addition & 3 deletions test/test_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down
1 change: 1 addition & 0 deletions test/test_other.py
Original file line number Diff line number Diff line change
Expand Up @@ -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')

Expand Down
2 changes: 2 additions & 0 deletions tools/emscripten.py
Original file line number Diff line number Diff line change
Expand Up @@ -1022,7 +1022,9 @@ 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',
'memalign': 'ppp',
'memcmp': '_ppp',
Expand Down
6 changes: 0 additions & 6 deletions tools/link.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading