diff options
author | Jacques Lucke <mail@jlucke.com> | 2020-01-23 16:17:13 +0300 |
---|---|---|
committer | Jacques Lucke <mail@jlucke.com> | 2020-01-23 16:21:48 +0300 |
commit | 9c9ea37770dcf2d8a77cab0bf267a5bcf76500eb (patch) | |
tree | 42949c951e57c88dc34ab62dc79763644aea5848 | |
parent | 237d03f3a308efb2155c438ada6feb347472db8c (diff) |
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
-rw-r--r-- | intern/guardedalloc/intern/mallocn.c | 3 | ||||
-rw-r--r-- | intern/guardedalloc/intern/mallocn_intern.h | 2 | ||||
-rw-r--r-- | intern/guardedalloc/intern/mallocn_lockfree_impl.c | 23 | ||||
-rw-r--r-- | tests/gtests/guardedalloc/guardedalloc_alignment_test.cc | 76 |
4 files changed, 89 insertions, 15 deletions
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); +} |