From 4b7ba11d675a2309e63ba9d0e046aea362b88e65 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ole=20Andr=C3=A9=20Vadla=20Ravn=C3=A5s?= Date: Fri, 6 Nov 2020 02:46:21 +0100 Subject: [PATCH] deps: V8: cherry-pick 4e077ff0444a Original commit message: [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 Reviewed-by: Michael Lippautz Reviewed-by: Clemens Backes Cr-Commit-Position: refs/heads/master@{#70379} PR-URL: https://github.com/nodejs/node/pull/35986 Reviewed-By: Anna Henningsen Reviewed-By: Richard Lau Reviewed-By: Michael Dawson Reviewed-By: Rich Trott Reviewed-By: Beth Griggs Reviewed-By: Jiawen Geng --- common.gypi | 2 +- deps/v8/src/base/page-allocator.cc | 10 ++++++++++ deps/v8/src/base/platform/platform-cygwin.cc | 1 + deps/v8/src/base/platform/platform-fuchsia.cc | 1 + deps/v8/src/base/platform/platform-posix.cc | 12 +++++------- deps/v8/src/base/platform/platform-win32.cc | 1 + deps/v8/src/base/platform/platform.h | 5 ++++- deps/v8/src/utils/allocation.cc | 10 ++++++---- deps/v8/src/utils/allocation.h | 6 ++++-- deps/v8/src/wasm/wasm-code-manager.cc | 6 +++++- deps/v8/test/cctest/cctest.status | 1 + 11 files changed, 39 insertions(+), 16 deletions(-) diff --git a/common.gypi b/common.gypi index cae6662364984c..154ba97d9f46cf 100644 --- a/common.gypi +++ b/common.gypi @@ -36,7 +36,7 @@ # Reset this number to 0 on major V8 upgrades. # Increment by one for each non-official patch applied to deps/v8. - 'v8_embedder_string': '-node.19', + 'v8_embedder_string': '-node.20', ##### V8 defaults for Node.js ##### diff --git a/deps/v8/src/base/page-allocator.cc b/deps/v8/src/base/page-allocator.cc index 98b2c690960336..62dfc94a83be07 100644 --- a/deps/v8/src/base/page-allocator.cc +++ b/deps/v8/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(access)); } diff --git a/deps/v8/src/base/platform/platform-cygwin.cc b/deps/v8/src/base/platform/platform-cygwin.cc index 92a5fbe490f4c3..b9da2f1cd592db 100644 --- a/deps/v8/src/base/platform/platform-cygwin.cc +++ b/deps/v8/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; diff --git a/deps/v8/src/base/platform/platform-fuchsia.cc b/deps/v8/src/base/platform/platform-fuchsia.cc index fa175c39177aea..35a508a140ebd7 100644 --- a/deps/v8/src/base/platform/platform-fuchsia.cc +++ b/deps/v8/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; diff --git a/deps/v8/src/base/platform/platform-posix.cc b/deps/v8/src/base/platform/platform-posix.cc index b765ad1897a3cf..4b49968baa053f 100644 --- a/deps/v8/src/base/platform/platform-posix.cc +++ b/deps/v8/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. + } +#if V8_OS_MACOSX && V8_HOST_ARCH_ARM64 && defined(MAP_JIT) + if (access == OS::MemoryPermission::kNoAccessWillJitLater) { flags |= MAP_JIT; -#endif } +#endif return flags; } diff --git a/deps/v8/src/base/platform/platform-win32.cc b/deps/v8/src/base/platform/platform-win32.cc index 5db3e343103dd0..6be63dee137a81 100644 --- a/deps/v8/src/base/platform/platform-win32.cc +++ b/deps/v8/src/base/platform/platform-win32.cc @@ -753,6 +753,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; diff --git a/deps/v8/src/base/platform/platform.h b/deps/v8/src/base/platform/platform.h index d5f59d1d7a8d8a..c4895a5b274374 100644 --- a/deps/v8/src/base/platform/platform.h +++ b/deps/v8/src/base/platform/platform.h @@ -167,7 +167,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(); diff --git a/deps/v8/src/utils/allocation.cc b/deps/v8/src/utils/allocation.cc index 6169acbfd6687a..022ac82ea6fa28 100644 --- a/deps/v8/src/utils/allocation.cc +++ b/deps/v8/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
( - AllocatePages(page_allocator_, hint, RoundUp(size, page_size), alignment, - PageAllocator::kNoAccess)); + PageAllocator::Permission permissions = + jit == kMapAsJittable ? PageAllocator::kNoAccessWillJitLater + : PageAllocator::kNoAccess; + Address address = reinterpret_cast
(AllocatePages( + page_allocator_, hint, RoundUp(size, page_size), alignment, permissions)); if (address != kNullAddress) { DCHECK(IsAligned(address, alignment)); region_ = base::AddressRegion(address, size); diff --git a/deps/v8/src/utils/allocation.h b/deps/v8/src/utils/allocation.h index 7106b1c749a893..a82012310b8efe 100644 --- a/deps/v8/src/utils/allocation.h +++ b/deps/v8/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. diff --git a/deps/v8/src/wasm/wasm-code-manager.cc b/deps/v8/src/wasm/wasm-code-manager.cc index 8bc6a353405831..fc657d634dba7c 100644 --- a/deps/v8/src/wasm/wasm-code-manager.cc +++ b/deps/v8/src/wasm/wasm-code-manager.cc @@ -1610,7 +1610,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 {}; diff --git a/deps/v8/test/cctest/cctest.status b/deps/v8/test/cctest/cctest.status index 40580c08cd3157..1f93ec28c299ff 100644 --- a/deps/v8/test/cctest/cctest.status +++ b/deps/v8/test/cctest/cctest.status @@ -496,6 +496,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],