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

Author: Akaricchi

ChangeLog.md

@@ -20,6 +20,13 @@ See docs/process.md for more on how version tagging works.
 
 2.0.25
 ------
+- 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
 -------------------

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_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 MALLOC_ALIGN 8
+#else
+#define MALLOC_ALIGN 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) % MALLOC_ALIGN == 0);
+    if (((uintptr_t)p) % 16 != 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/test_core.py

@@ -8508,6 +8508,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):
+    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')
-    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')
+    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')
@@ -1033,7 +1038,7 @@ def __init__(self, **kwargs):
     super().__init__(**kwargs)
 
   def get_files(self):
-    malloc_base = self.malloc.replace('-memvalidate', '').replace('-verbose', '').replace('-debug', '')
+    malloc_base = self.malloc.replace('-memvalidate', '').replace('-verbose', '').replace('-debug', '').replace('-align8', '')
     malloc = shared.path_from_root('system', 'lib', {
       'dlmalloc': 'dlmalloc.c', 'emmalloc': 'emmalloc.cpp',
     }[malloc_base])
@@ -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):
@@ -1075,7 +1082,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 +1091,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