Static Allocator - Aligned allocations (#31)

* Aligned memory pool allocations

Signed-off-by: Emerson Knapp <eknapp@amazon.com>

* Fix formatting

Signed-off-by: Emerson Knapp <eknapp@amazon.com>

* Address some review nitpicks

Signed-off-by: Emerson Knapp <eknapp@amazon.com>

* Fix formatting

Signed-off-by: Emerson Knapp <eknapp@amazon.com>

* A pass at adding unit test for StaticAllocator

Signed-off-by: Emerson Knapp <eknapp@amazon.com>

osrf_testing_tools_cpp/src/CMakeLists.txt

@@ -3,3 +3,5 @@ add_subdirectory(test_runner)
 
 set(memory_tools_extra_test_env "${memory_tools_extra_test_env}" PARENT_SCOPE)
 set(memory_tools_is_available "${memory_tools_is_available}" PARENT_SCOPE)
+set(memory_tools_src_dir_internal_testing_only
+  "${memory_tools_src_dir_internal_testing_only}" PARENT_SCOPE)

osrf_testing_tools_cpp/src/memory_tools/CMakeLists.txt

@@ -96,3 +96,5 @@ install(EXPORT memory_tools_interpose
 
 set(memory_tools_extra_test_env "${memory_tools_extra_test_env}" PARENT_SCOPE)
 set(memory_tools_is_available "${memory_tools_is_available}" PARENT_SCOPE)
+set(memory_tools_src_dir_internal_testing_only
+  "$<BUILD_INTERFACE:${CMAKE_SOURCE_DIR}/src>" PARENT_SCOPE)

osrf_testing_tools_cpp/src/memory_tools/impl/linux.cpp

@@ -42,13 +42,23 @@ find_original_function(const char * name)
   return original_function;
 }
 
+// An amount of memory that is greater than what is needed for static initialization
+// for any test we run. It was found experimentally on Ubuntu Linux 16.04 x86_64.
+static constexpr size_t STATIC_ALLOCATOR_SIZE = 0x800000;
 using osrf_testing_tools_cpp::memory_tools::impl::StaticAllocator;
-// the size was found experimentally on Ubuntu Linux 16.04 x86_64
-using StaticAllocatorT = StaticAllocator<8388608>;
-// used to fullfil calloc call from dlerror.c during initialization of original functions
-// constructor is called on first use with a placement-new and the static storage
+using StaticAllocatorT = StaticAllocator<STATIC_ALLOCATOR_SIZE>;
 static uint8_t g_static_allocator_storage[sizeof(StaticAllocatorT)];
-static StaticAllocatorT * g_static_allocator = nullptr;
+
+// Contains global allocator to make 100% sure to avoid Static Initialization Order Fiasco.
+// "Construct on first use" idiom
+static StaticAllocatorT *
+get_static_allocator()
+{
+  // placement-new the static allocator in preallocated storage
+  // which is used while finding the original memory functions
+  static StaticAllocatorT * alloc = new (g_static_allocator_storage) StaticAllocatorT;
+  return alloc;
+}
 
 // storage for original malloc/realloc/calloc/free
 using MallocSignature = void * (*)(size_t);
@@ -78,12 +88,7 @@ void *
 malloc(size_t size) noexcept
 {
   if (!get_static_initialization_complete()) {
-    if (nullptr == g_static_allocator) {
-      // placement-new the static allocator
-      // which is used while finding the original memory functions
-      g_static_allocator = new (g_static_allocator_storage) StaticAllocatorT;
-    }
-    return g_static_allocator->allocate(size);
+    return get_static_allocator()->allocate(size);
   }
   return unix_replacement_malloc(size, g_original_malloc);
 }
@@ -92,12 +97,7 @@ void *
 realloc(void * pointer, size_t size) noexcept
 {
   if (!get_static_initialization_complete()) {
-    if (nullptr == g_static_allocator) {
-      // placement-new the static allocator
-      // which is used while finding the original memory functions
-      g_static_allocator = new (g_static_allocator_storage) StaticAllocatorT;
-    }
-    return g_static_allocator->reallocate(pointer, size);
+    return get_static_allocator()->reallocate(pointer, size);
   }
   return unix_replacement_realloc(pointer, size, g_original_realloc);
 }
@@ -106,20 +106,15 @@ void *
 calloc(size_t count, size_t size) noexcept
 {
   if (!get_static_initialization_complete()) {
-    if (nullptr == g_static_allocator) {
-      // placement-new the static allocator
-      // which is used while finding the original memory functions
-      g_static_allocator = new (g_static_allocator_storage) StaticAllocatorT;
-    }
-    return g_static_allocator->zero_allocate(count, size);
+    return get_static_allocator()->zero_allocate(count, size);
   }
   return unix_replacement_calloc(count, size, g_original_calloc);
 }
 
 void
 free(void * pointer) noexcept
 {
-  if (nullptr == pointer || g_static_allocator->deallocate(pointer)) {
+  if (nullptr == pointer || get_static_allocator()->deallocate(pointer)) {
     // free of nullptr or,
     // memory was originally allocated by static allocator, no need to pass to "real" free
     return;

osrf_testing_tools_cpp/src/memory_tools/impl/static_allocator.hpp

@@ -29,6 +29,20 @@ namespace memory_tools
 namespace impl
 {
 
+// Alignment of the largest primitive type for this system.
+static constexpr size_t MAX_ALIGN = alignof(std::max_align_t);
+
+/// Round value up to a multiple of alignment.
+/**
+  * Implementation cribbed from Boost.
+  * https://github.com/boostorg/align/blob/develop/include/boost/align/align_up.hpp
+  */
+static constexpr inline std::size_t
+align_up(std::size_t value, std::size_t alignment) noexcept
+{
+  return (value + alignment - 1) & ~(alignment - 1);
+}
+
 template<size_t MemoryPoolSize>
 class StaticAllocator
 {
@@ -45,9 +59,10 @@ class StaticAllocator
   void *
   allocate(size_t size)
   {
-    if (size <= size_t(std::distance(stack_pointer_, end_))) {
+    const size_t aligned_size = align_up(size, MAX_ALIGN);
+    if (aligned_size <= static_cast<size_t>(std::distance(end_, stack_pointer_))) {
       uint8_t * result = stack_pointer_;
-      stack_pointer_ += size;
+      stack_pointer_ += aligned_size;
       return result;
     }
     SAFE_FWRITE(stderr, "StackAllocator.allocate() -> nullptr\n");
@@ -105,7 +120,8 @@ class StaticAllocator
   }
 
 private:
-  uint8_t memory_pool_[MemoryPoolSize];
+  // Make sure that our memory pool is aligned to the maximum primitive size for this system.
+  alignas(MAX_ALIGN) uint8_t memory_pool_[MemoryPoolSize];
   uint8_t * begin_;
   uint8_t * end_;
   uint8_t * stack_pointer_;
@@ -115,12 +131,4 @@ class StaticAllocator
 }  // namespace memory_tools
 }  // namespace osrf_testing_tools_cpp
 
-int main(void)
-{
-  osrf_testing_tools_cpp::memory_tools::impl::StaticAllocator<64> sa;
-  void * mem = sa.allocate(16);
-  (void)mem;
-  return 0;
-}
-
 #endif  // MEMORY_TOOLS__IMPL__STATIC_ALLOCATOR_HPP_

osrf_testing_tools_cpp/test/memory_tools/CMakeLists.txt

@@ -4,6 +4,8 @@ target_link_libraries(test_memory_tools
   memory_tools
   gtest_main
 )
+target_include_directories(test_memory_tools
+  PRIVATE ${memory_tools_src_dir_internal_testing_only})
 
 if(memory_tools_is_available)
   add_test(

osrf_testing_tools_cpp/test/memory_tools/test_memory_tools.cpp

@@ -19,6 +19,7 @@
 
 #include "osrf_testing_tools_cpp/memory_tools/memory_tools.hpp"
 #include "osrf_testing_tools_cpp/scope_exit.hpp"
+#include "memory_tools/impl/static_allocator.hpp"
 
 /**
  * Tests the dynamic memory checking tools.
@@ -232,3 +233,25 @@ TEST(TestMemoryTools, test_example) {
   });
   t2.join();
 }
+
+/**
+ * Tests the static allocator used during dynamic library loading.
+ */
+TEST(TestMemoryTools, test_static_allocation_alignment) {
+  static constexpr size_t allocator_size = 0x1000;
+  // Arbitrarily chosen values (somewhat observed values from OpenSSL static initialization).
+  // Not all aligned to std::max_align_t on most platforms.
+  static const std::vector<size_t> request_sizes = {
+    24, 24, 24, 7, 82, 2424
+  };
+  static constexpr size_t max_align = alignof(std::max_align_t);
+
+  osrf_testing_tools_cpp::memory_tools::impl::StaticAllocator<allocator_size> allocator;
+
+  // Check that all returned memory blocks are aligned with the max_align type.
+  for (const size_t request : request_sizes) {
+    void * memory = allocator.allocate(request);
+    ASSERT_EQ(reinterpret_cast<size_t>(memory) % max_align, size_t(0));
+    ASSERT_TRUE(allocator.deallocate(memory));
+  }
+}