[compiler-rt] r313294 - [scudo] Fix bad request handling when allocator has not been initialized
Diana Picus via llvm-commits
llvm-commits at lists.llvm.org
Wed Sep 27 17:13:08 PDT 2017
Ok, thanks!
On 27 September 2017 at 11:27, Kostya Kortchinsky <kostyak at google.com> wrote:
> 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/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
>>
>>
>
More information about the llvm-commits
mailing list