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

[Vitaly Buka via Phabricator via llvm-commits]

vitalybuka added inline comments.


================
Comment at: compiler-rt/lib/asan/asan_allocator.h:189
+template <class A32, class A64>
+class DoubleAllocator {
+  A32 a32;
----------------
I see 3 copies of this template.
We need to extract that into separate header in sanitizer_common/

In shared extracted version I'd like to avoid using 32/64 in naming, just something generic first/second 1/2 ... etc.


================
Comment at: compiler-rt/lib/asan/asan_allocator.h:200
+    void Init(AllocatorGlobalStats *s) {
+      if (UseAllocator32)
+        a32.Init(s);
----------------
we need to make UseAllocator32 as a static member of DoubleAllocator as we want to make difference instantiations if the template have own copy of this var.


================
Comment at: compiler-rt/lib/sanitizer_common/tests/sanitizer_allocator_test.cc:289
 TEST(SanitizerCommon, SizeClassAllocator32Compact) {
   TestSizeClassAllocator<Allocator32Compact>();
 }
----------------
There are other tests which use directly or indirectly SizeClassAllocator32/SizeClassAllocator64.
they all should work with DoubleAllocator.

Could you please define Allocator32or64Compact in the top of the file and extend other tests?

```
using Allocator32or64Compact = DoubleAllocator<Allocator32Compact, Allocator64Compact>;

...

TEST(SanitizerCommon, SizeClassAllocator32or64Compact) {
  Allocator32or64Compact::use1 = false;
  TestSizeClassAllocator<Allocator32or64Compact>();
  Allocator32or64Compact::use1 = true;
  TestSizeClassAllocator<Allocator32or64Compact>();
}
```


================
Comment at: compiler-rt/lib/sanitizer_common/tests/sanitizer_allocator_test.cc:802
+
+TEST(SanitizerCommon, CombinedDoubleAllocator) {
+  TestCombinedAllocator<
----------------
we want to test shared template not the one we define here.


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

https://reviews.llvm.org/D60243