Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cherry-pick V8 fixes needed for Apple Silicon #35986

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion common.gypi
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.17',
'v8_embedder_string': '-node.21',

##### V8 defaults for Node.js #####

Expand Down
1 change: 1 addition & 0 deletions deps/v8/BUILD.gn
Expand Up @@ -3237,6 +3237,7 @@ v8_source_set("v8_base_without_compiler") {
"src/wasm/baseline/liftoff-compiler.cc",
"src/wasm/baseline/liftoff-compiler.h",
"src/wasm/baseline/liftoff-register.h",
"src/wasm/code-space-access.h",
"src/wasm/compilation-environment.h",
"src/wasm/decoder.h",
"src/wasm/function-body-decoder-impl.h",
Expand Down
8 changes: 7 additions & 1 deletion deps/v8/include/v8-platform.h
Expand Up @@ -384,7 +384,13 @@ class PageAllocator {
kReadWrite,
// TODO(hpayer): Remove this flag. Memory should never be rwx.
kReadWriteExecute,
kReadExecute
kReadExecute,
// Set this when reserving memory that will later require kReadWriteExecute
// permissions. The resulting behavior is platform-specific, currently
// this is used to set the MAP_JIT flag on Apple Silicon.
// TODO(jkummerow): Remove this when Wasm has a platform-independent
// w^x implementation.
kNoAccessWillJitLater
};

/**
Expand Down
14 changes: 14 additions & 0 deletions deps/v8/src/base/page-allocator.cc
Expand Up @@ -6,6 +6,10 @@

#include "src/base/platform/platform.h"

#if V8_OS_MACOSX
#include <sys/mman.h> // For MAP_JIT.
#endif

namespace v8 {
namespace base {

Expand All @@ -21,6 +25,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 +44,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
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
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
6 changes: 6 additions & 0 deletions deps/v8/src/base/platform/platform-posix.cc
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 Down Expand Up @@ -152,6 +153,11 @@ int GetFlagsForMemoryPermission(OS::MemoryPermission access,
flags |= MAP_LAZY;
#endif // V8_OS_QNX
}
#if V8_OS_MACOSX && V8_HOST_ARCH_ARM64 && defined(MAP_JIT)
if (access == OS::MemoryPermission::kNoAccessWillJitLater) {
flags |= MAP_JIT;
}
#endif
return flags;
}

Expand Down
1 change: 1 addition & 0 deletions deps/v8/src/base/platform/platform-win32.cc
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
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
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
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
69 changes: 69 additions & 0 deletions deps/v8/src/wasm/code-space-access.h
@@ -0,0 +1,69 @@
// Copyright 2020 the V8 project authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#ifndef V8_WASM_CODE_SPACE_ACCESS_H_
#define V8_WASM_CODE_SPACE_ACCESS_H_

#include "src/base/build_config.h"
#include "src/base/macros.h"
#include "src/common/globals.h"

namespace v8 {
namespace internal {

#if defined(V8_OS_MACOSX) && defined(V8_HOST_ARCH_ARM64)

// Ignoring this warning is considered better than relying on
// __builtin_available.
#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wunguarded-availability-new"
inline void SwitchMemoryPermissionsToWritable() {
pthread_jit_write_protect_np(0);
}
inline void SwitchMemoryPermissionsToExecutable() {
pthread_jit_write_protect_np(1);
}
#pragma clang diagnostic pop

namespace wasm {

class CodeSpaceWriteScope {
public:
// TODO(jkummerow): Background threads could permanently stay in
// writable mode; only the main thread has to switch back and forth.
CodeSpaceWriteScope() {
if (code_space_write_nesting_level_ == 0) {
SwitchMemoryPermissionsToWritable();
}
code_space_write_nesting_level_++;
}
~CodeSpaceWriteScope() {
code_space_write_nesting_level_--;
if (code_space_write_nesting_level_ == 0) {
SwitchMemoryPermissionsToExecutable();
}
}

private:
static thread_local int code_space_write_nesting_level_;
};

#define CODE_SPACE_WRITE_SCOPE CodeSpaceWriteScope _write_access_;

} // namespace wasm

#else // Not Mac-on-arm64.

// Nothing to do, we map code memory with rwx permissions.
inline void SwitchMemoryPermissionsToWritable() {}
inline void SwitchMemoryPermissionsToExecutable() {}

#define CODE_SPACE_WRITE_SCOPE

#endif // V8_OS_MACOSX && V8_HOST_ARCH_ARM64

} // namespace internal
} // namespace v8

#endif // V8_WASM_CODE_SPACE_ACCESS_H_
21 changes: 20 additions & 1 deletion deps/v8/src/wasm/wasm-code-manager.cc
Expand Up @@ -6,6 +6,7 @@

#include <iomanip>

#include "src/base/build_config.h"
#include "src/base/iterator.h"
#include "src/base/macros.h"
#include "src/base/platform/platform.h"
Expand All @@ -21,6 +22,7 @@
#include "src/snapshot/embedded/embedded-data.h"
#include "src/utils/ostreams.h"
#include "src/utils/vector.h"
#include "src/wasm/code-space-access.h"
#include "src/wasm/compilation-environment.h"
#include "src/wasm/function-compiler.h"
#include "src/wasm/jump-table-assembler.h"
Expand All @@ -47,6 +49,10 @@ namespace wasm {

using trap_handler::ProtectedInstructionData;

#if defined(V8_OS_MACOSX) && defined(V8_HOST_ARCH_ARM64)
thread_local int CodeSpaceWriteScope::code_space_write_nesting_level_ = 0;
#endif

base::AddressRegion DisjointAllocationPool::Merge(
base::AddressRegion new_region) {
// Find the possible insertion position by identifying the first region whose
Expand Down Expand Up @@ -731,6 +737,7 @@ void WasmCodeAllocator::FreeCode(Vector<WasmCode* const> codes) {
// Zap code area and collect freed code regions.
DisjointAllocationPool freed_regions;
size_t code_size = 0;
CODE_SPACE_WRITE_SCOPE
for (WasmCode* code : codes) {
ZapCode(code->instruction_start(), code->instructions().size());
FlushInstructionCache(code->instruction_start(),
Expand Down Expand Up @@ -847,6 +854,7 @@ CompilationEnv NativeModule::CreateCompilationEnv() const {
}

WasmCode* NativeModule::AddCodeForTesting(Handle<Code> code) {
CODE_SPACE_WRITE_SCOPE
// For off-heap builtins, we create a copy of the off-heap instruction stream
// instead of the on-heap code object containing the trampoline. Ensure that
// we do not apply the on-heap reloc info to the off-heap instructions.
Expand Down Expand Up @@ -942,6 +950,7 @@ void NativeModule::UseLazyStub(uint32_t func_index) {
if (!lazy_compile_table_) {
uint32_t num_slots = module_->num_declared_functions;
WasmCodeRefScope code_ref_scope;
CODE_SPACE_WRITE_SCOPE
base::AddressRegion single_code_space_region;
{
base::MutexGuard guard(&allocation_mutex_);
Expand Down Expand Up @@ -1003,6 +1012,7 @@ std::unique_ptr<WasmCode> NativeModule::AddCodeWithCodeSpace(
const int code_comments_offset = desc.code_comments_offset;
const int instr_size = desc.instr_size;

CODE_SPACE_WRITE_SCOPE
memcpy(dst_code_bytes.begin(), desc.buffer,
static_cast<size_t>(desc.instr_size));

Expand Down Expand Up @@ -1138,6 +1148,7 @@ WasmCode* NativeModule::AddDeserializedCode(
Vector<const byte> protected_instructions_data,
Vector<const byte> reloc_info, Vector<const byte> source_position_table,
WasmCode::Kind kind, ExecutionTier tier) {
// CodeSpaceWriteScope is provided by the caller.
Vector<uint8_t> dst_code_bytes =
code_allocator_.AllocateForCode(this, instructions.size());
memcpy(dst_code_bytes.begin(), instructions.begin(), instructions.size());
Expand Down Expand Up @@ -1196,6 +1207,7 @@ WasmCode* NativeModule::CreateEmptyJumpTableInRegion(
Vector<uint8_t> code_space = code_allocator_.AllocateForCodeInRegion(
this, jump_table_size, region, allocator_lock);
DCHECK(!code_space.empty());
CODE_SPACE_WRITE_SCOPE
ZapCode(reinterpret_cast<Address>(code_space.begin()), code_space.size());
std::unique_ptr<WasmCode> code{
new WasmCode{this, // native_module
Expand All @@ -1221,6 +1233,7 @@ void NativeModule::PatchJumpTablesLocked(uint32_t slot_index, Address target) {
// The caller must hold the {allocation_mutex_}, thus we fail to lock it here.
DCHECK(!allocation_mutex_.TryLock());

CODE_SPACE_WRITE_SCOPE
for (auto& code_space_data : code_space_data_) {
DCHECK_IMPLIES(code_space_data.jump_table, code_space_data.far_jump_table);
if (!code_space_data.jump_table) continue;
Expand Down Expand Up @@ -1283,6 +1296,7 @@ void NativeModule::AddCodeSpace(
#endif // V8_OS_WIN64

WasmCodeRefScope code_ref_scope;
CODE_SPACE_WRITE_SCOPE
WasmCode* jump_table = nullptr;
WasmCode* far_jump_table = nullptr;
const uint32_t num_wasm_functions = module_->num_declared_functions;
Expand Down Expand Up @@ -1596,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 Expand Up @@ -1843,6 +1861,7 @@ std::vector<std::unique_ptr<WasmCode>> NativeModule::AddCompiledCode(
generated_code.reserve(results.size());

// Now copy the generated code into the code space and relocate it.
CODE_SPACE_WRITE_SCOPE
for (auto& result : results) {
DCHECK_EQ(result.code_desc.buffer, result.instr_buffer.get());
size_t code_size = RoundUp<kCodeAlignment>(result.code_desc.instr_size);
Expand Down
2 changes: 2 additions & 0 deletions deps/v8/src/wasm/wasm-serialization.cc
Expand Up @@ -13,6 +13,7 @@
#include "src/utils/ostreams.h"
#include "src/utils/utils.h"
#include "src/utils/version.h"
#include "src/wasm/code-space-access.h"
#include "src/wasm/function-compiler.h"
#include "src/wasm/module-compiler.h"
#include "src/wasm/module-decoder.h"
Expand Down Expand Up @@ -534,6 +535,7 @@ void NativeModuleDeserializer::ReadCode(int fn_index, Reader* reader) {
auto protected_instructions =
reader->ReadVector<byte>(protected_instructions_size);

CODE_SPACE_WRITE_SCOPE
WasmCode* code = native_module_->AddDeserializedCode(
fn_index, code_buffer, stack_slot_count, tagged_parameter_slots,
safepoint_table_offset, handler_table_offset, constant_pool_offset,
Expand Down
8 changes: 8 additions & 0 deletions deps/v8/test/cctest/cctest.status
Expand Up @@ -176,6 +176,13 @@
'test-debug/DebugBreakStackTrace': [PASS, SLOW],
}], # 'arch == arm64 and simulator_run'

['arch == arm64 and system == macos and not simulator_run', {
# printf, being a variadic function, has a different, stack-based ABI on
# Apple silicon. See:
# https://developer.apple.com/library/archive/documentation/Xcode/Conceptual/iPhoneOSABIReference/Articles/ARM64FunctionCallingConventions.html
'test-assembler-arm64/printf_no_preserve': [SKIP],
}], # arch == arm64 and system == macos and not simulator_run

##############################################################################
['variant == nooptimization and (arch == arm or arch == arm64) and simulator_run', {
# Slow tests: https://crbug.com/v8/7783
Expand Down Expand Up @@ -489,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
4 changes: 2 additions & 2 deletions deps/v8/test/cctest/test-assembler-arm64.cc
Expand Up @@ -11720,9 +11720,9 @@ TEST(system_msr) {
const uint64_t fpcr_core = 0x07C00000;

// All FPCR fields (including fields which may be read-as-zero):
// Stride, Len
// Stride, FZ16, Len
// IDE, IXE, UFE, OFE, DZE, IOE
const uint64_t fpcr_all = fpcr_core | 0x00379F00;
const uint64_t fpcr_all = fpcr_core | 0x003F9F00;

SETUP();

Expand Down