[PATCH] D54080: [unittest] Skip W+X MappedMemoryTests when MPROTECT is enabled

[Kamil Rytarowski via Phabricator via llvm-commits]

krytarowski accepted this revision.
krytarowski added inline comments.
This revision is now accepted and ready to land.


================
Comment at: unittests/Support/MemoryTest.cpp:90
+// MPROTECT prevents W+X mmaps
+#define CHECK_UNSUPPORTED() \
+  do { \
----------------
mgorny wrote:
> krytarowski wrote:
> > mgorny wrote:
> > > krytarowski wrote:
> > > > I would further refactor this into:
> > > > 
> > > > ```
> > > > // MPROTECT prevents W+X mmaps
> > > > #define CHECK_UNSUPPORTED(f) \
> > > > 	​  do { \
> > > > 	​    if ((f & (Memory::MF_WRITE | Memory::MF_EXEC)) && \
> > > > 	​        IsMPROTECT()) \
> > > > 	​      return; \
> > > > 	​  } while (0)
> > > > ```
> > > > 
> > > > And use `CHECK_UNSUPPORTED(Flags)` or `CHECK_UNSUPPORTED(Flags | Memory::MF_WRITE)` for `EnabledWrite`.
> > > Nah, that's too smart. Would go against KISS.
> > How about?
> > 
> > ```
> > #define CHECK_UNSUPPORTED_RAW(f) \
> >   ​  do { \
> >   ​    if ((f & (Memory::MF_WRITE | Memory::MF_EXEC)) && \
> >   ​        IsMPROTECT()) \
> >   ​      return; \
> >   ​  } while (0)
> > ```
> > 
> > and
> > 
> > `#define CHECK_UNSUPPORTED() CHECK_UNSUPPORTED_RAW(Flag)`
> > 
> > I would like to get rid of open-coding check in `EnabledWrite`.
> > 
> > In EnableWrite it would be: `CHECK_UNSUPPORTED_RAW(Flags | Memory::MF_WRITE)`.
> Same problem — it says 'write' when it checks for 'exec'. We could get away by making an extra function with RHS specified via parameter with a default but I don't think it's worth the hassle. The purpose of the macro is to avoid repeating the most common version, not solve all the problems. Note that some functions have additional 'skip conditions' which we leave intact.
OK, I defer this to others in review.


https://reviews.llvm.org/D54080