Skip to content

Commit 697d6a0

Browse files
oleavra60814billy
authored andcommittedApr 2, 2021
deps: V8: cherry-pick c6ec36a
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 <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 fe5e2e3 commit 697d6a0

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.20',
39+
'v8_embedder_string': '-node.21',
4040

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

‎deps/v8/BUILD.gn

+1
Original file line numberDiff line numberDiff line change
@@ -3080,6 +3080,7 @@ v8_source_set("v8_base_without_compiler") {
30803080
"src/wasm/baseline/liftoff-compiler.cc",
30813081
"src/wasm/baseline/liftoff-compiler.h",
30823082
"src/wasm/baseline/liftoff-register.h",
3083+
"src/wasm/code-space-access.h",
30833084
"src/wasm/compilation-environment.h",
30843085
"src/wasm/decoder.h",
30853086
"src/wasm/function-body-decoder-impl.h",

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

+8
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,14 @@ int GetFlagsForMemoryPermission(OS::MemoryPermission access) {
140140
#if V8_OS_QNX
141141
flags |= MAP_LAZY;
142142
#endif // V8_OS_QNX
143+
#if V8_OS_MACOSX && V8_HOST_ARCH_ARM64 && defined(MAP_JIT) && \
144+
!defined(V8_OS_IOS)
145+
// TODO(jkummerow): using the V8_OS_IOS define is a crude approximation
146+
// of the fact that we don't want to set the MAP_JIT flag when
147+
// FLAG_jitless == true, as src/base/ doesn't know any flags.
148+
// TODO(crbug.com/1117591): This is only needed for code spaces.
149+
flags |= MAP_JIT;
150+
#endif
143151
}
144152
return flags;
145153
}

‎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(),
@@ -842,6 +849,7 @@ CompilationEnv NativeModule::CreateCompilationEnv() const {
842849
}
843850

844851
WasmCode* NativeModule::AddCodeForTesting(Handle<Code> code) {
852+
CODE_SPACE_WRITE_SCOPE
845853
// For off-heap builtins, we create a copy of the off-heap instruction stream
846854
// instead of the on-heap code object containing the trampoline. Ensure that
847855
// 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) {
937945
if (!lazy_compile_table_) {
938946
uint32_t num_slots = module_->num_declared_functions;
939947
WasmCodeRefScope code_ref_scope;
948+
CODE_SPACE_WRITE_SCOPE
940949
base::AddressRegion single_code_space_region;
941950
{
942951
base::MutexGuard guard(&allocation_mutex_);
@@ -998,6 +1007,7 @@ std::unique_ptr<WasmCode> NativeModule::AddCodeWithCodeSpace(
9981007
const int code_comments_offset = desc.code_comments_offset;
9991008
const int instr_size = desc.instr_size;
10001009

1010+
CODE_SPACE_WRITE_SCOPE
10011011
memcpy(dst_code_bytes.begin(), desc.buffer,
10021012
static_cast<size_t>(desc.instr_size));
10031013

@@ -1122,6 +1132,7 @@ WasmCode* NativeModule::AddDeserializedCode(
11221132
Vector<const byte> protected_instructions_data,
11231133
Vector<const byte> reloc_info, Vector<const byte> source_position_table,
11241134
WasmCode::Kind kind, ExecutionTier tier) {
1135+
// CodeSpaceWriteScope is provided by the caller.
11251136
Vector<uint8_t> dst_code_bytes =
11261137
code_allocator_.AllocateForCode(this, instructions.size());
11271138
memcpy(dst_code_bytes.begin(), instructions.begin(), instructions.size());
@@ -1180,6 +1191,7 @@ WasmCode* NativeModule::CreateEmptyJumpTableInRegion(
11801191
Vector<uint8_t> code_space = code_allocator_.AllocateForCodeInRegion(
11811192
this, jump_table_size, region, allocator_lock);
11821193
DCHECK(!code_space.empty());
1194+
CODE_SPACE_WRITE_SCOPE
11831195
ZapCode(reinterpret_cast<Address>(code_space.begin()), code_space.size());
11841196
std::unique_ptr<WasmCode> code{
11851197
new WasmCode{this, // native_module
@@ -1205,6 +1217,7 @@ void NativeModule::PatchJumpTablesLocked(uint32_t slot_index, Address target) {
12051217
// The caller must hold the {allocation_mutex_}, thus we fail to lock it here.
12061218
DCHECK(!allocation_mutex_.TryLock());
12071219

1220+
CODE_SPACE_WRITE_SCOPE
12081221
for (auto& code_space_data : code_space_data_) {
12091222
DCHECK_IMPLIES(code_space_data.jump_table, code_space_data.far_jump_table);
12101223
if (!code_space_data.jump_table) continue;
@@ -1267,6 +1280,7 @@ void NativeModule::AddCodeSpace(
12671280
#endif // V8_OS_WIN64
12681281

12691282
WasmCodeRefScope code_ref_scope;
1283+
CODE_SPACE_WRITE_SCOPE
12701284
WasmCode* jump_table = nullptr;
12711285
WasmCode* far_jump_table = nullptr;
12721286
const uint32_t num_wasm_functions = module_->num_declared_functions;
@@ -1820,6 +1834,7 @@ std::vector<std::unique_ptr<WasmCode>> NativeModule::AddCompiledCode(
18201834
generated_code.reserve(results.size());
18211835

18221836
// Now copy the generated code into the code space and relocate it.
1837+
CODE_SPACE_WRITE_SCOPE
18231838
for (auto& result : results) {
18241839
DCHECK_EQ(result.code_desc.buffer, result.instr_buffer.get());
18251840
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"
@@ -520,6 +521,7 @@ bool NativeModuleDeserializer::ReadCode(int fn_index, Reader* reader) {
520521
auto protected_instructions =
521522
reader->ReadVector<byte>(protected_instructions_size);
522523

524+
CODE_SPACE_WRITE_SCOPE
523525
WasmCode* code = native_module_->AddDeserializedCode(
524526
fn_index, code_buffer, stack_slot_count, tagged_parameter_slots,
525527
safepoint_table_offset, handler_table_offset, constant_pool_offset,

‎deps/v8/test/cctest/cctest.status

+7
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,13 @@
169169
'test-debug/DebugBreakStackTrace': [PASS, SLOW],
170170
}], # 'arch == arm64 and simulator_run'
171171

172+
['arch == arm64 and system == macos and not simulator_run', {
173+
# printf, being a variadic function, has a different, stack-based ABI on
174+
# Apple silicon. See:
175+
# https://developer.apple.com/library/archive/documentation/Xcode/Conceptual/iPhoneOSABIReference/Articles/ARM64FunctionCallingConventions.html
176+
'test-assembler-arm64/printf_no_preserve': [SKIP],
177+
}], # arch == arm64 and system == macos and not simulator_run
178+
172179
##############################################################################
173180
['variant == nooptimization and (arch == arm or arch == arm64) and simulator_run', {
174181
# 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
@@ -11599,9 +11599,9 @@ TEST(system_msr) {
1159911599
const uint64_t fpcr_core = 0x07C00000;
1160011600

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

1160611606
SETUP();
1160711607

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

+3-2
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,9 @@ Handle<String> MakeName(const char* str, int suffix) {
5454
return MakeString(buffer.begin());
5555
}
5656

57-
int sum10(int a0, int a1, int a2, int a3, int a4, int a5, int a6, int a7,
58-
int a8, int a9) {
57+
intptr_t sum10(intptr_t a0, intptr_t a1, intptr_t a2, intptr_t a3, intptr_t a4,
58+
intptr_t a5, intptr_t a6, intptr_t a7, intptr_t a8,
59+
intptr_t a9) {
5960
return a0 + a1 + a2 + a3 + a4 + a5 + a6 + a7 + a8 + a9;
6061
}
6162

‎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 =
@@ -151,6 +157,7 @@ class JumpTableRunner : public v8::base::Thread {
151157

152158
void Run() override {
153159
TRACE("Runner #%d is starting ...\n", runner_id_);
160+
SwitchMemoryPermissionsToExecutable();
154161
GeneratedCode<void>::FromAddress(CcTest::i_isolate(), slot_address_).Call();
155162
TRACE("Runner #%d is stopping ...\n", runner_id_);
156163
USE(runner_id_);
@@ -173,6 +180,7 @@ class JumpTablePatcher : public v8::base::Thread {
173180

174181
void Run() override {
175182
TRACE("Patcher %p is starting ...\n", this);
183+
SwitchMemoryPermissionsToWritable();
176184
Address slot_address =
177185
slot_start_ + JumpTableAssembler::JumpSlotIndexToOffset(slot_index_);
178186
// First, emit code to the two thunks.
@@ -232,6 +240,7 @@ TEST(JumpTablePatchingStress) {
232240

233241
std::bitset<kAvailableBufferSlots> used_thunk_slots;
234242
buffer->MakeWritableAndExecutable();
243+
SwitchMemoryPermissionsToWritable();
235244

236245
// Iterate through jump-table slots to hammer at different alignments within
237246
// 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)
Please sign in to comment.