From 697d6a0e1f7477f0ee074851d4496717a68bf774 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:09 +0100 Subject: [PATCH] deps: V8: cherry-pick c6ec36a6 Original commit message: deps: V8: cherry-pick 27e1ac1a79ff [wasm][mac] Support w^x codespaces for Apple Silicon Apple's upcoming arm64 devices will prevent rwx access to memory, but in turn provide a new per-thread way to switch between write and execute permissions. This patch puts that system to use for the WebAssembly subsystem. The approach relies on CodeSpaceWriteScope objects for now. That isn't optimal for background threads (which could stay in "write" mode permanently instead of toggling), but its simplicity makes it a good first step. Background: https://developer.apple.com/documentation/apple_silicon/porting_just-in-time_compilers_to_apple_silicon Bug: chromium:1117591 Change-Id: I3b60f0efd34c0fed924dfc71ee2c7805801c5d42 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2378307 Commit-Queue: Jakob Kummerow Reviewed-by: Michael Lippautz Reviewed-by: Thibaud Michaud Cr-Commit-Position: refs/heads/master@{#69791} 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/BUILD.gn | 1 + deps/v8/src/base/platform/platform-posix.cc | 8 +++ deps/v8/src/wasm/code-space-access.h | 69 +++++++++++++++++++ deps/v8/src/wasm/wasm-code-manager.cc | 15 ++++ deps/v8/src/wasm/wasm-serialization.cc | 2 + deps/v8/test/cctest/cctest.status | 7 ++ deps/v8/test/cctest/test-assembler-arm64.cc | 4 +- .../test/cctest/test-code-stub-assembler.cc | 5 +- deps/v8/test/cctest/test-icache.cc | 5 ++ .../cctest/wasm/test-jump-table-assembler.cc | 9 +++ deps/v8/test/unittests/unittests.status | 21 ++++++ 12 files changed, 143 insertions(+), 5 deletions(-) create mode 100644 deps/v8/src/wasm/code-space-access.h diff --git a/common.gypi b/common.gypi index 154ba97d9f46cf..a3f155fde2861a 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.20', + 'v8_embedder_string': '-node.21', ##### V8 defaults for Node.js ##### diff --git a/deps/v8/BUILD.gn b/deps/v8/BUILD.gn index 167e63503c5535..eaaeadfce3f6cd 100644 --- a/deps/v8/BUILD.gn +++ b/deps/v8/BUILD.gn @@ -3080,6 +3080,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", diff --git a/deps/v8/src/base/platform/platform-posix.cc b/deps/v8/src/base/platform/platform-posix.cc index 17fc5b508394c8..87e5521d3fcd69 100644 --- a/deps/v8/src/base/platform/platform-posix.cc +++ b/deps/v8/src/base/platform/platform-posix.cc @@ -140,6 +140,14 @@ 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 } return flags; } diff --git a/deps/v8/src/wasm/code-space-access.h b/deps/v8/src/wasm/code-space-access.h new file mode 100644 index 00000000000000..5eeb980e17eddc --- /dev/null +++ b/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_ diff --git a/deps/v8/src/wasm/wasm-code-manager.cc b/deps/v8/src/wasm/wasm-code-manager.cc index 5477a18f33d7ea..b7fd17d887fb72 100644 --- a/deps/v8/src/wasm/wasm-code-manager.cc +++ b/deps/v8/src/wasm/wasm-code-manager.cc @@ -6,6 +6,7 @@ #include +#include "src/base/build_config.h" #include "src/base/iterator.h" #include "src/base/macros.h" #include "src/base/platform/platform.h" @@ -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" @@ -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 @@ -731,6 +737,7 @@ void WasmCodeAllocator::FreeCode(Vector 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(), @@ -842,6 +849,7 @@ CompilationEnv NativeModule::CreateCompilationEnv() const { } WasmCode* NativeModule::AddCodeForTesting(Handle 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. @@ -937,6 +945,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_); @@ -998,6 +1007,7 @@ std::unique_ptr 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(desc.instr_size)); @@ -1122,6 +1132,7 @@ WasmCode* NativeModule::AddDeserializedCode( Vector protected_instructions_data, Vector reloc_info, Vector source_position_table, WasmCode::Kind kind, ExecutionTier tier) { + // CodeSpaceWriteScope is provided by the caller. Vector dst_code_bytes = code_allocator_.AllocateForCode(this, instructions.size()); memcpy(dst_code_bytes.begin(), instructions.begin(), instructions.size()); @@ -1180,6 +1191,7 @@ WasmCode* NativeModule::CreateEmptyJumpTableInRegion( Vector code_space = code_allocator_.AllocateForCodeInRegion( this, jump_table_size, region, allocator_lock); DCHECK(!code_space.empty()); + CODE_SPACE_WRITE_SCOPE ZapCode(reinterpret_cast
(code_space.begin()), code_space.size()); std::unique_ptr code{ new WasmCode{this, // native_module @@ -1205,6 +1217,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; @@ -1267,6 +1280,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; @@ -1820,6 +1834,7 @@ std::vector> 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(result.code_desc.instr_size); diff --git a/deps/v8/src/wasm/wasm-serialization.cc b/deps/v8/src/wasm/wasm-serialization.cc index 8df5d4c88ee988..ea8c2820e2346c 100644 --- a/deps/v8/src/wasm/wasm-serialization.cc +++ b/deps/v8/src/wasm/wasm-serialization.cc @@ -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" @@ -520,6 +521,7 @@ bool NativeModuleDeserializer::ReadCode(int fn_index, Reader* reader) { auto protected_instructions = reader->ReadVector(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, diff --git a/deps/v8/test/cctest/cctest.status b/deps/v8/test/cctest/cctest.status index 73fdad69fe5358..65d7fab01005e3 100644 --- a/deps/v8/test/cctest/cctest.status +++ b/deps/v8/test/cctest/cctest.status @@ -169,6 +169,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 diff --git a/deps/v8/test/cctest/test-assembler-arm64.cc b/deps/v8/test/cctest/test-assembler-arm64.cc index 7b9a3ee8422c79..7bbcc71ee557e1 100644 --- a/deps/v8/test/cctest/test-assembler-arm64.cc +++ b/deps/v8/test/cctest/test-assembler-arm64.cc @@ -11599,9 +11599,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(); diff --git a/deps/v8/test/cctest/test-code-stub-assembler.cc b/deps/v8/test/cctest/test-code-stub-assembler.cc index eaad3180a55c7e..a371325340d177 100644 --- a/deps/v8/test/cctest/test-code-stub-assembler.cc +++ b/deps/v8/test/cctest/test-code-stub-assembler.cc @@ -54,8 +54,9 @@ Handle MakeName(const char* str, int suffix) { return MakeString(buffer.begin()); } -int sum10(int a0, int a1, int a2, int a3, int a4, int a5, int a6, int a7, - int a8, int a9) { +intptr_t sum10(intptr_t a0, intptr_t a1, intptr_t a2, intptr_t a3, intptr_t a4, + intptr_t a5, intptr_t a6, intptr_t a7, intptr_t a8, + intptr_t a9) { return a0 + a1 + a2 + a3 + a4 + a5 + a6 + a7 + a8 + a9; } diff --git a/deps/v8/test/cctest/test-icache.cc b/deps/v8/test/cctest/test-icache.cc index e8c89b7232b3b0..82baa9fe96212d 100644 --- a/deps/v8/test/cctest/test-icache.cc +++ b/deps/v8/test/cctest/test-icache.cc @@ -6,6 +6,7 @@ #include "src/codegen/macro-assembler-inl.h" #include "src/execution/simulator.h" #include "src/handles/handles-inl.h" +#include "src/wasm/code-space-access.h" #include "test/cctest/cctest.h" #include "test/common/assembler-tester.h" @@ -179,11 +180,15 @@ TEST(TestFlushICacheOfWritableAndExecutable) { CHECK(SetPermissions(GetPlatformPageAllocator(), buffer->start(), buffer->size(), v8::PageAllocator::kReadWriteExecute)); + SwitchMemoryPermissionsToWritable(); FloodWithInc(isolate, buffer.get()); FlushInstructionCache(buffer->start(), buffer->size()); + SwitchMemoryPermissionsToExecutable(); CHECK_EQ(23 + kNumInstr, f.Call(23)); // Call into generated code. + SwitchMemoryPermissionsToWritable(); FloodWithNop(isolate, buffer.get()); FlushInstructionCache(buffer->start(), buffer->size()); + SwitchMemoryPermissionsToExecutable(); CHECK_EQ(23, f.Call(23)); // Call into generated code. } } diff --git a/deps/v8/test/cctest/wasm/test-jump-table-assembler.cc b/deps/v8/test/cctest/wasm/test-jump-table-assembler.cc index 5ff91bc7786f64..350005d9572a45 100644 --- a/deps/v8/test/cctest/wasm/test-jump-table-assembler.cc +++ b/deps/v8/test/cctest/wasm/test-jump-table-assembler.cc @@ -8,6 +8,7 @@ #include "src/codegen/macro-assembler-inl.h" #include "src/execution/simulator.h" #include "src/utils/utils.h" +#include "src/wasm/code-space-access.h" #include "src/wasm/jump-table-assembler.h" #include "test/cctest/cctest.h" #include "test/common/assembler-tester.h" @@ -33,7 +34,12 @@ constexpr uint32_t kJumpTableSize = JumpTableAssembler::SizeForNumberOfSlots(kJumpTableSlotCount); // Must be a safe commit page size. +#if V8_OS_MACOSX && V8_HOST_ARCH_ARM64 +// See kAppleArmPageSize in platform-posix.cc. +constexpr size_t kThunkBufferSize = 1 << 14; +#else constexpr size_t kThunkBufferSize = 4 * KB; +#endif #if V8_TARGET_ARCH_ARM64 || V8_TARGET_ARCH_X64 constexpr uint32_t kAvailableBufferSlots = @@ -151,6 +157,7 @@ class JumpTableRunner : public v8::base::Thread { void Run() override { TRACE("Runner #%d is starting ...\n", runner_id_); + SwitchMemoryPermissionsToExecutable(); GeneratedCode::FromAddress(CcTest::i_isolate(), slot_address_).Call(); TRACE("Runner #%d is stopping ...\n", runner_id_); USE(runner_id_); @@ -173,6 +180,7 @@ class JumpTablePatcher : public v8::base::Thread { void Run() override { TRACE("Patcher %p is starting ...\n", this); + SwitchMemoryPermissionsToWritable(); Address slot_address = slot_start_ + JumpTableAssembler::JumpSlotIndexToOffset(slot_index_); // First, emit code to the two thunks. @@ -232,6 +240,7 @@ TEST(JumpTablePatchingStress) { std::bitset used_thunk_slots; buffer->MakeWritableAndExecutable(); + SwitchMemoryPermissionsToWritable(); // Iterate through jump-table slots to hammer at different alignments within // the jump-table, thereby increasing stress for variable-length ISAs. diff --git a/deps/v8/test/unittests/unittests.status b/deps/v8/test/unittests/unittests.status index 61bf5d1718eb27..048cf64215f317 100644 --- a/deps/v8/test/unittests/unittests.status +++ b/deps/v8/test/unittests/unittests.status @@ -17,6 +17,27 @@ 'RandomNumberGenerator.NextSampleSlowInvalidParam2': [SKIP], }], # system == macos and asan +['system == macos and arch == arm64 and not simulator_run', { + # Throwing C++ exceptions doesn't work; probably because the unittests + # binary is built with -fno-exceptions? + 'LanguageServerJson.LexerError': [SKIP], + 'LanguageServerJson.ParserError': [SKIP], + 'Torque.DoubleUnderScorePrefixIllegalForIdentifiers': [SKIP], + 'Torque.Enums': [SKIP], + 'Torque.ImportNonExistentFile': [SKIP], + + # Test uses fancy signal handling. Needs investigation. + 'MemoryAllocationPermissionsTest.DoTest': [SKIP], + + # cppgc::internal::kGuardPageSize is smaller than kAppleArmPageSize. + 'PageMemoryRegionTest.PlatformUsesGuardPages': [FAIL], + + # Time tick resolution appears to be ~42 microseconds. Tests expect 1 us. + 'TimeTicks.NowResolution': [FAIL], + 'RuntimeCallStatsTest.BasicJavaScript': [SKIP], + 'RuntimeCallStatsTest.FunctionLengthGetter': [SKIP], +}], # system == macos and arch == arm64 and not simulator_run + ############################################################################## ['lite_mode or variant == jitless', { # TODO(v8:7777): Re-enable once wasm is supported in jitless mode.