[compiler-rt] r313294 - [scudo] Fix bad request handling when allocator has not been initialized

Kostya Kortchinsky via llvm-commits llvm-commits at lists.llvm.org
Wed Sep 27 10:56:27 PDT 2017


I didn't notice that before, looking into it thanks!

On Wed, Sep 27, 2017 at 10:54 AM, Diana Picus <diana.picus at linaro.org>
wrote:

> Hi Kostya,
>
> The valloc test doesn't seem to work nicely on ARM:
> http://lab.llvm.org:8011/builders/clang-cmake-thumbv7-
> a15-full-sh/builds/2398
> http://lab.llvm.org:8011/builders/clang-cmake-armv7-a15-full/builds/10933
>
> Any idea what the problem might be?
>
> Thanks,
> Diana
>
> PS: Sorry I didn't notice earlier, I was on vacation.
>
> On 14 September 2017 at 13:34, Kostya Kortchinsky via llvm-commits
> <llvm-commits at lists.llvm.org> wrote:
> > Author: cryptoad
> > Date: Thu Sep 14 13:34:32 2017
> > New Revision: 313294
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=313294&view=rev
> > Log:
> > [scudo] Fix bad request handling when allocator has not been initialized
> >
> > Summary:
> > In a few functions (`scudoMemalign` and the like), we would call
> > `ScudoAllocator::FailureHandler::OnBadRequest` if the parameters didn't
> check
> > out. The issue is that if the allocator had not been initialized (eg: if
> this
> > is the first heap related function called), we would use variables like
> > `allocator_may_return_null` and `exitcode` that still had their default
> value
> > (as opposed to the one set by the user or the initialization path).
> >
> > To solve this, we introduce `handleBadRequest` that will call
> `initThreadMaybe`,
> > allowing the options to be correctly initialized.
> >
> > Unfortunately, the tests were passing because `exitcode` was still 0, so
> the
> > results looked like success. Change those tests to do what they were
> supposed
> > to.
> >
> > Reviewers: alekseyshl
> >
> > Reviewed By: alekseyshl
> >
> > Subscribers: llvm-commits
> >
> > Differential Revision: https://reviews.llvm.org/D37853
> >
> > Modified:
> >     compiler-rt/trunk/lib/scudo/scudo_allocator.cpp
> >     compiler-rt/trunk/test/scudo/memalign.cpp
> >     compiler-rt/trunk/test/scudo/valloc.cpp
> >
> > Modified: compiler-rt/trunk/lib/scudo/scudo_allocator.cpp
> > URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/
> scudo/scudo_allocator.cpp?rev=313294&r1=313293&r2=313294&view=diff
> > ============================================================
> ==================
> > --- compiler-rt/trunk/lib/scudo/scudo_allocator.cpp (original)
> > +++ compiler-rt/trunk/lib/scudo/scudo_allocator.cpp Thu Sep 14 13:34:32
> 2017
> > @@ -620,6 +620,11 @@ struct ScudoAllocator {
> >      BackendAllocator.getStats(stats);
> >      return stats[StatType];
> >    }
> > +
> > +  void *handleBadRequest() {
> > +    initThreadMaybe();
> > +    return FailureHandler::OnBadRequest();
> > +  }
> >  };
> >
> >  static ScudoAllocator Instance(LINKER_INITIALIZED);
> > @@ -677,7 +682,7 @@ void *scudoPvalloc(uptr Size) {
> >    uptr PageSize = GetPageSizeCached();
> >    if (UNLIKELY(CheckForPvallocOverflow(Size, PageSize))) {
> >      errno = errno_ENOMEM;
> > -    return ScudoAllocator::FailureHandler::OnBadRequest();
> > +    return Instance.handleBadRequest();
> >    }
> >    // pvalloc(0) should allocate one page.
> >    Size = Size ? RoundUpTo(Size, PageSize) : PageSize;
> > @@ -687,14 +692,14 @@ void *scudoPvalloc(uptr Size) {
> >  void *scudoMemalign(uptr Alignment, uptr Size) {
> >    if (UNLIKELY(!IsPowerOfTwo(Alignment))) {
> >      errno = errno_EINVAL;
> > -    return ScudoAllocator::FailureHandler::OnBadRequest();
> > +    return Instance.handleBadRequest();
> >    }
> >    return SetErrnoOnNull(Instance.allocate(Size, Alignment,
> FromMemalign));
> >  }
> >
> >  int scudoPosixMemalign(void **MemPtr, uptr Alignment, uptr Size) {
> >    if (UNLIKELY(!CheckPosixMemalignAlignment(Alignment))) {
> > -    ScudoAllocator::FailureHandler::OnBadRequest();
> > +    Instance.handleBadRequest();
> >      return errno_EINVAL;
> >    }
> >    void *Ptr = Instance.allocate(Size, Alignment, FromMemalign);
> > @@ -707,7 +712,7 @@ int scudoPosixMemalign(void **MemPtr, up
> >  void *scudoAlignedAlloc(uptr Alignment, uptr Size) {
> >    if (UNLIKELY(!CheckAlignedAllocAlignmentAndSize(Alignment, Size))) {
> >      errno = errno_EINVAL;
> > -    return ScudoAllocator::FailureHandler::OnBadRequest();
> > +    return Instance.handleBadRequest();
> >    }
> >    return SetErrnoOnNull(Instance.allocate(Size, Alignment,
> FromMalloc));
> >  }
> >
> > Modified: compiler-rt/trunk/test/scudo/memalign.cpp
> > URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/
> test/scudo/memalign.cpp?rev=313294&r1=313293&r2=313294&view=diff
> > ============================================================
> ==================
> > --- compiler-rt/trunk/test/scudo/memalign.cpp (original)
> > +++ compiler-rt/trunk/test/scudo/memalign.cpp Thu Sep 14 13:34:32 2017
> > @@ -1,6 +1,7 @@
> >  // RUN: %clang_scudo %s -o %t
> > -// RUN: %run %t valid   2>&1
> > -// RUN: %run %t invalid 2>&1
> > +// RUN:                                               %run %t valid
>  2>&1
> > +// RUN:                                           not %run %t invalid
> 2>&1
> > +// RUN: SCUDO_OPTIONS=allocator_may_return_null=1     %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.
> >
> > Modified: compiler-rt/trunk/test/scudo/valloc.cpp
> > URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/
> test/scudo/valloc.cpp?rev=313294&r1=313293&r2=313294&view=diff
> > ============================================================
> ==================
> > --- compiler-rt/trunk/test/scudo/valloc.cpp (original)
> > +++ compiler-rt/trunk/test/scudo/valloc.cpp Thu Sep 14 13:34:32 2017
> > @@ -1,6 +1,7 @@
> >  // RUN: %clang_scudo %s -o %t
> > -// RUN: %run %t valid   2>&1
> > -// RUN: %run %t invalid 2>&1
> > +// RUN:                                               %run %t valid
>  2>&1
> > +// RUN:                                           not %run %t invalid
> 2>&1
> > +// RUN: SCUDO_OPTIONS=allocator_may_return_null=1     %run %t invalid
> 2>&1
> >
> >  // Tests that valloc and pvalloc work as intended.
> >
> >
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170927/c773ffbb/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 4845 bytes
Desc: S/MIME Cryptographic Signature
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170927/c773ffbb/attachment.bin>


More information about the llvm-commits mailing list