[heap] Always allocate executable MemoryChunks rw.

This CL also narrows the rw scopes on various call sites.

Bug: chromium:774108,v8:6792
Change-Id: I41a6f5dc4948833baaa441fb998ef40d8a832619
Reviewed-on: https://chromium-review.googlesource.com/758370
Commit-Queue: Hannes Payer <hpayer@chromium.org>
Reviewed-by: Clemens Hammacher <clemensh@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#49411}

src/allocation.cc

@@ -138,9 +138,9 @@ void VirtualMemory::Reset() {
   size_ = 0;
 }
 
-bool VirtualMemory::Commit(void* address, size_t size, bool is_executable) {
+bool VirtualMemory::Commit(void* address, size_t size) {
   CHECK(InVM(address, size));
-  return base::OS::CommitRegion(address, size, is_executable);
+  return base::OS::CommitRegion(address, size);
 }
 
 bool VirtualMemory::Uncommit(void* address, size_t size) {

src/allocation.h

@@ -129,7 +129,7 @@ class V8_EXPORT_PRIVATE VirtualMemory {
   size_t size() const { return size_; }
 
   // Commits real memory. Returns whether the operation succeeded.
-  bool Commit(void* address, size_t size, bool is_executable);
+  bool Commit(void* address, size_t size);
 
   // Uncommit real memory.  Returns whether the operation succeeded.
   bool Uncommit(void* address, size_t size);

src/base/platform/platform-cygwin.cc

@@ -141,9 +141,11 @@ bool OS::Free(void* address, const size_t size) {
 }
 
 // static
-bool OS::CommitRegion(void* address, size_t size, bool is_executable) {
-  DWORD protect = is_executable ? PAGE_EXECUTE_READWRITE : PAGE_READWRITE;
-  return VirtualAlloc(address, size, MEM_COMMIT, protect) != nullptr;
+bool OS::CommitRegion(void* address, size_t size) {
+  if (nullptr == VirtualAlloc(address, size, MEM_COMMIT, PAGE_READWRITE)) {
+    return false;
+  }
+  return true;
 }
 
 // static

src/base/platform/platform-fuchsia.cc

@@ -87,12 +87,10 @@ void OS::Guard(void* address, size_t size) {
 }
 
 // static
-bool OS::CommitRegion(void* address, size_t size, bool is_executable) {
-  uint32_t prot = ZX_VM_FLAG_PERM_READ | ZX_VM_FLAG_PERM_WRITE |
-                  (is_executable ? ZX_VM_FLAG_PERM_EXECUTE : 0);
+bool OS::CommitRegion(void* address, size_t size) {
   return zx_vmar_protect(zx_vmar_root_self(),
                          reinterpret_cast<uintptr_t>(address), size,
-                         prot) == ZX_OK;
+                         ZX_VM_FLAG_PERM_READ | ZX_VM_FLAG_PERM_WRITE) == ZX_OK;
 }
 
 // static

src/base/platform/platform-posix.cc

@@ -314,16 +314,15 @@ void OS::SetReadWriteAndExecutable(void* address, const size_t size) {
 
 #if !V8_OS_CYGWIN && !V8_OS_FUCHSIA
 // static
-bool OS::CommitRegion(void* address, size_t size, bool is_executable) {
-  int prot = PROT_READ | PROT_WRITE | (is_executable ? PROT_EXEC : 0);
+bool OS::CommitRegion(void* address, size_t size) {
 #if !V8_OS_AIX
-  if (MAP_FAILED == mmap(address, size, prot,
+  if (MAP_FAILED == mmap(address, size, PROT_READ | PROT_WRITE,
                          MAP_PRIVATE | MAP_ANONYMOUS | MAP_FIXED, kMmapFd,
                          kMmapFdOffset)) {
     return false;
   }
 #else
-  if (mprotect(address, size, prot) == -1) return false;
+  if (mprotect(address, size, PROT_READ | PROT_WRITE) == -1) return false;
 #endif  // !V8_OS_AIX
   return true;
 }

src/base/platform/platform-win32.cc

@@ -846,9 +846,11 @@ void OS::SetReadWriteAndExecutable(void* address, const size_t size) {
 }
 
 // static
-bool OS::CommitRegion(void* address, size_t size, bool is_executable) {
-  DWORD protect = is_executable ? PAGE_EXECUTE_READWRITE : PAGE_READWRITE;
-  return VirtualAlloc(address, size, MEM_COMMIT, protect) != nullptr;
+bool OS::CommitRegion(void* address, size_t size) {
+  if (NULL == VirtualAlloc(address, size, MEM_COMMIT, PAGE_READWRITE)) {
+    return false;
+  }
+  return true;
 }
 
 // static

src/base/platform/platform.h

@@ -194,7 +194,7 @@ class V8_BASE_EXPORT OS {
   // function. This is only a temporary function and will go away soon.
   static void SetReadWriteAndExecutable(void* address, const size_t size);
 
-  static bool CommitRegion(void* address, size_t size, bool is_executable);
+  static bool CommitRegion(void* address, size_t size);
 
   static bool UncommitRegion(void* address, size_t size);
 

src/heap/spaces.cc

@@ -131,7 +131,7 @@ bool CodeRange::SetUp(size_t requested) {
   // On some platforms, specifically Win64, we need to reserve some pages at
   // the beginning of an executable space.
   if (reserved_area > 0) {
-    if (!reservation.Commit(base, reserved_area, true)) return false;
+    if (!reservation.Commit(base, reserved_area)) return false;
 
     base += reserved_area;
   }
@@ -414,7 +414,7 @@ bool MemoryAllocator::CanFreeMemoryChunk(MemoryChunk* chunk) {
 
 bool MemoryAllocator::CommitMemory(Address base, size_t size,
                                    Executability executable) {
-  if (!base::OS::CommitRegion(base, size, executable == EXECUTABLE)) {
+  if (!base::OS::CommitRegion(base, size)) {
     return false;
   }
   UpdateAllocatedSpaceLimits(base, base + size);
@@ -477,7 +477,7 @@ Address MemoryAllocator::AllocateAlignedMemory(
       base = nullptr;
     }
   } else {
-    if (reservation.Commit(base, commit_size, false)) {
+    if (reservation.Commit(base, commit_size)) {
       UpdateAllocatedSpaceLimits(base, base + commit_size);
     } else {
       base = nullptr;
@@ -618,6 +618,11 @@ MemoryChunk* MemoryChunk::Initialize(Heap* heap, Address base, size_t size,
 
   if (executable == EXECUTABLE) {
     chunk->SetFlag(IS_EXECUTABLE);
+    if (FLAG_write_protect_code_memory) {
+      chunk->write_unprotect_counter_ = 1;
+    } else {
+      base::OS::SetReadWriteAndExecutable(area_start, area_end - area_start);
+    }
   }
 
   if (reservation != nullptr) {
@@ -1163,13 +1168,13 @@ bool MemoryAllocator::CommitExecutableMemory(VirtualMemory* vm, Address start,
   // Commit page header (not executable).
   Address header = start;
   size_t header_size = CodePageGuardStartOffset();
-  if (vm->Commit(header, header_size, false)) {
+  if (vm->Commit(header, header_size)) {
     // Create guard page after the header.
     if (vm->Guard(start + CodePageGuardStartOffset())) {
       // Commit page body (executable).
       Address body = start + CodePageAreaStartOffset();
       size_t body_size = commit_size - CodePageGuardStartOffset();
-      if (vm->Commit(body, body_size, true)) {
+      if (vm->Commit(body, body_size)) {
         // Create guard page before the end.
         if (vm->Guard(start + reserved_size - CodePageGuardSize())) {
           UpdateAllocatedSpaceLimits(start, start + CodePageAreaStartOffset() +

src/heap/spaces.h

@@ -699,6 +699,10 @@ class MemoryChunk {
   // If Value() == 0 => The memory is read and executable.
   // If Value() >= 1 => The Memory is read and writable (and maybe executable).
   // The maximum value can right now only be 3.
+  // All executable MemoryChunks are allocated rw based on the assumption that
+  // they will be used immediatelly for an allocation. Hence they are
+  // initialized with 1. The caller that triggers the page allocation is
+  // responsible to make the MemoryChunk rx.
   uintptr_t write_unprotect_counter_;
 
   // Byte allocated on the page, which includes all objects on the page

src/heap/store-buffer.cc

@@ -57,8 +57,7 @@ void StoreBuffer::SetUp() {
   }
 
   if (!reservation.Commit(reinterpret_cast<Address>(start_[0]),
-                          kStoreBufferSize * kStoreBuffers,
-                          false)) {  // Not executable.
+                          kStoreBufferSize * kStoreBuffers)) {
     V8::FatalProcessOutOfMemory("StoreBuffer::SetUp");
   }
   current_ = 0;

src/isolate.cc

@@ -2863,6 +2863,7 @@ bool Isolate::Init(StartupDeserializer* des) {
   // If we are deserializing, read the state into the now-empty heap.
   {
     AlwaysAllocateScope always_allocate(this);
+    CodeSpaceMemoryModificationScope modification_scope(&heap_);
 
     if (!create_heap_objects) des->DeserializeInto(this);
     load_stub_cache_->Initialize();

src/snapshot/code-serializer.cc

@@ -242,9 +242,6 @@ MaybeHandle<FixedArray> WasmCompiledModuleSerializer::DeserializeWasmModule(
     return nothing;
   }
 
-  // TODO(6792): No longer needed once WebAssembly code is off heap.
-  CodeSpaceMemoryModificationScope modification_scope(isolate->heap());
-
   MaybeHandle<WasmCompiledModule> maybe_result =
       ObjectDeserializer::DeserializeWasmCompiledModule(isolate, &scd,
                                                         wire_bytes);
@@ -252,6 +249,8 @@ MaybeHandle<FixedArray> WasmCompiledModuleSerializer::DeserializeWasmModule(
   Handle<WasmCompiledModule> result;
   if (!maybe_result.ToHandle(&result)) return nothing;
 
+  // TODO(6792): No longer needed once WebAssembly code is off heap.
+  CodeSpaceMemoryModificationScope modification_scope(isolate->heap());
   WasmCompiledModule::ReinitializeAfterDeserialization(isolate, result);
   DCHECK(WasmCompiledModule::IsWasmCompiledModule(*result));
   return result;

src/snapshot/object-deserializer.cc

@@ -67,6 +67,9 @@ ObjectDeserializer::DeserializeWasmCompiledModule(
 
 MaybeHandle<HeapObject> ObjectDeserializer::Deserialize(Isolate* isolate) {
   Initialize(isolate);
+
+  CodeSpaceMemoryModificationScope modification_scope(isolate->heap());
+
   if (!allocator()->ReserveSpace()) return MaybeHandle<HeapObject>();
 
   DCHECK(deserializing_user_code());

src/wasm/module-compiler.cc

@@ -1248,9 +1248,6 @@ Handle<Code> EnsureExportedLazyDeoptData(Isolate* isolate,
     return code;
   }
 
-  // TODO(6792): No longer needed once WebAssembly code is off heap.
-  CodeSpaceMemoryModificationScope modification_scope(isolate->heap());
-
   // deopt_data:
   //   #0: weak instance
   //   #1: func_index
@@ -1262,6 +1259,8 @@ Handle<Code> EnsureExportedLazyDeoptData(Isolate* isolate,
     code = isolate->factory()->CopyCode(code);
     code_table->set(func_index, *code);
     deopt_data = isolate->factory()->NewFixedArray(2, TENURED);
+    // TODO(6792): No longer needed once WebAssembly code is off heap.
+    CodeSpaceMemoryModificationScope modification_scope(isolate->heap());
     code->set_deoptimization_data(*deopt_data);
     if (!instance.is_null()) {
       Handle<WeakCell> weak_instance =

src/wasm/wasm-code-specialization.cc

@@ -111,9 +111,6 @@ bool CodeSpecialization::ApplyToWholeInstance(
   bool changed = false;
   int func_index = module->num_imported_functions;
 
-  // TODO(6792): No longer needed once WebAssembly code is off heap.
-  CodeSpaceMemoryModificationScope modification_scope(instance->GetHeap());
-
   // Patch all wasm functions.
   for (int num_wasm_functions = static_cast<int>(wasm_functions->size());
        func_index < num_wasm_functions; ++func_index) {
@@ -122,6 +119,9 @@ bool CodeSpecialization::ApplyToWholeInstance(
     changed |= ApplyToWasmCode(wasm_function, icache_flush_mode);
   }
 
+  // TODO(6792): No longer needed once WebAssembly code is off heap.
+  CodeSpaceMemoryModificationScope modification_scope(instance->GetHeap());
+
   // Patch all exported functions (JS_TO_WASM_FUNCTION).
   int reloc_mode = 0;
   // We need to patch WASM_CONTEXT_REFERENCE to put the correct address.

test/cctest/test-platform.cc

@@ -41,7 +41,7 @@ TEST(OSReserveMemory) {
   CHECK_NE(0, page_size);
   CHECK_NOT_NULL(mem_addr);
   size_t commit_size = OS::CommitPageSize();
-  CHECK(OS::CommitRegion(mem_addr, commit_size, false));
+  CHECK(OS::CommitRegion(mem_addr, commit_size));
   // Check whether we can write to memory.
   int* addr = static_cast<int*>(mem_addr);
   addr[KB - 1] = 2;