Fix invalid root detection using wrong function to find space from object.
Fixes issue where a check would fail when we have a bad root instead of
returning NULL. This caused us to abort before outputting some of the
information.
Add large object space to DumpSpaces.
Add DumpSpaces calls to most places calling IsHeapAddress.
Change-Id: I34879fbaa3cd4d6589834a71b1069ca0cee9e701
diff --git a/src/check_jni.cc b/src/check_jni.cc
index e7590d3..7c5186a 100644
--- a/src/check_jni.cc
+++ b/src/check_jni.cc
@@ -208,6 +208,7 @@
// obj will be NULL. Otherwise, obj should always be non-NULL
// and valid.
if (!Runtime::Current()->GetHeap()->IsHeapAddress(obj)) {
+ Runtime::Current()->GetHeap()->DumpSpaces();
JniAbortF(function_name_, "field operation on invalid %s: %p",
ToStr<IndirectRefKind>(GetIndirectRefKind(java_object)).c_str(), java_object);
return;
@@ -244,6 +245,7 @@
SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) {
Object* o = soa_.Decode<Object*>(java_object);
if (o == NULL || !Runtime::Current()->GetHeap()->IsHeapAddress(o)) {
+ Runtime::Current()->GetHeap()->DumpSpaces();
JniAbortF(function_name_, "field operation on invalid %s: %p",
ToStr<IndirectRefKind>(GetIndirectRefKind(java_object)).c_str(), java_object);
return;
@@ -623,6 +625,7 @@
Object* obj = soa_.Decode<Object*>(java_object);
if (!Runtime::Current()->GetHeap()->IsHeapAddress(obj)) {
+ Runtime::Current()->GetHeap()->DumpSpaces();
JniAbortF(function_name_, "%s is an invalid %s: %p (%p)",
what, ToStr<IndirectRefKind>(GetIndirectRefKind(java_object)).c_str(), java_object, obj);
return false;
@@ -676,6 +679,7 @@
Array* a = soa_.Decode<Array*>(java_array);
if (!Runtime::Current()->GetHeap()->IsHeapAddress(a)) {
+ Runtime::Current()->GetHeap()->DumpSpaces();
JniAbortF(function_name_, "jarray is an invalid %s: %p (%p)",
ToStr<IndirectRefKind>(GetIndirectRefKind(java_array)).c_str(), java_array, a);
} else if (!a->IsArrayInstance()) {
@@ -696,6 +700,7 @@
}
Field* f = soa_.DecodeField(fid);
if (!Runtime::Current()->GetHeap()->IsHeapAddress(f) || !f->IsField()) {
+ Runtime::Current()->GetHeap()->DumpSpaces();
JniAbortF(function_name_, "invalid jfieldID: %p", fid);
return NULL;
}
@@ -709,6 +714,7 @@
}
AbstractMethod* m = soa_.DecodeMethod(mid);
if (!Runtime::Current()->GetHeap()->IsHeapAddress(m) || !m->IsMethod()) {
+ Runtime::Current()->GetHeap()->DumpSpaces();
JniAbortF(function_name_, "invalid jmethodID: %p", mid);
return NULL;
}
@@ -729,6 +735,7 @@
Object* o = soa_.Decode<Object*>(java_object);
if (!Runtime::Current()->GetHeap()->IsHeapAddress(o)) {
+ Runtime::Current()->GetHeap()->DumpSpaces();
// TODO: when we remove work_around_app_jni_bugs, this should be impossible.
JniAbortF(function_name_, "native code passing in reference to invalid %s: %p",
ToStr<IndirectRefKind>(GetIndirectRefKind(java_object)).c_str(), java_object);
diff --git a/src/gc/large_object_space.cc b/src/gc/large_object_space.cc
index 72c6c73..b066dd5 100644
--- a/src/gc/large_object_space.cc
+++ b/src/gc/large_object_space.cc
@@ -267,4 +267,10 @@
return reinterpret_cast<Object*>(addr);
}
+void FreeListSpace::Dump(std::ostream& os) const{
+ os << GetName() << " -"
+ << " begin: " << reinterpret_cast<void*>(Begin())
+ << " end: " << reinterpret_cast<void*>(End());
+}
+
}
diff --git a/src/gc/large_object_space.h b/src/gc/large_object_space.h
index 2bf6abf..979fce6 100644
--- a/src/gc/large_object_space.h
+++ b/src/gc/large_object_space.h
@@ -131,6 +131,9 @@
size_t Size() const {
return End() - Begin();
}
+
+ virtual void Dump(std::ostream& os) const;
+
private:
static const size_t kAlignment = kPageSize;
diff --git a/src/gc/mark_sweep.cc b/src/gc/mark_sweep.cc
index e4cb4d6..da6a593 100644
--- a/src/gc/mark_sweep.cc
+++ b/src/gc/mark_sweep.cc
@@ -179,7 +179,7 @@
void MarkSweep::VerifyRoot(const Object* root, size_t vreg, const AbstractMethod* method) {
// See if the root is on any space bitmap.
- if (heap_->FindSpaceFromObject(root) == NULL) {
+ if (GetHeap()->GetLiveBitmap()->GetSpaceBitmap(root) == NULL) {
LargeObjectSpace* large_object_space = GetHeap()->GetLargeObjectsSpace();
if (large_object_space->Contains(root)) {
LOG(ERROR) << "Found invalid root: " << root;
diff --git a/src/gc/space.cc b/src/gc/space.cc
index 4d5ce93..a7a5942 100644
--- a/src/gc/space.cc
+++ b/src/gc/space.cc
@@ -434,7 +434,6 @@
new_size = current_space_size;
}
mspace_set_footprint_limit(mspace_, new_size);
- LOG(INFO) << "Setting footprint limit to " << new_size;
}
size_t ImageSpace::bitmap_index_ = 0;
diff --git a/src/heap.cc b/src/heap.cc
index 4ca80a6..84030ba 100644
--- a/src/heap.cc
+++ b/src/heap.cc
@@ -536,7 +536,9 @@
<< live_bitmap << " " << *live_bitmap << "\n"
<< mark_bitmap << " " << *mark_bitmap;
}
- // TODO: Dump large object space?
+ if (large_object_space_.get() != NULL) {
+ large_object_space_->Dump(LOG(INFO));
+ }
}
void Heap::VerifyObjectBody(const Object* obj) {
@@ -1459,9 +1461,13 @@
if (space->GetGcRetentionPolicy() == kGcRetentionPolicyAlwaysCollect ||
(gc_type == kGcTypeFull &&
space->GetGcRetentionPolicy() == kGcRetentionPolicyFullCollect)) {
- live_bitmap_->ReplaceBitmap(space->GetLiveBitmap(), space->GetMarkBitmap());
- mark_bitmap_->ReplaceBitmap(space->GetMarkBitmap(), space->GetLiveBitmap());
- space->AsAllocSpace()->SwapBitmaps();
+ SpaceBitmap* live_bitmap = space->GetLiveBitmap();
+ SpaceBitmap* mark_bitmap = space->GetMarkBitmap();
+ if (live_bitmap != mark_bitmap) {
+ live_bitmap_->ReplaceBitmap(live_bitmap, mark_bitmap);
+ mark_bitmap_->ReplaceBitmap(mark_bitmap, live_bitmap);
+ space->AsAllocSpace()->SwapBitmaps();
+ }
}
}
SwapLargeObjects();
@@ -1700,11 +1706,12 @@
if (verify_post_gc_heap_) {
WriterMutexLock mu(self, *Locks::heap_bitmap_lock_);
- SwapBitmaps(gc_type);
+ // Swapping bound bitmaps does nothing.
+ SwapBitmaps(kGcTypeFull);
if (!VerifyHeapReferences()) {
LOG(FATAL) << "Post " << gc_type_str.str() << "Gc verification failed";
}
- SwapBitmaps(gc_type);
+ SwapBitmaps(kGcTypeFull);
timings.AddSplit("VerifyHeapReferencesPostGC");
}
