Merge "Make aligned_alloc match the standard." am: b8ef55a4f8 am: 4917f6f575

am: 5cbbc36698

Change-Id: I396705b8822e1fca827e214d5a622f0c2651c979

docs/status.md

@@ -48,8 +48,10 @@ New libc behavior in Q (API level 29):
   * Whole printf family now supports the GNU `%m` extension, rather than a special-case hack in `syslog`
   * `popen` now always uses `O_CLOEXEC`, not just with the `e` extension
   * Bug fixes to handling of UTF-8 U+fffe/U+ffff and code points above U+10ffff
+  * `aligned_alloc` correctly verifies that `size` is a multiple of `alignment`
 
 New libc functions in P (API level 28):
+  * `aligned_alloc`
   * `__freading`/`__fwriting` (completing <stdio_ext.h>)
   * `endhostent`/`endnetent`/`endprotoent`/`getnetent`/`getprotoent`/`sethostent`/`setnetent`/`setprotoent` (completing <netdb.h>)
   * `fexecve`

libc/bionic/jemalloc.h

@@ -22,14 +22,19 @@
 // Need to wrap memalign since je_memalign fails on non-power of 2 alignments.
 #define je_memalign je_memalign_round_up_boundary
 
+// Need to wrap aligned_alloc since je_aligned_alloc does not enforce
+// that size is a multiple of alignment.
+#define je_aligned_alloc je_aligned_alloc_wrapper
+
 __BEGIN_DECLS
 
-struct mallinfo je_mallinfo();
-int je_mallopt(int, int);
+void* je_aligned_alloc_wrapper(size_t, size_t);
 int je_iterate(uintptr_t, size_t, void (*)(uintptr_t, size_t, void*), void*);
+int je_mallctl(const char *name, void *oldp, size_t *oldlenp, void *newp, size_t newlen);
+struct mallinfo je_mallinfo();
 void je_malloc_disable();
 void je_malloc_enable();
-int je_mallctl(const char *name, void *oldp, size_t *oldlenp, void *newp, size_t newlen);
+int je_mallopt(int, int);
 void* je_memalign_round_up_boundary(size_t, size_t);
 void* je_pvalloc(size_t);
 

libc/bionic/jemalloc_wrapper.cpp

@@ -14,6 +14,7 @@
  * limitations under the License.
  */
 
+#include <errno.h>
 #include <malloc.h>
 #include <sys/param.h>
 #include <unistd.h>
@@ -48,6 +49,20 @@ void* je_memalign_round_up_boundary(size_t boundary, size_t size) {
   return je_memalign(boundary, size);
 }
 
+#ifdef je_aligned_alloc
+#undef je_aligned_alloc
+#endif
+
+// The aligned_alloc function requires that size is a multiple of alignment.
+// jemalloc doesn't enforce this, so add enforcement here.
+void* je_aligned_alloc_wrapper(size_t alignment, size_t size) {
+  if ((size % alignment) != 0) {
+    errno = EINVAL;
+    return nullptr;
+  }
+  return je_aligned_alloc(alignment, size);
+}
+
 int je_mallopt(int param, int value) {
   // The only parameter we currently understand is M_DECAY_TIME.
   if (param == M_DECAY_TIME) {

libc/malloc_debug/malloc_debug.cpp

@@ -729,7 +729,7 @@ void* debug_aligned_alloc(size_t alignment, size_t size) {
   if (DebugCallsDisabled()) {
     return g_dispatch->aligned_alloc(alignment, size);
   }
-  if (!powerof2(alignment)) {
+  if (!powerof2(alignment) || (size % alignment) != 0) {
     errno = EINVAL;
     return nullptr;
   }

libc/malloc_debug/tests/malloc_debug_unit_tests.cpp

@@ -318,7 +318,7 @@ TEST_F(MallocDebugTest, expand_alloc) {
   ASSERT_LE(1039U, debug_malloc_usable_size(pointer));
   debug_free(pointer);
 
-  pointer = debug_aligned_alloc(128, 15);
+  pointer = debug_aligned_alloc(16, 16);
   ASSERT_TRUE(pointer != nullptr);
   ASSERT_LE(1039U, debug_malloc_usable_size(pointer));
   debug_free(pointer);
@@ -2144,9 +2144,9 @@ void VerifyRecordAllocs() {
   debug_free(pointer);
   expected += android::base::StringPrintf("%d: free %p\n", getpid(), pointer);
 
-  pointer = debug_aligned_alloc(32, 50);
+  pointer = debug_aligned_alloc(32, 64);
   ASSERT_TRUE(pointer != nullptr);
-  expected += android::base::StringPrintf("%d: memalign %p 32 50\n", getpid(), pointer);
+  expected += android::base::StringPrintf("%d: memalign %p 32 64\n", getpid(), pointer);
   debug_free(pointer);
   expected += android::base::StringPrintf("%d: free %p\n", getpid(), pointer);
 

libc/malloc_hooks/malloc_hooks.cpp

@@ -176,7 +176,7 @@ int hooks_mallopt(int param, int value) {
 
 void* hooks_aligned_alloc(size_t alignment, size_t size) {
   if (__memalign_hook != nullptr && __memalign_hook != default_memalign_hook) {
-    if (!powerof2(alignment)) {
+    if (!powerof2(alignment) || (size % alignment) != 0) {
       errno = EINVAL;
       return nullptr;
     }

tests/stdlib_test.cpp

@@ -272,11 +272,11 @@ TEST(stdlib, aligned_alloc_sweep) {
   for (size_t align = 1; align <= 2048; align <<= 1) {
     // Try all of the non power of 2 values from the last until this value.
     for (size_t fail_align = last_align + 1; fail_align < align; fail_align++) {
-      ASSERT_TRUE(aligned_alloc(fail_align, 256) == nullptr)
+      ASSERT_TRUE(aligned_alloc(fail_align, fail_align) == nullptr)
           << "Unexpected success at align " << fail_align;
       ASSERT_EQ(EINVAL, errno) << "Unexpected errno at align " << fail_align;
     }
-    void* ptr = aligned_alloc(align, 256);
+    void* ptr = aligned_alloc(align, 2 * align);
     ASSERT_TRUE(ptr != nullptr) << "Unexpected failure at align " << align;
     ASSERT_EQ(0U, reinterpret_cast<uintptr_t>(ptr) & (align - 1))
         << "Did not return a valid aligned ptr " << ptr << " expected alignment " << align;
@@ -292,11 +292,11 @@ TEST(stdlib, aligned_alloc_overflow) {
 
 TEST(stdlib, aligned_alloc_size_not_multiple_of_alignment) {
   SKIP_WITH_HWASAN;
-  for (size_t size = 1; size <= 2048; size++) {
-    void* ptr = aligned_alloc(2048, size);
-    ASSERT_TRUE(ptr != nullptr) << "Failed at size " << std::to_string(size);
-    free(ptr);
-  }
+
+  ASSERT_TRUE(aligned_alloc(2048, 1) == nullptr);
+  ASSERT_TRUE(aligned_alloc(4, 3) == nullptr);
+  ASSERT_TRUE(aligned_alloc(4, 7) == nullptr);
+  ASSERT_TRUE(aligned_alloc(16, 8) == nullptr);
 }
 
 TEST(stdlib, realpath__NULL_filename) {