Merge pull request #84 from ldorau/Make_sure_pointers_are_correctly_a…

…ligned_in_os_mmap_aligned

Make sure pointers are correctly aligned in os_mmap_aligned()

src/provider/provider_os_memory.c

@@ -268,8 +268,8 @@ static umf_result_t os_alloc(void *provider, size_t size, size_t alignment,
 
     void *addr = NULL;
     errno = 0;
-    ret =
-        os_mmap_aligned(NULL, size, alignment, protection, flags, -1, 0, &addr);
+    ret = os_mmap_aligned(NULL, size, alignment, page_size, protection, flags,
+                          -1, 0, &addr);
     if (ret) {
         os_store_last_native_error(UMF_OS_RESULT_ERROR_ALLOC_FAILED, errno);
         if (os_config->traces) {

src/provider/provider_os_memory_internal.h

@@ -32,8 +32,9 @@ long os_mbind(void *addr, size_t len, int mode, const unsigned long *nodemask,
 long os_get_mempolicy(int *mode, unsigned long *nodemask, unsigned long maxnode,
                       void *addr);
 
-int os_mmap_aligned(void *hint_addr, size_t length, size_t alignment, int prot,
-                    int flags, int fd, long offset, void **out_addr);
+int os_mmap_aligned(void *hint_addr, size_t length, size_t alignment,
+                    size_t page_size, int prot, int flags, int fd, long offset,
+                    void **out_addr);
 
 int os_munmap(void *addr, size_t length);
 

src/provider/provider_os_memory_linux.c

@@ -117,12 +117,18 @@ long os_get_mempolicy(int *mode, unsigned long *nodemask, unsigned long maxnode,
                          MPOL_F_NODE | MPOL_F_ADDR);
 }
 
-int os_mmap_aligned(void *hint_addr, size_t length, size_t alignment, int prot,
-                    int flags, int fd, long offset, void **out_addr) {
+static inline void assert_is_page_aligned(uintptr_t ptr, size_t page_size) {
+    assert((ptr & (page_size - 1)) == 0);
+    (void)ptr;       // unused in Release build
+    (void)page_size; // unused in Release build
+}
+
+int os_mmap_aligned(void *hint_addr, size_t length, size_t alignment,
+                    size_t page_size, int prot, int flags, int fd, long offset,
+                    void **out_addr) {
     assert(out_addr);
 
     size_t extended_length = length;
-    size_t page_size = os_get_page_size();
 
     if (alignment > page_size) {
         // We have to increase length by alignment to be able to "cut out"
@@ -141,15 +147,30 @@ int os_mmap_aligned(void *hint_addr, size_t length, size_t alignment, int prot,
 
     if (alignment > page_size) {
         uintptr_t addr = (uintptr_t)ptr;
-        uintptr_t aligned_addr = (addr + alignment) & ~(alignment - 1);
+        uintptr_t aligned_addr = addr;
+        uintptr_t rest_of_div = aligned_addr % alignment;
+
+        if (rest_of_div) {
+            aligned_addr += alignment - rest_of_div;
+        }
+
+        assert_is_page_aligned(aligned_addr, page_size);
 
         size_t head_len = aligned_addr - addr;
         if (head_len > 0) {
             munmap(ptr, head_len);
         }
 
+        // tail address has to page-aligned
         uintptr_t tail = aligned_addr + length;
-        size_t tail_len = (addr + extended_length) - (aligned_addr + length);
+        if (tail & (page_size - 1)) {
+            tail = (tail + page_size) & ~(page_size - 1);
+        }
+
+        assert_is_page_aligned(tail, page_size);
+        assert(tail >= aligned_addr + length);
+
+        size_t tail_len = (addr + extended_length) - tail;
         if (tail_len > 0) {
             munmap((void *)tail, tail_len);
         }

test/provider_os_memory.cpp

@@ -12,6 +12,9 @@ using umf_test::test;
 
 #define SIZE_4K (4096)
 
+#define ASSERT_IS_ALIGNED(ptr, alignment)                                      \
+    ASSERT_EQ(((uintptr_t)ptr % alignment), 0)
+
 static umf_os_memory_provider_params_t UMF_OS_MEMORY_PROVIDER_PARAMS_TEST = {
     /* .protection = */ UMF_PROTECTION_READ | UMF_PROTECTION_WRITE,
     /* .visibility = */ UMF_VISIBILITY_PRIVATE,
@@ -137,6 +140,7 @@ TEST_F(test, provider_os_memory_alloc_free_2k_alignment_2k) {
         umfMemoryProviderAlloc(os_memory_provider, size, alignment, &ptr);
     ASSERT_EQ(umf_result, UMF_RESULT_SUCCESS);
     ASSERT_NE(ptr, nullptr);
+    ASSERT_IS_ALIGNED(ptr, alignment);
 
     memset(ptr, 0xFF, size);
 
@@ -165,6 +169,7 @@ TEST_F(test, provider_os_memory_alloc_free_8k_alignment_8k) {
         umfMemoryProviderAlloc(os_memory_provider, size, alignment, &ptr);
     ASSERT_EQ(umf_result, UMF_RESULT_SUCCESS);
     ASSERT_NE(ptr, nullptr);
+    ASSERT_IS_ALIGNED(ptr, alignment);
 
     memset(ptr, 0xFF, size);