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

Wed Sep 27 11:27:45 PDT 2017

Sent out https://reviews.llvm.org/D38324 to temporary disable the test on
armhf.

On Wed, Sep 27, 2017 at 10:56 AM, Kostya Kortchinsky <kostyak at google.com>
wrote:

> 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/sc
>> udo/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/d148272d/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/d148272d/attachment.bin>