Default to 16-byte alignment in malloc

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
 -------------------

embuilder.py

@@ -42,8 +42,10 @@
     'libc++-noexcept',
     'libal',
     'libdlmalloc',
+    'libdlmalloc-align8',
     'libdlmalloc-debug',
     'libemmalloc',
+    'libemmalloc-align8',
     'libemmalloc-debug',
     'libemmalloc-memvalidate',
     'libemmalloc-verbose',

system/lib/dlmalloc.c

@@ -24,6 +24,12 @@
 #define USE_SPIN_LOCKS 0 // Ensure we use pthread_mutex_t.
 #endif
 
+#ifndef MALLOC_ALIGNMENT
+#include <stddef.h>
+/* `malloc`ed pointers must be aligned at least as strictly as max_align_t. */
+#define MALLOC_ALIGNMENT (__alignof__(max_align_t))
+#endif
+
 #endif
 
 

system/lib/emmalloc.cpp

@@ -39,6 +39,7 @@
  *    malloc.
  */
 
+#include <stddef.h>
 #include <stdint.h>
 #include <unistd.h>
 #include <memory.h>
@@ -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")))
 

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;

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

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 <assert.h>
+#include <stdalign.h>
+#include <stdbool.h>
+#include <stddef.h>
+#include <stdint.h>
+#include <stdlib.h>
+
+#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;
+}

tests/core/test_setjmp_noleak.c

@@ -16,27 +16,30 @@ 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)
 {
   printf("work %d\n", n);
   dump();
 
-  if(!setjmp(env)){
+  if (!setjmp(env)){
     luaWork(n);
   }
 
   if (n > 0) work(n-1);
 }
 
 int main() {
+  void* x = malloc(10);
   struct mallinfo m1 = mallinfo();
   dump();
   work(10);
   dump();
   struct mallinfo m2 = mallinfo();
   assert(m1.uordblks == m2.uordblks);
   printf("ok.\n");
+  dump();
+  return 0;
 }

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):

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,14 +1075,16 @@ def get_base_name(self):
       name += '-noerrno'
     if self.is_tracing:
       name += '-tracing'
+    if self.align8:
+      name += '-align8'
     return name
 
   def can_use(self):
     return super().can_use() and settings.MALLOC != 'none'
 
   @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