[heap] Remove page header tag from owner field.

This reverts commit 8d7522bc67 and fixes
the TSAN issue.

Bug: chromium:800251
Change-Id: Ie88e5281f7543bb3420703e798416d4a6dbbd91a
Reviewed-on: https://chromium-review.googlesource.com/864042
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Commit-Queue: Hannes Payer <hpayer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#50535}

src/globals.h

@@ -413,16 +413,10 @@ const int kCodeAlignmentBits = 5;
 const intptr_t kCodeAlignment = 1 << kCodeAlignmentBits;
 const intptr_t kCodeAlignmentMask = kCodeAlignment - 1;
 
-// The owner field of a page is tagged with the page header tag. We need that
-// to find out if a slot is part of a large object. If we mask out the lower
-// 0xfffff bits (1M pages), go to the owner offset, and see that this field
-// is tagged with the page header tag, we can just look up the owner.
-// Otherwise, we know that we are somewhere (not within the first 1M) in a
-// large object.
-const int kPageHeaderTag = 3;
-const int kPageHeaderTagSize = 2;
-const intptr_t kPageHeaderTagMask = (1 << kPageHeaderTagSize) - 1;
-
+// Weak references are tagged using the second bit in a pointer.
+const int kWeakReferenceTag = 3;
+const int kWeakReferenceTagSize = 2;
+const intptr_t kWeakReferenceTagMask = (1 << kWeakReferenceTagSize) - 1;
 
 // Zap-value: The value used for zapping dead objects.
 // Should be a recognizable hex value tagged as a failure.
@@ -544,7 +538,6 @@ enum AllocationSpace {
   LAST_PAGED_SPACE = MAP_SPACE
 };
 const int kSpaceTagSize = 3;
-const int kSpaceTagMask = (1 << kSpaceTagSize) - 1;
 
 enum AllocationAlignment { kWordAligned, kDoubleAligned, kDoubleUnaligned };
 

src/heap/incremental-marking.cc

@@ -680,7 +680,7 @@ void IncrementalMarking::RevisitObject(HeapObject* obj) {
   DCHECK(IsMarking());
   DCHECK(FLAG_concurrent_marking || marking_state()->IsBlack(obj));
   Page* page = Page::FromAddress(obj->address());
-  if ((page->owner() != nullptr) && (page->owner()->identity() == LO_SPACE)) {
+  if (page->owner()->identity() == LO_SPACE) {
     page->ResetProgressBar();
   }
   Map* map = obj->map();

src/heap/spaces-inl.h

@@ -144,14 +144,13 @@ void MemoryChunk::InitializeFreeListCategories() {
 }
 
 bool PagedSpace::Contains(Address addr) {
+  if (heap_->lo_space()->FindPage(addr)) return false;
   return MemoryChunk::FromAnyPointerAddress(heap(), addr)->owner() == this;
 }
 
 bool PagedSpace::Contains(Object* o) {
   if (!o->IsHeapObject()) return false;
-  Page* p = Page::FromAddress(HeapObject::cast(o)->address());
-  if (!Page::IsValid(p)) return false;
-  return p->owner() == this;
+  return Page::FromAddress(HeapObject::cast(o)->address())->owner() == this;
 }
 
 void PagedSpace::UnlinkFreeListCategories(Page* page) {

src/heap/spaces.cc

@@ -3217,7 +3217,6 @@ LargePage* LargeObjectSpace::FindPage(Address a) {
   auto it = chunk_map_.find(reinterpret_cast<Address>(key));
   if (it != chunk_map_.end()) {
     LargePage* page = it->second;
-    DCHECK(LargePage::IsValid(page));
     if (page->Contains(a)) {
       return page;
     }

src/heap/spaces.h

@@ -423,8 +423,6 @@ class MemoryChunk {
              !chunk->high_water_mark_.TrySetValue(old_mark, new_mark));
   }
 
-  static bool IsValid(MemoryChunk* chunk) { return chunk != nullptr; }
-
   Address address() const {
     return reinterpret_cast<Address>(const_cast<MemoryChunk*>(this));
   }
@@ -608,25 +606,9 @@ class MemoryChunk {
 
   void set_prev_chunk(MemoryChunk* prev) { prev_chunk_.SetValue(prev); }
 
-  Space* owner() const {
-    uintptr_t owner_value = base::AsAtomicWord::Acquire_Load(
-        reinterpret_cast<const uintptr_t*>(&owner_));
-    return ((owner_value & kPageHeaderTagMask) == kPageHeaderTag)
-               ? reinterpret_cast<Space*>(owner_value - kPageHeaderTag)
-               : nullptr;
-  }
+  Space* owner() const { return owner_.Value(); }
 
-  void set_owner(Space* space) {
-    DCHECK_EQ(0, reinterpret_cast<uintptr_t>(space) & kPageHeaderTagMask);
-    base::AsAtomicWord::Release_Store(
-        reinterpret_cast<uintptr_t*>(&owner_),
-        reinterpret_cast<uintptr_t>(space) + kPageHeaderTag);
-    DCHECK_EQ(kPageHeaderTag, base::AsAtomicWord::Relaxed_Load(
-                                  reinterpret_cast<const uintptr_t*>(&owner_)) &
-                                  kPageHeaderTagMask);
-  }
-
-  bool HasPageHeader() { return owner() != nullptr; }
+  void set_owner(Space* space) { owner_.SetValue(space); }
 
   void InsertAfter(MemoryChunk* other);
   void Unlink();
@@ -661,10 +643,8 @@ class MemoryChunk {
   // If the chunk needs to remember its memory reservation, it is stored here.
   VirtualMemory reservation_;
 
-  // The identity of the owning space.  This is tagged as a failure pointer, but
-  // no failure can be in an object, so this can be distinguished from any entry
-  // in a fixed array.
-  Address owner_;
+  // The space owning this memory chunk.
+  base::AtomicValue<Space*> owner_;
 
   Heap* heap_;
 
@@ -1000,7 +980,7 @@ class Space : public Malloced {
   std::vector<AllocationObserver*> allocation_observers_;
   bool allocation_observers_paused_;
 
- private:
+ protected:
   Heap* heap_;
   AllocationSpace id_;
   Executability executable_;

test/cctest/heap/test-spaces.cc

@@ -230,7 +230,6 @@ TEST(MemoryAllocator) {
         NOT_EXECUTABLE);
 
     first_page->InsertAfter(faked_space.anchor()->prev_page());
-    CHECK(Page::IsValid(first_page));
     CHECK(first_page->next_page() == faked_space.anchor());
     total_pages++;
 
@@ -242,7 +241,6 @@ TEST(MemoryAllocator) {
     Page* other = memory_allocator->AllocatePage(
         faked_space.AreaSize(), static_cast<PagedSpace*>(&faked_space),
         NOT_EXECUTABLE);
-    CHECK(Page::IsValid(other));
     total_pages++;
     other->InsertAfter(first_page);
     int page_count = 0;
@@ -253,7 +251,7 @@ TEST(MemoryAllocator) {
     CHECK(total_pages == page_count);
 
     Page* second_page = first_page->next_page();
-    CHECK(Page::IsValid(second_page));
+    CHECK_NOT_NULL(second_page);
 
     // OldSpace's destructor will tear down the space and free up all pages.
   }