Skip to content

Commit c6ec36a

Browse files
oleavrnodejs-github-bot
authored andcommitted
deps: V8: cherry-pick 27e1ac1a79ff
Original commit message: [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 <jkummerow@chromium.org> Reviewed-by: Michael Lippautz <mlippautz@chromium.org> Reviewed-by: Thibaud Michaud <thibaudm@chromium.org> Cr-Commit-Position: refs/heads/master@{#69791} PR-URL: nodejs#35986 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Richard Lau <rlau@redhat.com> Reviewed-By: Michael Dawson <midawson@redhat.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Beth Griggs <bgriggs@redhat.com> Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
1 parent 929c51f commit c6ec36a

12 files changed

+143
-5
lines changed

common.gypi

+1-1
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@
3636

3737
# Reset this number to 0 on major V8 upgrades.
3838
# Increment by one for each non-official patch applied to deps/v8.
39-
'v8_embedder_string': '-node.17',
39+
'v8_embedder_string': '-node.18',
4040

4141
##### V8 defaults for Node.js #####
4242

deps/v8/BUILD.gn

+1
Original file line numberDiff line numberDiff line change
@@ -3237,6 +3237,7 @@ v8_source_set("v8_base_without_compiler") {
32373237
"src/wasm/baseline/liftoff-compiler.cc",
32383238
"src/wasm/baseline/liftoff-compiler.h",
32393239
"src/wasm/baseline/liftoff-register.h",
3240+
"src/wasm/code-space-access.h",
32403241
"src/wasm/compilation-environment.h",
32413242
"src/wasm/decoder.h",
32423243
"src/wasm/function-body-decoder-impl.h",

deps/v8/src/base/platform/platform-posix.cc

+8
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,14 @@ int GetFlagsForMemoryPermission(OS::MemoryPermission access,
151151
#if V8_OS_QNX
152152
flags |= MAP_LAZY;
153153
#endif // V8_OS_QNX
154+
#if V8_OS_MACOSX && V8_HOST_ARCH_ARM64 && defined(MAP_JIT) && \
155+
!defined(V8_OS_IOS)
156+
// TODO(jkummerow): using the V8_OS_IOS define is a crude approximation
157+
// of the fact that we don't want to set the MAP_JIT flag when
158+
// FLAG_jitless == true, as src/base/ doesn't know any flags.
159+
// TODO(crbug.com/1117591): This is only needed for code spaces.
160+
flags |= MAP_JIT;
161+
#endif
154162
}
155163
return flags;
156164
}

deps/v8/src/wasm/code-space-access.h

+69
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
// Copyright 2020 the V8 project authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
5+
#ifndef V8_WASM_CODE_SPACE_ACCESS_H_
6+
#define V8_WASM_CODE_SPACE_ACCESS_H_
7+
8+
#include "src/base/build_config.h"
9+
#include "src/base/macros.h"
10+
#include "src/common/globals.h"
11+
12+
namespace v8 {
13+
namespace internal {
14+
15+
#if defined(V8_OS_MACOSX) && defined(V8_HOST_ARCH_ARM64)
16+
17+
// Ignoring this warning is considered better than relying on
18+
// __builtin_available.
19+
#pragma clang diagnostic push
20+
#pragma clang diagnostic ignored "-Wunguarded-availability-new"
21+
inline void SwitchMemoryPermissionsToWritable() {
22+
pthread_jit_write_protect_np(0);
23+
}
24+
inline void SwitchMemoryPermissionsToExecutable() {
25+
pthread_jit_write_protect_np(1);
26+
}
27+
#pragma clang diagnostic pop
28+
29+
namespace wasm {
30+
31+
class CodeSpaceWriteScope {
32+
public:
33+
// TODO(jkummerow): Background threads could permanently stay in
34+
// writable mode; only the main thread has to switch back and forth.
35+
CodeSpaceWriteScope() {
36+
if (code_space_write_nesting_level_ == 0) {
37+
SwitchMemoryPermissionsToWritable();
38+
}
39+
code_space_write_nesting_level_++;
40+
}
41+
~CodeSpaceWriteScope() {
42+
code_space_write_nesting_level_--;
43+
if (code_space_write_nesting_level_ == 0) {
44+
SwitchMemoryPermissionsToExecutable();
45+
}
46+
}
47+
48+
private:
49+
static thread_local int code_space_write_nesting_level_;
50+
};
51+
52+
#define CODE_SPACE_WRITE_SCOPE CodeSpaceWriteScope _write_access_;
53+
54+
} // namespace wasm
55+
56+
#else // Not Mac-on-arm64.
57+
58+
// Nothing to do, we map code memory with rwx permissions.
59+
inline void SwitchMemoryPermissionsToWritable() {}
60+
inline void SwitchMemoryPermissionsToExecutable() {}
61+
62+
#define CODE_SPACE_WRITE_SCOPE
63+
64+
#endif // V8_OS_MACOSX && V8_HOST_ARCH_ARM64
65+
66+
} // namespace internal
67+
} // namespace v8
68+
69+
#endif // V8_WASM_CODE_SPACE_ACCESS_H_

deps/v8/src/wasm/wasm-code-manager.cc

+15
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
#include <iomanip>
88

9+
#include "src/base/build_config.h"
910
#include "src/base/iterator.h"
1011
#include "src/base/macros.h"
1112
#include "src/base/platform/platform.h"
@@ -21,6 +22,7 @@
2122
#include "src/snapshot/embedded/embedded-data.h"
2223
#include "src/utils/ostreams.h"
2324
#include "src/utils/vector.h"
25+
#include "src/wasm/code-space-access.h"
2426
#include "src/wasm/compilation-environment.h"
2527
#include "src/wasm/function-compiler.h"
2628
#include "src/wasm/jump-table-assembler.h"
@@ -47,6 +49,10 @@ namespace wasm {
4749

4850
using trap_handler::ProtectedInstructionData;
4951

52+
#if defined(V8_OS_MACOSX) && defined(V8_HOST_ARCH_ARM64)
53+
thread_local int CodeSpaceWriteScope::code_space_write_nesting_level_ = 0;
54+
#endif
55+
5056
base::AddressRegion DisjointAllocationPool::Merge(
5157
base::AddressRegion new_region) {
5258
// Find the possible insertion position by identifying the first region whose
@@ -731,6 +737,7 @@ void WasmCodeAllocator::FreeCode(Vector<WasmCode* const> codes) {
731737
// Zap code area and collect freed code regions.
732738
DisjointAllocationPool freed_regions;
733739
size_t code_size = 0;
740+
CODE_SPACE_WRITE_SCOPE
734741
for (WasmCode* code : codes) {
735742
ZapCode(code->instruction_start(), code->instructions().size());
736743
FlushInstructionCache(code->instruction_start(),
@@ -847,6 +854,7 @@ CompilationEnv NativeModule::CreateCompilationEnv() const {
847854
}
848855

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

1015+
CODE_SPACE_WRITE_SCOPE
10061016
memcpy(dst_code_bytes.begin(), desc.buffer,
10071017
static_cast<size_t>(desc.instr_size));
10081018

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

1236+
CODE_SPACE_WRITE_SCOPE
12241237
for (auto& code_space_data : code_space_data_) {
12251238
DCHECK_IMPLIES(code_space_data.jump_table, code_space_data.far_jump_table);
12261239
if (!code_space_data.jump_table) continue;
@@ -1283,6 +1296,7 @@ void NativeModule::AddCodeSpace(
12831296
#endif // V8_OS_WIN64
12841297

12851298
WasmCodeRefScope code_ref_scope;
1299+
CODE_SPACE_WRITE_SCOPE
12861300
WasmCode* jump_table = nullptr;
12871301
WasmCode* far_jump_table = nullptr;
12881302
const uint32_t num_wasm_functions = module_->num_declared_functions;
@@ -1843,6 +1857,7 @@ std::vector<std::unique_ptr<WasmCode>> NativeModule::AddCompiledCode(
18431857
generated_code.reserve(results.size());
18441858

18451859
// Now copy the generated code into the code space and relocate it.
1860+
CODE_SPACE_WRITE_SCOPE
18461861
for (auto& result : results) {
18471862
DCHECK_EQ(result.code_desc.buffer, result.instr_buffer.get());
18481863
size_t code_size = RoundUp<kCodeAlignment>(result.code_desc.instr_size);

deps/v8/src/wasm/wasm-serialization.cc

+2
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
#include "src/utils/ostreams.h"
1414
#include "src/utils/utils.h"
1515
#include "src/utils/version.h"
16+
#include "src/wasm/code-space-access.h"
1617
#include "src/wasm/function-compiler.h"
1718
#include "src/wasm/module-compiler.h"
1819
#include "src/wasm/module-decoder.h"
@@ -534,6 +535,7 @@ void NativeModuleDeserializer::ReadCode(int fn_index, Reader* reader) {
534535
auto protected_instructions =
535536
reader->ReadVector<byte>(protected_instructions_size);
536537

538+
CODE_SPACE_WRITE_SCOPE
537539
WasmCode* code = native_module_->AddDeserializedCode(
538540
fn_index, code_buffer, stack_slot_count, tagged_parameter_slots,
539541
safepoint_table_offset, handler_table_offset, constant_pool_offset,

deps/v8/test/cctest/cctest.status

+7
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,13 @@
176176
'test-debug/DebugBreakStackTrace': [PASS, SLOW],
177177
}], # 'arch == arm64 and simulator_run'
178178

179+
['arch == arm64 and system == macos and not simulator_run', {
180+
# printf, being a variadic function, has a different, stack-based ABI on
181+
# Apple silicon. See:
182+
# https://developer.apple.com/library/archive/documentation/Xcode/Conceptual/iPhoneOSABIReference/Articles/ARM64FunctionCallingConventions.html
183+
'test-assembler-arm64/printf_no_preserve': [SKIP],
184+
}], # arch == arm64 and system == macos and not simulator_run
185+
179186
##############################################################################
180187
['variant == nooptimization and (arch == arm or arch == arm64) and simulator_run', {
181188
# Slow tests: https://crbug.com/v8/7783

deps/v8/test/cctest/test-assembler-arm64.cc

+2-2
Original file line numberDiff line numberDiff line change
@@ -11720,9 +11720,9 @@ TEST(system_msr) {
1172011720
const uint64_t fpcr_core = 0x07C00000;
1172111721

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

1172711727
SETUP();
1172811728

deps/v8/test/cctest/test-code-stub-assembler.cc

+3-2
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,9 @@ template <class T>
4141
using TVariable = TypedCodeAssemblerVariable<T>;
4242
using PromiseResolvingFunctions = TorqueStructPromiseResolvingFunctions;
4343

44-
int sum10(int a0, int a1, int a2, int a3, int a4, int a5, int a6, int a7,
45-
int a8, int a9) {
44+
intptr_t sum10(intptr_t a0, intptr_t a1, intptr_t a2, intptr_t a3, intptr_t a4,
45+
intptr_t a5, intptr_t a6, intptr_t a7, intptr_t a8,
46+
intptr_t a9) {
4647
return a0 + a1 + a2 + a3 + a4 + a5 + a6 + a7 + a8 + a9;
4748
}
4849

deps/v8/test/cctest/test-icache.cc

+5
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
#include "src/codegen/macro-assembler-inl.h"
77
#include "src/execution/simulator.h"
88
#include "src/handles/handles-inl.h"
9+
#include "src/wasm/code-space-access.h"
910
#include "test/cctest/cctest.h"
1011
#include "test/common/assembler-tester.h"
1112

@@ -179,11 +180,15 @@ TEST(TestFlushICacheOfWritableAndExecutable) {
179180

180181
CHECK(SetPermissions(GetPlatformPageAllocator(), buffer->start(),
181182
buffer->size(), v8::PageAllocator::kReadWriteExecute));
183+
SwitchMemoryPermissionsToWritable();
182184
FloodWithInc(isolate, buffer.get());
183185
FlushInstructionCache(buffer->start(), buffer->size());
186+
SwitchMemoryPermissionsToExecutable();
184187
CHECK_EQ(23 + kNumInstr, f.Call(23)); // Call into generated code.
188+
SwitchMemoryPermissionsToWritable();
185189
FloodWithNop(isolate, buffer.get());
186190
FlushInstructionCache(buffer->start(), buffer->size());
191+
SwitchMemoryPermissionsToExecutable();
187192
CHECK_EQ(23, f.Call(23)); // Call into generated code.
188193
}
189194
}

deps/v8/test/cctest/wasm/test-jump-table-assembler.cc

+9
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
#include "src/codegen/macro-assembler-inl.h"
99
#include "src/execution/simulator.h"
1010
#include "src/utils/utils.h"
11+
#include "src/wasm/code-space-access.h"
1112
#include "src/wasm/jump-table-assembler.h"
1213
#include "test/cctest/cctest.h"
1314
#include "test/common/assembler-tester.h"
@@ -33,7 +34,12 @@ constexpr uint32_t kJumpTableSize =
3334
JumpTableAssembler::SizeForNumberOfSlots(kJumpTableSlotCount);
3435

3536
// Must be a safe commit page size.
37+
#if V8_OS_MACOSX && V8_HOST_ARCH_ARM64
38+
// See kAppleArmPageSize in platform-posix.cc.
39+
constexpr size_t kThunkBufferSize = 1 << 14;
40+
#else
3641
constexpr size_t kThunkBufferSize = 4 * KB;
42+
#endif
3743

3844
#if V8_TARGET_ARCH_ARM64 || V8_TARGET_ARCH_X64
3945
constexpr uint32_t kAvailableBufferSlots =
@@ -154,6 +160,7 @@ class JumpTableRunner : public v8::base::Thread {
154160

155161
void Run() override {
156162
TRACE("Runner #%d is starting ...\n", runner_id_);
163+
SwitchMemoryPermissionsToExecutable();
157164
GeneratedCode<void>::FromAddress(CcTest::i_isolate(), slot_address_).Call();
158165
TRACE("Runner #%d is stopping ...\n", runner_id_);
159166
USE(runner_id_);
@@ -176,6 +183,7 @@ class JumpTablePatcher : public v8::base::Thread {
176183

177184
void Run() override {
178185
TRACE("Patcher %p is starting ...\n", this);
186+
SwitchMemoryPermissionsToWritable();
179187
Address slot_address =
180188
slot_start_ + JumpTableAssembler::JumpSlotIndexToOffset(slot_index_);
181189
// First, emit code to the two thunks.
@@ -235,6 +243,7 @@ TEST(JumpTablePatchingStress) {
235243

236244
std::bitset<kAvailableBufferSlots> used_thunk_slots;
237245
buffer->MakeWritableAndExecutable();
246+
SwitchMemoryPermissionsToWritable();
238247

239248
// Iterate through jump-table slots to hammer at different alignments within
240249
// the jump-table, thereby increasing stress for variable-length ISAs.

deps/v8/test/unittests/unittests.status

+21
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,27 @@
1717
'RandomNumberGenerator.NextSampleSlowInvalidParam2': [SKIP],
1818
}], # system == macos and asan
1919

20+
['system == macos and arch == arm64 and not simulator_run', {
21+
# Throwing C++ exceptions doesn't work; probably because the unittests
22+
# binary is built with -fno-exceptions?
23+
'LanguageServerJson.LexerError': [SKIP],
24+
'LanguageServerJson.ParserError': [SKIP],
25+
'Torque.DoubleUnderScorePrefixIllegalForIdentifiers': [SKIP],
26+
'Torque.Enums': [SKIP],
27+
'Torque.ImportNonExistentFile': [SKIP],
28+
29+
# Test uses fancy signal handling. Needs investigation.
30+
'MemoryAllocationPermissionsTest.DoTest': [SKIP],
31+
32+
# cppgc::internal::kGuardPageSize is smaller than kAppleArmPageSize.
33+
'PageMemoryRegionTest.PlatformUsesGuardPages': [FAIL],
34+
35+
# Time tick resolution appears to be ~42 microseconds. Tests expect 1 us.
36+
'TimeTicks.NowResolution': [FAIL],
37+
'RuntimeCallStatsTest.BasicJavaScript': [SKIP],
38+
'RuntimeCallStatsTest.FunctionLengthGetter': [SKIP],
39+
}], # system == macos and arch == arm64 and not simulator_run
40+
2041
##############################################################################
2142
['lite_mode or variant == jitless', {
2243
# TODO(v8:7777): Re-enable once wasm is supported in jitless mode.

0 commit comments

Comments
 (0)