[scudo] Change aligned alloc functions to be more compliant & perf changes

Summary:
We were not following the `man` documented behaviors for invalid arguments to
`memalign` and associated functions. Using `CHECK` for those was a bit extreme,
so we relax the behavior to return null pointers as expected when this happens.
Adapt the associated test.

I am using this change also to change a few more minor performance improvements:
- mark as `UNLIKELY` a bunch of unlikely conditions;
- the current `CHECK` in `__sanitizer::RoundUpTo` is redundant for us in *all*
  calls. So I am introducing our own version without said `CHECK`.
- change our combined allocator `GetActuallyAllocatedSize`. We already know if
  the pointer is from the Primary or Secondary, so the `PointerIsMine` check is
  redundant as well, and costly for the 32-bit Primary. So we get the size by
  directly using the available Primary functions.

Finally, change a `int` to `uptr` to avoid a warning/error when compiling on
Android.

Reviewers: alekseyshl

Reviewed By: alekseyshl

Subscribers: llvm-commits

Differential Revision: https://reviews.llvm.org/D34782

llvm-svn: 306698

compiler-rt/lib/scudo/scudo_allocator.cpp

@@ -22,8 +22,7 @@
 #include "sanitizer_common/sanitizer_allocator_interface.h"
 #include "sanitizer_common/sanitizer_quarantine.h"
 
-#include <limits.h>
-#include <pthread.h>
+#include <errno.h>
 #include <string.h>
 
 namespace __scudo {
@@ -341,7 +340,7 @@ struct ScudoAllocator {
   // Helper function that checks for a valid Scudo chunk. nullptr isn't.
   bool isValidPointer(const void *UserPtr) {
     initThreadMaybe();
-    if (!UserPtr)
+    if (UNLIKELY(!UserPtr))
       return false;
     uptr UserBeg = reinterpret_cast<uptr>(UserPtr);
     if (!IsAligned(UserBeg, MinAlignment))
@@ -353,22 +352,19 @@ struct ScudoAllocator {
   void *allocate(uptr Size, uptr Alignment, AllocType Type,
                  bool ForceZeroContents = false) {
     initThreadMaybe();
-    if (UNLIKELY(!IsPowerOfTwo(Alignment))) {
-      dieWithMessage("ERROR: alignment is not a power of 2\n");
-    }
-    if (Alignment > MaxAlignment)
+    if (UNLIKELY(Alignment > MaxAlignment))
       return FailureHandler::OnBadRequest();
-    if (Alignment < MinAlignment)
+    if (UNLIKELY(Alignment < MinAlignment))
       Alignment = MinAlignment;
-    if (Size >= MaxAllowedMallocSize)
+    if (UNLIKELY(Size >= MaxAllowedMallocSize))
       return FailureHandler::OnBadRequest();
-    if (Size == 0)
+    if (UNLIKELY(Size == 0))
       Size = 1;
 
     uptr NeededSize = RoundUpTo(Size, MinAlignment) + AlignedChunkHeaderSize;
     uptr AlignedSize = (Alignment > MinAlignment) ?
         NeededSize + (Alignment - AlignedChunkHeaderSize) : NeededSize;
-    if (AlignedSize >= MaxAllowedMallocSize)
+    if (UNLIKELY(AlignedSize >= MaxAllowedMallocSize))
       return FailureHandler::OnBadRequest();
 
     // Primary and Secondary backed allocations have a different treatment. We
@@ -393,7 +389,7 @@ struct ScudoAllocator {
       Ptr = BackendAllocator.Allocate(&FallbackAllocatorCache, AllocationSize,
                                       AllocationAlignment, FromPrimary);
     }
-    if (!Ptr)
+    if (UNLIKELY(!Ptr))
       return FailureHandler::OnOOM();
 
     // If requested, we will zero out the entire contents of the returned chunk.
@@ -404,7 +400,7 @@ struct ScudoAllocator {
     UnpackedHeader Header = {};
     uptr AllocBeg = reinterpret_cast<uptr>(Ptr);
     uptr UserBeg = AllocBeg + AlignedChunkHeaderSize;
-    if (!IsAligned(UserBeg, Alignment)) {
+    if (UNLIKELY(!IsAligned(UserBeg, Alignment))) {
       // Since the Secondary takes care of alignment, a non-aligned pointer
       // means it is from the Primary. It is also the only case where the offset
       // field of the header would be non-zero.
@@ -481,7 +477,7 @@ struct ScudoAllocator {
   void deallocate(void *UserPtr, uptr DeleteSize, AllocType Type) {
     initThreadMaybe();
     // if (&__sanitizer_free_hook) __sanitizer_free_hook(UserPtr);
-    if (!UserPtr)
+    if (UNLIKELY(!UserPtr))
       return;
     uptr UserBeg = reinterpret_cast<uptr>(UserPtr);
     if (UNLIKELY(!IsAligned(UserBeg, MinAlignment))) {
@@ -568,7 +564,7 @@ struct ScudoAllocator {
   // Helper function that returns the actual usable size of a chunk.
   uptr getUsableSize(const void *Ptr) {
     initThreadMaybe();
-    if (!Ptr)
+    if (UNLIKELY(!Ptr))
       return 0;
     uptr UserBeg = reinterpret_cast<uptr>(Ptr);
     ScudoChunk *Chunk = getScudoChunk(UserBeg);
@@ -584,10 +580,10 @@ struct ScudoAllocator {
 
   void *calloc(uptr NMemB, uptr Size) {
     initThreadMaybe();
-    uptr Total = NMemB * Size;
-    if (Size != 0 && Total / Size != NMemB)  // Overflow check
+    // TODO(kostyak): switch to CheckForCallocOverflow once D34799 lands.
+    if (CallocShouldReturnNullDueToOverflow(NMemB, Size))
       return FailureHandler::OnBadRequest();
-    return allocate(Total, MinAlignment, FromMalloc, true);
+    return allocate(NMemB * Size, MinAlignment, FromMalloc, true);
   }
 
   void commitBack(ScudoThreadContext *ThreadContext) {
@@ -655,10 +651,6 @@ void *scudoValloc(uptr Size) {
   return Instance.allocate(Size, GetPageSizeCached(), FromMemalign);
 }
 
-void *scudoMemalign(uptr Alignment, uptr Size) {
-  return Instance.allocate(Size, Alignment, FromMemalign);
-}
-
 void *scudoPvalloc(uptr Size) {
   uptr PageSize = GetPageSizeCached();
   Size = RoundUpTo(Size, PageSize);
@@ -669,16 +661,27 @@ void *scudoPvalloc(uptr Size) {
   return Instance.allocate(Size, PageSize, FromMemalign);
 }
 
+void *scudoMemalign(uptr Alignment, uptr Size) {
+  if (UNLIKELY(!IsPowerOfTwo(Alignment)))
+    return ScudoAllocator::FailureHandler::OnBadRequest();
+  return Instance.allocate(Size, Alignment, FromMemalign);
+}
+
 int scudoPosixMemalign(void **MemPtr, uptr Alignment, uptr Size) {
+  if (UNLIKELY(!IsPowerOfTwo(Alignment) || (Alignment % sizeof(void *)) != 0)) {
+    *MemPtr = ScudoAllocator::FailureHandler::OnBadRequest();
+    return EINVAL;
+  }
   *MemPtr = Instance.allocate(Size, Alignment, FromMemalign);
+  if (!*MemPtr)
+    return ENOMEM;
   return 0;
 }
 
 void *scudoAlignedAlloc(uptr Alignment, uptr Size) {
-  // size must be a multiple of the alignment. To avoid a division, we first
-  // make sure that alignment is a power of 2.
-  CHECK(IsPowerOfTwo(Alignment));
-  CHECK_EQ((Size & (Alignment - 1)), 0);
+  // Alignment must be a power of 2, Size must be a multiple of Alignment.
+  if (UNLIKELY(!IsPowerOfTwo(Alignment) || (Size & (Alignment - 1)) != 0))
+    return ScudoAllocator::FailureHandler::OnBadRequest();
   return Instance.allocate(Size, Alignment, FromMalloc);
 }
 

compiler-rt/lib/scudo/scudo_allocator.h

@@ -116,6 +116,11 @@ struct AP32 {
 typedef SizeClassAllocator32<AP32> PrimaryAllocator;
 #endif  // SANITIZER_CAN_USE_ALLOCATOR64
 
+// __sanitizer::RoundUp has a CHECK that is extraneous for us. Use our own.
+INLINE uptr RoundUpTo(uptr Size, uptr Boundary) {
+  return (Size + Boundary - 1) & ~(Boundary - 1);
+}
+
 #include "scudo_allocator_secondary.h"
 #include "scudo_allocator_combined.h"
 

compiler-rt/lib/scudo/scudo_allocator_combined.h

@@ -45,7 +45,7 @@ class ScudoCombinedAllocator {
 
   uptr GetActuallyAllocatedSize(void *Ptr, bool FromPrimary) {
     if (FromPrimary)
-      return Primary.GetActuallyAllocatedSize(Ptr);
+      return PrimaryAllocator::ClassIdToSize(Primary.GetSizeClass(Ptr));
     return Secondary.GetActuallyAllocatedSize(Ptr);
   }
 

compiler-rt/lib/scudo/scudo_tls_android.cpp

@@ -45,7 +45,7 @@ static void initOnce() {
   NumberOfContexts = getNumberOfCPUs();
   ThreadContexts = reinterpret_cast<ScudoThreadContext *>(
       MmapOrDie(sizeof(ScudoThreadContext) * NumberOfContexts, __func__));
-  for (int i = 0; i < NumberOfContexts; i++)
+  for (uptr i = 0; i < NumberOfContexts; i++)
     ThreadContexts[i].init();
 }
 

compiler-rt/test/scudo/memalign.cpp

@@ -1,11 +1,12 @@
 // RUN: %clang_scudo %s -o %t
-// RUN:     %run %t valid   2>&1
-// RUN: not %run %t invalid 2>&1 | FileCheck %s
+// RUN: %run %t valid   2>&1
+// RUN: %run %t invalid 2>&1
 
 // Tests that the various aligned allocation functions work as intended. Also
 // tests for the condition where the alignment is not a power of 2.
 
 #include <assert.h>
+#include <errno.h>
 #include <malloc.h>
 #include <stdlib.h>
 #include <string.h>
@@ -24,6 +25,7 @@ int main(int argc, char **argv)
   void *p = nullptr;
   size_t alignment = 1U << 12;
   size_t size = 1U << 12;
+  int err;
 
   assert(argc == 2);
 
@@ -57,10 +59,22 @@ int main(int argc, char **argv)
     }
   }
   if (!strcmp(argv[1], "invalid")) {
+    // Alignment is not a power of 2.
     p = memalign(alignment - 1, size);
-    free(p);
+    assert(!p);
+    // Size is not a multiple of alignment.
+    p = aligned_alloc(alignment, size >> 1);
+    assert(!p);
+    p = (void *)0x42UL;
+    // Alignment is not a power of 2.
+    err = posix_memalign(&p, 3, size);
+    assert(!p);
+    assert(err == EINVAL);
+    p = (void *)0x42UL;
+    // Alignment is a power of 2, but not a multiple of size(void *).
+    err = posix_memalign(&p, 2, size);
+    assert(!p);
+    assert(err == EINVAL);
   }
   return 0;
 }
-
-// CHECK: ERROR: alignment is not a power of 2