Reland "[wasm] Reset PKRU before spawning new threads"

This is a reland of commit 8218c06158.
Compile errors on mac arm64 are fixed.

Original change's description:
> [wasm] Reset PKRU before spawning new threads
>
> We sometimes hit the DCHECK in the wasm code manager:
>   DCHECK_IMPLIES(writable, !MemoryProtectionKeyWritable());
>
> This is because we spawn new threads while having a
> {CodeSpaceWriteScope} open. In the case of PKU, this changes the PKRU
> register to allow writes to the code space, and the value of that
> register is inherited by any new thread. If this thread then tries to
> switch to writable code spaces, it hits the DCHECK. It would hit a
> similar DCHECK when trying to execute code.
>
> We fix this issue by temporarily resetting the PKRU register to
> non-writable while we call the {NotifyConcurrencyIncrease} method. This
> is not a very robust solution, as any new call that potentially happens
> inside a {CodeSpaceWriteScope} needs to do the same, but refactoring the
> code to avoid spawning new threads while being in writable state would
> be a lot of work with other downsides.
>
> R=jkummerow@chromium.org
>
> Bug: v8:13075
> Change-Id: Ibc7270aa597902dc6d9649cb6bcdfce8b1a9bafc
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3762579
> Commit-Queue: Clemens Backes <clemensb@chromium.org>
> Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#81729}

Bug: v8:13075
Cq-Include-Trybots: luci.v8.try:v8_mac_arm64_compile_rel
Cq-Include-Trybots: luci.v8.try:v8_mac_arm64_compile_dbg
Change-Id: I2e634959c969fc022393ae51c391397c7195ee54
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3769829
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Cr-Commit-Position: refs/heads/main@{#81781}

src/wasm/code-space-access.cc

@@ -96,6 +96,17 @@ bool CodeSpaceWriteScope::SwitchingPerNativeModule() {
          FLAG_wasm_write_protect_code_memory;
 }
 
+ResetPKUPermissionsForThreadSpawning::ResetPKUPermissionsForThreadSpawning() {
+  auto* code_manager = GetWasmCodeManager();
+  was_writable_ = code_manager->MemoryProtectionKeysEnabled() &&
+                  code_manager->MemoryProtectionKeyWritable();
+  if (was_writable_) code_manager->SetThreadWritable(false);
+}
+
+ResetPKUPermissionsForThreadSpawning::~ResetPKUPermissionsForThreadSpawning() {
+  if (was_writable_) GetWasmCodeManager()->SetThreadWritable(true);
+}
+
 #endif  // !V8_HAS_PTHREAD_JIT_WRITE_PROTECT
 
 }  // namespace wasm

src/wasm/code-space-access.h

@@ -12,10 +12,7 @@
 #include "src/base/build_config.h"
 #include "src/base/macros.h"
 
-namespace v8 {
-namespace internal {
-
-namespace wasm {
+namespace v8::internal::wasm {
 
 class NativeModule;
 
@@ -75,8 +72,27 @@ class V8_NODISCARD CodeSpaceWriteScope final {
   NativeModule* const previous_native_module_;
 };
 
-}  // namespace wasm
-}  // namespace internal
-}  // namespace v8
+// Sometimes we need to call a function which will / might spawn a new thread,
+// like {JobHandle::NotifyConcurrencyIncrease}, while holding a
+// {CodeSpaceWriteScope}. This is problematic since the new thread will inherit
+// the parent thread's PKU permissions.
+// The {ResetPKUPermissionsForThreadSpawning} scope will thus reset the PKU
+// permissions as long as it is in scope, such that it is safe to spawn new
+// threads.
+class V8_NODISCARD ResetPKUPermissionsForThreadSpawning {
+ public:
+#if !V8_HAS_PTHREAD_JIT_WRITE_PROTECT
+  ResetPKUPermissionsForThreadSpawning();
+  ~ResetPKUPermissionsForThreadSpawning();
+
+ private:
+  bool was_writable_;
+#else
+  // Define an empty constructor to avoid "unused variable" warnings.
+  ResetPKUPermissionsForThreadSpawning() {}
+#endif
+};
+
+}  // namespace v8::internal::wasm
 
 #endif  // V8_WASM_CODE_SPACE_ACCESS_H_

src/wasm/module-compiler.cc

@@ -3404,6 +3404,7 @@ void CompilationStateImpl::CommitCompilationUnits(
     compilation_unit_queues_.AddUnits(baseline_units, top_tier_units,
                                       native_module_->module());
   }
+  ResetPKUPermissionsForThreadSpawning pku_reset_scope;
   compile_job_->NotifyConcurrencyIncrease();
 }
 
@@ -3415,6 +3416,10 @@ void CompilationStateImpl::CommitTopTierCompilationUnit(
 void CompilationStateImpl::AddTopTierPriorityCompilationUnit(
     WasmCompilationUnit unit, size_t priority) {
   compilation_unit_queues_.AddTopTierPriorityUnit(unit, priority);
+  // We should not have a {CodeSpaceWriteScope} open at this point, as
+  // {NotifyConcurrencyIncrease} can spawn new threads which could inherit PKU
+  // permissions (which would be a security issue).
+  DCHECK(!CodeSpaceWriteScope::IsInScope());
   compile_job_->NotifyConcurrencyIncrease();
 }
 

src/wasm/wasm-serialization.cc

@@ -596,6 +596,7 @@ class DeserializeCodeTask : public JobTask {
         deserializer_->CopyAndRelocate(unit);
       }
       publish_queue_.Add(std::move(batch));
+      ResetPKUPermissionsForThreadSpawning pku_reset_scope;
       delegate->NotifyConcurrencyIncrease();
     }
   }
@@ -678,6 +679,7 @@ bool NativeModuleDeserializer::Read(Reader* reader) {
       reloc_queue.Add(std::move(batch));
       DCHECK(batch.empty());
       batch_size = 0;
+      ResetPKUPermissionsForThreadSpawning pku_reset_scope;
       job_handle->NotifyConcurrencyIncrease();
     }
   }
@@ -689,6 +691,7 @@ bool NativeModuleDeserializer::Read(Reader* reader) {
 
   if (!batch.empty()) {
     reloc_queue.Add(std::move(batch));
+    ResetPKUPermissionsForThreadSpawning pku_reset_scope;
     job_handle->NotifyConcurrencyIncrease();
   }