[PATCH] D60243: [LSan][AArch64] Speed-up leak and address sanitizers on AArch64 for 47-bit VMA

[Brian Rzycki via Phabricator via llvm-commits]

brzycki added a comment.

Hello @vitalybuka , I have uploaded a WIP diff in D61766 <https://reviews.llvm.org/D61766> and I appreciate any insight you can give.

First, this approach still requires `ifdefs` for `__aarch64__`. The problem is I cannot use `SizeClassAllocatorPair` for arches where there is only one Allocator. This unfortunately prevents the request of using this approach to build this code even on platforms that do not need to select an allocator at runtime. This goes against the original intent of what you and @kcc asked for after the initial patch review.

Second, there are compile-time errors due to issues with replacing  `SizeClassAllocatorXX` with the new `SizeClassAllocatorPair` class on Aaarch64. It's not a 1:1 drop-in replacement and in some cases I don't know what the correct answer is. For example, here's an LLVM error when building `lsan_allocator.cc`:

  In file included from /home/cc/bmr/llvm-project/llvm/projects/compiler-rt/lib/lsan/lsan_allocator.cc:14:
  In file included from /home/cc/bmr/llvm-project/llvm/projects/compiler-rt/lib/lsan/lsan_allocator.h:17:
  In file included from /home/cc/bmr/llvm-project/llvm/projects/compiler-rt/lib/lsan/../sanitizer_common/sanitzer_allocator.h:77:
  /home/cc/bmr/llvm-project/llvm/projects/compiler-rt/lib/lsan/../sanitizer_common/sanitizer_allocator_combine.h:28:53: error: no type named 'MapUnmapCallback' in '__sanitizer::SizeClassAllocatorPair<__lsan::AP64<__santizer::LocalAddressSpaceView>, __lsan::AP32<__sanitizer::LocalAddressSpaceView> >'
        LargeMmapAllocator<typename PrimaryAllocator::MapUnmapCallback,
                           ~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~
  /home/cc/bmr/llvm-project/llvm/projects/compiler-rt/lib/lsan/lsan_allocator.h:122:24: note: in instantiationof template class '__sanitizer::CombinedAllocator<__sanitizer::SizeClassAllocatorPair<__lsan::AP64<__sanitizr::LocalAddressSpaceView>, __lsan::AP32<__sanitizer::LocalAddressSpaceView> >, __sanitizer::LargeMmapAllocatrPtrArrayDynamic>' requested here
  using AllocatorCache = Allocator::AllocatorCache;
                         ^

In `SizeClassAllocatorXX` the `MapUnmapCallback` is passed in as a `typedef` inside one of the two `APXX` structs. The `SizeClassAllocatorPair` class cannot select one of the two `MapUnmapCallback` `A1` or `A2` template classes until the runtime memory check determines the size of the VMA. But by then it's too late: we need to instantiate a `CombinedAllocator` class in order to create `AllocatorCache` in `lsan_allocator.h`. We are back to this needing to be a compile-time property for another dependent class.  I could always select `A1`'s typedef of `MapUnmapCallback`, similar to what you do with `PrimaryAllocator::MapUnmapCallback` but I am uncertain if this is correct.

Please let me know if I  misunderstood your testcase in D61401 <https://reviews.llvm.org/D61401> or if I'm inserting the templated class in the wrong location.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60243/new/

https://reviews.llvm.org/D60243