From: Mathieu Chartier <mathieuc@google.com>
Date: Mon, 21 Mar 2016 21:05:56 +0000 (-0700)
Subject: Add sanity checking to declaring class visiting
X-Git-Tag: android-x86-7.1-r1~315
X-Git-Url: http://git.osdn.net/view?a=commitdiff_plain;h=b33b1dc78d178060ff0f4327d448fd3a89df51de;p=android-x86%2Fart.git

Add sanity checking to declaring class visiting

When we have an unmarked declaring class in the image, it will
SIGSEGV due to the bitmap being read-only. Print some useful
info to try and debug this.

This probably hurts performance a bit, TODO disable when the bug is
fixed.

Bug: 27493510

Change-Id: Ida0c09cb8e41c1c2ff5cd9d07cbf0d34d6802511
---

diff --git a/runtime/gc/space/image_space.cc b/runtime/gc/space/image_space.cc
index 5ff1cb7a2..22bf5f935 100644
--- a/runtime/gc/space/image_space.cc
+++ b/runtime/gc/space/image_space.cc
@@ -1520,6 +1520,17 @@ ImageSpace* ImageSpace::CreateFromAppImage(const char* image,
                                      /*out*/error_msg);
 }
 
+void ImageSpace::DumpSections(std::ostream& os) const {
+  const uint8_t* base = Begin();
+  const ImageHeader& header = GetImageHeader();
+  for (size_t i = 0; i < ImageHeader::kSectionCount; ++i) {
+    auto section_type = static_cast<ImageHeader::ImageSections>(i);
+    const ImageSection& section = header.GetImageSection(section_type);
+    os << section_type << " " << reinterpret_cast<const void*>(base + section.Offset())
+       << "-" << reinterpret_cast<const void*>(base + section.End()) << "\n";
+  }
+}
+
 }  // namespace space
 }  // namespace gc
 }  // namespace art
diff --git a/runtime/gc/space/image_space.h b/runtime/gc/space/image_space.h
index f2f416377..c9741d064 100644
--- a/runtime/gc/space/image_space.h
+++ b/runtime/gc/space/image_space.h
@@ -149,6 +149,8 @@ class ImageSpace : public MemMapSpace {
     return GetImageHeader().GetOatFileEnd();
   }
 
+  void DumpSections(std::ostream& os) const;
+
  protected:
   // Tries to initialize an ImageSpace from the given image path, returning null on error.
   //
diff --git a/runtime/thread.cc b/runtime/thread.cc
index 217644417..02aecfb15 100644
--- a/runtime/thread.cc
+++ b/runtime/thread.cc
@@ -46,7 +46,7 @@
 #include "gc/accounting/card_table-inl.h"
 #include "gc/allocator/rosalloc.h"
 #include "gc/heap.h"
-#include "gc/space/space.h"
+#include "gc/space/space-inl.h"
 #include "handle_scope-inl.h"
 #include "indirect_reference_table-inl.h"
 #include "jni_internal.h"
@@ -90,6 +90,8 @@ bool Thread::is_started_ = false;
 pthread_key_t Thread::pthread_key_self_;
 ConditionVariable* Thread::resume_cond_ = nullptr;
 const size_t Thread::kStackOverflowImplicitCheckSize = GetStackOverflowReservedBytes(kRuntimeISA);
+// Enabled for b/27493510. TODO: disable when fixed.
+static constexpr bool kVerifyImageObjectsMarked = true;
 
 // For implicit overflow checks we reserve an extra piece of memory at the bottom
 // of the stack (lowest memory).  The higher portion of the memory
@@ -2715,11 +2717,37 @@ class ReferenceMapVisitor : public StackVisitor {
 
  private:
   // Visiting the declaring class is necessary so that we don't unload the class of a method that
-  // is executing. We need to ensure that the code stays mapped.
-  void VisitDeclaringClass(ArtMethod* method) SHARED_REQUIRES(Locks::mutator_lock_) {
+  // is executing. We need to ensure that the code stays mapped. NO_THREAD_SAFETY_ANALYSIS since
+  // the threads do not all hold the heap bitmap lock for parallel GC.
+  void VisitDeclaringClass(ArtMethod* method)
+      SHARED_REQUIRES(Locks::mutator_lock_)
+      NO_THREAD_SAFETY_ANALYSIS {
     mirror::Class* klass = method->GetDeclaringClassUnchecked<kWithoutReadBarrier>();
     // klass can be null for runtime methods.
     if (klass != nullptr) {
+      if (kVerifyImageObjectsMarked) {
+        gc::Heap* const heap = Runtime::Current()->GetHeap();
+        gc::space::ContinuousSpace* space = heap->FindContinuousSpaceFromObject(klass,
+                                                                                /*fail_ok*/true);
+        if (space != nullptr && space->IsImageSpace()) {
+          bool failed = false;
+          if (!space->GetLiveBitmap()->Test(klass)) {
+            failed = true;
+            LOG(INTERNAL_FATAL) << "Unmarked object in image " << *space;
+          } else if (!heap->GetLiveBitmap()->Test(klass)) {
+            failed = true;
+            LOG(INTERNAL_FATAL) << "Unmarked object in image through live bitmap " << *space;
+          }
+          if (failed) {
+            GetThread()->Dump(LOG(INTERNAL_FATAL));
+            space->AsImageSpace()->DumpSections(LOG(INTERNAL_FATAL));
+            LOG(INTERNAL_FATAL) << "Method@" << method->GetDexMethodIndex() << ":" << method
+                                << " klass@" << klass;
+            // Pretty info last in case it crashes.
+            LOG(FATAL) << "Method " << PrettyMethod(method) << " klass " << PrettyClass(klass);
+          }
+        }
+      }
       mirror::Object* new_ref = klass;
       visitor_(&new_ref, -1, this);
       if (new_ref != klass) {