[PATCH] D48445: [sanitizer] Use "fast mmap" kernel flag for shadow memory on macOS 10.13.4+

Dan Liew via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jul 26 03:56:04 PDT 2018


delcypher added inline comments.


================
Comment at: lib/sanitizer_common/sanitizer_posix_libcdep.cc:337
   uptr PageSize = GetPageSizeCached();
-  uptr p = internal_mmap((void *)(fixed_addr & ~(PageSize - 1)),
-                         RoundUpTo(size, PageSize), PROT_READ | PROT_WRITE,
-                         flags, fd, 0);
+  uptr p = internal_mmap_shadow((void *)(fixed_addr & ~(PageSize - 1)),
+                                RoundUpTo(size, PageSize),
----------------
kubamracek wrote:
> delcypher wrote:
> > The name `internal_mmap_shadow(...)` implies that this function is only for use in allocating shadow regions. However the name `MmapFixedNoReserve()` doesn't imply that. However, looking at the uses 
> >  of `MmapFixedNoReserve()` it looks like they are only used for allocating "shadow" memory currently, so this is probably okay but maybe it should be named `internal_mmap_large()` instead because semantically that's what differentiates it from `internal_mmap()`?
> “large” to me doesn’t imply what I mean here. A 10 MB mmap is “large”. But “shadow” regions imply they are proportional in size to the entire VM address space.
Ok. By "large" I mean extremely "large" but I guess that could also be confusing. I guess the remaining thing to address is `MmapFixedNoReserve()` calling `internal_mmap_shadow()`. I don't like `MmapFixedNoReserve()` (which has a name that does not imply it is for shadow memory) calling a function whose name implies it is for use for shadow memory.

A comment on `MmapFixedNoReserve()` saying it is meant for allocating shadow memory might be sufficient.


https://reviews.llvm.org/D48445





More information about the llvm-commits mailing list