[mac] Set MAP_JIT only when necessary

This is a "minimal" change to achieve the required goal: seeing that
there is only one place where we need to indicate that memory should
be reserved with MAP_JIT, we can add a value to the Permissions enum
instead of adding a second, orthogonal parameter.
That way we avoid changing public API functions, which makes this CL
easier to undo once we have platform-independent w^x in Wasm.

Bug: chromium:1117591
Change-Id: I6333d69ab29d5900c689f08dcc892a5f1c1159b8
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2435365
Commit-Queue: Jakob Kummerow <jkummerow@chromium.org>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#70379}

src/base/page-allocator.cc

@@ -21,6 +21,8 @@ STATIC_ASSERT_ENUM(PageAllocator::kReadWriteExecute,
                    base::OS::MemoryPermission::kReadWriteExecute);
 STATIC_ASSERT_ENUM(PageAllocator::kReadExecute,
                    base::OS::MemoryPermission::kReadExecute);
+STATIC_ASSERT_ENUM(PageAllocator::kNoAccessWillJitLater,
+                   base::OS::MemoryPermission::kNoAccessWillJitLater);
 
 #undef STATIC_ASSERT_ENUM
 
@@ -38,6 +40,14 @@ void* PageAllocator::GetRandomMmapAddr() {
 
 void* PageAllocator::AllocatePages(void* hint, size_t size, size_t alignment,
                                    PageAllocator::Permission access) {
+#if !(V8_OS_MACOSX && V8_HOST_ARCH_ARM64 && defined(MAP_JIT))
+  // kNoAccessWillJitLater is only used on Apple Silicon. Map it to regular
+  // kNoAccess on other platforms, so code doesn't have to handle both enum
+  // values.
+  if (access == PageAllocator::kNoAccessWillJitLater) {
+    access = PageAllocator::kNoAccess;
+  }
+#endif
   return base::OS::Allocate(hint, size, alignment,
                             static_cast<base::OS::MemoryPermission>(access));
 }

src/base/platform/platform-cygwin.cc

@@ -33,6 +33,7 @@ namespace {
 DWORD GetProtectionFromMemoryPermission(OS::MemoryPermission access) {
   switch (access) {
     case OS::MemoryPermission::kNoAccess:
+    case OS::MemoryPermission::kNoAccessWillJitLater:
       return PAGE_NOACCESS;
     case OS::MemoryPermission::kRead:
       return PAGE_READONLY;

src/base/platform/platform-fuchsia.cc

@@ -18,6 +18,7 @@ namespace {
 uint32_t GetProtectionFromMemoryPermission(OS::MemoryPermission access) {
   switch (access) {
     case OS::MemoryPermission::kNoAccess:
+    case OS::MemoryPermission::kNoAccessWillJitLater:
       return 0;  // no permissions
     case OS::MemoryPermission::kRead:
       return ZX_VM_PERM_READ;

src/base/platform/platform-posix.cc

@@ -125,6 +125,7 @@ const int kMmapFdOffset = 0;
 int GetProtectionFromMemoryPermission(OS::MemoryPermission access) {
   switch (access) {
     case OS::MemoryPermission::kNoAccess:
+    case OS::MemoryPermission::kNoAccessWillJitLater:
       return PROT_NONE;
     case OS::MemoryPermission::kRead:
       return PROT_READ;
@@ -151,15 +152,12 @@ int GetFlagsForMemoryPermission(OS::MemoryPermission access,
 #if V8_OS_QNX
     flags |= MAP_LAZY;
 #endif  // V8_OS_QNX
-#if V8_OS_MACOSX && V8_HOST_ARCH_ARM64 && defined(MAP_JIT) && \
-    !defined(V8_OS_IOS)
-    // TODO(jkummerow): using the V8_OS_IOS define is a crude approximation
-    // of the fact that we don't want to set the MAP_JIT flag when
-    // FLAG_jitless == true, as src/base/ doesn't know any flags.
-    // TODO(crbug.com/1117591): This is only needed for code spaces.
-    flags |= MAP_JIT;
-#endif
   }
+#if V8_OS_MACOSX && V8_HOST_ARCH_ARM64 && defined(MAP_JIT)
+  if (access == OS::MemoryPermission::kNoAccessWillJitLater) {
+    flags |= MAP_JIT;
+  }
+#endif
   return flags;
 }
 

src/base/platform/platform-win32.cc

@@ -752,6 +752,7 @@ namespace {
 DWORD GetProtectionFromMemoryPermission(OS::MemoryPermission access) {
   switch (access) {
     case OS::MemoryPermission::kNoAccess:
+    case OS::MemoryPermission::kNoAccessWillJitLater:
       return PAGE_NOACCESS;
     case OS::MemoryPermission::kRead:
       return PAGE_READONLY;

src/base/platform/platform.h

@@ -172,7 +172,10 @@ class V8_BASE_EXPORT OS {
     kReadWrite,
     // TODO(hpayer): Remove this flag. Memory should never be rwx.
     kReadWriteExecute,
-    kReadExecute
+    kReadExecute,
+    // TODO(jkummerow): Remove this when Wasm has a platform-independent
+    // w^x implementation.
+    kNoAccessWillJitLater
   };
 
   static bool HasLazyCommits();

src/utils/allocation.cc

@@ -213,15 +213,17 @@ bool OnCriticalMemoryPressure(size_t length) {
 VirtualMemory::VirtualMemory() = default;
 
 VirtualMemory::VirtualMemory(v8::PageAllocator* page_allocator, size_t size,
-                             void* hint, size_t alignment)
+                             void* hint, size_t alignment, JitPermission jit)
     : page_allocator_(page_allocator) {
   DCHECK_NOT_NULL(page_allocator);
   DCHECK(IsAligned(size, page_allocator_->CommitPageSize()));
   size_t page_size = page_allocator_->AllocatePageSize();
   alignment = RoundUp(alignment, page_size);
-  Address address = reinterpret_cast<Address>(
-      AllocatePages(page_allocator_, hint, RoundUp(size, page_size), alignment,
-                    PageAllocator::kNoAccess));
+  PageAllocator::Permission permissions =
+      jit == kMapAsJittable ? PageAllocator::kNoAccessWillJitLater
+                            : PageAllocator::kNoAccess;
+  Address address = reinterpret_cast<Address>(AllocatePages(
+      page_allocator_, hint, RoundUp(size, page_size), alignment, permissions));
   if (address != kNullAddress) {
     DCHECK(IsAligned(address, alignment));
     region_ = base::AddressRegion(address, size);

src/utils/allocation.h

@@ -156,6 +156,8 @@ V8_EXPORT_PRIVATE bool OnCriticalMemoryPressure(size_t length);
 // Represents and controls an area of reserved memory.
 class VirtualMemory final {
  public:
+  enum JitPermission { kNoJit, kMapAsJittable };
+
   // Empty VirtualMemory object, controlling no reserved memory.
   V8_EXPORT_PRIVATE VirtualMemory();
 
@@ -164,8 +166,8 @@ class VirtualMemory final {
   // size. The |size| must be aligned with |page_allocator|'s commit page size.
   // This may not be at the position returned by address().
   V8_EXPORT_PRIVATE VirtualMemory(v8::PageAllocator* page_allocator,
-                                  size_t size, void* hint,
-                                  size_t alignment = 1);
+                                  size_t size, void* hint, size_t alignment = 1,
+                                  JitPermission jit = kNoJit);
 
   // Construct a virtual memory by assigning it some already mapped address
   // and size.

src/wasm/wasm-code-manager.cc

@@ -1646,7 +1646,11 @@ VirtualMemory WasmCodeManager::TryAllocate(size_t size, void* hint) {
   if (!BackingStore::ReserveAddressSpace(size)) return {};
   if (hint == nullptr) hint = page_allocator->GetRandomMmapAddr();
 
-  VirtualMemory mem(page_allocator, size, hint, allocate_page_size);
+  // When we start exposing Wasm in jitless mode, then the jitless flag
+  // will have to determine whether we set kMapAsJittable or not.
+  DCHECK(!FLAG_jitless);
+  VirtualMemory mem(page_allocator, size, hint, allocate_page_size,
+                    VirtualMemory::kMapAsJittable);
   if (!mem.IsReserved()) {
     BackingStore::ReleaseReservation(size);
     return {};

test/cctest/cctest.status

@@ -503,6 +503,7 @@
   'test-jump-table-assembler/*': [SKIP],
   'test-gc/*': [SKIP],
   'test-grow-memory/*': [SKIP],
+  'test-liftoff-inspection/*': [SKIP],
   'test-run-wasm-64/*': [SKIP],
   'test-run-wasm-asmjs/*': [SKIP],
   'test-run-wasm-atomics64/*': [SKIP],