[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_myriad.h:26
+
+#define kHighMemBeg    (kMyriadMemoryEnd32 + 1)
+
----------------
vitalybuka wrote:
> waltl wrote:
> > waltl wrote:
> > > alekseyshl wrote:
> > > > 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.
> > > That's what I did originally.  Vitally suggested that I define things this way.  I'm fine with either.  The tradeoffs I see are:
> > > 
> > > - define as 0: cleaner conceptually, consistent with handling of mid memory.
> > > 
> > > - define as range [begin, begin - 1]: fewer changes required.
> > > 
> > I went back to setting kHighMemBeg to 0.
> > 
> It's not just fewer changes, with [begin, begin - 1] you don't need sentinel value like 0.  This also confusing because kLowMemBeg is 0 and it's not sentinel in that case.
> Still we already use 0 as sentinel for kMidMemBeg.
> 
> So I'd prefer to convert all code into [begin, begin - 1] for unavailable regions, but feel free to keep as 0, as it's already a mix of approaches.
> 
Yep, that was my point, let's keep the code consistent, it is pretty non-trivial as it is.


Repository:
  rL LLVM

https://reviews.llvm.org/D46456