[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

276

we should query this from kernel, like in CanUseTaggingAbi

[vitalybuka]

compiler-rt/lib/lsan/lsan_common.cpp

275–277

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

276

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

275–277

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

276

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

276

not done?

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

9–17 ↗

(On Diff #541254)

Maybe something more stronger

[kstoimenov]

Addressed comments.

[kstoimenov]

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

9–17 ↗

(On Diff #541254)

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

14 ↗

(On Diff #541283)

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

14 ↗

(On Diff #541283)

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);
-#  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.
+#  if defined(__x86_64__) || defined(__aarch64__)
+  // This logic strips down the top byte and the lower 48 bits from the 64 bit
+  // value and checks if the remaining 8 bits are 0. The assumption is that we
+  // have 48 bit VMA and that the top byte can be used to store additional
+  // information on both AArch64 and X86_64. Even though on X86_64 we might use
+  // less than 8 bits from the top byte we want to error on the side of
+  // returning true rather than creating false positive memory leak report.
   constexpr uptr kPointerMask = 255ULL << 48;
   // Accept up to 48 bit VMA.
   return ((p & kPointerMask) == 0);
+#  elif defined(__mips64)
+  return ((p >> 40) == 0);
 #  elif defined(__loongarch_lp64)
   // Allow 47-bit user-space VMA at current.
   return ((p >> 47) == 0);
 #  else
   return true;
 #  endif
 }