Commit a4d12a86 authored by Igor Sheludko's avatar Igor Sheludko Committed by V8 LUCI CQ

[heap] Fix race in MemoryChunk protection logic

... when allocating Code objects from background thread.

Bug: chromium:1329012, chromium:1330887
Change-Id: Ia2731ba463381c826d14591f4ba3b3fe15d15a0b
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3688517
Commit-Queue: Igor Sheludko <ishell@chromium.org>
Reviewed-by: 's avatarDominik Inführ <dinfuehr@chromium.org>
Cr-Commit-Position: refs/heads/main@{#80948}
parent 620388b1
...@@ -651,10 +651,7 @@ CodePageCollectionMemoryModificationScope:: ...@@ -651,10 +651,7 @@ CodePageCollectionMemoryModificationScope::
CodePageCollectionMemoryModificationScope:: CodePageCollectionMemoryModificationScope::
~CodePageCollectionMemoryModificationScope() { ~CodePageCollectionMemoryModificationScope() {
if (heap_->write_protect_code_memory()) { if (heap_->write_protect_code_memory()) {
heap_->DecrementCodePageCollectionMemoryModificationScopeDepth(); heap_->ProtectUnprotectedMemoryChunks();
if (heap_->code_page_collection_memory_modification_scope_depth() == 0) {
heap_->ProtectUnprotectedMemoryChunks();
}
} }
} }
......
...@@ -2768,8 +2768,12 @@ void Heap::ComputeFastPromotionMode() { ...@@ -2768,8 +2768,12 @@ void Heap::ComputeFastPromotionMode() {
void Heap::UnprotectAndRegisterMemoryChunk(MemoryChunk* chunk, void Heap::UnprotectAndRegisterMemoryChunk(MemoryChunk* chunk,
UnprotectMemoryOrigin origin) { UnprotectMemoryOrigin origin) {
if (!write_protect_code_memory()) return; if (!write_protect_code_memory()) return;
base::MutexGuard guard(&unprotected_memory_chunks_mutex_);
// CodePageCollectionMemoryModificationScope can be used in multiple threads,
// so we have to check its depth behind the lock.
if (code_page_collection_memory_modification_scope_depth_ > 0) { if (code_page_collection_memory_modification_scope_depth_ > 0) {
base::MutexGuard guard(&unprotected_memory_chunks_mutex_);
if (unprotected_memory_chunks_.insert(chunk).second) { if (unprotected_memory_chunks_.insert(chunk).second) {
chunk->SetCodeModificationPermissions(); chunk->SetCodeModificationPermissions();
} }
...@@ -2790,6 +2794,11 @@ void Heap::UnregisterUnprotectedMemoryChunk(MemoryChunk* chunk) { ...@@ -2790,6 +2794,11 @@ void Heap::UnregisterUnprotectedMemoryChunk(MemoryChunk* chunk) {
void Heap::ProtectUnprotectedMemoryChunks() { void Heap::ProtectUnprotectedMemoryChunks() {
base::MutexGuard guard(&unprotected_memory_chunks_mutex_); base::MutexGuard guard(&unprotected_memory_chunks_mutex_);
// CodePageCollectionMemoryModificationScope can be used in multiple threads,
// so we have to check its depth behind the lock.
if (--code_page_collection_memory_modification_scope_depth_ > 0) return;
for (auto chunk = unprotected_memory_chunks_.begin(); for (auto chunk = unprotected_memory_chunks_.begin();
chunk != unprotected_memory_chunks_.end(); chunk++) { chunk != unprotected_memory_chunks_.end(); chunk++) {
DCHECK(memory_allocator()->IsMemoryChunkExecutable(*chunk)); DCHECK(memory_allocator()->IsMemoryChunkExecutable(*chunk));
......
...@@ -675,14 +675,6 @@ class Heap { ...@@ -675,14 +675,6 @@ class Heap {
code_page_collection_memory_modification_scope_depth_++; code_page_collection_memory_modification_scope_depth_++;
} }
void DecrementCodePageCollectionMemoryModificationScopeDepth() {
code_page_collection_memory_modification_scope_depth_--;
}
uintptr_t code_page_collection_memory_modification_scope_depth() {
return code_page_collection_memory_modification_scope_depth_;
}
inline HeapState gc_state() const { inline HeapState gc_state() const {
return gc_state_.load(std::memory_order_relaxed); return gc_state_.load(std::memory_order_relaxed);
} }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment