Reviewers: Hannes Payer,
Message:
Hi Hannes, here are the changes we discussed.
Description:
To diagnose chromium bug 284577, some additional CHECKS. TODOs are
added so these can be backed out once the cause of the bug is determined.
BUG=
Please review this at https://codereview.chromium.org/23936007/
SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge
Affected files (+41, -10 lines):
M src/hydrogen.cc
M src/objects-debug.cc
M src/objects.cc
Index: src/hydrogen.cc
diff --git a/src/hydrogen.cc b/src/hydrogen.cc
index
437d29b82e75a900410352d1e5261c4f6b7791be..8f1f411eec5914e366ebb13e8d277bba8c1e491e
100644
--- a/src/hydrogen.cc
+++ b/src/hydrogen.cc
@@ -1824,7 +1824,8 @@ void HGraphBuilder::BuildCompareNil(
HValue* HGraphBuilder::BuildCreateAllocationMemento(HValue*
previous_object,
int
previous_object_size,
HValue* alloc_site) {
- ASSERT(alloc_site != NULL);
+ // TODO(mvstanton): ASSERT altered to CHECK to diagnose chromium bug
284577
+ CHECK(alloc_site != NULL);
HInnerAllocatedObject* alloc_memento = Add<HInnerAllocatedObject>(
previous_object, previous_object_size);
Handle<Map> alloc_memento_map(
@@ -1832,6 +1833,12 @@ HValue*
HGraphBuilder::BuildCreateAllocationMemento(HValue* previous_object,
AddStoreMapConstant(alloc_memento, alloc_memento_map);
HObjectAccess access = HObjectAccess::ForAllocationMementoSite();
Add<HStoreNamedField>(alloc_memento, access, alloc_site);
+ // TODO(mvstanton): to diagnose chromium bug 284577 we could/should
+ // dereference the alloc_site here. We could do a map check. However it
has
+ // a performance impact so I'm not doing that right away.
+ // for example, something like:
+ // Add<HLoadNamedField>(alloc_site,
+ //
HObjectAccess::ForAllocationSiteTransitionInfo());
return alloc_memento;
}
Index: src/objects-debug.cc
diff --git a/src/objects-debug.cc b/src/objects-debug.cc
index
3eb7817b1aa2623ecf9d6d2e184612c6ae0b09cf..87e657a8a86bd55bd55c68b1fed8ebec56704de4
100644
--- a/src/objects-debug.cc
+++ b/src/objects-debug.cc
@@ -328,11 +328,17 @@ void JSObject::JSObjectVerify() {
}
}
}
- CHECK_EQ((map()->has_fast_smi_or_object_elements() ||
- (elements() == GetHeap()->empty_fixed_array())),
- (elements()->map() == GetHeap()->fixed_array_map() ||
- elements()->map() == GetHeap()->fixed_cow_array_map()));
- CHECK(map()->has_fast_object_elements() == HasFastObjectElements());
+
+ // TODO(hpayer): deal gracefully with partially constructed JSObjects,
when
+ // allocation folding is turned off.
+ if (reinterpret_cast<Map*>(elements()) !=
+ GetHeap()->one_pointer_filler_map()) {
+ CHECK_EQ((map()->has_fast_smi_or_object_elements() ||
+ (elements() == GetHeap()->empty_fixed_array())),
+ (elements()->map() == GetHeap()->fixed_array_map() ||
+ elements()->map() == GetHeap()->fixed_cow_array_map()));
+ CHECK(map()->has_fast_object_elements() == HasFastObjectElements());
+ }
}
@@ -675,9 +681,19 @@ void Code::VerifyEmbeddedMapsDependency() {
void JSArray::JSArrayVerify() {
JSObjectVerify();
CHECK(length()->IsNumber() || length()->IsUndefined());
- CHECK(elements()->IsUndefined() ||
- elements()->IsFixedArray() ||
- elements()->IsFixedDoubleArray());
+ // TODO(hpayer): deal gracefully with partially constructed JSObjects,
when
+ // allocation folding is turned off.
+ if (reinterpret_cast<Map*>(elements()) !=
+ GetHeap()->one_pointer_filler_map()) {
+ 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();
+ }
+ }
}
Index: src/objects.cc
diff --git a/src/objects.cc b/src/objects.cc
index
507fbfc1142ccf057a70995c112c143232a1a402..c6596c6789a53d8fe48b592eea201cce31ec321a
100644
--- a/src/objects.cc
+++ b/src/objects.cc
@@ -8985,6 +8985,8 @@ 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));
Address ptr_end = (reinterpret_cast<Address>(object) - kHeapObjectTag)
+
object->Size();
if ((ptr_end + AllocationMemento::kSize) <=
@@ -8994,8 +8996,14 @@ AllocationMemento*
AllocationMemento::FindForJSObject(JSObject* object) {
reinterpret_cast<Map**>(ptr_end);
if (*possible_allocation_memento_map ==
object->GetHeap()->allocation_memento_map()) {
+ Address ptr_object = reinterpret_cast<Address>(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<Object*>(ptr_end + 1));
+ reinterpret_cast<Object*>(ptr_end + kHeapObjectTag));
return memento;
}
}
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.
