From 62c6ab614836e439ff9895ffe4148d62f665a9e7 Mon Sep 17 00:00:00 2001 From: Andrei Alexeyev Date: Fri, 10 Jan 2020 01:18:47 +0200 Subject: [PATCH] Default to 16-byte alignment in malloc By default we now use `alignof(max_align_t)` in both `emmalloc` and `dlmalloc`. This is a requirement of the C and C++ standards. Developers can opt into the old behviour using `-s MALLOC=dlmalloc-align8` or `-s MALLOC=emmalloc-align8`. Based on #10110 which was authored by @Akaricchi. Fixes: #10072 --- ChangeLog.md | 7 ++++ embuilder.py | 2 + system/lib/dlmalloc.c | 6 +++ system/lib/emmalloc.cpp | 5 ++- tests/core/test_emmalloc.cpp | 4 +- .../core/test_emmalloc_memory_statistics.out | 2 +- tests/core/test_malloc_alignment.c | 38 +++++++++++++++++++ tests/core/test_setjmp_noleak.c | 7 +++- tests/test_core.py | 14 +++++++ tools/system_libs.py | 18 +++++++-- 10 files changed, 93 insertions(+), 10 deletions(-) create mode 100644 tests/core/test_malloc_alignment.c diff --git a/ChangeLog.md b/ChangeLog.md index bec4f7f504f33..54c6421f672a9 100644 --- a/ChangeLog.md +++ b/ChangeLog.md @@ -24,6 +24,13 @@ See docs/process.md for more on how version tagging works. existence of `Buffer.from` which was added in v5.10.0. If it turns out there is still a need to support these older node versions we can add a polyfil under LEGACY_VM_SUPPORT (#14447). +- The alignment of memory returned from the malloc implementations in emscripten + (dlmalloc and emmalloc) defaults to 16 rather than 8. This is technically + correct (since `alignof(max_align_t)` is 16 for the WebAssembly clang target) + and fixes several issues we have seen in the wild. Since some programs can + benefit having a lower alignment we have added `dlmalloc-align8` and + `emmalloc-align8` variants which can use used with the `-s MALLOC` option to + opt into the old (non-compliant) behavior. 2.0.24 - 06/10/2021 ------------------- diff --git a/embuilder.py b/embuilder.py index dcf3af137c18a..637c5255e4f59 100755 --- a/embuilder.py +++ b/embuilder.py @@ -42,8 +42,10 @@ 'libc++-noexcept', 'libal', 'libdlmalloc', + 'libdlmalloc-align8', 'libdlmalloc-debug', 'libemmalloc', + 'libemmalloc-align8', 'libemmalloc-debug', 'libemmalloc-memvalidate', 'libemmalloc-verbose', diff --git a/system/lib/dlmalloc.c b/system/lib/dlmalloc.c index a500268f323f8..780983de317a2 100644 --- a/system/lib/dlmalloc.c +++ b/system/lib/dlmalloc.c @@ -24,6 +24,12 @@ #define USE_SPIN_LOCKS 0 // Ensure we use pthread_mutex_t. #endif +#ifndef MALLOC_ALIGNMENT +#include +/* `malloc`ed pointers must be aligned at least as strictly as max_align_t. */ +#define MALLOC_ALIGNMENT (__alignof__(max_align_t)) +#endif + #endif diff --git a/system/lib/emmalloc.cpp b/system/lib/emmalloc.cpp index 187063ef96814..3ae2b5164be5e 100644 --- a/system/lib/emmalloc.cpp +++ b/system/lib/emmalloc.cpp @@ -39,6 +39,7 @@ * malloc. */ +#include #include #include #include @@ -60,7 +61,9 @@ extern "C" // Configuration: specifies the minimum alignment that malloc()ed memory outputs. Allocation requests with smaller alignment // than this will yield an allocation with this much alignment. -#define MALLOC_ALIGNMENT 8 +#ifndef MALLOC_ALIGNMENT +#define MALLOC_ALIGNMENT __alignof__(max_align_t) +#endif #define EMMALLOC_EXPORT __attribute__((weak, __visibility__("default"))) diff --git a/tests/core/test_emmalloc.cpp b/tests/core/test_emmalloc.cpp index 7e0ce62207d13..82ef7d1947d98 100644 --- a/tests/core/test_emmalloc.cpp +++ b/tests/core/test_emmalloc.cpp @@ -114,7 +114,7 @@ void test_realloc() { // realloc copies char* ptr = (char*)malloc(10); *ptr = 123; - for (int i = 5; i <= 16; i++) { + for (int i = 5; i <= 24; i++) { char* temp = (char*)realloc(ptr, i); assert(*temp == 123); assert(temp == ptr); @@ -123,7 +123,7 @@ void test_realloc() { malloc(1); malloc(100); { - char* temp = (char*)realloc(ptr, 17); + char* temp = (char*)realloc(ptr, 25); assert(*temp == 123); assert(temp != ptr); ptr = temp; diff --git a/tests/core/test_emmalloc_memory_statistics.out b/tests/core/test_emmalloc_memory_statistics.out index 0daf8d72f0209..a808aa3060a1f 100644 --- a/tests/core/test_emmalloc_memory_statistics.out +++ b/tests/core/test_emmalloc_memory_statistics.out @@ -1,7 +1,7 @@ 1 0 106971424 -4210892 +4210868 3 0 0 0 0 0 0 0 1 0 0 0 0 0 0 1 0 0 0 0 0 0 0 1 0 0 0 0 0 0 0 0 0 21999616 diff --git a/tests/core/test_malloc_alignment.c b/tests/core/test_malloc_alignment.c new file mode 100644 index 0000000000000..3351cd0149254 --- /dev/null +++ b/tests/core/test_malloc_alignment.c @@ -0,0 +1,38 @@ +/* + * Copyright 2021 The Emscripten Authors. All rights reserved. + * Emscripten is available under two separate licenses, the MIT license and the + * University of Illinois/NCSA Open Source License. Both these licenses can be + * found in the LICENSE file. + */ + +#include +#include +#include +#include +#include +#include + +#ifdef ALIGN8 +#define EXPECTED_ALIGNMENT 8 +#else +#define EXPECTED_ALIGNMENT alignof(max_align_t) +#endif + +int main(int argc, char **argv) { + bool underaligned = false; + + for (int size = 0; size < 16; ++size) { + void *p = malloc(size); + assert(((uintptr_t)p) % EXPECTED_ALIGNMENT == 0); + if (((uintptr_t)p) % alignof(max_align_t) != 0) { + underaligned = true; + } + } + +#if ALIGN8 + // Ensure that we have have at least one allocation that is under 16-byte + // alignment when using the align8 variants of malloc. + assert(underaligned); +#endif + return 0; +} diff --git a/tests/core/test_setjmp_noleak.c b/tests/core/test_setjmp_noleak.c index ff68352a59085..3972f969b4690 100644 --- a/tests/core/test_setjmp_noleak.c +++ b/tests/core/test_setjmp_noleak.c @@ -16,7 +16,7 @@ void luaWork(int d){ void dump() { struct mallinfo m = mallinfo(); - printf("dump: %d , %d\n", m.arena, m.uordblks); + printf("dump: %d , %d, %d\n", m.arena, m.uordblks, m.fordblks); } void work(int n) @@ -24,7 +24,7 @@ void work(int n) printf("work %d\n", n); dump(); - if(!setjmp(env)){ + if (!setjmp(env)){ luaWork(n); } @@ -32,6 +32,7 @@ void work(int n) } int main() { + void* x = malloc(10); struct mallinfo m1 = mallinfo(); dump(); work(10); @@ -39,4 +40,6 @@ int main() { struct mallinfo m2 = mallinfo(); assert(m1.uordblks == m2.uordblks); printf("ok.\n"); + dump(); + return 0; } diff --git a/tests/test_core.py b/tests/test_core.py index 5e1dc32c6ca82..312883ca458e7 100644 --- a/tests/test_core.py +++ b/tests/test_core.py @@ -1018,6 +1018,7 @@ def test_setjmp_many_2(self): self.do_run(src, r'''d is at 24''') def test_setjmp_noleak(self): + self.emcc_args.append('-fno-builtin') self.do_runf(test_file('core/test_setjmp_noleak.c'), 'ok.') @with_both_exception_handling @@ -8508,6 +8509,19 @@ def test_REVERSE_DEPS(self): err = self.expect_fail([EMCC, 'connect.c', '-sREVERSE_DEPS=none']) self.assertContained('undefined symbol: ntohs', err) + @parameterized({ + 'dlmalloc': ['dlmalloc'], + 'emmalloc': ['emmalloc'], + 'dlmalloc_align8': ['dlmalloc-align8'], + 'emmalloc_align8': ['emmalloc-align8'], + }) + def test_malloc_alignment(self, malloc): + self.set_setting('MALLOC', malloc) + if 'align8' in malloc: + self.emcc_args += ['-DALIGN8'] + self.emcc_args += ['-std=gnu11', '-fno-builtin'] + self.do_runf(test_file('core/test_malloc_alignment.c')) + # Generate tests for everything def make_run(name, emcc_args, settings=None, env=None): diff --git a/tools/system_libs.py b/tools/system_libs.py index 670e04c2f401c..a98c036284c1f 100644 --- a/tools/system_libs.py +++ b/tools/system_libs.py @@ -1020,10 +1020,15 @@ class libmalloc(MTLibrary): cflags = ['-O2', '-fno-builtin'] def __init__(self, **kwargs): - self.malloc = kwargs.pop('malloc') - if self.malloc not in ('dlmalloc', 'emmalloc', 'emmalloc-debug', 'emmalloc-memvalidate', 'emmalloc-verbose', 'emmalloc-memvalidate-verbose', 'none'): - raise Exception('malloc must be one of "emmalloc[-debug|-memvalidate][-verbose]", "dlmalloc" or "none", see settings.js') + valid = ['dlmalloc', 'emmalloc', 'emmalloc-debug', 'emmalloc-memvalidate', 'emmalloc-verbose', 'emmalloc-memvalidate-verbose'] + valid += [v + '-align8' for v in valid] + valid.append('none') + self.malloc = kwargs.pop('malloc').replace('-align8', '') + if self.malloc not in valid: + shared.exit_with_error(f'invalid MALLOC setting: `{self.malloc}`. must be one of "emmalloc[-debug|-memvalidate][-verbose]", "dlmalloc" or "none", see settings.js') + + self.align8 = kwargs.pop('align8') self.use_errno = kwargs.pop('use_errno') self.is_tracing = kwargs.pop('is_tracing') self.memvalidate = kwargs.pop('memvalidate') @@ -1054,6 +1059,8 @@ def get_cflags(self): cflags += ['-DMALLOC_FAILURE_ACTION=', '-DEMSCRIPTEN_NO_ERRNO'] if self.is_tracing: cflags += ['--tracing'] + if self.align8: + cflags += ['-DMALLOC_ALIGNMENT=8'] return cflags def get_base_name_prefix(self): @@ -1068,6 +1075,8 @@ def get_base_name(self): name += '-noerrno' if self.is_tracing: name += '-tracing' + if self.align8: + name += '-align8' return name def can_use(self): @@ -1075,7 +1084,7 @@ def can_use(self): @classmethod def vary_on(cls): - return super().vary_on() + ['is_debug', 'use_errno', 'is_tracing', 'memvalidate', 'verbose'] + return super().vary_on() + ['is_debug', 'use_errno', 'is_tracing', 'memvalidate', 'verbose', 'align8'] @classmethod def get_default_variation(cls, **kwargs): @@ -1084,6 +1093,7 @@ def get_default_variation(cls, **kwargs): is_debug=settings.ASSERTIONS >= 2, use_errno=settings.SUPPORT_ERRNO, is_tracing=settings.EMSCRIPTEN_TRACING, + align8='align8' in settings.MALLOC, memvalidate='memvalidate' in settings.MALLOC, verbose='verbose' in settings.MALLOC, **kwargs