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

[Walter Lee via Phabricator via llvm-commits]

waltl marked 16 inline comments as done.
waltl added inline comments.


================
Comment at: compiler-rt/lib/asan/asan_mapping_myriad.h:26
+
+#define kHighMemBeg    (kMyriadMemoryEnd32 + 1)
+
----------------
alekseyshl wrote:
> 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.
Using 0 for now.


Repository:
  rL LLVM

https://reviews.llvm.org/D46456