[heap] Sweeper sets code memory to rwx to allow concurrent sweeping while executing code.

Bug: chromium:774108,v8:6792
Change-Id: Ibdb00bee4dc563663ef2151b489600a2b100f146
Reviewed-on: https://chromium-review.googlesource.com/739601
Commit-Queue: Hannes Payer <hpayer@chromium.org>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#49143}

src/base/platform/platform-posix.cc

@@ -260,6 +260,17 @@ void OS::SetReadAndWritable(void* address, const size_t size, bool commit) {
 #endif
 }
 
+// Make a region of memory readable, writable, and executable.
+void OS::SetReadWriteAndExecutable(void* address, const size_t size) {
+#if V8_OS_CYGWIN
+  DWORD oldprotect;
+  CHECK_NOT_NULL(
+      VirtualProtect(address, size, PAGE_EXECUTE_READWRITE, &oldprotect));
+#else
+  CHECK_EQ(0, mprotect(address, size, PROT_READ | PROT_WRITE | PROT_EXEC));
+#endif
+}
+
 #if !V8_OS_CYGWIN && !V8_OS_FUCHSIA
 // static
 void* OS::ReserveRegion(size_t size, void* hint) {

src/base/platform/platform-win32.cc

@@ -818,6 +818,13 @@ void OS::SetReadAndWritable(void* address, const size_t size, bool commit) {
   }
 }
 
+// Make a region of memory readable, writable, and executable.
+void OS::SetReadWriteAndExecutable(void* address, const size_t size) {
+  DWORD oldprotect;
+  CHECK_NE(NULL,
+           VirtualProtect(address, size, PAGE_EXECUTE_READWRITE, &oldprotect));
+}
+
 // static
 void* OS::ReserveRegion(size_t size, void* hint) {
   return RandomizedVirtualAlloc(size, MEM_RESERVE, PAGE_NOACCESS, hint);

src/base/platform/platform.h

@@ -188,6 +188,10 @@ class V8_BASE_EXPORT OS {
   // Make a region of memory non-executable but readable and writable.
   static void SetReadAndWritable(void* address, const size_t size, bool commit);
 
+  // Make a region of memory read, write, and executable. Do not use this
+  // function. This is only a temporary function and will go away soon.
+  static void SetReadWriteAndExecutable(void* address, const size_t size);
+
   static void* ReserveRegion(size_t size, void* hint);
 
   static void* ReserveAlignedRegion(size_t size, size_t alignment, void* hint,

src/deoptimizer.cc

@@ -2018,7 +2018,8 @@ void Deoptimizer::EnsureCodeForDeoptimizationEntry(Isolate* isolate,
 
   // TODO(mstarzinger,6792): This code-space modification section should be
   // moved into {Heap} eventually and a safe wrapper be provided.
-  CodePageMemoryModificationScope modification_scope(chunk);
+  CodePageMemoryModificationScope modification_scope(
+      chunk, CodePageMemoryModificationScope::READ_WRITE);
 
   CHECK(static_cast<int>(Deoptimizer::GetMaxDeoptTableSize()) >=
         desc.instr_size);

src/heap/heap-inl.h

@@ -611,7 +611,7 @@ CodeSpaceMemoryModificationScope::~CodeSpaceMemoryModificationScope() {
 }
 
 CodePageMemoryModificationScope::CodePageMemoryModificationScope(
-    MemoryChunk* chunk)
+    MemoryChunk* chunk, CodePageModificationMode mode)
     : chunk_(chunk),
       scope_active_(FLAG_write_protect_code_memory &&
                     chunk_->IsFlagSet(MemoryChunk::IS_EXECUTABLE)) {
@@ -622,7 +622,11 @@ CodePageMemoryModificationScope::CodePageMemoryModificationScope(
            chunk_->owner()->identity() == CODE_SPACE ||
            (chunk_->owner()->identity() == LO_SPACE &&
             chunk_->IsFlagSet(MemoryChunk::IS_EXECUTABLE)));
-    chunk_->SetReadAndWritable();
+    if (mode == READ_WRITE_EXECUTABLE) {
+      chunk_->SetReadWriteAndExecutable();
+    } else {
+      chunk_->SetReadAndWritable();
+    }
   }
 }
 

src/heap/heap.h

@@ -2541,7 +2541,11 @@ class CodeSpaceMemoryModificationScope {
 
 class CodePageMemoryModificationScope {
  public:
-  explicit inline CodePageMemoryModificationScope(MemoryChunk* chunk);
+  enum CodePageModificationMode { READ_WRITE, READ_WRITE_EXECUTABLE };
+  // TODO(hpayer): Remove set_executable from the constructor. Code pages should
+  // never be executable and writable at the same time.
+  inline CodePageMemoryModificationScope(MemoryChunk* chunk,
+                                         CodePageModificationMode mode);
   inline ~CodePageMemoryModificationScope();
 
  private:

src/heap/mark-compact.cc

@@ -4449,8 +4449,10 @@ int MarkCompactCollector::Sweeper::ParallelSweepPage(Page* page,
     if (page->SweepingDone()) return 0;
 
     // If the page is a code page, the CodePageMemoryModificationScope changes
-    // the page protection mode from read+execute to read+write while sweeping.
-    CodePageMemoryModificationScope code_page_scope(page);
+    // the page protection mode from rx -> rwx while sweeping.
+    // TODO(hpayer): Allow only rx -> rw transitions.
+    CodePageMemoryModificationScope code_page_scope(
+        page, CodePageMemoryModificationScope::READ_WRITE_EXECUTABLE);
 
     DCHECK_EQ(Page::kSweepingPending,
               page->concurrent_sweeping_state().Value());

src/heap/scavenger.cc

@@ -96,7 +96,8 @@ void Scavenger::AddPageToSweeperIfNecessary(MemoryChunk* page) {
 }
 
 void Scavenger::ScavengePage(MemoryChunk* page) {
-  CodePageMemoryModificationScope memory_modification_scope(page);
+  CodePageMemoryModificationScope memory_modification_scope(
+      page, CodePageMemoryModificationScope::READ_WRITE);
   RememberedSet<OLD_TO_NEW>::Iterate(
       page,
       [this](Address addr) { return CheckAndScavengeObject(heap_, addr); },

src/heap/spaces.cc

@@ -574,6 +574,25 @@ void MemoryChunk::SetReadAndWritable() {
   }
 }
 
+void MemoryChunk::SetReadWriteAndExecutable() {
+  DCHECK(IsFlagSet(MemoryChunk::IS_EXECUTABLE));
+  // TODO(hpayer): owner() can only be null if we use the MemoryChunk outside
+  // of spaces. We actually should not do that and we should untangle this.
+  DCHECK(owner() == nullptr || owner()->identity() == CODE_SPACE ||
+         owner()->identity() == LO_SPACE);
+  // Incrementing the write_unprotect_counter_ and changing the page
+  // protection mode has to be atomic.
+  base::LockGuard<base::Mutex> guard(page_protection_change_mutex_);
+  write_unprotect_counter_++;
+  DCHECK_LE(write_unprotect_counter_, 2);
+  Address unprotect_start =
+      address() + MemoryAllocator::CodePageAreaStartOffset();
+  size_t unprotect_size = size() - MemoryAllocator::CodePageAreaStartOffset();
+  DCHECK(
+      IsAddressAligned(unprotect_start, MemoryAllocator::GetCommitPageSize()));
+  base::OS::SetReadWriteAndExecutable(unprotect_start, unprotect_size);
+}
+
 MemoryChunk* MemoryChunk::Initialize(Heap* heap, Address base, size_t size,
                                      Address area_start, Address area_end,
                                      Executability executable, Space* owner,

src/heap/spaces.h

@@ -638,6 +638,9 @@ class MemoryChunk {
   void SetReadAndExecutable();
   void SetReadAndWritable();
 
+  // TODO(hpayer): Remove this method. Memory should never be rwx.
+  void SetReadWriteAndExecutable();
+
  protected:
   static MemoryChunk* Initialize(Heap* heap, Address base, size_t size,
                                  Address area_start, Address area_end,