[compiler-rt] [tsan] Fix calculation of shadow end address in MemoryAccessRangeT (PR #98404)

[Vitaly Buka via llvm-commits]

https://github.com/vitalybuka updated https://github.com/llvm/llvm-project/pull/98404

>From 0f495dbb407b6c2e5b6fb908d9d201d14abaf4d8 Mon Sep 17 00:00:00 2001
From: Thurston Dang <thurston at google.com>
Date: Wed, 10 Jul 2024 22:42:03 +0000
Subject: [PATCH 1/2] [tsan] Fix calculation of shadow end address in
 MemoryAccessRangeT

MemoryAccessRangeT overestimates the size of the shadow region by 8x, occasionally leading to assertion failure:
```
  RawShadow* shadow_mem = MemToShadow(addr);
  ...
  // Check that end of shadow is valid
  if (!IsShadowMem(shadow_mem + size * kShadowCnt - 1)) {
    DCHECK(IsShadowMem(shadow_mem + size * kShadowCnt - 1));
```
It is erroneous for two separate reasons:
- it uses kShadowCnt (== 4) instead of kShadowMultiplier (== 2)
- since shadow_mem is a RawShadow*, pointer arithmetic is multiplied by sizeof(RawShadow) == 4

This patch fixes the calculation, and also improves the debugging information.

The assertion error was observed on a buildbot (https://lab.llvm.org/staging/#/builders/89/builds/656/steps/13/logs/stdio):
```
Bad shadow addr 0x3000000190bc (7fffffffe85f)
ThreadSanitizer: CHECK failed: tsan_rtl_access.cpp:690 "((IsShadowMem(shadow_mem + size * kShadowCnt - 1))) != (0)" (0x0, 0x0) (tid=2202676)
```
Notice that 0x3000000190bc is not the correct shadow for the end address 0x7fffffffe85f.

This error is more commonly observed on high-entropy ASLR systems, since ASLR may be disabled (if the randomized memory layout is incompatible), leading to an allocation near the boundaries of the high app memory region (and therefore a shadow end that may be erroneously calculated to be past the end of the shadow region). Also note that the assertion is guarded by SANITIZER_DEBUG.
---
 compiler-rt/lib/tsan/rtl/tsan_rtl_access.cpp | 24 +++++++++++++-------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/compiler-rt/lib/tsan/rtl/tsan_rtl_access.cpp b/compiler-rt/lib/tsan/rtl/tsan_rtl_access.cpp
index 8b20984a01000..d03b1863735df 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_rtl_access.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_rtl_access.cpp
@@ -672,22 +672,30 @@ void MemoryAccessRangeT(ThreadState* thr, uptr pc, uptr addr, uptr size) {
 
 #if SANITIZER_DEBUG
   if (!IsAppMem(addr)) {
-    Printf("Access to non app mem %zx\n", addr);
+    Printf("Access to non app mem start: 0x%zx\n", addr);
     DCHECK(IsAppMem(addr));
   }
   if (!IsAppMem(addr + size - 1)) {
-    Printf("Access to non app mem %zx\n", addr + size - 1);
+    Printf("Access to non app mem end: 0x%zx\n", addr + size - 1);
     DCHECK(IsAppMem(addr + size - 1));
   }
   if (!IsShadowMem(shadow_mem)) {
-    Printf("Bad shadow addr %p (%zx)\n", static_cast<void*>(shadow_mem), addr);
+    Printf("Bad shadow start addr: %p (0x%zx)\n",
+           static_cast<void*>(shadow_mem), addr);
     DCHECK(IsShadowMem(shadow_mem));
   }
-  if (!IsShadowMem(shadow_mem + size * kShadowCnt - 1)) {
-    Printf("Bad shadow addr %p (%zx)\n",
-           static_cast<void*>(shadow_mem + size * kShadowCnt - 1),
-           addr + size - 1);
-    DCHECK(IsShadowMem(shadow_mem + size * kShadowCnt - 1));
+
+  // Be careful to cast to uptr before performing arithmetic.
+  RawShadow* shadow_mem_end = static_cast<RawShadow*>(
+      (void*)(((uptr)shadow_mem) + size * kShadowMultiplier - 1));
+  if (!IsShadowMem(shadow_mem_end)) {
+    Printf("Bad shadow end addr: %p (0x%zx)\n",
+           static_cast<void*>(shadow_mem_end), addr + size - 1);
+    Printf(
+        "Shadow start addr (ok): %p (0x%zx); size: 0x%zx; kShadowMultiplier: "
+        "%zx\n",
+        static_cast<void*>(shadow_mem), addr, size, kShadowMultiplier);
+    DCHECK(IsShadowMem(shadow_mem_end));
   }
 #endif
 

>From b392137050c55f1c00351c601e8fd1d5cd1f3418 Mon Sep 17 00:00:00 2001
From: Vitaly Buka <vitalybuka at gmail.com>
Date: Wed, 10 Jul 2024 19:32:12 -0700
Subject: [PATCH 2/2] Print pointers with %p

---
 compiler-rt/lib/tsan/rtl/tsan_rtl_access.cpp | 20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/compiler-rt/lib/tsan/rtl/tsan_rtl_access.cpp b/compiler-rt/lib/tsan/rtl/tsan_rtl_access.cpp
index d03b1863735df..cf07686d968dc 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_rtl_access.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_rtl_access.cpp
@@ -672,29 +672,27 @@ void MemoryAccessRangeT(ThreadState* thr, uptr pc, uptr addr, uptr size) {
 
 #if SANITIZER_DEBUG
   if (!IsAppMem(addr)) {
-    Printf("Access to non app mem start: 0x%zx\n", addr);
+    Printf("Access to non app mem start: %p\n", (void*)addr);
     DCHECK(IsAppMem(addr));
   }
   if (!IsAppMem(addr + size - 1)) {
-    Printf("Access to non app mem end: 0x%zx\n", addr + size - 1);
+    Printf("Access to non app mem end: %p\n", (void*)(addr + size - 1));
     DCHECK(IsAppMem(addr + size - 1));
   }
   if (!IsShadowMem(shadow_mem)) {
-    Printf("Bad shadow start addr: %p (0x%zx)\n",
-           static_cast<void*>(shadow_mem), addr);
+    Printf("Bad shadow start addr: %p (%p)\n", shadow_mem, (void*)addr);
     DCHECK(IsShadowMem(shadow_mem));
   }
 
-  // Be careful to cast to uptr before performing arithmetic.
-  RawShadow* shadow_mem_end = static_cast<RawShadow*>(
-      (void*)(((uptr)shadow_mem) + size * kShadowMultiplier - 1));
+  RawShadow* shadow_mem_end = reinterpret_cast<RawShadow*>(
+      reinterpret_cast<uptr>(shadow_mem) + size * kShadowMultiplier - 1);
   if (!IsShadowMem(shadow_mem_end)) {
-    Printf("Bad shadow end addr: %p (0x%zx)\n",
-           static_cast<void*>(shadow_mem_end), addr + size - 1);
+    Printf("Bad shadow end addr: %p (%p)\n", shadow_mem_end,
+           (void*)(addr + size - 1));
     Printf(
-        "Shadow start addr (ok): %p (0x%zx); size: 0x%zx; kShadowMultiplier: "
+        "Shadow start addr (ok): %p (%p); size: 0x%zx; kShadowMultiplier: "
         "%zx\n",
-        static_cast<void*>(shadow_mem), addr, size, kShadowMultiplier);
+        shadow_mem, (void*)addr, size, kShadowMultiplier);
     DCHECK(IsShadowMem(shadow_mem_end));
   }
 #endif