[compiler-rt] [Darwin] Strip MTE tags from ASAN and TSAN

[ndrewh]

ASAN and TSAN need to strip tags in order to compute the correct shadow addresses.

rdar://163518624

[llvmbot]

@llvm/pr-subscribers-compiler-rt-sanitizer

Author: Andrew Haberlandt (ndrewh)

Changes

ASAN and TSAN need to strip tags in order to compute the correct shadow addresses.

---

Full diff: https://github.com/llvm/llvm-project/pull/166453.diff

2 Files Affected:

• (modified) compiler-rt/lib/asan/asan_mapping.h (+10-1)
• (modified) compiler-rt/lib/tsan/rtl/tsan_platform.h (+13-3)

diff --git a/compiler-rt/lib/asan/asan_mapping.h b/compiler-rt/lib/asan/asan_mapping.h
index bddae9a074056..9fa7f9014445c 100644
--- a/compiler-rt/lib/asan/asan_mapping.h
+++ b/compiler-rt/lib/asan/asan_mapping.h
@@ -281,11 +281,18 @@ extern uptr kHighMemEnd, kMidMemBeg, kMidMemEnd;  // Initialized in __asan_init.
 
 }  // namespace __asan
 
+#  if SANITIZER_APPLE && SANITIZER_WORDSIZE == 64
+#    define TAG_MASK ((uptr)0x0f << 56)  // Lower half of top byte
+#    define STRIP_TAG(addr) ((addr) & ~TAG_MASK)
+#  else
+#    define STRIP_TAG(addr) (addr)
+#  endif
+
 #  if defined(__sparc__) && SANITIZER_WORDSIZE == 64
 #    include "asan_mapping_sparc64.h"
 #  else
 #    define MEM_TO_SHADOW(mem) \
-      (((mem) >> ASAN_SHADOW_SCALE) + (ASAN_SHADOW_OFFSET))
+      ((STRIP_TAG(mem) >> ASAN_SHADOW_SCALE) + (ASAN_SHADOW_OFFSET))
 #    define SHADOW_TO_MEM(mem) \
       (((mem) - (ASAN_SHADOW_OFFSET)) << (ASAN_SHADOW_SCALE))
 
@@ -377,6 +384,7 @@ static inline uptr MemToShadowSize(uptr size) {
 
 static inline bool AddrIsInMem(uptr a) {
   PROFILE_ASAN_MAPPING();
+  a = STRIP_TAG(a);
   return AddrIsInLowMem(a) || AddrIsInMidMem(a) || AddrIsInHighMem(a) ||
          (flags()->protect_shadow_gap == 0 && AddrIsInShadowGap(a));
 }
@@ -389,6 +397,7 @@ static inline uptr MemToShadow(uptr p) {
 
 static inline bool AddrIsInShadow(uptr a) {
   PROFILE_ASAN_MAPPING();
+  a = STRIP_TAG(a);
   return AddrIsInLowShadow(a) || AddrIsInMidShadow(a) || AddrIsInHighShadow(a);
 }
 
diff --git a/compiler-rt/lib/tsan/rtl/tsan_platform.h b/compiler-rt/lib/tsan/rtl/tsan_platform.h
index 00b493bf2d931..fc7d1be61ca4c 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_platform.h
+++ b/compiler-rt/lib/tsan/rtl/tsan_platform.h
@@ -947,6 +947,16 @@ uptr MetaShadowBeg(void) { return SelectMapping<MappingField>(kMetaShadowBeg); }
 ALWAYS_INLINE
 uptr MetaShadowEnd(void) { return SelectMapping<MappingField>(kMetaShadowEnd); }
 
+ALWAYS_INLINE
+uptr StripTag(uptr addr) {
+#if SANITIZER_APPLE
+  constexpr uptr kTagMask = ((uptr)0x0f << 56);  // Lower half of top byte
+  return addr & ~kTagMask;
+#else
+  return addr;
+#endif
+}
+
 struct IsAppMemImpl {
   template <typename Mapping>
   static bool Apply(uptr mem) {
@@ -958,7 +968,7 @@ struct IsAppMemImpl {
 };
 
 ALWAYS_INLINE
-bool IsAppMem(uptr mem) { return SelectMapping<IsAppMemImpl>(mem); }
+bool IsAppMem(uptr mem) { return SelectMapping<IsAppMemImpl>(StripTag(mem)); }
 
 struct IsShadowMemImpl {
   template <typename Mapping>
@@ -997,7 +1007,7 @@ struct MemToShadowImpl {
 
 ALWAYS_INLINE
 RawShadow *MemToShadow(uptr x) {
-  return reinterpret_cast<RawShadow *>(SelectMapping<MemToShadowImpl>(x));
+  return reinterpret_cast<RawShadow *>(SelectMapping<MemToShadowImpl>(StripTag(mem)));
 }
 
 struct MemToMetaImpl {
@@ -1011,7 +1021,7 @@ struct MemToMetaImpl {
 };
 
 ALWAYS_INLINE
-u32 *MemToMeta(uptr x) { return SelectMapping<MemToMetaImpl>(x); }
+u32 *MemToMeta(uptr x) { return SelectMapping<MemToMetaImpl>(StripTag(x)); }
 
 struct ShadowToMemImpl {
   template <typename Mapping>

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[thurstond]

Would StripTag be better off in sanitizer_common?

P.S. HWASan has a similar function, UntagAddr.

[vitalybuka]

ASAN and TSAN need to strip tags in order to compute the correct shadow addresses.

What is the point of running Asan with MTE?

[vitalybuka]

ASAN and TSAN need to strip tags in order to compute the correct shadow addresses.

What is the point of running Asan with MTE?

Also Tsan has its own allocator, which does not tag.

[DanBlackwell]

Would StripTag be better off in sanitizer_common?

P.S. HWASan has a similar function, UntagAddr.

This is true, but HWASan uses all 8 bits of TBI [63:56]; while MTE uses only [59:56]. Perhaps StripTag should be named StripMTEtag in order to avoid future confusion? I do agree that this could move up to sanitizer_common.

[DanBlackwell]

ASAN and TSAN need to strip tags in order to compute the correct shadow addresses.

What is the point of running Asan with MTE?

Also Tsan has its own allocator, which does not tag.

Other linked (uninstrumented) libraries can use their own MTE-backed allocator, but Asan / Tsan will still intercept their calls and then IsAddrInMem and friends could misclassify them (due to the tag making it appear to be an ultra-high address).

[thurstond]

Would StripTag be better off in sanitizer_common?
P.S. HWASan has a similar function, UntagAddr.

This is true, but HWASan uses all 8 bits of TBI [63:56]; while MTE uses only [59:56].

Ah, good point!

Perhaps StripTag should be named StripMTEtag in order to avoid future confusion?

Sure. I would also be partial to something like "UntagMTEAddr", for consistency with HWASan.

[ndrewh]

I've renamed the macro to STRIP_MTE_TAG and moved it into sanitizer_common (sanitizer_platform.h seemed appropriate) and updated asan and tsan accordingly.

I have a slight preference for the macro over an inline function, is there a good reason to also have the inline function?