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

[Walter Lee via Phabricator via llvm-commits]

waltl added inline comments.


================
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));
----------------
alekseyshl wrote:
> waltl wrote:
> > alekseyshl wrote:
> > > 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?
> > > Why the address is shadow is also considered in mem?
> > 
> > I had a few places where I had to check AddrIsInMem(addr) && AddrIsInShadow(addr).  Vitaly suggested that I define AddIsInMem accordingly so I don't have to do both checks.  This makes sense to me conceptually and I checked that it shouldn't break existing uses of the function.
> > 
> > > AddrIsInShadowGap(a) is covered by the next check, protect_shadow_gap is always 0 in
> > > your config, right?
> > 
> > True.  I'll simplify.
> > 
> I see. What are these places? I'd rather have those checks explicit, because, on top of being pretty confusing to read, for one example, GetShadowAddressInformation() won't work on Myriad2.
They are in __asan_region_is_poisoned (2 places) and
ASAN_MEMORY_ACCESS_CALLBACK_BODY, where there are now checks for just !AddrIsInMem(addr).



================
Comment at: compiler-rt/lib/asan/asan_mapping.h:365
+  a = RawAddr(a);
   return a >= kHighShadowBeg && a <= kHighShadowEnd;
 }
----------------
alekseyshl wrote:
> alekseyshl wrote:
> > return kHighShadowBeg && a >= kHighShadowBeg && a <= kHighShadowEnd;
> > 
> > otherwise nullptr will pass as a high shadow.
> kHighMemBeg -> kHighShadowBeg
This should be ok, if we keep kHighMemBeg = 0?  I was keeping consistent with AddrIsInMidShadow().


Repository:
  rL LLVM

https://reviews.llvm.org/D46456