[PATCH] D123814: [compiler-rt][lsan] Update CanBeAHeapPointer for AArch64

[Leonard Chan via Phabricator via llvm-commits]

leonardchan created this revision.
leonardchan added reviewers: mcgrathr, phosek, vitalybuka.
leonardchan added a project: Sanitizers.
Herald added subscribers: kristof.beyls, dberris.
Herald added a project: All.
leonardchan requested review of this revision.
Herald added a subscriber: Sanitizers.

While attempting to get the 64-bit lsan allocator working for Fuchsia, I noticed this function would incorrectly return false for pointers returned by the 64-bit allocator. On AArch64, this function attempts to get the VMA size dynamically by counting the number of leading zeros from the function frame address. This will fail if the frame address is significantly below an allocated pointer (that is, the frame address has more leading zeros than an allocated pointer). This is possible on Fuchsia and linux (when not called from the initial thread stack).

It seems the intended use of this function is to speed up pointer scanning by filtering out addresses that user code might not be able to access. Other platforms this check is done on seem to hardcode the VMA size/shift, so it seems appropriate to do this for aarch64 as well. This implies pointers on aarch64 where the VMA size is <64 will pass through, but bad pointers will still be caught by subsequent scan checks.

This patch also renames the function to something more fitting of what it's trying to do.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D123814

Files:
  compiler-rt/lib/lsan/lsan_common.cpp


Index: compiler-rt/lib/lsan/lsan_common.cpp
===================================================================
--- compiler-rt/lib/lsan/lsan_common.cpp
+++ compiler-rt/lib/lsan/lsan_common.cpp
@@ -240,7 +240,7 @@
   const char *Leak() { return Blue(); }
 };
 
-static inline bool CanBeAHeapPointer(uptr p) {
+static inline bool CouldBeUserPointer(uptr p) {
   // Since our heap is located in mmap-ed memory, we can assume a sensible lower
   // bound on heap addresses.
   const uptr kMinAddress = 4 * 4096;
@@ -252,8 +252,8 @@
 #  elif defined(__mips64)
   return ((p >> 40) == 0);
 #  elif defined(__aarch64__)
-  unsigned runtimeVMA = (MostSignificantSetBitIndex(GET_CURRENT_FRAME()) + 1);
-  return ((p >> runtimeVMA) == 0);
+  // Accept up to 48 bit VMA.
+  return ((p >> 48) == 0);
 #  else
   return true;
 #  endif
@@ -276,7 +276,7 @@
     pp = pp + alignment - pp % alignment;
   for (; pp + sizeof(void *) <= end; pp += alignment) {
     void *p = *reinterpret_cast<void **>(pp);
-    if (!CanBeAHeapPointer(reinterpret_cast<uptr>(p)))
+    if (!CouldBeUserPointer(reinterpret_cast<uptr>(p)))
       continue;
     uptr chunk = PointsIntoChunk(p);
     if (!chunk)


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D123814.422933.patch
Type: text/x-patch
Size: 1176 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220414/d0e5e6e5/attachment.bin>