tsan: unit-test all mappings

[dvyukov]

Summary

Move the mapping checking logic from startup to unit tests
and test all mapping instead of just the active one.
This makes it much more feasible to make any global changes
to the mappings since we have 17 of them.

Depends on D107740.

[melver]

compiler-rt/lib/tsan/rtl/tsan_platform.h

211

minor: align '=' as well?

268

minor: Ignore clang-format and align with others.

[dvyukov]

compiler-rt/lib/tsan/rtl/tsan_platform.h

211

I can't. All new code is formatted by clang-format. And I can't even do 'arc diff' w/o it running clang-format and amending results...
I agree this is ugly. We can either give up on alignment and re-format everything with clang-format, or add annotations to disable clang-format and re-align everything. Any preferences?
But let me do it separately after landing these series because it causes conflicts.

268

same here

[melver]

compiler-rt/lib/tsan/rtl/tsan_platform.h

211

Sigh. Annotations to disable ("// clang-format off"?) are probably worse.

Somehow there must be a way to hack it so 'arc diff' doesn't do this. I hate that clang-format is "the law" now, because it's not perfect and makes mistakes, too (especially all the cases where it does something like "type var =<newline>" is just awful).

But no point in wasting time on it, so disregard.

[dvyukov]

compiler-rt/lib/tsan/rtl/tsan_platform.h

211

Sigh. Annotations to disable ("// clang-format off"?) are probably worse.

Yes, these annotations.
I don't think they will be too bad here. We will surround very limited and specific part of code formatted as a table.

Somehow there must be a way to hack it so 'arc diff' doesn't do this. I hate that clang-format is "the law" now, because it's not perfect and makes mistakes, too

FWIW I like it :)
I don't need to argue about formatting anymore.
If one can randomly disregard it, then we get to where we start. Arguing about style. And do you know if I intentionally disregarded it or just forgot to format?

(especially all the cases where it does something like "type var =<newline>" is just awful).

Yes, not very nice. I think part of the problem with this particular one is interaction of our verbose naming style with 80 column requirement. That's why I wanted to increase line width.

But no point in wasting time on it, so disregard.

[vitalybuka]

compiler-rt/lib/tsan/rtl/tsan_platform.h

211

Yeh, leave it to clang-format and let it to what ever it does.
Also "AlignConsecutiveAssignments: true" into tsan/.clang-format ?

743

This thing is exactly for this with better output
https://github.com/google/googletest/blob/master/docs/advanced.md#typed-tests

[dvyukov]

changed SelectMapping to use template Arg and rebased

[dvyukov]

compiler-rt/lib/tsan/rtl/tsan_platform.h

211

AlignConsecutiveAssignments produces quite a diff, and some of the edits are quite dubious:
https://github.com/dvyukov/llvm-project/commit/c0065f73f601514ff7f0c5a2f534af2fd09d64a6
So I don't think we want to enable it because of tables in this file.

[dvyukov]

compiler-rt/lib/tsan/rtl/tsan_platform.h

743

Interesting. But it does not produce better output :)

[----------] 1 test from MappingTest/0, where TypeParam = <type>
[ RUN      ] MappingTest/0.Test
[       OK ] MappingTest/0.Test (0 ms)
[----------] 1 test from MappingTest/0 (0 ms total)

[----------] 1 test from MappingTest/1, where TypeParam = <type>
[ RUN      ] MappingTest/1.Test
[       OK ] MappingTest/1.Test (0 ms)
[----------] 1 test from MappingTest/1 (0 ms total)

[----------] 1 test from MappingTest/2, where TypeParam = <type>
[ RUN      ] MappingTest/2.Test
[       OK ] MappingTest/2.Test (0 ms)
[----------] 1 test from MappingTest/2 (0 ms total)

I guess that's because we don't have rtti enabled.

The reason I wanted the mapping list to be in tsan_platform.h is because when a new mapping is added, the chances that it will be added to tests/unit/tsan_shadow_test.cpp are close to 0 :)

I don't have strong preference either way. Do you?

[dvyukov]

compiler-rt/lib/tsan/rtl/tsan_platform.h

743

I will proceed with merging this series (I have other pending changes on top).
If you prefer any additional changes, please say and I will fix separately.

[vitalybuka]

compiler-rt/lib/tsan/rtl/tsan_platform.h

743

no strong preference, feel free to keep either way

compiler-rt/lib/tsan/rtl/tsan_platform.h

 # error "Only 64-bit is supported"
 #endif
 
 #include "tsan_defs.h"
 #include "tsan_trace.h"
 
 namespace __tsan {
 
+enum {
+  // App memory is not mapped onto shadow memory range.
+  kBrokenMapping = 1 << 0,
+  // Mapping app memory and back does not produce the same address,
+  // this can lead to wrong addresses in reports and potentially
+  // other bad consequences.
+  kBrokenReverseMapping = 1 << 1,
+  // Mapping is non-linear for linear user range.
+  // This is bad and can lead to unpredictable memory corruptions, etc
+  // because range access functions assume linearity.
+  kBrokenLinearity = 1 << 2,
+};
+
 /*
 C/C++ on linux/x86_64 and freebsd/x86_64
 0000 0000 1000 - 0080 0000 0000: main binary and/or MAP_32BIT mappings (512GB)
 0040 0000 0000 - 0100 0000 0000: -
 0100 0000 0000 - 2000 0000 0000: shadow
 2000 0000 0000 - 3000 0000 0000: -
 3000 0000 0000 - 4000 0000 0000: metainfo (memory blocks and sync objects)
 4000 0000 0000 - 5500 0000 0000: -

 2aa00 0000 00 - 2ab00 0000 00: main binary (PIE)
 2ab00 0000 00 - 36200 0000 00: -
 36200 0000 00 - 36240 0000 00: traces
 36240 0000 00 - 3e000 0000 00: -
 3e000 0000 00 - 3f000 0000 00: heap
 3f000 0000 00 - 3ffff ffff ff: modules and main thread stack
 */
 struct MappingAarch64_42 {
+  static const uptr kBroken = kBrokenReverseMapping;
   static const uptr kLoAppMemBeg   = 0x00000001000ull;
   static const uptr kLoAppMemEnd   = 0x01000000000ull;
   static const uptr kShadowBeg     = 0x10000000000ull;
   static const uptr kShadowEnd     = 0x20000000000ull;
   static const uptr kMetaShadowBeg = 0x26000000000ull;
   static const uptr kMetaShadowEnd = 0x28000000000ull;
   static const uptr kMidAppMemBeg  = 0x2aa00000000ull;
   static const uptr kMidAppMemEnd  = 0x2ab00000000ull;

 0d00 0000 0000 - 0d00 0000 0000: -
 0d00 0000 0000 - 0f00 0000 0000: traces
 0f00 0000 0000 - 0f00 0000 0000: -
 0f00 0000 0000 - 0f50 0000 0000: heap
 0f50 0000 0000 - 0f60 0000 0000: -
 0f60 0000 0000 - 1000 0000 0000: modules and main thread stack
 */
 struct MappingPPC64_44 {
+  static const uptr kBroken =
+      kBrokenMapping | kBrokenReverseMapping | kBrokenLinearity;
   static const uptr kMetaShadowBeg = 0x0b0000000000ull;
   static const uptr kMetaShadowEnd = 0x0d0000000000ull;
   static const uptr kTraceMemBeg   = 0x0d0000000000ull;
   static const uptr kTraceMemEnd   = 0x0f0000000000ull;
   static const uptr kShadowBeg     = 0x000100000000ull;
   static const uptr kShadowEnd     = 0x0b0000000000ull;
   static const uptr kLoAppMemBeg   = 0x000000000100ull;
   static const uptr kLoAppMemEnd   = 0x000100000000ull;
#  endif
   }
   DCHECK(0);
   return 0;
 #else
   return Func::template Apply<Mapping>(arg);
 #endif
 }
 
+template <typename Func>
+void ForEachMapping() {
+  Func::template Apply<Mapping48AddressSpace>();
+  Func::template Apply<MappingMips64_40>();
+  Func::template Apply<MappingAppleAarch64>();
+  Func::template Apply<MappingAarch64_39>();
+  Func::template Apply<MappingAarch64_42>();
+  Func::template Apply<MappingAarch64_48>();
+  Func::template Apply<MappingPPC64_44>();
+  Func::template Apply<MappingPPC64_46>();
+  Func::template Apply<MappingPPC64_47>();
+  Func::template Apply<MappingS390x>();
+  Func::template Apply<MappingGo48>();
+  Func::template Apply<MappingGoWindows>();
+  Func::template Apply<MappingGoPPC64_46>();
+  Func::template Apply<MappingGoPPC64_47>();
+  Func::template Apply<MappingGoAarch64>();
+  Func::template Apply<MappingGoMips64_47>();
+  Func::template Apply<MappingGoS390x>();
+}
+
 enum MappingType {
   MAPPING_LO_APP_BEG,
   MAPPING_LO_APP_END,
   MAPPING_HI_APP_BEG,
   MAPPING_HI_APP_END,
   MAPPING_MID_APP_BEG,
   MAPPING_MID_APP_END,
   MAPPING_HEAP_BEG,

 ALWAYS_INLINE
 uptr HiAppMemEnd(void) {
   return SelectMapping<MappingField>(MAPPING_HI_APP_END);
 }
 
 ALWAYS_INLINE
 uptr VdsoBeg(void) { return SelectMapping<MappingField>(MAPPING_VDSO_BEG); }
 
-static inline
-bool GetUserRegion(int i, uptr *start, uptr *end) {
-  switch (i) {
-  case 0:
-    *start = LoAppMemBeg();
-    *end = LoAppMemEnd();
-    return true;
-  case 1:
-    *start = HiAppMemBeg();
-    *end = HiAppMemEnd();
-    return true;
-  case 2:
-    *start = HeapMemBeg();
-    *end = HeapMemEnd();
-    return true;
-  case 3:
-    if (MidAppMemBeg()) {
-      *start = MidAppMemBeg();
-      *end = MidAppMemEnd();
-      return true;
-    }
-    FALLTHROUGH;
-  default:
-    return false;
-  }
-}
-
 ALWAYS_INLINE
 uptr ShadowBeg(void) { return SelectMapping<MappingField>(MAPPING_SHADOW_BEG); }
 ALWAYS_INLINE
 uptr ShadowEnd(void) { return SelectMapping<MappingField>(MAPPING_SHADOW_END); }
 
 ALWAYS_INLINE
 uptr MetaShadowBeg(void) {
   return SelectMapping<MappingField>(MAPPING_META_SHADOW_BEG);

 };
 
 ALWAYS_INLINE
 u32 *MemToMeta(uptr x) { return SelectMapping<MemToMetaImpl>(x); }
 
 struct ShadowToMemImpl {
   template <typename Mapping>
   static uptr Apply(uptr sp) {
-    DCHECK(IsShadowMemImpl::Apply<Mapping>(sp));
-  // The shadow mapping is non-linear and we've lost some bits, so we don't have
-  // an easy way to restore the original app address. But the mapping is a
-  // bijection, so we try to restore the address as belonging to low/mid/high
-  // range consecutively and see if shadow->app->shadow mapping gives us the
-  // same address.
+    if (!IsShadowMemImpl::Apply<Mapping>(sp))
+      return 0;
+    // The shadow mapping is non-linear and we've lost some bits, so we don't
+    // have an easy way to restore the original app address. But the mapping is
+    // a bijection, so we try to restore the address as belonging to
+    // low/mid/high range consecutively and see if shadow->app->shadow mapping
+    // gives us the same address.
     uptr p = ((sp - Mapping::kShadowAdd) / kShadowCnt) ^ Mapping::kShadowXor;
     if (p >= Mapping::kLoAppMemBeg && p < Mapping::kLoAppMemEnd &&
         MemToShadowImpl::Apply<Mapping>(p) == sp)
       return p;
     if (Mapping::kMidAppMemBeg) {
       uptr p_mid = p + (Mapping::kMidAppMemBeg & Mapping::kShadowMsk);
       if (p_mid >= Mapping::kMidAppMemBeg && p_mid < Mapping::kMidAppMemEnd &&
           MemToShadowImpl::Apply<Mapping>(p_mid) == sp)

compiler-rt/lib/tsan/rtl/tsan_rtl.cpp

void MapThreadTrace(uptr addr, uptr size, const char *name) {
   CHECK_EQ(addr, addr & ~((64 << 10) - 1));  // windows wants 64K alignment
   if (!MmapFixedSuperNoReserve(addr, size, name)) {
     Printf("FATAL: ThreadSanitizer can not mmap thread trace (%p/%p)\n",
         addr, size);
     Die();
   }
 }
 
-static void CheckShadowMapping() {
-  uptr beg, end;
-  for (int i = 0; GetUserRegion(i, &beg, &end); i++) {
-    // Skip cases for empty regions (heap definition for architectures that
-    // do not use 64-bit allocator).
-    if (beg == end)
-      continue;
-    VPrintf(3, "checking shadow region %p-%p\n", beg, end);
-    uptr prev = 0;
-    for (uptr p0 = beg; p0 <= end; p0 += (end - beg) / 4) {
-      for (int x = -(int)kShadowCell; x <= (int)kShadowCell; x += kShadowCell) {
-        const uptr p = RoundDown(p0 + x, kShadowCell);
-        if (p < beg || p >= end)
-          continue;
-        RawShadow *const s = MemToShadow(p);
-        u32 *const m = MemToMeta(p);
-        VPrintf(3, "  checking pointer %p: shadow=%p meta=%p\n", p, s, m);
-        CHECK(IsAppMem(p));
-        CHECK(IsShadowMem(s));
-        CHECK_EQ(p, ShadowToMem(s));
-        CHECK(IsMetaMem(m));
-        if (prev) {
-          // Ensure that shadow and meta mappings are linear within a single
-          // user range. Lots of code that processes memory ranges assumes it.
-          RawShadow *const prev_s = MemToShadow(prev);
-          u32 *const prev_m = MemToMeta(prev);
-          CHECK_EQ((s - prev_s) * kShadowSize, (p - prev) * kShadowMultiplier);
-          CHECK_EQ(m - prev_m, (p - prev) / kMetaShadowCell);
-        }
-        prev = p;
-      }
-    }
-  }
-}
-
 #if !SANITIZER_GO
 static void OnStackUnwind(const SignalContext &sig, const void *,
                           BufferedStackTrace *stack) {
   stack->Unwind(StackTrace::GetNextInstructionPc(sig.pc), sig.bp, sig.context,
                 common_flags()->fast_unwind_on_fatal);
 }
 
 static void TsanOnDeadlySignal(int signo, void *siginfo, void *context) {
#if !SANITIZER_GO
   InitializeAllocator();
   ReplaceSystemMalloc();
 #endif
   if (common_flags()->detect_deadlocks)
     ctx->dd = DDetector::Create(flags());
   Processor *proc = ProcCreate();
   ProcWire(proc, thr);
   InitializeInterceptors();
-  CheckShadowMapping();
   InitializePlatform();
   InitializeDynamicAnnotations();
 #if !SANITIZER_GO
   InitializeShadowMemory();
   InitializeAllocatorLate();
   InstallDeadlySignalHandlers(TsanOnDeadlySignal);
 #endif
   // Setup correct file descriptor for error reports.

compiler-rt/lib/tsan/tests/unit/tsan_shadow_test.cpp

TEST(Shadow, Celling) {
   for (unsigned i = 1; i < kShadowCell; i++)
     CHECK_EQ(s0, MemToShadow((uptr)&data[i]));
   for (unsigned i = kShadowCell; i < 2*kShadowCell; i++)
     CHECK_EQ(s0 + kShadowCnt, MemToShadow((uptr)&data[i]));
   for (unsigned i = 2*kShadowCell; i < 3*kShadowCell; i++)
     CHECK_EQ(s0 + 2 * kShadowCnt, MemToShadow((uptr)&data[i]));
 }
 
+// Detect is the Mapping has kBroken field.
+template <uptr>
+struct Has {
+  typedef bool Result;
+};
+
+template <typename Mapping>
+bool broken(...) {
+  return false;
+}
+
+template <typename Mapping>
+bool broken(uptr what, typename Has<Mapping::kBroken>::Result = false) {
+  return Mapping::kBroken & what;
+}
+
+struct MappingTest {
+  template <typename Mapping>
+  static void Apply() {
+    // Easy (but ugly) way to print the mapping name.
+    Printf("%s\n", __PRETTY_FUNCTION__);
+    TestRegion<Mapping>(Mapping::kLoAppMemBeg, Mapping::kLoAppMemEnd);
+    TestRegion<Mapping>(Mapping::kMidAppMemBeg, Mapping::kMidAppMemEnd);
+    TestRegion<Mapping>(Mapping::kHiAppMemBeg, Mapping::kHiAppMemEnd);
+    TestRegion<Mapping>(Mapping::kHeapMemBeg, Mapping::kHeapMemEnd);
+  }
+
+  template <typename Mapping>
+  static void TestRegion(uptr beg, uptr end) {
+    if (beg == end)
+      return;
+    Printf("checking region [%p-%p)\n", beg, end);
+    uptr prev = 0;
+    for (uptr p0 = beg; p0 <= end; p0 += (end - beg) / 256) {
+      for (int x = -(int)kShadowCell; x <= (int)kShadowCell; x += kShadowCell) {
+        const uptr p = RoundDown(p0 + x, kShadowCell);
+        if (p < beg || p >= end)
+          continue;
+        const uptr s = MemToShadowImpl::Apply<Mapping>(p);
+        u32 *const m = MemToMetaImpl::Apply<Mapping>(p);
+        const uptr r = ShadowToMemImpl::Apply<Mapping>(s);
+        Printf("  addr=%p: shadow=%p meta=%p reverse=%p\n", p, s, m, r);
+        CHECK(IsAppMemImpl::Apply<Mapping>(p));
+        if (!broken<Mapping>(kBrokenMapping))
+          CHECK(IsShadowMemImpl::Apply<Mapping>(s));
+        CHECK(IsMetaMemImpl::Apply<Mapping>(reinterpret_cast<uptr>(m)));
+        if (!broken<Mapping>(kBrokenReverseMapping))
+          CHECK_EQ(p, r);
+        if (prev && !broken<Mapping>(kBrokenLinearity)) {
+          // Ensure that shadow and meta mappings are linear within a single
+          // user range. Lots of code that processes memory ranges assumes it.
+          const uptr prev_s = MemToShadowImpl::Apply<Mapping>(prev);
+          u32 *const prev_m = MemToMetaImpl::Apply<Mapping>(prev);
+          CHECK_EQ(s - prev_s, (p - prev) * kShadowMultiplier);
+          CHECK_EQ(m - prev_m, (p - prev) / kMetaShadowCell);
+        }
+        prev = p;
+      }
+    }
+  }
+};
+
+TEST(Shadow, AllMappings) { ForEachMapping<MappingTest>(); }
+
 }  // namespace __tsan