[wasm] Move writer tracking to CodeSpaceWriteScope

We had two implementations of a thread-local counter for the number of
writers: One in {CodeSpaceWriteScope} and one in
{WasmCodeManager::SetThreadWritable}. This CL removes the latter, and
uses the counter in {CodeSpaceWriteScope} for all implementations.

R=jkummerow@chromium.org

Bug: v8:11974
Cq-Include-Trybots: luci.v8.try:v8_mac_arm64_rel_ng
Cq-Include-Trybots: luci.v8.try:v8_mac_arm64_dbg_ng
Change-Id: I683131296c6106a2b12986942bb18e6c0e716612
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3024148Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#75733}

src/wasm/code-space-access.cc

@@ -11,59 +11,55 @@ namespace v8 {
 namespace internal {
 namespace wasm {
 
-#if defined(V8_OS_MACOSX) && defined(V8_HOST_ARCH_ARM64)
-
 thread_local int CodeSpaceWriteScope::code_space_write_nesting_level_ = 0;
 
-// The {NativeModule} argument is unused; it is just here for a common API with
-// the non-M1 implementation.
 // TODO(jkummerow): Background threads could permanently stay in
 // writable mode; only the main thread has to switch back and forth.
+#if defined(V8_OS_MACOSX) && defined(V8_HOST_ARCH_ARM64)
 CodeSpaceWriteScope::CodeSpaceWriteScope(NativeModule*) {
-  if (code_space_write_nesting_level_ == 0) {
-    SwitchMemoryPermissionsToWritable();
-  }
+#else
+CodeSpaceWriteScope::CodeSpaceWriteScope(NativeModule* native_module)
+    : native_module_(native_module) {
+#endif
+  if (code_space_write_nesting_level_ == 0) SetWritable();
   code_space_write_nesting_level_++;
 }
 
 CodeSpaceWriteScope::~CodeSpaceWriteScope() {
   code_space_write_nesting_level_--;
-  if (code_space_write_nesting_level_ == 0) {
-    SwitchMemoryPermissionsToExecutable();
-  }
+  if (code_space_write_nesting_level_ == 0) SetExecutable();
 }
 
-#else  // Not on MacOS on ARM64 (M1 hardware): Use Intel PKU and/or mprotect.
+#if defined(V8_OS_MACOSX) && defined(V8_HOST_ARCH_ARM64)
 
-CodeSpaceWriteScope::CodeSpaceWriteScope(NativeModule* native_module)
-    : native_module_(native_module) {
+void CodeSpaceWriteScope::SetWritable() const {
+  SwitchMemoryPermissionsToWritable();
+}
+
+void CodeSpaceWriteScope::SetExecutable() const {
+  SwitchMemoryPermissionsToExecutable();
+}
+
+#else  // Not Mac-on-arm64.
+
+void CodeSpaceWriteScope::SetWritable() const {
   DCHECK_NOT_NULL(native_module_);
-  if (FLAG_wasm_memory_protection_keys) {
-    auto* code_manager = GetWasmCodeManager();
-    if (code_manager->HasMemoryProtectionKeySupport()) {
-      code_manager->SetThreadWritable(true);
-      return;
-    }
-    // Fallback to mprotect-based write protection, if enabled.
-  }
-  if (FLAG_wasm_write_protect_code_memory) {
-    bool success = native_module_->SetWritable(true);
-    CHECK(success);
+  auto* code_manager = GetWasmCodeManager();
+  if (code_manager->HasMemoryProtectionKeySupport()) {
+    DCHECK(FLAG_wasm_memory_protection_keys);
+    code_manager->SetThreadWritable(true);
+  } else if (FLAG_wasm_write_protect_code_memory) {
+    CHECK(native_module_->SetWritable(true));
   }
 }
 
-CodeSpaceWriteScope::~CodeSpaceWriteScope() {
-  if (FLAG_wasm_memory_protection_keys) {
-    auto* code_manager = GetWasmCodeManager();
-    if (code_manager->HasMemoryProtectionKeySupport()) {
-      code_manager->SetThreadWritable(false);
-      return;
-    }
-    // Fallback to mprotect-based write protection, if enabled.
-  }
-  if (FLAG_wasm_write_protect_code_memory) {
-    bool success = native_module_->SetWritable(false);
-    CHECK(success);
+void CodeSpaceWriteScope::SetExecutable() const {
+  auto* code_manager = GetWasmCodeManager();
+  if (code_manager->HasMemoryProtectionKeySupport()) {
+    DCHECK(FLAG_wasm_memory_protection_keys);
+    code_manager->SetThreadWritable(false);
+  } else if (FLAG_wasm_write_protect_code_memory) {
+    CHECK(native_module_->SetWritable(false));
   }
 }
 

src/wasm/code-space-access.h

@@ -54,15 +54,18 @@ class V8_NODISCARD CodeSpaceWriteScope final {
   CodeSpaceWriteScope& operator=(const CodeSpaceWriteScope&) = delete;
 
  private:
-#if defined(V8_OS_MACOSX) && defined(V8_HOST_ARCH_ARM64)
   static thread_local int code_space_write_nesting_level_;
-#else   // On non-M1 hardware:
+
+  void SetWritable() const;
+  void SetExecutable() const;
+
   // The M1 implementation knows implicitly from the {MAP_JIT} flag during
-  // allocation which region to switch permissions for. On non-M1 hardware,
-  // however, we either need the protection key or code space from the
+  // allocation which region to switch permissions for. On non-M1 hardware
+  // without memory protection key support, we need the code space from the
   // {native_module_}.
-  NativeModule* native_module_;
-#endif  // defined(V8_OS_MACOSX) && defined(V8_HOST_ARCH_ARM64)
+#if !defined(V8_OS_MACOSX) || !defined(V8_HOST_ARCH_ARM64)
+  NativeModule* const native_module_;
+#endif
 };
 
 }  // namespace wasm

src/wasm/wasm-code-manager.cc

@@ -1970,14 +1970,6 @@ size_t WasmCodeManager::EstimateNativeModuleMetaDataSize(
 
 void WasmCodeManager::SetThreadWritable(bool writable) {
   DCHECK(HasMemoryProtectionKeySupport());
-  static thread_local int writable_nesting_level = 0;
-  if (writable) {
-    if (++writable_nesting_level > 1) return;
-  } else {
-    DCHECK_GT(writable_nesting_level, 0);
-    if (--writable_nesting_level > 0) return;
-  }
-  writable = writable_nesting_level > 0;
 
   MemoryProtectionKeyPermission permissions =
       writable ? kNoRestrictions : kDisableWrite;