[PATCH] D46456: [asan] Add support for Myriad RTEMS memory map

[Aleksey Shlyapnikov via Phabricator via llvm-commits]

alekseyshl added inline comments.


================
Comment at: compiler-rt/lib/asan/asan_mapping.h:307
+  return SANITIZER_MYRIAD2 ? (a & ~kMyriadCacheBitMask32) : a;
+}
+
----------------
It would be more in the spirit of this file to define a macro here:
#define RAW_ADDR(mem) (mem)
and in asan_mapping_myriad.h:
#define RAW_ADDR(mem) (mem & ~kMyriadCacheBitMask32)


================
Comment at: compiler-rt/lib/asan/asan_mapping.h:352
   return AddrIsInLowMem(a) || AddrIsInMidMem(a) || AddrIsInHighMem(a) ||
+      (SANITIZER_MYRIAD2 && (AddrIsInLowShadow(a) || AddrIsInShadowGap(a))) ||
       (flags()->protect_shadow_gap == 0 && AddrIsInShadowGap(a));
----------------
Why the address is shadow is also considered in mem?

AddrIsInShadowGap(a) is covered by the next check, protect_shadow_gap is always 0 in your config, right?


================
Comment at: compiler-rt/lib/asan/asan_mapping.h:365
+  a = RawAddr(a);
   return a >= kHighShadowBeg && a <= kHighShadowEnd;
 }
----------------
return kHighShadowBeg && a >= kHighShadowBeg && a <= kHighShadowEnd;

otherwise nullptr will pass as a high shadow.


================
Comment at: compiler-rt/lib/asan/asan_mapping_myriad.h:26
+
+#define kHighMemBeg    (kMyriadMemoryEnd32 + 1)
+
----------------
If you do not have a concept of high mem, why not set kHighMemBeg and kHighMemEnd to 0? It'll make other conditions more explicit.


================
Comment at: compiler-rt/lib/asan/asan_rtl.cc:59
     } else {
-      UnmapOrDie((void*)kLowShadowBeg, kHighShadowEnd - kLowShadowBeg);
+      if (!SANITIZER_MYRIAD2)
+        UnmapOrDie((void*)kLowShadowBeg, kHighShadowEnd - kLowShadowBeg);
----------------
Instead of this condition, add CHECK(!f->unmap_shadow_on_exit) to InitializeFlags(), the same way you do for SANITIZER_RTEMS. By the way, isn't that existing check enough already?


================
Comment at: compiler-rt/lib/asan/asan_rtl.cc:144
 #define ASAN_MEMORY_ACCESS_CALLBACK_BODY(type, is_write, size, exp_arg, fatal) \
+    if (SANITIZER_RTEMS && !AddrIsInMem(addr))                                 \
+      return;                                                                  \
----------------
Why __asan_region_is_poisoned special cases SANITIZER_MYRIAD2 and here you check for SANITIZER_RTEMS? Shouldn't it be the same?


================
Comment at: compiler-rt/lib/asan/asan_rtl.cc:322
 void PrintAddressSpaceLayout() {
-  Printf("|| `[%p, %p]` || HighMem    ||\n",
-         (void*)kHighMemBeg, (void*)kHighMemEnd);
-  Printf("|| `[%p, %p]` || HighShadow ||\n",
-         (void*)kHighShadowBeg, (void*)kHighShadowEnd);
+  if (!SANITIZER_MYRIAD2) {
+    Printf("|| `[%p, %p]` || HighMem    ||\n",
----------------
"if (kHighShadowBeg) {" or, if you set kHighMemBeg to 0, "if (kHighMemBeg) {"


================
Comment at: compiler-rt/lib/asan/asan_rtl.cc:349
+         (void*)MEM_TO_SHADOW(kLowShadowEnd));
+  if (!SANITIZER_MYRIAD2) {
+    Printf(" %p %p",
----------------
if (kHighShadowBeg) {


Repository:
  rL LLVM

https://reviews.llvm.org/D46456