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

[kubamracek]

Summary

This speeds up process startup and teardown and also reduces lock contention when running multiple ASanified/TSanified processes simultaneously. Should greatly improve lit testing time.

[vitalybuka]

lib/sanitizer_common/sanitizer_mac.cc

764

Would you like to resolve this by moving into your new function?
Probably in the next patch.

[kubamracek]

lib/sanitizer_common/sanitizer_mac.cc

764

Yes, nice catch! Will send another patch after this one.

[delcypher]

lib/sanitizer_common/sanitizer_posix_libcdep.cc

331 ↗

(On Diff #152344)

Is there a reason the use_xnu_fast_mmap isn't pushed into the Darwin implementation of internal_mmap()? Wouldn't it make more sense to be there given that xnu_fast_mmap is Darwin specific?

391 ↗

(On Diff #152344)

Again can't we move this into the Darwin implementation of internal_mmap(). This will also mean we avoid duplicating code.

lib/tsan/rtl/tsan_rtl.cc

362–364

__sanitizer::InitializePlatformEarly() is different from InitializePlatformEarly() that's called immediately afterwards?

[kubamracek]

lib/sanitizer_common/sanitizer_posix_libcdep.cc

331 ↗

(On Diff #152344)

We don't want to use this flag in internal_mmap, that would apply to all mmap calls. We really only want this flag for the shadow memory. I can add another argument to internal_mmap, but that looks to me like it would complicate things more than clarify them.

lib/tsan/rtl/tsan_rtl.cc

362–364

Yes. Tsan has __tsan::InitializePlatformEarly.

[delcypher]

lib/sanitizer_common/sanitizer_posix_libcdep.cc

331 ↗

(On Diff #152344)

Why wouldn't you want to take the fast path for all mmap calls? I suppose it's the fast path for large allocations but not necessarily smaller ones?

If we're going with this approach there should probably be a comment or note in the commit message explaining why the decision to use fast_mmap has to belong in these locations and not in internal_mmap.

I'd kind of feel that the fast mmap option really should be a flag because that would allow us to pass that flag and then handle it in internal_mmap() (i.e. on non Darwin platforms, strip out the flag and use it appropriately on Darwin). That would avoid having to change all call sites to internal_mmap().

The problem is mmap flag values are likely reserved and adding our own special value makes me feel uneasy because if another platform (or even Darwin) ever uses that flag value to mean something else, then semantics of internal_mmap will be broken. So I guess we have to do something else.

lib/tsan/rtl/tsan_rtl.cc

362–364

Ah. Personally I'd rather it read as

__sanitizer::InitializePlatformEarly();
__tsan::InitializePlatformEarly();

for clarity but I won't insist on this if you really want to write it how it is now.

[kubamracek]

lib/sanitizer_common/sanitizer_posix_libcdep.cc

331 ↗

(On Diff #152344)

I'll add the comment with a bit of explanation, but I think you won't be fully satisfied because I don't understand what exactly the behavior of this flag is either. It was added into xnu specifically for the sanitizers and it's supposed to only be used for the shadow memory (giant region sizes), and not for other memory allocations. I don't know why.

The interface of internal_mmap is exactly the same as mmap, so I don't think we want to change it, or pass "fd" into "flags".

lib/tsan/rtl/tsan_rtl.cc

362–364

Sure, I'll change it.

[delcypher]

lib/sanitizer_common/sanitizer_posix_libcdep.cc

338 ↗

(On Diff #152344)

Side note. This implementation will prevent the VM tag being set (see https://reviews.llvm.org/D49603 ) with fast mmap is being used. This needs to be fixed.

391 ↗

(On Diff #152344)

Side note. This implementation will prevent the VM tag being set (see https://reviews.llvm.org/D49603 ) with fast mmap is being used. This needs to be fixed.

[kubamracek]

Updating patch.

[delcypher]

lib/sanitizer_common/sanitizer_mac.cc

138

Thanks for fixing the VM tag issue!

lib/sanitizer_common/sanitizer_posix_libcdep.cc

337 ↗

(On Diff #157177)

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()?

[kubamracek]

lib/sanitizer_common/sanitizer_posix_libcdep.cc

337 ↗

(On Diff #157177)

“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.

[delcypher]

lib/sanitizer_common/sanitizer_posix_libcdep.cc

337 ↗

(On Diff #157177)

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.

[kubamracek]

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.

I've updated the patch to now split out MmapFixedNoReserve into MmapFixedNoReserveShadow. The latter is used when allocating shadow regions.

[delcypher]

Other than the kXnuFastMmapFd declaration. LGTM.

lib/sanitizer_common/sanitizer_mac.cc

131

Any particular reason to use a macro rather than static const int kXnuFastMmapFd = 0x4;? The later style seems used more in the sanitizer code base (at least the bits I've seen). This is a very minor issue though so I'll leave changing this to your discretion.

[kubamracek]

Changing the #define to a constant. Rebasing patch.

[kubamracek]

Adding MmapFixedNoAccessShadow.

[kubamracek]

This revision is marked as "Accepted", but the last round of changes are touching other files than before. @kcc, @dvyukov, @eugenis, are you okay with these changes?

[eugenis]

I don't like the new Mmap*Shadow functions. The number of different Mmap* functions is growing exponentially with any new feature, let's use flags and/or arguments instead.

It is also not clear from the user side what this Shadow suffix means exactly. If that's an optimization for huge mappings, can't we define a size threshold when it is more efficient than the alternative, and use it under the hood in common Mmap* calls?

lib/sanitizer_common/sanitizer_mac.cc

119

Leftover debug? More of the same below.

[kubamracek]

Okay, simplifying this patch back, and only applying the "fast mmap" flag based on a size threshold.

[eugenis]

LGTM

[kubamracek]

@earthdok

r346264 | d0k | 2018-11-06 12:42:19 -0800 (Tue, 06 Nov 2018) | 11 lines

[dfsan] Fix build after r346262

compiler-rt/lib/dfsan/dfsan.cc:426:3: error: call to 'InitializePlatformEarly' is ambiguous
  InitializePlatformEarly();
  ^~~~~~~~~~~~~~~~~~~~~~~
compiler-rt/lib/dfsan/../sanitizer_common/sanitizer_common.h:901:6: note: candidate function
void InitializePlatformEarly();
     ^
compiler-rt/lib/dfsan/dfsan.cc:391:13: note: candidate function
static void InitializePlatformEarly() {
            ^

Thanks and sorry about this! Please let me know if the failure is still there!

lib/asan/asan_rtl.cc

static void AsanInitInternal() {
   SetCheckFailedCallback(AsanCheckFailed);
   SetPrintfAndReportCallback(AppendToErrorMessageBuffer);
 
   __sanitizer_set_report_path(common_flags()->log_path);
 
   __asan_option_detect_stack_use_after_return =
       flags()->detect_stack_use_after_return;
 
+  __sanitizer::InitializePlatformEarly();
+
   // Re-exec ourselves if we need to set additional env or command line args.
   MaybeReexec();
 
   // Setup internal allocator callback.
   SetLowLevelAllocateMinAlignment(SHADOW_GRANULARITY);
   SetLowLevelAllocateCallback(OnLowLevelAllocate);
 
   InitializeAsanInterceptors();

lib/sanitizer_common/sanitizer_common.h

  private:
   // Platform specific initialization.
   void InitPcSpBp();
   uptr GetAddress() const;
   WriteFlag GetWriteFlag() const;
   bool IsMemoryAccess() const;
 };
 
+void InitializePlatformEarly();
 void MaybeReexec();
 
 template <typename Fn>
 class RunOnDestruction {
  public:
   explicit RunOnDestruction(Fn fn) : fn_(fn) {}
   ~RunOnDestruction() { fn_(); }
 

lib/sanitizer_common/sanitizer_fuchsia.cc

void GetThreadStackTopAndBottom(bool, uptr *stack_top, uptr *stack_bottom) {
   size_t size;
   CHECK_EQ(pthread_attr_getstack(&attr, &base, &size), 0);
   CHECK_EQ(pthread_attr_destroy(&attr), 0);
 
   *stack_bottom = reinterpret_cast<uptr>(base);
   *stack_top = *stack_bottom + size;
 }
 
+void InitializePlatformEarly() {}
 void MaybeReexec() {}
 void CheckASLR() {}
 void PlatformPrepareForSandboxing(__sanitizer_sandbox_arguments *args) {}
 void DisableCoreDumperIfNecessary() {}
 void InstallDeadlySignalHandlers(SignalHandlerType handler) {}
 void SetAlternateSignalStack() {}
 void UnsetAlternateSignalStack() {}
 void InitTlsSize() {}

lib/sanitizer_common/sanitizer_linux.cc

# endif
   *sp = ucontext->uc_mcontext.gregs[15];
 #else
 # error "Unsupported arch"
 #endif
 }
 
 void SignalContext::InitPcSpBp() { GetPcSpBp(context, &pc, &sp, &bp); }
 
+void InitializePlatformEarly() {
+  // Do nothing.
+}
+
 void MaybeReexec() {
   // No need to re-exec on Linux.
 }
 
 void CheckASLR() {
 #if SANITIZER_NETBSD
   int mib[3];
   int paxflags;

lib/sanitizer_common/sanitizer_mac.cc

 
 // ---------------------- sanitizer_libc.h
 
 // From <mach/vm_statistics.h>, but not on older OSs.
 #ifndef VM_MEMORY_SANITIZER
 #define VM_MEMORY_SANITIZER 99
 #endif
 
+// XNU on Darwin provides a mmap flag that optimizes allocation/deallocation of
+// giant memory regions (i.e. shadow memory regions).
+#define kXnuFastMmapFd 0x4
+static size_t kXnuFastMmapThreshold = 2 << 30; // 2 GB
+static bool use_xnu_fast_mmap = false;
+
 uptr internal_mmap(void *addr, size_t length, int prot, int flags,
                    int fd, u64 offset) {
-  if (fd == -1) fd = VM_MAKE_TAG(VM_MEMORY_SANITIZER);
+  if (fd == -1) {
+    fd = VM_MAKE_TAG(VM_MEMORY_SANITIZER);
+    if (length >= kXnuFastMmapThreshold) {
+      if (use_xnu_fast_mmap) fd |= kXnuFastMmapFd;
+    }
+  }
   if (&__mmap) return (uptr)__mmap(addr, length, prot, flags, fd, offset);
   return (uptr)mmap(addr, length, prot, flags, fd, offset);
 }
 
 uptr internal_munmap(void *addr, uptr length) {
   if (&__munmap) return __munmap(addr, length);
   return munmap(addr, length);
 }
 
 int internal_mprotect(void *addr, uptr length, int prot) {
   return mprotect(addr, length, prot);
 }
 
 uptr internal_close(fd_t fd) {
   return close(fd);
 }
 
 uptr internal_open(const char *filename, int flags) {
   return open(filename, flags);
 }
 
 uptr internal_open(const char *filename, int flags, u32 mode) {
# elif defined(__i386__)
   *sp = ucontext->uc_mcontext->__ss.__esp;
 # else
 # error "Unknown architecture"
 # endif
 }
 
 void SignalContext::InitPcSpBp() { GetPcSpBp(context, &pc, &sp, &bp); }
 
+void InitializePlatformEarly() {
+  // Only use xnu_fast_mmap when on x86_64 and the OS supports it.
+  use_xnu_fast_mmap =
+#if defined(__x86_64__)
+      GetMacosVersion() >= MACOS_VERSION_HIGH_SIERRA_DOT_RELEASE_4;
+#else
+      false;
+#endif
+}
+
 #if !SANITIZER_GO
 static const char kDyldInsertLibraries[] = "DYLD_INSERT_LIBRARIES";
 LowLevelAllocator allocator_for_env;
 
 // Change the value of the env var |name|, leaking the original value.
 // If |name_value| is NULL, the variable is deleted from the environment,
 // otherwise the corresponding "NAME=value" string is replaced with
 // |name_value|.
bool DyldNeedsEnvVariable() {
   // DYLD_INSERT_LIBRARIES is not set. However, checking OS version via
   // GetMacosVersion() doesn't work for the simulator. Let's instead check
   // `dyldVersionNumber`, which is exported by dyld, against a known version
   // number from the first OS release where this appeared.
   return dyldVersionNumber < kMinDyldVersionWithAutoInterposition;
 }
 
 void MaybeReexec() {
   // FIXME: This should really live in some "InitializePlatform" method.
   MonotonicNanoTime();
 
   if (ReexecDisabled()) return;
 
   // Make sure the dynamic runtime library is preloaded so that the
   // wrappers work. If it is not, set DYLD_INSERT_LIBRARIES and re-exec
   // ourselves.
   Dl_info info;

lib/sanitizer_common/sanitizer_rtems.cc

void GetThreadStackAndTls(bool main, uptr *stk_addr, uptr *stk_size,
                           uptr *tls_addr, uptr *tls_size) {
   uptr stack_top, stack_bottom;
   GetThreadStackTopAndBottom(main, &stack_top, &stack_bottom);
   *stk_addr = stack_bottom;
   *stk_size = stack_top - stack_bottom;
   *tls_addr = *tls_size = 0;
 }
 
+void InitializePlatformEarly() {}
 void MaybeReexec() {}
 void CheckASLR() {}
 void DisableCoreDumperIfNecessary() {}
 void InstallDeadlySignalHandlers(SignalHandlerType handler) {}
 void SetAlternateSignalStack() {}
 void UnsetAlternateSignalStack() {}
 void InitTlsSize() {}
 

lib/sanitizer_common/sanitizer_win.cc

 uptr ReadLongProcessName(/*out*/char *buf, uptr buf_len) {
   return ReadBinaryName(buf, buf_len);
 }
 
 void CheckVMASize() {
   // Do nothing.
 }
 
+void InitializePlatformEarly() {
+  // Do nothing.
+}
+
 void MaybeReexec() {
   // No need to re-exec on Windows.
 }
 
 void CheckASLR() {
   // Do nothing
 }
 

lib/tsan/rtl/tsan_rtl.cc

void Initialize(ThreadState *thr) {
   SetCheckFailedCallback(TsanCheckFailed);
 
   ctx = new(ctx_placeholder) Context;
   const char *options = GetEnv(SANITIZER_GO ? "GORACE" : "TSAN_OPTIONS");
   CacheBinaryName();
   CheckASLR();
   InitializeFlags(&ctx->flags, options);
   AvoidCVE_2016_2143();
-  InitializePlatformEarly();
+  __sanitizer::InitializePlatformEarly();
+  __tsan::InitializePlatformEarly();
+
 #if !SANITIZER_GO
   // Re-exec ourselves if we need to set additional env or command line args.
   MaybeReexec();
 
   InitializeAllocator();
   ReplaceSystemMalloc();
 #endif
   if (common_flags()->detect_deadlocks)