[PATCH] D123822: [compiler-rt][lsan] Share platform allocator settings between ASan and LSan

[Vitaly Buka via Phabricator via llvm-commits]

vitalybuka added a comment.

In D123822#3534734 <https://reviews.llvm.org/D123822#3534734>, @leonardchan wrote:

> *ping* any more comments?

No objections, but some small concerns that it will be annoying to split them again if we need different setups for these sanitizers.



================
Comment at: compiler-rt/lib/asan/asan_allocator.h:129-142
-# elif defined(__aarch64__) && SANITIZER_ANDROID
-// Android needs to support 39, 42 and 48 bit VMA.
-const uptr kAllocatorSpace =  ~(uptr)0;
-const uptr kAllocatorSize  =  0x2000000000ULL;  // 128G.
-typedef VeryCompactSizeClassMap SizeClassMap;
-#elif SANITIZER_RISCV64
-const uptr kAllocatorSpace = ~(uptr)0;
----------------
leonardchan wrote:
> vitalybuka wrote:
> > i suspect these smaller values has something to do with ability allocate shadow, which is not an issue for lsan
> Do you recommend I should remove them?
we can't just remove them for asan, because there probably was a reason
but now, with shared config, it's behavior change for lsan


================
Comment at: compiler-rt/lib/lsan/lsan_common.h:80
+typedef VeryDenseSizeClassMap SizeClassMap;
+# elif defined(__aarch64__)
+// AArch64/SANITIZER_CAN_USE_ALLOCATOR64 is only for 42-bit VMA
----------------
SANITIZER_ARM64 and the rest?


================
Comment at: compiler-rt/lib/sanitizer_common/sanitizer_platform.h:252-257
 #ifndef SANITIZER_CAN_USE_ALLOCATOR64
-#  if (SANITIZER_ANDROID && defined(__aarch64__)) || SANITIZER_FUCHSIA
+#  if (SANITIZER_ANDROID && defined(__aarch64__)) || SANITIZER_FUCHSIA || \
+      defined(__aarch64__)
 #    define SANITIZER_CAN_USE_ALLOCATOR64 1
-#  elif defined(__mips64) || defined(__aarch64__)
+#  elif defined(__mips64) || defined(__i386__) || defined(__arm__) || \
+      SANITIZER_RISCV64 || defined(__hexagon__)
----------------
this part looks a little bit unrelated, would it be better to have a separate patch?

Can you swticth to  stuff like SANITIZER_ARM?

Also maybe it's better to have condition SANITIZER_WORDSIZE != 64 || the rest?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123822