From 95b7d03067b32b8bd3bad196694e944a9c957b3e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dominik=20Inf=C3=BChr?= <dinfuehr@chromium.org>
Date: Mon, 6 Feb 2023 12:57:26 +0100
Subject: [PATCH] [heap] Move ActiveSystemPages out of page header
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

ActiveSystemPages uses std::bitset internally for convenience. Our
MemoryChunk fields implicitly assume that all fields only require
system pointer alignment.

However MSVC on 32-bit uses uint64_t internally to implement
std::bitset. Because of this ActiveSystemPages needs 8-bytes alignment
on 32-bit architectures. We can solve this by moving
ActiveSystemPages out of the page header and storing it in the
malloc()'ed heap.

Bug: v8:13716
Change-Id: Iecb17372d065c612bbdbca7d854c76d3256bc01d
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4223005
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Auto-Submit: Dominik Inführ <dinfuehr@chromium.org>
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/main@{#85681}
---
 src/heap/memory-chunk-layout.h |  6 +-----
 src/heap/memory-chunk.cc       | 15 +++++++++++----
 src/heap/memory-chunk.h        |  2 +-
 src/heap/spaces.h              |  2 +-
 4 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/src/heap/memory-chunk-layout.h b/src/heap/memory-chunk-layout.h
index 67e3a7b9dd..fd65684b12 100644
--- a/src/heap/memory-chunk-layout.h
+++ b/src/heap/memory-chunk-layout.h
@@ -39,11 +39,7 @@ class V8_EXPORT_PRIVATE MemoryChunkLayout {
  public:
   static constexpr int kNumSets = NUMBER_OF_REMEMBERED_SET_TYPES;
   static constexpr int kNumTypes = ExternalBackingStoreType::kNumTypes;
-#if V8_CC_MSVC && V8_TARGET_ARCH_IA32
-  static constexpr int kMemoryChunkAlignment = 8;
-#else
   static constexpr int kMemoryChunkAlignment = sizeof(size_t);
-#endif  // V8_CC_MSVC && V8_TARGET_ARCH_IA32
 #define FIELD(Type, Name) \
   k##Name##Offset, k##Name##End = k##Name##Offset + sizeof(Type) - 1
   enum Header {
@@ -73,7 +69,7 @@ class V8_EXPORT_PRIVATE MemoryChunkLayout {
     FIELD(FreeListCategory**, Categories),
     FIELD(CodeObjectRegistry*, CodeObjectRegistry),
     FIELD(PossiblyEmptyBuckets, PossiblyEmptyBuckets),
-    FIELD(ActiveSystemPages, ActiveSystemPages),
+    FIELD(ActiveSystemPages*, ActiveSystemPages),
 #ifdef V8_ENABLE_INNER_POINTER_RESOLUTION_OSB
     FIELD(ObjectStartBitmap, ObjectStartBitmap),
 #endif  // V8_ENABLE_INNER_POINTER_RESOLUTION_OSB
diff --git a/src/heap/memory-chunk.cc b/src/heap/memory-chunk.cc
index a2238416f5..6a98d11ffe 100644
--- a/src/heap/memory-chunk.cc
+++ b/src/heap/memory-chunk.cc
@@ -182,11 +182,13 @@ MemoryChunk::MemoryChunk(Heap* heap, BaseSpace* space, size_t chunk_size,
   possibly_empty_buckets_.Initialize();
 
   if (page_size == PageSize::kRegular) {
-    active_system_pages_.Init(MemoryChunkLayout::kMemoryChunkHeaderSize,
-                              MemoryAllocator::GetCommitPageSizeBits(), size());
+    active_system_pages_ = new ActiveSystemPages;
+    active_system_pages_->Init(MemoryChunkLayout::kMemoryChunkHeaderSize,
+                               MemoryAllocator::GetCommitPageSizeBits(),
+                               size());
   } else {
     // We do not track active system pages for large pages.
-    active_system_pages_.Clear();
+    active_system_pages_ = nullptr;
   }
 
   // All pages of a shared heap need to be marked with this flag.
@@ -202,7 +204,7 @@ MemoryChunk::MemoryChunk(Heap* heap, BaseSpace* space, size_t chunk_size,
 
 size_t MemoryChunk::CommittedPhysicalMemory() const {
   if (!base::OS::HasLazyCommits() || IsLargePage()) return size();
-  return active_system_pages_.Size(MemoryAllocator::GetCommitPageSizeBits());
+  return active_system_pages_->Size(MemoryAllocator::GetCommitPageSizeBits());
 }
 
 void MemoryChunk::SetOldGenerationPageFlags(bool is_marking) {
@@ -245,6 +247,11 @@ void MemoryChunk::ReleaseAllocatedMemoryNeededForWritableChunk() {
     code_object_registry_ = nullptr;
   }
 
+  if (active_system_pages_ != nullptr) {
+    delete active_system_pages_;
+    active_system_pages_ = nullptr;
+  }
+
   possibly_empty_buckets_.Release();
   ReleaseSlotSet<OLD_TO_NEW>();
   ReleaseSlotSet<OLD_TO_OLD>();
diff --git a/src/heap/memory-chunk.h b/src/heap/memory-chunk.h
index 8e74f9edc2..f532e28de0 100644
--- a/src/heap/memory-chunk.h
+++ b/src/heap/memory-chunk.h
@@ -297,7 +297,7 @@ class MemoryChunk : public BasicMemoryChunk {
 
   PossiblyEmptyBuckets possibly_empty_buckets_;
 
-  ActiveSystemPages active_system_pages_;
+  ActiveSystemPages* active_system_pages_;
 
 #ifdef V8_ENABLE_INNER_POINTER_RESOLUTION_OSB
   ObjectStartBitmap object_start_bitmap_;
diff --git a/src/heap/spaces.h b/src/heap/spaces.h
index 53e35d129c..29ac3ad0ed 100644
--- a/src/heap/spaces.h
+++ b/src/heap/spaces.h
@@ -315,7 +315,7 @@ class Page : public MemoryChunk {
   void AllocateFreeListCategories();
   void ReleaseFreeListCategories();
 
-  ActiveSystemPages* active_system_pages() { return &active_system_pages_; }
+  ActiveSystemPages* active_system_pages() { return active_system_pages_; }
 
   template <RememberedSetType remembered_set>
   void ClearTypedSlotsInFreeMemory(const TypedSlotSet::FreeRangesMap& ranges) {