[rwx][mac] Introduce RwxMemoryWriteScope

... as a single bottleneck that encapsulates the semantics and
implementation of fast per-thread W^X permission switching supported
by Apple Silicon (arm64 M1).
On other architectures this class is a no-op.

Bug: v8:12797
Change-Id: Ica842ff9f843e20b7f61fd7e80591e7a1fd29771
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3586986Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Commit-Queue: Igor Sheludko <ishell@chromium.org>
Cr-Commit-Position: refs/heads/main@{#79994}

BUILD.bazel

@@ -1205,6 +1205,9 @@ filegroup(
         "src/common/assert-scope.h",
         "src/common/allow-deprecated.h",
         "src/common/checks.h",
+        "src/common/code-memory-access-inl.h",
+        "src/common/code-memory-access.cc",
+        "src/common/code-memory-access.h",
         "src/common/high-allocation-throughput-scope.h",
         "src/common/message-template.h",
         "src/common/operation.h",

BUILD.gn

@@ -2750,6 +2750,8 @@ v8_header_set("v8_internal_headers") {
     "src/common/allow-deprecated.h",
     "src/common/assert-scope.h",
     "src/common/checks.h",
+    "src/common/code-memory-access-inl.h",
+    "src/common/code-memory-access.h",
     "src/common/high-allocation-throughput-scope.h",
     "src/common/message-template.h",
     "src/common/operation.h",
@@ -4163,6 +4165,9 @@ v8_source_set("v8_base_without_compiler") {
     "src/codegen/turbo-assembler.cc",
     "src/codegen/unoptimized-compilation-info.cc",
     "src/common/assert-scope.cc",
+    "src/common/code-memory-access-inl.h",
+    "src/common/code-memory-access.cc",
+    "src/common/code-memory-access.h",
     "src/compiler-dispatcher/lazy-compile-dispatcher.cc",
     "src/compiler-dispatcher/optimizing-compile-dispatcher.cc",
     "src/date/date.cc",

src/common/code-memory-access-inl.h

+// Copyright 2022 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_COMMON_CODE_MEMORY_ACCESS_INL_H_
+#define V8_COMMON_CODE_MEMORY_ACCESS_INL_H_
+
+#include "src/common/code-memory-access.h"
+#include "src/logging/log.h"
+
+namespace v8 {
+namespace internal {
+
+RwxMemoryWriteScope::RwxMemoryWriteScope() { SetWritable(); }
+
+RwxMemoryWriteScope::~RwxMemoryWriteScope() { SetExecutable(); }
+
+#if V8_HAS_PTHREAD_JIT_WRITE_PROTECT
+
+// Ignoring this warning is considered better than relying on
+// __builtin_available.
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wunguarded-availability-new"
+// static
+void RwxMemoryWriteScope::SetWritable() {
+  if (code_space_write_nesting_level_ == 0) {
+    pthread_jit_write_protect_np(0);
+  }
+  code_space_write_nesting_level_++;
+}
+
+// static
+void RwxMemoryWriteScope::SetExecutable() {
+  code_space_write_nesting_level_--;
+  if (code_space_write_nesting_level_ == 0) {
+    pthread_jit_write_protect_np(1);
+  }
+}
+#pragma clang diagnostic pop
+
+#else  // !V8_HAS_PTHREAD_JIT_WRITE_PROTECT
+
+void RwxMemoryWriteScope::SetWritable() {}
+
+void RwxMemoryWriteScope::SetExecutable() {}
+
+#endif  // V8_HAS_PTHREAD_JIT_WRITE_PROTECT
+
+}  // namespace internal
+}  // namespace v8
+
+#endif  // V8_COMMON_CODE_MEMORY_ACCESS_INL_H_

src/common/code-memory-access.cc

+// Copyright 2022 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.
+
+#include "src/common/code-memory-access.h"
+
+namespace v8 {
+namespace internal {
+
+#if V8_HAS_PTHREAD_JIT_WRITE_PROTECT
+
+thread_local int RwxMemoryWriteScope::code_space_write_nesting_level_ = 0;
+
+#endif  // V8_HAS_PTHREAD_JIT_WRITE_PROTECT
+
+}  // namespace internal
+}  // namespace v8

src/common/code-memory-access.h

+// Copyright 2022 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_COMMON_CODE_MEMORY_ACCESS_H_
+#define V8_COMMON_CODE_MEMORY_ACCESS_H_
+
+#include "src/base/build_config.h"
+
+namespace v8 {
+namespace internal {
+
+class CodePageCollectionMemoryModificationScope;
+class CodePageMemoryModificationScope;
+class CodeSpaceMemoryModificationScope;
+namespace wasm {
+class CodeSpaceWriteScope;
+}
+
+// This scope is a wrapper for APRR/MAP_JIT machinery on MacOS on ARM64
+// ("Apple M1"/Apple Silicon) with respective semantics.
+// See pthread_jit_write_protect_np() for details.
+// On other platforms the scope is a no-op.
+//
+// The semantics is the following: the scope switches permissions between
+// writable and executable for all the pages allocated with RWX permissions.
+// Only current thread is affected. This achieves "real" W^X and it's fast.
+// By default it is assumed that the state is executable.
+//
+// The scope is reentrant and thread safe.
+class V8_NODISCARD RwxMemoryWriteScope final {
+ public:
+  V8_INLINE explicit RwxMemoryWriteScope();
+  V8_INLINE virtual ~RwxMemoryWriteScope();
+
+  // Disable copy constructor and copy-assignment operator, since this manages
+  // a resource and implicit copying of the scope can yield surprising errors.
+  RwxMemoryWriteScope(const RwxMemoryWriteScope&) = delete;
+  RwxMemoryWriteScope& operator=(const RwxMemoryWriteScope&) = delete;
+
+ private:
+  friend class CodePageCollectionMemoryModificationScope;
+  friend class CodePageMemoryModificationScope;
+  friend class CodeSpaceMemoryModificationScope;
+  friend class wasm::CodeSpaceWriteScope;
+
+  // {SetWritable} and {SetExecutable} implicitly enters/exits the scope.
+  // These methods are exposed only for the purpose of implementing other
+  // scope classes that affect executable pages permissions.
+  V8_INLINE static void SetWritable();
+  V8_INLINE static void SetExecutable();
+
+#if V8_HAS_PTHREAD_JIT_WRITE_PROTECT
+  // This counter is used for supporting scope reentrance.
+  static thread_local int code_space_write_nesting_level_;
+#endif  // V8_HAS_PTHREAD_JIT_WRITE_PROTECT || defined(DEBUG)
+};
+
+}  // namespace internal
+}  // namespace v8
+
+#endif  // V8_COMMON_CODE_MEMORY_ACCESS_H_

src/wasm/code-space-access.cc

@@ -4,6 +4,7 @@
 
 #include "src/wasm/code-space-access.h"
 
+#include "src/common/code-memory-access-inl.h"
 #include "src/wasm/wasm-code-manager.h"
 #include "src/wasm/wasm-engine.h"
 
@@ -36,20 +37,13 @@ CodeSpaceWriteScope::~CodeSpaceWriteScope() {
 
 #if V8_HAS_PTHREAD_JIT_WRITE_PROTECT
 
-// Ignoring this warning is considered better than relying on
-// __builtin_available.
-#pragma clang diagnostic push
-#pragma clang diagnostic ignored "-Wunguarded-availability-new"
 // static
-void CodeSpaceWriteScope::SetWritable() {
-  pthread_jit_write_protect_np(0);
-}
+void CodeSpaceWriteScope::SetWritable() { RwxMemoryWriteScope::SetWritable(); }
 
 // static
 void CodeSpaceWriteScope::SetExecutable() {
-  pthread_jit_write_protect_np(1);
+  RwxMemoryWriteScope::SetExecutable();
 }
-#pragma clang diagnostic pop
 
 // static
 bool CodeSpaceWriteScope::SwitchingPerNativeModule() { return false; }

test/cctest/test-assembler-arm64.cc

@@ -122,9 +122,11 @@ static void InitializeVM() {
   Isolate* isolate = CcTest::i_isolate();                                     \
   HandleScope scope(isolate);                                                 \
   CHECK_NOT_NULL(isolate);                                                    \
-  std::unique_ptr<byte[]> owned_buf{new byte[buf_size]};                   \
+  auto owned_buf =                                                            \
+      AllocateAssemblerBuffer(buf_size, nullptr, VirtualMemory::kNoJit);      \
   MacroAssembler masm(isolate, v8::internal::CodeObjectRequired::kYes,        \
-                      ExternalAssemblerBuffer(owned_buf.get(), buf_size)); \
+                      ExternalAssemblerBuffer(owned_buf->start(), buf_size)); \
+  std::optional<AssemblerBufferWriteScope> rw_buffer_scope;                   \
   Decoder<DispatchingDecoderVisitor>* decoder =                               \
       new Decoder<DispatchingDecoderVisitor>();                               \
   Simulator simulator(decoder);                                               \
@@ -177,6 +179,7 @@ static void InitializeVM() {
   HandleScope scope(isolate);                                          \
   CHECK_NOT_NULL(isolate);                                             \
   auto owned_buf = AllocateAssemblerBuffer(buf_size);                  \
+  std::optional<AssemblerBufferWriteScope> rw_buffer_scope;            \
   MacroAssembler masm(isolate, v8::internal::CodeObjectRequired::kYes, \
                       owned_buf->CreateView());                        \
   HandleScope handle_scope(isolate);                                   \
@@ -184,7 +187,7 @@ static void InitializeVM() {
   RegisterDump core;
 
 #define RESET()                                                \
-  owned_buf->MakeWritable();                                   \
+  rw_buffer_scope.emplace(*owned_buf);                         \
   __ Reset();                                                  \
   __ CodeEntry();                                              \
   /* Reset the machine state (like simulator.ResetState()). */ \
@@ -200,6 +203,8 @@ static void InitializeVM() {
 
 #define RUN()                                                  \
   {                                                            \
+    /* Reset the scope and thus make the buffer executable. */ \
+    rw_buffer_scope.reset();                                   \
     auto f = GeneratedCode<void>::FromCode(*code);             \
     f.Call();                                                  \
   }
@@ -14880,6 +14885,7 @@ TEST(pool_size) {
 
   // This test does not execute any code. It only tests that the size of the
   // pools is read correctly from the RelocInfo.
+  rw_buffer_scope.emplace(*owned_buf);
 
   Label exit;
   __ b(&exit);

test/cctest/test-icache.cc

@@ -113,19 +113,18 @@ TEST(TestFlushICacheOfWritable) {
     // Allow calling the function from C++.
     auto f = GeneratedCode<F0>::FromBuffer(isolate, buffer->start());
 
-    CHECK(SetPermissions(GetPlatformPageAllocator(), buffer->start(),
-                         buffer->size(), v8::PageAllocator::kReadWrite));
+    {
+      AssemblerBufferWriteScope rw_buffer_scope(*buffer);
       FloodWithInc(isolate, buffer.get());
       FlushInstructionCache(buffer->start(), buffer->size());
-    CHECK(SetPermissions(GetPlatformPageAllocator(), buffer->start(),
-                         buffer->size(), v8::PageAllocator::kReadExecute));
+    }
     CHECK_EQ(23 + kNumInstr, f.Call(23));  // Call into generated code.
-    CHECK(SetPermissions(GetPlatformPageAllocator(), buffer->start(),
-                         buffer->size(), v8::PageAllocator::kReadWrite));
+
+    {
+      AssemblerBufferWriteScope rw_buffer_scope(*buffer);
       FloodWithNop(isolate, buffer.get());
       FlushInstructionCache(buffer->start(), buffer->size());
-    CHECK(SetPermissions(GetPlatformPageAllocator(), buffer->start(),
-                         buffer->size(), v8::PageAllocator::kReadExecute));
+    }
     CHECK_EQ(23, f.Call(23));  // Call into generated code.
   }
 }
@@ -184,24 +183,6 @@ TEST(TestFlushICacheOfWritableAndExecutable) {
   Isolate* isolate = CcTest::i_isolate();
   HandleScope handles(isolate);
 
-  struct V8_NODISCARD EnableWritePermissionsOnMacArm64Scope {
-#if defined(V8_OS_DARWIN) && 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"
-    EnableWritePermissionsOnMacArm64Scope() { pthread_jit_write_protect_np(0); }
-    ~EnableWritePermissionsOnMacArm64Scope() {
-      pthread_jit_write_protect_np(1);
-    }
-#pragma clang diagnostic pop
-#else
-    EnableWritePermissionsOnMacArm64Scope() {
-      // Define a constructor to avoid unused variable warnings.
-    }
-#endif
-  };
-
   for (int i = 0; i < kNumIterations; ++i) {
     auto buffer = AllocateAssemblerBuffer(kBufferSize, nullptr,
                                           VirtualMemory::kMapAsJittable);
@@ -209,16 +190,16 @@ TEST(TestFlushICacheOfWritableAndExecutable) {
     // Allow calling the function from C++.
     auto f = GeneratedCode<F0>::FromBuffer(isolate, buffer->start());
 
-    CHECK(SetPermissions(GetPlatformPageAllocator(), buffer->start(),
-                         buffer->size(), v8::PageAllocator::kReadWriteExecute));
+    buffer->MakeWritableAndExecutable();
+
     {
-      EnableWritePermissionsOnMacArm64Scope write_scope;
+      RwxMemoryWriteScope rw_scope;
       FloodWithInc(isolate, buffer.get());
       FlushInstructionCache(buffer->start(), buffer->size());
     }
     CHECK_EQ(23 + kNumInstr, f.Call(23));  // Call into generated code.
     {
-      EnableWritePermissionsOnMacArm64Scope write_scope;
+      RwxMemoryWriteScope rw_scope;
       FloodWithNop(isolate, buffer.get());
       FlushInstructionCache(buffer->start(), buffer->size());
     }

test/cctest/test-macro-assembler-arm64.cc

@@ -57,6 +57,8 @@ TEST(EmbeddedObj) {
   MacroAssembler masm(isolate, v8::internal::CodeObjectRequired::kYes,
                       buffer->CreateView());
 
+  AssemblerBufferWriteScope rw_scope(*buffer);
+
   Handle<HeapObject> old_array = isolate->factory()->NewFixedArray(2000);
   Handle<HeapObject> my_array = isolate->factory()->NewFixedArray(1000);
   __ Mov(w4, Immediate(my_array, RelocInfo::COMPRESSED_EMBEDDED_OBJECT));
@@ -100,6 +102,8 @@ TEST(DeoptExitSizeIsFixed) {
   MacroAssembler masm(isolate, v8::internal::CodeObjectRequired::kYes,
                       buffer->CreateView());
 
+  AssemblerBufferWriteScope rw_scope(*buffer);
+
   STATIC_ASSERT(static_cast<int>(kFirstDeoptimizeKind) == 0);
   for (int i = 0; i < kDeoptimizeKindCount; i++) {
     DeoptimizeKind kind = static_cast<DeoptimizeKind>(i);

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

@@ -55,17 +55,6 @@ constexpr uint32_t kAvailableBufferSlots = 0;
 constexpr uint32_t kBufferSlotStartOffset = 0;
 #endif
 
-void EnsureThreadHasWritePermissions() {
-#if defined(V8_OS_DARWIN) && 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"
-  pthread_jit_write_protect_np(0);
-#pragma clang diagnostic pop
-#endif
-}
-
 Address AllocateJumpTableThunk(
     Address jump_target, byte* thunk_slot_buffer,
     std::bitset<kAvailableBufferSlots>* used_slots,
@@ -214,7 +203,7 @@ class JumpTablePatcher : public v8::base::Thread {
 
   void Run() override {
     TRACE("Patcher %p is starting ...\n", this);
-    EnsureThreadHasWritePermissions();
+    RwxMemoryWriteScope rwx_write_scope;
     Address slot_address =
         slot_start_ + JumpTableAssembler::JumpSlotIndexToOffset(slot_index_);
     // First, emit code to the two thunks.
@@ -265,11 +254,11 @@ TEST(JumpTablePatchingStress) {
 
   std::bitset<kAvailableBufferSlots> used_thunk_slots;
   buffer->MakeWritableAndExecutable();
+  RwxMemoryWriteScope rwx_write_scope;
 
   // Iterate through jump-table slots to hammer at different alignments within
   // the jump-table, thereby increasing stress for variable-length ISAs.
   Address slot_start = reinterpret_cast<Address>(buffer->start());
-  EnsureThreadHasWritePermissions();
   for (int slot = 0; slot < kJumpTableSlotCount; ++slot) {
     TRACE("Hammering on jump table slot #%d ...\n", slot);
     uint32_t slot_offset = JumpTableAssembler::JumpSlotIndexToOffset(slot);

test/common/assembler-tester.h

@@ -9,6 +9,7 @@
 
 #include "src/codegen/assembler.h"
 #include "src/codegen/code-desc.h"
+#include "src/common/code-memory-access-inl.h"
 
 namespace v8 {
 namespace internal {
@@ -27,7 +28,7 @@ class TestingAssemblerBuffer : public AssemblerBuffer {
     MakeWritable();
   }
 
-  ~TestingAssemblerBuffer() { reservation_.Free(); }
+  ~TestingAssemblerBuffer() override { reservation_.Free(); }
 
   byte* start() const override {
     return reinterpret_cast<byte*>(reservation_.address());
@@ -62,7 +63,6 @@ class TestingAssemblerBuffer : public AssemblerBuffer {
     CHECK(result);
   }
 
-  // TODO(wasm): Only needed for the "test-jump-table-assembler.cc" tests.
   void MakeWritableAndExecutable() {
     bool result = SetPermissions(GetPlatformPageAllocator(), start(), size(),
                                  v8::PageAllocator::kReadWriteExecute);
@@ -73,6 +73,31 @@ class TestingAssemblerBuffer : public AssemblerBuffer {
   VirtualMemory reservation_;
 };
 
+// This scope class is mostly necesasry for arm64 tests running on Apple Silicon
+// (M1) which prohibits reconfiguration of page permissions for RWX pages.
+// Instead of altering the page permissions one must flip the X-W state by
+// calling pthread_jit_write_protect_np() function.
+// See RwxMemoryWriteScope for details.
+class V8_NODISCARD AssemblerBufferWriteScope final {
+ public:
+  explicit AssemblerBufferWriteScope(TestingAssemblerBuffer& buffer)
+      : buffer_(buffer) {
+    buffer_.MakeWritable();
+  }
+
+  ~AssemblerBufferWriteScope() { buffer_.MakeExecutable(); }
+
+  // Disable copy constructor and copy-assignment operator, since this manages
+  // a resource and implicit copying of the scope can yield surprising errors.
+  AssemblerBufferWriteScope(const AssemblerBufferWriteScope&) = delete;
+  AssemblerBufferWriteScope& operator=(const AssemblerBufferWriteScope&) =
+      delete;
+
+ private:
+  RwxMemoryWriteScope rwx_write_scope_;
+  TestingAssemblerBuffer& buffer_;
+};
+
 static inline std::unique_ptr<TestingAssemblerBuffer> AllocateAssemblerBuffer(
     size_t requested = v8::internal::AssemblerBase::kDefaultBufferSize,
     void* address = nullptr,

test/unittests/assembler/turbo-assembler-arm64-unittest.cc

@@ -37,13 +37,16 @@ TEST_F(TurboAssemblerTest, TestHardAbort) {
   __ set_root_array_available(false);
   __ set_abort_hard(true);
 
+  {
+    AssemblerBufferWriteScope rw_scope(*buffer);
+
     __ CodeEntry();
 
     __ Abort(AbortReason::kNoReason);
 
     CodeDesc desc;
     tasm.GetCode(isolate(), &desc);
-  buffer->MakeExecutable();
+  }
   // We need an isolate here to execute in the simulator.
   auto f = GeneratedCode<void>::FromBuffer(isolate(), buffer->start());
 
@@ -57,6 +60,9 @@ TEST_F(TurboAssemblerTest, TestCheck) {
   __ set_root_array_available(false);
   __ set_abort_hard(true);
 
+  {
+    AssemblerBufferWriteScope rw_scope(*buffer);
+
     __ CodeEntry();
 
     // Fail if the first parameter is 17.
@@ -67,7 +73,7 @@ TEST_F(TurboAssemblerTest, TestCheck) {
 
     CodeDesc desc;
     tasm.GetCode(isolate(), &desc);
-  buffer->MakeExecutable();
+  }
   // We need an isolate here to execute in the simulator.
   auto f = GeneratedCode<void, int>::FromBuffer(isolate(), buffer->start());
 
@@ -119,6 +125,9 @@ TEST_P(TurboAssemblerTestMoveObjectAndSlot, MoveObjectAndSlot) {
     TurboAssembler tasm(nullptr, AssemblerOptions{}, CodeObjectRequired::kNo,
                         buffer->CreateView());
 
+    {
+      AssemblerBufferWriteScope rw_buffer_scope(*buffer);
+
       __ CodeEntry();
       __ Push(x0, padreg);
       __ Mov(test_case.object, x1);
@@ -161,12 +170,13 @@ TEST_P(TurboAssemblerTestMoveObjectAndSlot, MoveObjectAndSlot) {
       tasm.GetCode(nullptr, &desc);
       if (FLAG_print_code) {
         Handle<Code> code =
-          Factory::CodeBuilder(isolate(), desc, CodeKind::FOR_TESTING).Build();
+            Factory::CodeBuilder(isolate(), desc, CodeKind::FOR_TESTING)
+                .Build();
         StdoutStream os;
         code->Print(os);
       }
+    }
 
-    buffer->MakeExecutable();
     // We need an isolate here to execute in the simulator.
     auto f = GeneratedCode<void, byte**, byte*>::FromBuffer(isolate(),
                                                             buffer->start());