[scudo] Add C++17 aligned new/delete operators support

[cryptoad]

Summary

This CL adds support for aligned new/delete operators (C++17). Currently we
do not support alignment inconsistency detection on deallocation, as this
requires a header change, but the APIs are introduced and are functional.

Add a smoke test for the aligned version of the operators.

[alekseyshl]

lib/scudo/scudo_allocator.cpp

633

IsPowerOfTwo(0) is true, which is not correct, but you can use this feature to drop Alignment check.

We mostly use IsPowerOfTwo in places where we know x != 0, so no need to check for 0 and here we just don't care.

I just submitted D47924, which tightened alignment check for posix_memalign and aligned_alloc. I am curious whether it will break anyone, according to spec, aligned_alloc should not accept non-power-of-two alignments, although my current libc implementation happily accepts any value, 0 or not, power of two or not. posix_memalign returns EINVAL as expected. Anyway, it was just a side note.

lib/scudo/scudo_new_delete.cpp

38

I was thinking of merging OPERATOR_NEW_BODY/OPERATOR_NEW_BODY_ALIGN defines for other allocators, so maybe you can start the trend :)

#define OPERATOR_NEW_BODY_ALIGN(Type, Align, NoThrow) \
  ...
#define OPERATOR_NEW_BODY(Type, Align, NoThrow) \
  OPERATOR_NEW_BODY_ALIGN(Type, 0, NoThrow)

or pass the zero explicitly in the operators.

[cryptoad]

lib/scudo/scudo_allocator.cpp

633

Ack. I was sort of thinking that checking for 0 had the least performance cost since it would be the path taken the most. WDYT?

lib/scudo/scudo_new_delete.cpp

38

Ok will do. Are you doing it for delete as well?

[alekseyshl]

lib/scudo/scudo_allocator.cpp

633

Perf data and looking at the generated code would be the only way to pick the right way to do it.

lib/scudo/scudo_new_delete.cpp

38

delete does not worth it, they are all one liners. New worries me because there's quite a lot of duplicated logic there, especially for ASan.

[cryptoad]

OPERATOR_NEW_BODY now wraps OPERATOR_NEW_BODY_ALIGN.

[cryptoad]

lib/scudo/scudo_allocator.cpp

633

If that's ok with you I'd rather stick with checking for Alignment rather than expecting IsPowerOfTwo(0) to be true.

lib/scudo/scudo_allocator.h

 
 typedef SizeClassAllocatorLocalCache<PrimaryAllocator> AllocatorCache;
 typedef ScudoLargeMmapAllocator SecondaryAllocator;
 typedef ScudoCombinedAllocator<PrimaryAllocator, AllocatorCache,
     SecondaryAllocator> ScudoBackendAllocator;
 
 void initScudo();
 
-void *scudoMalloc(uptr Size, AllocType Type);
-void scudoFree(void *Ptr, AllocType Type);
-void scudoSizedFree(void *Ptr, uptr Size, AllocType Type);
+void *scudoAllocate(uptr Size, uptr Alignment, AllocType Type);
+void scudoDeallocate(void *Ptr, uptr Size, uptr Alignment, AllocType Type);
 void *scudoRealloc(void *Ptr, uptr Size);
 void *scudoCalloc(uptr NMemB, uptr Size);
-void *scudoMemalign(uptr Alignment, uptr Size);
 void *scudoValloc(uptr Size);
 void *scudoPvalloc(uptr Size);
 int scudoPosixMemalign(void **MemPtr, uptr Alignment, uptr Size);
 void *scudoAlignedAlloc(uptr Alignment, uptr Size);
 uptr scudoMallocUsableSize(void *Ptr);
 
 }  // namespace __scudo
 
 #endif  // SCUDO_ALLOCATOR_H_

lib/scudo/scudo_allocator.cpp

if (BypassQuarantine) {
                               EstimatedSize);
       if (UnlockRequired)
         TSD->unlock();
     }
   }
 
   // Deallocates a Chunk, which means either adding it to the quarantine or
   // directly returning it to the backend if criteria are met.
-  void deallocate(void *Ptr, uptr DeleteSize, AllocType Type) {
+  void deallocate(void *Ptr, uptr DeleteSize, uptr DeleteAlignment,
+                  AllocType Type) {
     // For a deallocation, we only ensure minimal initialization, meaning thread
     // local data will be left uninitialized for now (when using ELF TLS). The
     // fallback cache will be used instead. This is a workaround for a situation
     // where the only heap operation performed in a thread would be a free past
     // the TLS destructors, ending up in initialized thread specific data never
     // being destroyed properly. Any other heap operation will do a full init.
     initThreadMaybe(/*MinimalInit=*/true);
     if (SCUDO_CAN_USE_HOOKS && &__sanitizer_free_hook)
if (DeallocationTypeMismatch) {
       }
     }
     const uptr Size = Chunk::getSize(Ptr, &Header);
     if (DeleteSizeMismatch) {
       if (DeleteSize && DeleteSize != Size)
         dieWithMessage("invalid sized delete when deallocating address %p\n",
                        Ptr);
     }
+    (void)DeleteAlignment;  // TODO(kostyak): verify that the alignment matches.
     quarantineOrDeallocateChunk(Ptr, &Header, Size);
   }
 
   // Reallocates a chunk. We can save on a new allocation if the new requested
   // size still fits in the chunk.
   void *reallocate(void *OldPtr, uptr NewSize) {
     initThreadMaybe();
     if (UNLIKELY(!Chunk::isAligned(OldPtr)))
void ScudoTSD::init() {
   getBackendAllocator().initCache(&Cache);
   memset(QuarantineCachePlaceHolder, 0, sizeof(QuarantineCachePlaceHolder));
 }
 
 void ScudoTSD::commitBack() {
   Instance.commitBack(this);
 }
 
-void *scudoMalloc(uptr Size, AllocType Type) {
-  return SetErrnoOnNull(Instance.allocate(Size, MinAlignment, Type));
+void *scudoAllocate(uptr Size, uptr Alignment, AllocType Type) {
+  if (Alignment && UNLIKELY(!IsPowerOfTwo(Alignment))) {
+    errno = EINVAL;
+    return Instance.handleBadRequest();
   }
-
-void scudoFree(void *Ptr, AllocType Type) {
-  Instance.deallocate(Ptr, 0, Type);
+  return SetErrnoOnNull(Instance.allocate(Size, Alignment, Type));
 }
 
-void scudoSizedFree(void *Ptr, uptr Size, AllocType Type) {
-  Instance.deallocate(Ptr, Size, Type);
+void scudoDeallocate(void *Ptr, uptr Size, uptr Alignment, AllocType Type) {
+  Instance.deallocate(Ptr, Size, Alignment, Type);
 }
 
 void *scudoRealloc(void *Ptr, uptr Size) {
   if (!Ptr)
     return SetErrnoOnNull(Instance.allocate(Size, MinAlignment, FromMalloc));
   if (Size == 0) {
-    Instance.deallocate(Ptr, 0, FromMalloc);
+    Instance.deallocate(Ptr, 0, 0, FromMalloc);
     return nullptr;
   }
   return SetErrnoOnNull(Instance.reallocate(Ptr, Size));
 }
 
 void *scudoCalloc(uptr NMemB, uptr Size) {
   return SetErrnoOnNull(Instance.calloc(NMemB, Size));
 }
if (UNLIKELY(CheckForPvallocOverflow(Size, PageSize))) {
     errno = ENOMEM;
     return Instance.handleBadRequest();
   }
   // pvalloc(0) should allocate one page.
   Size = Size ? RoundUpTo(Size, PageSize) : PageSize;
   return SetErrnoOnNull(Instance.allocate(Size, PageSize, FromMemalign));
 }
 
-void *scudoMemalign(uptr Alignment, uptr Size) {
-  if (UNLIKELY(!IsPowerOfTwo(Alignment))) {
-    errno = EINVAL;
-    return Instance.handleBadRequest();
-  }
-  return SetErrnoOnNull(Instance.allocate(Size, Alignment, FromMemalign));
-}
-
 int scudoPosixMemalign(void **MemPtr, uptr Alignment, uptr Size) {
   if (UNLIKELY(!CheckPosixMemalignAlignment(Alignment))) {
     Instance.handleBadRequest();
     return EINVAL;
   }
   void *Ptr = Instance.allocate(Size, Alignment, FromMemalign);
   if (UNLIKELY(!Ptr))
     return ENOMEM;

lib/scudo/scudo_malloc.cpp

 
 #include "interception/interception.h"
 #include "sanitizer_common/sanitizer_platform_interceptors.h"
 
 using namespace __scudo;
 
 extern "C" {
 INTERCEPTOR_ATTRIBUTE void free(void *ptr) {
-  scudoFree(ptr, FromMalloc);
+  scudoDeallocate(ptr, 0, 0, FromMalloc);
 }
 
 INTERCEPTOR_ATTRIBUTE void *malloc(SIZE_T size) {
-  return scudoMalloc(size, FromMalloc);
+  return scudoAllocate(size, 0, FromMalloc);
 }
 
 INTERCEPTOR_ATTRIBUTE void *realloc(void *ptr, SIZE_T size) {
   return scudoRealloc(ptr, size);
 }
 
 INTERCEPTOR_ATTRIBUTE void *calloc(SIZE_T nmemb, SIZE_T size) {
   return scudoCalloc(nmemb, size);

 }
 
 #if SANITIZER_INTERCEPT_CFREE
 INTERCEPTOR_ATTRIBUTE void cfree(void *ptr) ALIAS("free");
 #endif
 
 #if SANITIZER_INTERCEPT_MEMALIGN
 INTERCEPTOR_ATTRIBUTE void *memalign(SIZE_T alignment, SIZE_T size) {
-  return scudoMemalign(alignment, size);
+  return scudoAllocate(size, alignment, FromMemalign);
 }
 
 INTERCEPTOR_ATTRIBUTE
 void *__libc_memalign(SIZE_T alignment, SIZE_T size) ALIAS("memalign");
 #endif
 
 #if SANITIZER_INTERCEPT_PVALLOC
 INTERCEPTOR_ATTRIBUTE void *pvalloc(SIZE_T size) {

lib/scudo/scudo_new_delete.cpp

 
 using namespace __scudo;
 
 #define CXX_OPERATOR_ATTRIBUTE INTERCEPTOR_ATTRIBUTE
 
 // Fake std::nothrow_t to avoid including <new>.
 namespace std {
 struct nothrow_t {};
+enum class align_val_t: size_t {};
 }  // namespace std
 
 // TODO(alekseys): throw std::bad_alloc instead of dying on OOM.
+#define OPERATOR_NEW_BODY(Type, NoThrow)                 \
+  void *Ptr = scudoAllocate(size, 0, Type);              \
+  if (!NoThrow && UNLIKELY(!Ptr)) DieOnFailure::OnOOM(); \
+  return Ptr;
+#define OPERATOR_NEW_BODY_ALIGN(Type, NoThrow)                     \
+  void *Ptr = scudoAllocate(size, static_cast<uptr>(align), Type); \
+  if (!NoThrow && UNLIKELY(!Ptr)) DieOnFailure::OnOOM();           \
+  return Ptr;
+
+CXX_OPERATOR_ATTRIBUTE
+void *operator new(size_t size)
+{ OPERATOR_NEW_BODY(FromNew, /*NoThrow=*/false); }
+CXX_OPERATOR_ATTRIBUTE
+void *operator new[](size_t size)
+{ OPERATOR_NEW_BODY(FromNewArray, /*NoThrow=*/false); }
+CXX_OPERATOR_ATTRIBUTE
+void *operator new(size_t size, std::nothrow_t const&)
+{ OPERATOR_NEW_BODY(FromNew, /*NoThrow=*/true); }
+CXX_OPERATOR_ATTRIBUTE
+void *operator new[](size_t size, std::nothrow_t const&)
+{ OPERATOR_NEW_BODY(FromNewArray, /*NoThrow=*/true); }
+CXX_OPERATOR_ATTRIBUTE
+void *operator new(size_t size, std::align_val_t align)
+{ OPERATOR_NEW_BODY_ALIGN(FromNew, /*NoThrow=*/false); }
+CXX_OPERATOR_ATTRIBUTE
+void *operator new[](size_t size, std::align_val_t align)
+{ OPERATOR_NEW_BODY_ALIGN(FromNewArray, /*NoThrow=*/false); }
+CXX_OPERATOR_ATTRIBUTE
+void *operator new(size_t size, std::align_val_t align, std::nothrow_t const&)
+{ OPERATOR_NEW_BODY_ALIGN(FromNew, /*NoThrow=*/true); }
+CXX_OPERATOR_ATTRIBUTE
+void *operator new[](size_t size, std::align_val_t align, std::nothrow_t const&)
+{ OPERATOR_NEW_BODY_ALIGN(FromNewArray, /*NoThrow=*/true); }
+
+#define OPERATOR_DELETE_BODY(Type) \
+  scudoDeallocate(ptr, 0, 0, Type);
+#define OPERATOR_DELETE_BODY_SIZE(Type) \
+  scudoDeallocate(ptr, size, 0, Type);
+#define OPERATOR_DELETE_BODY_ALIGN(Type) \
+  scudoDeallocate(ptr, 0, static_cast<uptr>(align), Type);
+#define OPERATOR_DELETE_BODY_SIZE_ALIGN(Type) \
+  scudoDeallocate(ptr, size, static_cast<uptr>(align), Type);
+
+CXX_OPERATOR_ATTRIBUTE
+void operator delete(void *ptr) NOEXCEPT
+{ OPERATOR_DELETE_BODY(FromNew); }
+CXX_OPERATOR_ATTRIBUTE
+void operator delete[](void *ptr) NOEXCEPT
+{ OPERATOR_DELETE_BODY(FromNewArray); }
+CXX_OPERATOR_ATTRIBUTE
+void operator delete(void *ptr, std::nothrow_t const&)
+{ OPERATOR_DELETE_BODY(FromNew); }
+CXX_OPERATOR_ATTRIBUTE
+void operator delete[](void *ptr, std::nothrow_t const&)
+{ OPERATOR_DELETE_BODY(FromNewArray); }
+CXX_OPERATOR_ATTRIBUTE
+void operator delete(void *ptr, size_t size) NOEXCEPT
+{ OPERATOR_DELETE_BODY_SIZE(FromNew); }
+CXX_OPERATOR_ATTRIBUTE
+void operator delete[](void *ptr, size_t size) NOEXCEPT
+{ OPERATOR_DELETE_BODY_SIZE(FromNewArray); }
+CXX_OPERATOR_ATTRIBUTE
+void operator delete(void *ptr, std::align_val_t align) NOEXCEPT
+{ OPERATOR_DELETE_BODY_ALIGN(FromNew); }
+CXX_OPERATOR_ATTRIBUTE
+void operator delete[](void *ptr, std::align_val_t align) NOEXCEPT
+{ OPERATOR_DELETE_BODY_ALIGN(FromNewArray); }
+CXX_OPERATOR_ATTRIBUTE
+void operator delete(void *ptr, std::align_val_t align, std::nothrow_t const&)
+{ OPERATOR_DELETE_BODY_ALIGN(FromNew); }
+CXX_OPERATOR_ATTRIBUTE
+void operator delete[](void *ptr, std::align_val_t align, std::nothrow_t const&)
+{ OPERATOR_DELETE_BODY_ALIGN(FromNewArray); }
+CXX_OPERATOR_ATTRIBUTE
+void operator delete(void *ptr, size_t size, std::align_val_t align) NOEXCEPT
+{ OPERATOR_DELETE_BODY_SIZE_ALIGN(FromNew); }
 CXX_OPERATOR_ATTRIBUTE
-void *operator new(size_t size) {
-  void *res = scudoMalloc(size, FromNew);
-  if (UNLIKELY(!res)) DieOnFailure::OnOOM();
-  return res;
-}
-CXX_OPERATOR_ATTRIBUTE
-void *operator new[](size_t size) {
-  void *res = scudoMalloc(size, FromNewArray);
-  if (UNLIKELY(!res)) DieOnFailure::OnOOM();
-  return res;
-}
-CXX_OPERATOR_ATTRIBUTE
-void *operator new(size_t size, std::nothrow_t const&) {
-  return scudoMalloc(size, FromNew);
-}
-CXX_OPERATOR_ATTRIBUTE
-void *operator new[](size_t size, std::nothrow_t const&) {
-  return scudoMalloc(size, FromNewArray);
-}
-
-CXX_OPERATOR_ATTRIBUTE
-void operator delete(void *ptr) NOEXCEPT {
-  return scudoFree(ptr, FromNew);
-}
-CXX_OPERATOR_ATTRIBUTE
-void operator delete[](void *ptr) NOEXCEPT {
-  return scudoFree(ptr, FromNewArray);
-}
-CXX_OPERATOR_ATTRIBUTE
-void operator delete(void *ptr, std::nothrow_t const&) NOEXCEPT {
-  return scudoFree(ptr, FromNew);
-}
-CXX_OPERATOR_ATTRIBUTE
-void operator delete[](void *ptr, std::nothrow_t const&) NOEXCEPT {
-  return scudoFree(ptr, FromNewArray);
-}
-CXX_OPERATOR_ATTRIBUTE
-void operator delete(void *ptr, size_t size) NOEXCEPT {
-  scudoSizedFree(ptr, size, FromNew);
-}
-CXX_OPERATOR_ATTRIBUTE
-void operator delete[](void *ptr, size_t size) NOEXCEPT {
-  scudoSizedFree(ptr, size, FromNewArray);
-}
+void operator delete[](void *ptr, size_t size, std::align_val_t align) NOEXCEPT
+{ OPERATOR_DELETE_BODY_SIZE_ALIGN(FromNewArray); }

test/scudo/aligned-new.cpp

+// RUN: %clangxx_scudo -std=c++1z -faligned-allocation %s -o %t
+// RUN:                                             %run %t valid   2>&1
+// RUN: %env_scudo_opts=allocator_may_return_null=1 %run %t invalid 2>&1
+
+// Tests that the C++17 aligned new/delete operators are working as expected.
+// Currently we do not check the consistency of the alignment on deallocation,
+// so this just tests that the APIs work.
+
+#include <assert.h>
+#include <stdio.h>
+#include <stdint.h>
+#include <string.h>
+
+// Define all new/delete to not depend on the version provided by the platform.
+
+namespace std {
+struct nothrow_t {};
+static const nothrow_t nothrow;
+enum class align_val_t : size_t {};
+}  // namespace std
+
+void *operator new(size_t);
+void *operator new[](size_t);
+void *operator new(size_t, std::nothrow_t const&);
+void *operator new[](size_t, std::nothrow_t const&);
+void *operator new(size_t, std::align_val_t);
+void *operator new[](size_t, std::align_val_t);
+void *operator new(size_t, std::align_val_t, std::nothrow_t const&);
+void *operator new[](size_t, std::align_val_t, std::nothrow_t const&);
+
+void operator delete(void*) throw();
+void operator delete[](void*) throw();
+void operator delete(void*, std::nothrow_t const&);
+void operator delete[](void*, std::nothrow_t const&);
+void operator delete(void*, size_t) throw();
+void operator delete[](void*, size_t) throw();
+void operator delete(void*, std::align_val_t) throw();
+void operator delete[](void*, std::align_val_t) throw();
+void operator delete(void*, std::align_val_t, std::nothrow_t const&);
+void operator delete[](void*, std::align_val_t, std::nothrow_t const&);
+void operator delete(void*, size_t, std::align_val_t) throw();
+void operator delete[](void*, size_t, std::align_val_t) throw();
+
+template<typename T>
+inline T* break_optimization(T *arg) {
+  __asm__ __volatile__("" : : "r" (arg) : "memory");
+  return arg;
+}
+
+struct S12 { int a, b, c; };
+struct alignas(128) S12_128 { int a, b, c; };
+struct alignas(256) S12_256 { int a, b, c; };
+struct alignas(512) S1024_512 { char a[1024]; };
+struct alignas(1024) S1024_1024 { char a[1024]; };
+
+int main(int argc, char **argv) {
+  assert(argc == 2);
+
+  if (!strcmp(argv[1], "valid")) {
+    // Standard use case.
+    delete break_optimization(new S12);
+    delete break_optimization(new S12_128);
+    delete[] break_optimization(new S12_128[4]);
+    delete break_optimization(new S12_256);
+    delete break_optimization(new S1024_512);
+    delete[] break_optimization(new S1024_512[4]);
+    delete break_optimization(new S1024_1024);
+
+    // Call directly the aligned versions of the operators.
+    const size_t alignment = 1U << 8;
+    void *p = operator new(1, static_cast<std::align_val_t>(alignment));
+    assert((reinterpret_cast<uintptr_t>(p) & (alignment - 1)) == 0);
+    operator delete(p, static_cast<std::align_val_t>(alignment));
+  }
+  if (!strcmp(argv[1], "invalid")) {
+    // Alignment must be a power of 2.
+    const size_t alignment = (1U << 8) - 1;
+    void *p = operator new(1, static_cast<std::align_val_t>(alignment),
+                           std::nothrow);
+    assert(!p);
+  }
+
+  return 0;
+}