[PATCH] D35275: [Sanitizers] MSan allocator set errno on failure.
Vitaly Buka via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Jul 14 19:29:21 PDT 2017
vitalybuka accepted this revision.
vitalybuka added inline comments.
This revision is now accepted and ready to land.
================
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;
----------------
alekseyshl wrote:
> alekseyshl wrote:
> > vitalybuka wrote:
> > > 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)
> > My man memalign says exactly as quoted earlier, the power of two requirement affects both memalign and aligned_alloc.
> >
> >
> (size & (alignment - 1)) != 0) leads to appreciably more compact and faster assembly generated than (size % alignment), because div required for the latter touches a lot of registers, which forces compiler to push other vars around.
>
> The difference is 0x41 vs 0x51 bytes of the function size.
then why msan_posix_memalign uses % when you do the same?
================
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;
----------------
vitalybuka wrote:
> alekseyshl wrote:
> > alekseyshl wrote:
> > > vitalybuka wrote:
> > > > 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)
> > > My man memalign says exactly as quoted earlier, the power of two requirement affects both memalign and aligned_alloc.
> > >
> > >
> > (size & (alignment - 1)) != 0) leads to appreciably more compact and faster assembly generated than (size % alignment), because div required for the latter touches a lot of registers, which forces compiler to push other vars around.
> >
> > The difference is 0x41 vs 0x51 bytes of the function size.
> then why msan_posix_memalign uses % when you do the same?
your man is for posix
================
Comment at: lib/msan/msan_allocator.cc:231
+inline void *check_ptr(void *ptr) {
+ if (UNLIKELY(!ptr))
----------------
maybe move to some common header?
https://reviews.llvm.org/D35275
More information about the llvm-commits
mailing list