Fix the default alignment of the allocations.

In order to enforce this constraint:

The pointer returned if the allocation succeeds shall be suitably
aligned so that it may be assigned to a pointer to any type of object
and then used to access such an object in the space allocated.

Force all allocations on 32 bit systems to have 8 byte alignment,
and all allocations on 64 bit systems to have 16 byte alignment.

Add a test to verify that the allocator returns the correct alignments.

Bug: 26739265
Change-Id: I9af53279617408676b94e4ec6481b3ed7ffafc6a

libc/malloc_debug/BacktraceData.cpp

@@ -44,7 +44,7 @@
 BacktraceData::BacktraceData(const Config& config, size_t* offset) {
   size_t hdr_len = sizeof(BacktraceHeader) + sizeof(uintptr_t) * config.backtrace_frames;
   alloc_offset_ = *offset;
-  *offset += BIONIC_ALIGN(hdr_len, sizeof(uintptr_t));
+  *offset += BIONIC_ALIGN(hdr_len, MINIMUM_ALIGNMENT_BYTES);
 }
 
 static BacktraceData* g_backtrace_data = nullptr;

libc/malloc_debug/Config.cpp

@@ -347,10 +347,10 @@ bool Config::SetFromProperties() {
   valid = valid && parser.Done();
 
   if (valid) {
-    // It's necessary to align the front guard to sizeof(uintptr_t) to
+    // It's necessary to align the front guard to MINIMUM_ALIGNMENT_BYTES to
     // make sure that the header is aligned properly.
     if (options & FRONT_GUARD) {
-      front_guard_bytes = BIONIC_ALIGN(front_guard_bytes, sizeof(uintptr_t));
+      front_guard_bytes = BIONIC_ALIGN(front_guard_bytes, MINIMUM_ALIGNMENT_BYTES);
     }
 
     // This situation can occur if the free_track option is specified and

libc/malloc_debug/Config.h

@@ -41,6 +41,14 @@ constexpr uint64_t FREE_TRACK = 0x40;
 constexpr uint64_t TRACK_ALLOCS = 0x80;
 constexpr uint64_t LEAK_TRACK = 0x100;
 
+// In order to guarantee posix compliance, set the minimum alignment
+// to 8 bytes for 32 bit systems and 16 bytes for 64 bit systems.
+#if defined(__LP64__)
+constexpr size_t MINIMUM_ALIGNMENT_BYTES = 16;
+#else
+constexpr size_t MINIMUM_ALIGNMENT_BYTES = 8;
+#endif
+
 // If only one or more of these options is set, then no special header is needed.
 constexpr uint64_t NO_HEADER_OPTIONS = FILL_ON_ALLOC | FILL_ON_FREE | EXPAND_ALLOC;
 

libc/malloc_debug/DebugData.cpp

@@ -47,7 +47,7 @@ bool DebugData::Initialize() {
     need_header_ = true;
 
     // Initialize all of the static header offsets.
-    pointer_offset_ = BIONIC_ALIGN(sizeof(Header), sizeof(uintptr_t));
+    pointer_offset_ = BIONIC_ALIGN(sizeof(Header), MINIMUM_ALIGNMENT_BYTES);
 
     if (config_.options & BACKTRACE) {
       backtrace.reset(new BacktraceData(config_, &pointer_offset_));

libc/malloc_debug/GuardData.cpp

@@ -75,7 +75,7 @@ FrontGuardData::FrontGuardData(const Config& config, size_t* offset)
   // Create a buffer for fast comparisons of the front guard.
   cmp_mem_.resize(config.front_guard_bytes);
   memset(cmp_mem_.data(), config.front_guard_value, cmp_mem_.size());
-  // Assumes that front_bytes is a multiple of sizeof(uintptr_t).
+  // Assumes that front_bytes is a multiple of MINIMUM_ALIGNMENT_BYTES.
   offset_ = *offset;
   *offset += config.front_guard_bytes;
 }

libc/malloc_debug/malloc_debug.cpp

@@ -39,6 +39,7 @@
 #include <private/bionic_malloc_dispatch.h>
 
 #include "backtrace.h"
+#include "Config.h"
 #include "DebugData.h"
 #include "debug_disable.h"
 #include "debug_log.h"
@@ -264,7 +265,8 @@ void* debug_malloc(size_t size) {
       return nullptr;
     }
 
-    Header* header = reinterpret_cast<Header*>(g_dispatch->memalign(sizeof(uintptr_t), real_size));
+    Header* header = reinterpret_cast<Header*>(
+        g_dispatch->memalign(MINIMUM_ALIGNMENT_BYTES, real_size));
     if (header == nullptr) {
       return nullptr;
     }
@@ -355,10 +357,10 @@ void* debug_memalign(size_t alignment, size_t bytes) {
     if (!powerof2(alignment)) {
       alignment = BIONIC_ROUND_UP_POWER_OF_2(alignment);
     }
-    // Force the alignment to at least sizeof(uintptr_t) to guarantee
+    // Force the alignment to at least MINIMUM_ALIGNMENT_BYTES to guarantee
     // that the header is aligned properly.
-    if (alignment < sizeof(uintptr_t)) {
-      alignment = sizeof(uintptr_t);
+    if (alignment < MINIMUM_ALIGNMENT_BYTES) {
+      alignment = MINIMUM_ALIGNMENT_BYTES;
     }
 
     // We don't have any idea what the natural alignment of
@@ -512,7 +514,8 @@ void* debug_calloc(size_t nmemb, size_t bytes) {
     }
 
     // Need to guarantee the alignment of the header.
-    Header* header = reinterpret_cast<Header*>(g_dispatch->memalign(sizeof(uintptr_t), real_size));
+    Header* header = reinterpret_cast<Header*>(
+        g_dispatch->memalign(MINIMUM_ALIGNMENT_BYTES, real_size));
     if (header == nullptr) {
       return nullptr;
     }

libc/malloc_debug/malloc_debug.h

@@ -39,7 +39,7 @@
 // will still be in this order.
 //   Header          (Required)
 //   BacktraceHeader (Optional: For the allocation backtrace)
-//   uint8_t data    (Optional: Front guard, will be a multiple of sizeof(uintptr_t))
+//   uint8_t data    (Optional: Front guard, will be a multiple of MINIMUM_ALIGNMENT_BYTES)
 //   allocation data
 //   uint8_t data    (Optional: End guard)
 //

libc/malloc_debug/tests/malloc_debug_config_tests.cpp

@@ -230,6 +230,10 @@ TEST_F(MallocDebugConfigTest, front_guard) {
   ASSERT_EQ(FRONT_GUARD, config->options);
   ASSERT_EQ(40U, config->front_guard_bytes);
 
+  ASSERT_TRUE(InitConfig("front_guard=41"));
+  ASSERT_EQ(FRONT_GUARD, config->options);
+  ASSERT_EQ(48U, config->front_guard_bytes);
+
   ASSERT_STREQ("", getFakeLogBuf().c_str());
   ASSERT_STREQ("", getFakeLogPrint().c_str());
 }

libc/malloc_debug/tests/malloc_debug_unit_tests.cpp

@@ -34,6 +34,7 @@
 #include <private/bionic_macros.h>
 #include <private/bionic_malloc_dispatch.h>
 
+#include "Config.h"
 #include "malloc_debug.h"
 
 #include "log_fake.h"
@@ -70,9 +71,9 @@ constexpr char DIVIDER[] =
 constexpr uint32_t BACKTRACE_HEADER = 0x1;
 
 static size_t get_tag_offset(uint32_t flags = 0, size_t backtrace_frames = 0) {
-  size_t offset = BIONIC_ALIGN(sizeof(Header), sizeof(uintptr_t));
+  size_t offset = BIONIC_ALIGN(sizeof(Header), MINIMUM_ALIGNMENT_BYTES);
   if (flags & BACKTRACE_HEADER) {
-    offset += BIONIC_ALIGN(sizeof(BacktraceHeader) + sizeof(uintptr_t) * backtrace_frames, sizeof(uintptr_t));
+    offset += BIONIC_ALIGN(sizeof(BacktraceHeader) + sizeof(uintptr_t) * backtrace_frames, MINIMUM_ALIGNMENT_BYTES);
   }
   return offset;
 }

tests/malloc_test.cpp

@@ -430,3 +430,73 @@ TEST(malloc, realloc_0) {
   void* p2 = realloc(p, 0);
   ASSERT_TRUE(p2 == nullptr);
 }
+
+constexpr size_t MAX_LOOPS = 200;
+
+// Make sure that memory returned by malloc is aligned to allow these data types.
+TEST(malloc, verify_alignment) {
+  uint32_t** values_32 = new uint32_t*[MAX_LOOPS];
+  uint64_t** values_64 = new uint64_t*[MAX_LOOPS];
+  long double** values_ldouble = new long double*[MAX_LOOPS];
+  // Use filler to attempt to force the allocator to get potentially bad alignments.
+  void** filler = new void*[MAX_LOOPS];
+
+  for (size_t i = 0; i < MAX_LOOPS; i++) {
+    // Check uint32_t pointers.
+    filler[i] = malloc(1);
+    ASSERT_TRUE(filler[i] != nullptr);
+
+    values_32[i] = reinterpret_cast<uint32_t*>(malloc(sizeof(uint32_t)));
+    ASSERT_TRUE(values_32[i] != nullptr);
+    *values_32[i] = i;
+    ASSERT_EQ(*values_32[i], i);
+    ASSERT_EQ(0U, reinterpret_cast<uintptr_t>(values_32[i]) & (sizeof(uint32_t) - 1));
+
+    free(filler[i]);
+  }
+
+  for (size_t i = 0; i < MAX_LOOPS; i++) {
+    // Check uint64_t pointers.
+    filler[i] = malloc(1);
+    ASSERT_TRUE(filler[i] != nullptr);
+
+    values_64[i] = reinterpret_cast<uint64_t*>(malloc(sizeof(uint64_t)));
+    ASSERT_TRUE(values_64[i] != nullptr);
+    *values_64[i] = 0x1000 + i;
+    ASSERT_EQ(*values_64[i], 0x1000 + i);
+    ASSERT_EQ(0U, reinterpret_cast<uintptr_t>(values_64[i]) & (sizeof(uint64_t) - 1));
+
+    free(filler[i]);
+  }
+
+  for (size_t i = 0; i < MAX_LOOPS; i++) {
+    // Check long double pointers.
+    filler[i] = malloc(1);
+    ASSERT_TRUE(filler[i] != nullptr);
+
+    values_ldouble[i] = reinterpret_cast<long double*>(malloc(sizeof(long double)));
+    ASSERT_TRUE(values_ldouble[i] != nullptr);
+    *values_ldouble[i] = 5.5 + i;
+    ASSERT_DOUBLE_EQ(*values_ldouble[i], 5.5 + i);
+    // 32 bit glibc has a long double size of 12 bytes, so hardcode the
+    // required alignment to 0x7.
+#if !defined(__BIONIC__) && !defined(__LP64__)
+    ASSERT_EQ(0U, reinterpret_cast<uintptr_t>(values_ldouble[i]) & 0x7);
+#else
+    ASSERT_EQ(0U, reinterpret_cast<uintptr_t>(values_ldouble[i]) & (sizeof(long double) - 1));
+#endif
+
+    free(filler[i]);
+  }
+
+  for (size_t i = 0; i < MAX_LOOPS; i++) {
+    free(values_32[i]);
+    free(values_64[i]);
+    free(values_ldouble[i]);
+  }
+
+  delete[] filler;
+  delete[] values_32;
+  delete[] values_64;
+  delete[] values_ldouble;
+}