[HWASAN][LSAN] Fix false positive memory leak reports on X86_64

[kstoimenov]

Summary

Before this patch when running HWASAN on x86_64 with memory tagging support we got a bunch of false memory leak reports. The reason for that is that the heuristic used to detect if an 8 bytes could be a user pointer was not valid when memory tagging is used as the top byte could contain non-zero information.

[vitalybuka]

compiler-rt/lib/lsan/lsan_common.cpp

279

we should query this from kernel, like in CanUseTaggingAbi

[vitalybuka]

compiler-rt/lib/lsan/lsan_common.cpp

271–273

I am confused.
This one masking out 55:48, not the top byte?

279

Maybe it's fine keep it simple and handle the worst case https://lwn.net/Articles/902094/

[vitalybuka]

compiler-rt/lib/lsan/lsan_common.cpp

271–273

Never mind, the point to distinguish user space, so we check top bits AFTER top byte.

279

I measured "return true" and on average it's 1.5% regressions.

So it's measureable, current 17 bit vs 8 bit, I can't measure a difference, but still think it's safe to keep as much as possible bits, 11 should work for us.

Please drop TODO that we will need to refactore this code for LAM48, or 5 level page tables.

[kstoimenov]

Added a test.

[kstoimenov]

Updated comment.

[kstoimenov]

Removed include.

[vitalybuka]

compiler-rt/lib/lsan/lsan_common.cpp

279

not done?

compiler-rt/test/lsan/TestCases/user_pointer.cpp

10–18

Maybe something more stronger

[kstoimenov]

Addressed comments.

[kstoimenov]

compiler-rt/test/lsan/TestCases/user_pointer.cpp

10–18

This code actually generates memory leaks because the allocated memory is not referenced by any pointer. I had a version where there are 2 arrays: one with the pointer and the other one with & ' p & (255ULL << 48)', but I am not sure how with would help. Probably the best is just to leave with one

[vitalybuka]

compiler-rt/test/lsan/TestCases/user_pointer.cpp

15

I see. So if we set masked bits, it will still fail other LSAN checks

so the test as is is not very useful, but not easy way change that.

[kstoimenov]

compiler-rt/test/lsan/TestCases/user_pointer.cpp

15

I will catch false positives. This test failed with HWASAN on x86 without my fix. Granted a bunch of other tests would have also failed, but it is good to have an explicit one.

compiler-rt/lib/lsan/lsan_common.cpp

 
 static inline bool MaybeUserPointer(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;
   if (p < kMinAddress)
     return false;
 #  if defined(__x86_64__)
-  // TODO: add logic similar to ARM when Intel LAM is available.
-  // Accept only canonical form user-space addresses.
-  return ((p >> 47) == 0);
+  // TODO: support LAM48 and 5 level page tables.
+  // LAM_U57 mask format
+  //  * top byte: 0x81 because the format is: [0] [6-bit tag] [0]
+  //  * top-1 byte: 0xff because it should be 0
+  //  * top-2 byte: 0x80 because Linux uses 128 TB VMA ending at 0x7fffffffffff
+  constexpr uptr kLAM_U57Mask = 0x81ff80;
+  constexpr uptr kPointerMask = kLAM_U57Mask << 40;
+  return ((p & kPointerMask) == 0);
 #  elif defined(__mips64)
   return ((p >> 40) == 0);
 #  elif defined(__aarch64__)
   // TBI (Top Byte Ignore) feature of AArch64: bits [63:56] are ignored in
   // address translation and can be used to store a tag.
   constexpr uptr kPointerMask = 255ULL << 48;
   // Accept up to 48 bit VMA.
   return ((p & kPointerMask) == 0);
 #  elif defined(__loongarch_lp64)
   // Allow 47-bit user-space VMA at current.
   return ((p >> 47) == 0);
 #  else
   return true;
 #  endif
 }
 
 // Scans the memory range, looking for byte patterns that point into allocator

compiler-rt/test/lsan/TestCases/user_pointer.cpp

+// Checks if a user pointer is found by the leak sanitizer.
+// RUN: %clang_lsan %s -o %t
+// RUN: %run %t 2>&1
+
+#include <cstdint>
+#include <stdio.h>
+#include <stdlib.h>
+
+uintptr_t glob[1024];
+
+int main() {
+  for (int i = 0; i < 1024; ++i) {
+    // Check that the pointers will not be falsely reported as leaks.
+    glob[i] = (uintptr_t)malloc(sizeof(int *));
+  }
+  return 0;
+}