[PATCH] D14199: [compiler-rt] [tsan] Unify aarch64 mapping

[Dmitry Vyukov via llvm-commits]

dvyukov added inline comments.

================
Comment at: lib/tsan/rtl/tsan_platform.h:224
@@ +223,3 @@
+# if !defined(SANITIZER_GO)
+static USED uptr UserRegions[] = {
+  Mapping::kLoAppMemBeg, Mapping::kLoAppMemEnd,
----------------
UserRegions are used only once during startup, there is no point in having both const and dynamic versions of it. Also you initialize the dynamic variable only on linux, but it actually needs to be initialized when TSAN_RUNTIME_VMA (e.g. on mac/aarch64 in future).

Please leave only one version and define it in this header.
Something along the lines of:

bool GetUserRegion(int i, uptr *start, uptr *end) {
  switch (i) {
  default:
    return false;
  case 0:
    *start = LoAppMemBeg();
    *end = LoAppMemEnd();
  case 1:
    ...
  case 2:
    ...
  }
}

And loop in CheckShadowMapping until this function returns false.

================
Comment at: lib/tsan/rtl/tsan_platform.h:272
@@ +271,3 @@
+    case MAPPING_VDSO_BEG: return Mapping::kVdsoBeg;
+#elif defined(SANITIZER_GO) && !SANITIZER_WINDOWS
+    case MAPPING_APP_BEG: return Mapping::kAppMemBeg;
----------------
Why !SANITIZER_WINDOWS?
There is also no windows-Go-specific code, so these definitions must be necessary either on all Go platforms or on no Go platforms.


================
Comment at: lib/tsan/rtl/tsan_platform_posix.cc:50
@@ -48,3 +49,3 @@
 #elif defined(__aarch64__)
-  const uptr kMadviseRangeBeg  = 0x7e00000000ull;
-  const uptr kMadviseRangeSize = 0x0100000000ull;
+  unsigned runtimeVMA =
+    (MostSignificantSetBitIndex(GET_CURRENT_FRAME()) + 1);
----------------
vmaSize should be already set by now, so use it.
Add

  } else {
    CHECK(0);
  }

branch below. It will catch both case when vmaSize is somehow not initialized by now and case when support for a new vma size is added.


http://reviews.llvm.org/D14199