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

[Diana Picus via llvm-commits]

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