Skip to content

Commit

Permalink
deps: V8: cherry-pick 4e077ff0444a
Browse files Browse the repository at this point in the history
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 <[email protected]>
    Reviewed-by: Michael Lippautz <[email protected]>
    Reviewed-by: Clemens Backes <[email protected]>
    Cr-Commit-Position: refs/heads/master@{#70379}

PR-URL: #35986
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Beth Griggs <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
  • Loading branch information
oleavr authored and codebytere committed Nov 22, 2020
1 parent 098a5b1 commit 4b7ba11
Show file tree
Hide file tree
Showing 11 changed files with 39 additions and 16 deletions.
2 changes: 1 addition & 1 deletion common.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -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 #####

Expand Down
10 changes: 10 additions & 0 deletions deps/v8/src/base/page-allocator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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));
}
Expand Down
1 change: 1 addition & 0 deletions deps/v8/src/base/platform/platform-cygwin.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
1 change: 1 addition & 0 deletions deps/v8/src/base/platform/platform-fuchsia.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
12 changes: 5 additions & 7 deletions deps/v8/src/base/platform/platform-posix.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
}

Expand Down
1 change: 1 addition & 0 deletions deps/v8/src/base/platform/platform-win32.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
5 changes: 4 additions & 1 deletion deps/v8/src/base/platform/platform.h
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
10 changes: 6 additions & 4 deletions deps/v8/src/utils/allocation.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
6 changes: 4 additions & 2 deletions deps/v8/src/utils/allocation.h
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand All @@ -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.
Expand Down
6 changes: 5 additions & 1 deletion deps/v8/src/wasm/wasm-code-manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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 {};
Expand Down
1 change: 1 addition & 0 deletions deps/v8/test/cctest/cctest.status
Original file line number Diff line number Diff line change
Expand Up @@ -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],
Expand Down

0 comments on commit 4b7ba11

Please sign in to comment.