diff --git a/osrf_testing_tools_cpp/src/CMakeLists.txt b/osrf_testing_tools_cpp/src/CMakeLists.txt index f09e6a6..607a1cc 100644 --- a/osrf_testing_tools_cpp/src/CMakeLists.txt +++ b/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) diff --git a/osrf_testing_tools_cpp/src/memory_tools/CMakeLists.txt b/osrf_testing_tools_cpp/src/memory_tools/CMakeLists.txt index 6bbce36..3a892ee 100644 --- a/osrf_testing_tools_cpp/src/memory_tools/CMakeLists.txt +++ b/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 + "$" PARENT_SCOPE) diff --git a/osrf_testing_tools_cpp/src/memory_tools/impl/linux.cpp b/osrf_testing_tools_cpp/src/memory_tools/impl/linux.cpp index f92c9e7..cf69e73 100644 --- a/osrf_testing_tools_cpp/src/memory_tools/impl/linux.cpp +++ b/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 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,12 +106,7 @@ 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); } @@ -119,7 +114,7 @@ calloc(size_t count, size_t size) noexcept 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; diff --git a/osrf_testing_tools_cpp/src/memory_tools/impl/static_allocator.hpp b/osrf_testing_tools_cpp/src/memory_tools/impl/static_allocator.hpp index fa1ca06..f59d3b0 100644 --- a/osrf_testing_tools_cpp/src/memory_tools/impl/static_allocator.hpp +++ b/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 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(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_ diff --git a/osrf_testing_tools_cpp/test/memory_tools/CMakeLists.txt b/osrf_testing_tools_cpp/test/memory_tools/CMakeLists.txt index 3e2f225..5eecdec 100644 --- a/osrf_testing_tools_cpp/test/memory_tools/CMakeLists.txt +++ b/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( diff --git a/osrf_testing_tools_cpp/test/memory_tools/test_memory_tools.cpp b/osrf_testing_tools_cpp/test/memory_tools/test_memory_tools.cpp index 2a62c7b..e401b10 100644 --- a/osrf_testing_tools_cpp/test/memory_tools/test_memory_tools.cpp +++ b/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 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; + + // 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(memory) % max_align, size_t(0)); + ASSERT_TRUE(allocator.deallocate(memory)); + } +}