From 61a3605ad2b051efd486cb4467411517b9caba85 Mon Sep 17 00:00:00 2001 From: "mvstanton@chromium.org" Date: Mon, 16 Sep 2013 16:50:41 +0000 Subject: [PATCH] Chromium 284577 needs a mitigation CL added. There is a TODO to remove the mitigation when the cause of the bug is discovered. BUG= R=hpayer@chromium.org, mstarzinger@chromium.org Review URL: https://codereview.chromium.org/23606032 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@16742 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/heap.cc | 15 +++------------ src/hydrogen.cc | 16 +--------------- src/objects-debug.cc | 5 ----- src/objects.cc | 22 +++++++++++++--------- 4 files changed, 17 insertions(+), 41 deletions(-) diff --git a/src/heap.cc b/src/heap.cc index 108cfb3..86c2b4a 100644 --- a/src/heap.cc +++ b/src/heap.cc @@ -4310,10 +4310,7 @@ MaybeObject* Heap::AllocateWithAllocationSite(Map* map, AllocationSpace space, AllocationMemento* alloc_memento = reinterpret_cast( reinterpret_cast
(result) + map->instance_size()); alloc_memento->set_map_no_write_barrier(allocation_memento_map()); - - // TODO(mvstanton): To diagnose bug 284577, some extra checks - CHECK(allocation_site->map() == allocation_site_map()); - + ASSERT(allocation_site->map() == allocation_site_map()); alloc_memento->set_allocation_site(*allocation_site, SKIP_WRITE_BARRIER); return result; } @@ -5057,10 +5054,7 @@ MaybeObject* Heap::CopyJSObjectWithAllocationSite( AllocationMemento* alloc_memento; if (maybe_alloc_memento->To(&alloc_memento)) { alloc_memento->set_map_no_write_barrier(allocation_memento_map()); - - // TODO(mvstanton): To diagnose bug 284577, some extra checks - CHECK(site->map() == allocation_site_map()); - + ASSERT(site->map() == allocation_site_map()); alloc_memento->set_allocation_site(site, SKIP_WRITE_BARRIER); } } @@ -5083,10 +5077,7 @@ MaybeObject* Heap::CopyJSObjectWithAllocationSite( AllocationMemento* alloc_memento = reinterpret_cast( reinterpret_cast
(clone) + object_size); alloc_memento->set_map_no_write_barrier(allocation_memento_map()); - - // TODO(mvstanton): To diagnose bug 284577, some extra checks - CHECK(site->map() == allocation_site_map()); - + ASSERT(site->map() == allocation_site_map()); alloc_memento->set_allocation_site(site, SKIP_WRITE_BARRIER); } diff --git a/src/hydrogen.cc b/src/hydrogen.cc index b567f61..9401c4e 100644 --- a/src/hydrogen.cc +++ b/src/hydrogen.cc @@ -1823,26 +1823,12 @@ void HGraphBuilder::BuildCompareNil( HValue* HGraphBuilder::BuildCreateAllocationMemento(HValue* previous_object, int previous_object_size, HValue* alloc_site) { - // TODO(mvstanton): ASSERT altered to CHECK to diagnose chromium bug 284577 - CHECK(alloc_site != NULL); + ASSERT(alloc_site != NULL); HInnerAllocatedObject* alloc_memento = Add( previous_object, previous_object_size); Handle alloc_memento_map( isolate()->heap()->allocation_memento_map()); AddStoreMapConstant(alloc_memento, alloc_memento_map); - - { - // TODO(mvstanton): the code below is turned on to diagnose chromium bug - // 284577. - Handle alloc_site_map(isolate()->heap()->allocation_site_map()); - IfBuilder builder(this); - builder.If(alloc_site, alloc_site_map); - builder.Then(); - builder.Else(); - Add(); - builder.End(); - } - HObjectAccess access = HObjectAccess::ForAllocationMementoSite(); Add(alloc_memento, access, alloc_site); return alloc_memento; diff --git a/src/objects-debug.cc b/src/objects-debug.cc index acb00da..ad13d7f 100644 --- a/src/objects-debug.cc +++ b/src/objects-debug.cc @@ -692,11 +692,6 @@ void JSArray::JSArrayVerify() { CHECK(elements()->IsUndefined() || elements()->IsFixedArray() || elements()->IsFixedDoubleArray()); - // TODO(mvstanton): to diagnose chromium bug 284577, remove after. - AllocationMemento* memento = AllocationMemento::FindForJSObject(this); - if (memento != NULL && memento->IsValid()) { - memento->AllocationMementoVerify(); - } } } diff --git a/src/objects.cc b/src/objects.cc index d586c4a..bddc5d7 100644 --- a/src/objects.cc +++ b/src/objects.cc @@ -9016,8 +9016,7 @@ AllocationMemento* AllocationMemento::FindForJSObject(JSObject* object) { // involves carefully checking the object immediately after the JSArray // (if there is one) to see if it's an AllocationMemento. if (FLAG_track_allocation_sites && object->GetHeap()->InNewSpace(object)) { - // TODO(mvstanton): CHECK to diagnose chromium bug 284577, remove after. - CHECK(object->GetHeap()->InToSpace(object)); + ASSERT(object->GetHeap()->InToSpace(object)); Address ptr_end = (reinterpret_cast
(object) - kHeapObjectTag) + object->Size(); if ((ptr_end + AllocationMemento::kSize) <= @@ -9027,15 +9026,20 @@ AllocationMemento* AllocationMemento::FindForJSObject(JSObject* object) { reinterpret_cast(ptr_end); if (*possible_allocation_memento_map == object->GetHeap()->allocation_memento_map()) { - Address ptr_object = reinterpret_cast
(object); - // TODO(mvstanton): CHECK to diagnose chromium bug 284577, remove after. - // If this check fails it points to the very unlikely case that we've - // misinterpreted a page header as an allocation memento. Follow up - // with a real fix. - CHECK(Page::FromAddress(ptr_object) == Page::FromAddress(ptr_end)); AllocationMemento* memento = AllocationMemento::cast( reinterpret_cast(ptr_end + kHeapObjectTag)); - return memento; + + // TODO(mvstanton): because of chromium bug 284577, put extra care + // into validating that the memento points to a valid AllocationSite. + // This check is expensive so remove it asap. Also, this check + // HIDES bug 284577, so it must be disabled to debug/diagnose. + Object* site = memento->allocation_site(); + Heap* heap = object->GetHeap(); + if (heap->InOldPointerSpace(site) && + site->IsHeapObject() && + HeapObject::cast(site)->map() == heap->allocation_site_map()) { + return memento; + } } } } -- 2.7.4