[PATCH] D35275: [Sanitizers] ASan/MSan/LSan allocators set errno on failure.

[Vitaly Buka via Phabricator via llvm-commits]

vitalybuka added inline comments.


================
Comment at: lib/msan/msan_allocator.cc:269
+void *msan_aligned_alloc(uptr alignment, uptr size, StackTrace *stack) {
+  if (UNLIKELY(!IsPowerOfTwo(alignment) || (size & (alignment - 1)) != 0)) {
+    errno = errno_EINVAL;
----------------
morehouse wrote:
> vitalybuka wrote:
> > why not just (size % alignmet == 0) and ignore "power of two" check?
> From memalign and aligned_alloc man page:
> > The  obsolete  function  memalign()  allocates size bytes and returns a
> > pointer to the allocated memory.  The memory address will be a multiple
> > of alignment, which must be a power of two.
> 
> > The  function aligned_alloc() is the same as memalign(), except for the
> > added restriction that size should be a multiple of alignment.
power of two is posix specific requirement
http://en.cppreference.com/w/c/memory/aligned_alloc

Anyway  (size & (alignment - 1)) != 0) can be replaced with simple (size % alignmet)


================
Comment at: lib/msan/msan_allocator.cc:287
+  if (UNLIKELY(!IsPowerOfTwo(alignment) ||
+               (alignment % sizeof(void *)) != 0)) {  // NOLINT
+    Allocator::FailureHandler::OnBadRequest();
----------------
morehouse wrote:
> vitalybuka wrote:
> > maybe just, pointer should be always power of two?
> > if (UNLIKELY((alignment % sizeof(void *)))) { 
> > 
> > 
> From the posix_memalign man page:
> 
> > The function posix_memalign()  allocates  size  bytes  and  places  the
> > address  of  the allocated memory in *memptr.  The address of the allo‐
> > cated memory will be a multiple of alignment, which must be a power  of
> > two  and  a  multiple  of sizeof(void *).
> 
> 
yes, but  sizeof(void *) is already power of two


https://reviews.llvm.org/D35275