[Bf-blender-cvs] [9c9ea37770d] master: Fix: Use a minimal alignment of 8 in MEM_lockfree_mallocN_aligned

[Jacques Lucke]

Commit: 9c9ea37770dcf2d8a77cab0bf267a5bcf76500eb
Author: Jacques Lucke
Date:   Thu Jan 23 14:17:13 2020 +0100
Branches: master
https://developer.blender.org/rB9c9ea37770dcf2d8a77cab0bf267a5bcf76500eb

Fix: Use a minimal alignment of 8 in MEM_lockfree_mallocN_aligned

`posix_memalign` requires the `alignment` to be at least `sizeof(void *)`.
Previously, `MEM_mallocN_aligned` would simply return `NULL` if a too small
`alignment` was used. This was an OS specific issue.

The solution is to use a minimal alignment of `8` for all aligned allocations.
The unit tests have been extended to test more possible alignments (some
of which were broken before).

Reviewers: brecht

Differential Revision: https://developer.blender.org/D6660

===================================================================

M	intern/guardedalloc/intern/mallocn.c
M	intern/guardedalloc/intern/mallocn_intern.h
M	intern/guardedalloc/intern/mallocn_lockfree_impl.c
M	tests/gtests/guardedalloc/guardedalloc_alignment_test.cc

===================================================================

diff --git a/intern/guardedalloc/intern/mallocn.c b/intern/guardedalloc/intern/mallocn.c
index fa2d0d1e334..d24437c85f2 100644
--- a/intern/guardedalloc/intern/mallocn.c
+++ b/intern/guardedalloc/intern/mallocn.c
@@ -70,6 +70,9 @@ const char *(*MEM_name_ptr)(void *vmemh) = MEM_lockfree_name_ptr;
 
 void *aligned_malloc(size_t size, size_t alignment)
 {
+  /* posix_memalign requires alignment to be a multiple of sizeof(void *). */
+  assert(alignment >= ALIGNED_MALLOC_MINIMUM_ALIGNMENT);
+
 #ifdef _WIN32
   return _aligned_malloc(size, alignment);
 #elif defined(__FreeBSD__) || defined(__NetBSD__) || defined(__APPLE__)
diff --git a/intern/guardedalloc/intern/mallocn_intern.h b/intern/guardedalloc/intern/mallocn_intern.h
index e6e090703d4..876607fdb77 100644
--- a/intern/guardedalloc/intern/mallocn_intern.h
+++ b/intern/guardedalloc/intern/mallocn_intern.h
@@ -107,6 +107,8 @@ size_t malloc_usable_size(void *ptr);
 
 #include "mallocn_inline.h"
 
+#define ALIGNED_MALLOC_MINIMUM_ALIGNMENT sizeof(void *)
+
 void *aligned_malloc(size_t size, size_t alignment);
 void aligned_free(void *ptr);
 
diff --git a/intern/guardedalloc/intern/mallocn_lockfree_impl.c b/intern/guardedalloc/intern/mallocn_lockfree_impl.c
index e8fd8de738b..87091bb9862 100644
--- a/intern/guardedalloc/intern/mallocn_lockfree_impl.c
+++ b/intern/guardedalloc/intern/mallocn_lockfree_impl.c
@@ -346,7 +346,17 @@ void *MEM_lockfree_malloc_arrayN(size_t len, size_t size, const char *str)
 
 void *MEM_lockfree_mallocN_aligned(size_t len, size_t alignment, const char *str)
 {
-  MemHeadAligned *memh;
+  /* Huge alignment values doesn't make sense and they wouldn't fit into 'short' used in the
+   * MemHead. */
+  assert(alignment < 1024);
+
+  /* We only support alignments that are a power of two. */
+  assert(IS_POW2(alignment));
+
+  /* Some OS specific aligned allocators require a certain minimal alignment. */
+  if (alignment < ALIGNED_MALLOC_MINIMUM_ALIGNMENT) {
+    alignment = ALIGNED_MALLOC_MINIMUM_ALIGNMENT;
+  }
 
   /* It's possible that MemHead's size is not properly aligned,
    * do extra padding to deal with this.
@@ -356,17 +366,10 @@ void *MEM_lockfree_mallocN_aligned(size_t len, size_t alignment, const char *str
    */
   size_t extra_padding = MEMHEAD_ALIGN_PADDING(alignment);
 
-  /* Huge alignment values doesn't make sense and they
-   * wouldn't fit into 'short' used in the MemHead.
-   */
-  assert(alignment < 1024);
-
-  /* We only support alignment to a power of two. */
-  assert(IS_POW2(alignment));
-
   len = SIZET_ALIGN_4(len);
 
-  memh = (MemHeadAligned *)aligned_malloc(len + extra_padding + sizeof(MemHeadAligned), alignment);
+  MemHeadAligned *memh = (MemHeadAligned *)aligned_malloc(
+      len + extra_padding + sizeof(MemHeadAligned), alignment);
 
   if (LIKELY(memh)) {
     /* We keep padding in the beginning of MemHead,
diff --git a/tests/gtests/guardedalloc/guardedalloc_alignment_test.cc b/tests/gtests/guardedalloc/guardedalloc_alignment_test.cc
index efb29a6088d..4866ac44e3c 100644
--- a/tests/gtests/guardedalloc/guardedalloc_alignment_test.cc
+++ b/tests/gtests/guardedalloc/guardedalloc_alignment_test.cc
@@ -34,6 +34,50 @@ void DoBasicAlignmentChecks(const int alignment)
 
 }  // namespace
 
+TEST(guardedalloc, LockfreeAlignedAlloc1)
+{
+  DoBasicAlignmentChecks(1);
+}
+
+TEST(guardedalloc, GuardedAlignedAlloc1)
+{
+  MEM_use_guarded_allocator();
+  DoBasicAlignmentChecks(1);
+}
+
+TEST(guardedalloc, LockfreeAlignedAlloc2)
+{
+  DoBasicAlignmentChecks(2);
+}
+
+TEST(guardedalloc, GuardedAlignedAlloc2)
+{
+  MEM_use_guarded_allocator();
+  DoBasicAlignmentChecks(2);
+}
+
+TEST(guardedalloc, LockfreeAlignedAlloc4)
+{
+  DoBasicAlignmentChecks(4);
+}
+
+TEST(guardedalloc, GuardedAlignedAlloc4)
+{
+  MEM_use_guarded_allocator();
+  DoBasicAlignmentChecks(4);
+}
+
+TEST(guardedalloc, LockfreeAlignedAlloc8)
+{
+  DoBasicAlignmentChecks(8);
+}
+
+TEST(guardedalloc, GuardedAlignedAlloc8)
+{
+  MEM_use_guarded_allocator();
+  DoBasicAlignmentChecks(8);
+}
+
 TEST(guardedalloc, LockfreeAlignedAlloc16)
 {
   DoBasicAlignmentChecks(16);
@@ -45,13 +89,35 @@ TEST(guardedalloc, GuardedAlignedAlloc16)
   DoBasicAlignmentChecks(16);
 }
 
-// On Apple we currently support 16 bit alignment only.
-// Harmless for Blender, but would be nice to support
-// eventually.
-#ifndef __APPLE__
+TEST(guardedalloc, LockfreeAlignedAlloc32)
+{
+  DoBasicAlignmentChecks(32);
+}
+
 TEST(guardedalloc, GuardedAlignedAlloc32)
 {
   MEM_use_guarded_allocator();
   DoBasicAlignmentChecks(32);
 }
-#endif
+
+TEST(guardedalloc, LockfreeAlignedAlloc256)
+{
+  DoBasicAlignmentChecks(256);
+}
+
+TEST(guardedalloc, GuardedAlignedAlloc256)
+{
+  MEM_use_guarded_allocator();
+  DoBasicAlignmentChecks(256);
+}
+
+TEST(guardedalloc, LockfreeAlignedAlloc512)
+{
+  DoBasicAlignmentChecks(512);
+}
+
+TEST(guardedalloc, GuardedAlignedAlloc512)
+{
+  MEM_use_guarded_allocator();
+  DoBasicAlignmentChecks(512);
+}