[wasm] W^X mechanism for WasmCodeManager.

This enables the invariant that a NativeModule's code may either be
executable or writable, but never both at the same time.

Bug: v8:7105
Change-Id: If2abfce6796a365bb675a82140f32e8f45bb923f
Reviewed-on: https://chromium-review.googlesource.com/804208
Commit-Queue: Mircea Trofin <mtrofin@chromium.org>
Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#49840}

src/wasm/module-compiler.cc

@@ -497,6 +497,28 @@ void InstanceFinalizer(const v8::WeakCallbackInfo<void>& data) {
   TRACE("}\n");
 }
 
+// This is used in ProcessImports.
+// When importing other modules' exports, we need to ask
+// the exporter for a WasmToWasm wrapper. To do that, we need to
+// switch that module to RW. To avoid flip-floping the same module
+// RW <->RX, we create a scope for a set of NativeModules.
+class SetOfNativeModuleModificationScopes final {
+ public:
+  void Add(NativeModule* module) {
+    module->SetExecutable(false);
+    native_modules_.insert(module);
+  }
+
+  ~SetOfNativeModuleModificationScopes() {
+    for (NativeModule* module : native_modules_) {
+      module->SetExecutable(true);
+    }
+  }
+
+ private:
+  std::unordered_set<NativeModule*> native_modules_;
+};
+
 }  // namespace
 
 bool SyncValidate(Isolate* isolate, const ModuleWireBytes& bytes) {
@@ -780,6 +802,9 @@ Address CompileLazy(Isolate* isolate) {
           ->get();
   DCHECK(!orchestrator->IsFrozenForTesting());
 
+  NativeModuleModificationScope native_module_modification_scope(
+      compiled_module->GetNativeModule());
+
   const wasm::WasmCode* result = nullptr;
   // The caller may be js to wasm calling a function
   // also available for indirect calls.
@@ -2278,6 +2303,11 @@ MaybeHandle<WasmInstanceObject> InstanceBuilder::Build() {
     }
     compiled_module_->set_native_context(isolate_->native_context());
   }
+  base::Optional<wasm::NativeModuleModificationScope>
+      native_module_modification_scope;
+  if (native_module != nullptr) {
+    native_module_modification_scope.emplace(native_module);
+  }
 
   //--------------------------------------------------------------------------
   // Allocate the instance object.
@@ -2540,8 +2570,9 @@ MaybeHandle<WasmInstanceObject> InstanceBuilder::Build() {
     RecordStats(startup_code, counters());
     // Call the JS function.
     Handle<Object> undefined = factory->undefined_value();
-    // Close the CodeSpaceMemoryModificationScope to execute the start function.
+    // Close the modification scopes, so we can execute the start function.
     modification_scope.reset();
+    native_module_modification_scope.reset();
     {
       // We're OK with JS execution here. The instance is fully setup.
       AllowJavascriptExecution allow_js(isolate_);
@@ -2767,6 +2798,8 @@ int InstanceBuilder::ProcessImports(Handle<FixedArray> code_table,
   int num_imported_tables = 0;
   Handle<FixedArray> js_imports_table = SetupWasmToJSImportsTable(instance);
   WasmInstanceMap imported_wasm_instances(isolate_->heap());
+  SetOfNativeModuleModificationScopes set_of_native_module_scopes;
+
   DCHECK_EQ(module_->import_table.size(), sanitized_imports_.size());
   for (int index = 0; index < static_cast<int>(module_->import_table.size());
        ++index) {
@@ -2894,6 +2927,7 @@ int InstanceBuilder::ProcessImports(Handle<FixedArray> code_table,
               Handle<Code> wrapper = compiler::CompileWasmToWasmWrapper(
                   isolate_, target->GetWasmCode(), sig,
                   reinterpret_cast<Address>(other_context));
+              set_of_native_module_scopes.Add(exporting_module);
               wrapper_code = exporting_module->AddExportedWrapper(
                   wrapper, exported_code->index());
             }

src/wasm/wasm-debug.cc

@@ -6,6 +6,7 @@
 
 #include "src/assembler-inl.h"
 #include "src/assert-scope.h"
+#include "src/base/optional.h"
 #include "src/compiler/wasm-compiler.h"
 #include "src/debug/debug-scopes.h"
 #include "src/debug/debug.h"
@@ -728,7 +729,15 @@ void WasmDebugInfo::RedirectToInterpreter(Handle<WasmDebugInfo> debug_info,
   Handle<FixedArray> code_table = instance->compiled_module()->code_table();
   CodeRelocationMapGC code_to_relocate_gc(isolate->heap());
   // TODO(6792): No longer needed once WebAssembly code is off heap.
-  CodeSpaceMemoryModificationScope modification_scope(isolate->heap());
+  base::Optional<CodeSpaceMemoryModificationScope> modification_scope;
+  base::Optional<wasm::NativeModuleModificationScope>
+      native_module_modification_scope;
+  if (!FLAG_wasm_jit_to_native) {
+    modification_scope.emplace(isolate->heap());
+  } else {
+    native_module_modification_scope.emplace(native_module);
+  }
+
   for (int func_index : func_indexes) {
     DCHECK_LE(0, func_index);
     DCHECK_GT(debug_info->wasm_instance()->module()->functions.size(),

src/wasm/wasm-heap.cc

@@ -687,9 +687,11 @@ bool WasmCodeManager::Commit(Address start, size_t size) {
     remaining_uncommitted_.Increment(size);
     return false;
   }
-  // TODO(v8:7105) Enable W^X instead of setting W|X permissions below.
-  bool ret = base::OS::SetPermissions(
-      start, size, base::OS::MemoryPermission::kReadWriteExecute);
+  bool ret = base::OS::SetPermissions(start, size,
+                                      base::OS::MemoryPermission::kReadWrite);
+  TRACE_HEAP("Setting rw permissions for %p:%p\n",
+             reinterpret_cast<void*>(start),
+             reinterpret_cast<void*>(start + size));
   if (!ret) {
     // Highly unlikely.
     remaining_uncommitted_.Increment(size);
@@ -783,6 +785,51 @@ std::unique_ptr<NativeModule> WasmCodeManager::NewNativeModule(
   return nullptr;
 }
 
+bool NativeModule::SetExecutable(bool executable) {
+  if (is_executable_ == executable) return true;
+  TRACE_HEAP("Setting module %zu as executable: %d.\n", instance_id,
+             executable);
+  base::OS::MemoryPermission permission =
+      executable ? base::OS::MemoryPermission::kReadExecute
+                 : base::OS::MemoryPermission::kReadWrite;
+
+#if V8_OS_WIN
+  // On windows, we need to switch permissions per separate virtual memory
+  // reservation. This is really just a problem when the NativeModule is
+  // growable (meaning can_request_more_memory_). That's 32-bit in production,
+  // or unittests.
+  // For now, in that case, we commit at reserved memory granularity.
+  // Technically, that may be a waste, because we may reserve more than we use.
+  // On 32-bit though, the scarce resource is the address space - committed or
+  // not.
+  if (can_request_more_memory_) {
+    for (auto& vmem : owned_memory_) {
+      if (!base::OS::SetPermissions(vmem.address(), vmem.size(), permission)) {
+        return false;
+      }
+      TRACE_HEAP("Set %p:%p to executable:%d\n", vmem.address(), vmem.end(),
+                 executable);
+    }
+    is_executable_ = executable;
+    return true;
+  }
+#endif
+  for (auto& range : allocated_memory_.ranges()) {
+    // allocated_memory_ is fine-grained, so we need to
+    // page-align it.
+    size_t range_size = RoundUp(static_cast<size_t>(range.second - range.first),
+                                base::OS::AllocatePageSize());
+    if (!base::OS::SetPermissions(range.first, range_size, permission)) {
+      return false;
+    }
+    TRACE_HEAP("Set %p:%p to executable:%d\n",
+               reinterpret_cast<void*>(range.first),
+               reinterpret_cast<void*>(range.second), executable);
+  }
+  is_executable_ = executable;
+  return true;
+}
+
 std::unique_ptr<NativeModule> NativeModule::Clone() {
   std::unique_ptr<NativeModule> ret = wasm_code_manager_->NewNativeModule(
       owned_memory_.front().size(), FunctionCount(), num_imported_functions(),
@@ -913,6 +960,18 @@ void WasmCodeManager::FlushICache(Address start, size_t size) {
                          size);
 }
 
+NativeModuleModificationScope::NativeModuleModificationScope(
+    NativeModule* native_module)
+    : native_module_(native_module) {
+  bool success = native_module_->SetExecutable(false);
+  CHECK(success);
+}
+
+NativeModuleModificationScope::~NativeModuleModificationScope() {
+  bool success = native_module_->SetExecutable(true);
+  CHECK(success);
+}
+
 }  // namespace wasm
 }  // namespace internal
 }  // namespace v8

src/wasm/wasm-heap.h

@@ -231,6 +231,8 @@ class V8_EXPORT_PRIVATE NativeModule final {
   // this change.
   WasmCode* CloneLazyBuiltinInto(uint32_t);
 
+  bool SetExecutable(bool executable);
+
   // For cctests, where we build both WasmModule and the runtime objects
   // on the fly, and bypass the instance builder pipeline.
   void ResizeCodeTableForTest(size_t);
@@ -328,6 +330,7 @@ class V8_EXPORT_PRIVATE NativeModule final {
   Handle<WasmCompiledModule> compiled_module_;
   size_t committed_memory_ = 0;
   bool can_request_more_memory_;
+  bool is_executable_ = false;
 
   // Specialization data that needs to be serialized and cloned.
   // Keeping it groupped together because it makes cloning of all these
@@ -392,6 +395,25 @@ class V8_EXPORT_PRIVATE WasmCodeManager final {
   v8::Isolate* isolate_;
 };
 
+// Within the scope, the native_module is writable and not executable.
+// At the scope's destruction, the native_module is executable and not writable.
+// The states inside the scope and at the scope termination are irrespective of
+// native_module's state when entering the scope.
+// We currently mark the entire module's memory W^X:
+//  - for AOT, that's as efficient as it can be.
+//  - for Lazy, we don't have a heuristic for functions that may need patching,
+//    and even if we did, the resulting set of pages may be fragmented.
+//    Currently, we try and keep the number of syscalls low.
+// -  similar argument for debug time.
+class NativeModuleModificationScope final {
+ public:
+  explicit NativeModuleModificationScope(NativeModule* native_module);
+  ~NativeModuleModificationScope();
+
+ private:
+  NativeModule* native_module_;
+};
+
 }  // namespace wasm
 }  // namespace internal
 }  // namespace v8

src/wasm/wasm-objects.cc

@@ -269,6 +269,8 @@ void WasmTableObject::Grow(Isolate* isolate, uint32_t count) {
           WasmInstanceObject::cast(dispatch_tables->get(i));
       WasmCompiledModule* compiled_module = instance->compiled_module();
       wasm::NativeModule* native_module = compiled_module->GetNativeModule();
+      wasm::NativeModuleModificationScope native_module_modification_scope(
+          native_module);
       GlobalHandleAddress old_function_table_addr =
           native_module->function_tables()[table_index];
       GlobalHandleAddress old_signature_table_addr =
@@ -334,6 +336,7 @@ void WasmTableObject::Set(Isolate* isolate, Handle<WasmTableObject> table,
     WasmCodeWrapper wasm_code = exported_function->GetWasmCode();
     if (!wasm_code.IsCodeObject()) {
       wasm::NativeModule* native_module = wasm_code.GetWasmCode()->owner();
+      wasm::NativeModuleModificationScope modification_scope(native_module);
       // we create the wrapper on the module exporting the function. This
       // wrapper will only be called as indirect call.
       wasm::WasmCode* exported_wrapper =
@@ -1250,6 +1253,8 @@ void WasmCompiledModule::Reset(Isolate* isolate,
   compiled_module->reset_next_instance();
   wasm::NativeModule* native_module = compiled_module->GetNativeModule();
   if (native_module == nullptr) return;
+  native_module->SetExecutable(false);
+
   TRACE("Resetting %zu\n", native_module->instance_id);
   if (trap_handler::UseTrapHandler()) {
     for (uint32_t i = native_module->num_imported_functions(),

test/cctest/wasm/test-c-wasm-entry.cc

@@ -71,6 +71,9 @@ class CWasmEntryArgTester {
     static_assert(
         arraysize(call_args) == compiler::CWasmEntryParameters::kNumParameters,
         "adapt this test");
+    if (FLAG_wasm_jit_to_native) {
+      wasm_code_.GetWasmCode()->owner()->SetExecutable(true);
+    }
     MaybeHandle<Object> return_obj = Execution::Call(
         isolate_, c_wasm_entry_fn_, receiver, arraysize(call_args), call_args);
     CHECK(!return_obj.is_null());

test/cctest/wasm/wasm-run-utils.h

@@ -222,6 +222,7 @@ class TestingModuleBuilder {
     if (!linked_) {
       native_module_->LinkAll();
       linked_ = true;
+      native_module_->SetExecutable(true);
     }
   }