Fix object verification.

Refactor VERIFY_OBJECT_ENABLED to become less brittle to change enum and global
constant.

Change-Id: Ie405106be81dce9a913730c7f46a5659582fa18b

19 files changed, 78 insertions, 51 deletions

diff --git a/src/base/mutex.cc b/src/base/mutex.cc
index 1975f3bfd2..912e7fd0c8 100644
--- a/src/base/mutex.cc
+++ b/src/base/mutex.cc
@@ -25,7 +25,7 @@
 #include "mutex-inl.h"
 #include "runtime.h"
 #include "scoped_thread_state_change.h"
-#include "thread.h"
+#include "thread-inl.h"
 #include "utils.h"
 
 namespace art {
diff --git a/src/compiler/driver/compiler_driver_test.cc b/src/compiler/driver/compiler_driver_test.cc
index cbfc2ae948..aef3a33e08 100644
--- a/src/compiler/driver/compiler_driver_test.cc
+++ b/src/compiler/driver/compiler_driver_test.cc
@@ -29,6 +29,7 @@
 #include "mirror/dex_cache.h"
 #include "mirror/abstract_method-inl.h"
 #include "mirror/object_array-inl.h"
+#include "mirror/object-inl.h"
 
 namespace art {
 
diff --git a/src/compiler/jni/jni_compiler_test.cc b/src/compiler/jni/jni_compiler_test.cc
index 77dd51ef6f..6c9a6dfda1 100644
--- a/src/compiler/jni/jni_compiler_test.cc
+++ b/src/compiler/jni/jni_compiler_test.cc
@@ -25,6 +25,7 @@
 #include "mirror/class_loader.h"
 #include "mirror/abstract_method-inl.h"
 #include "mirror/object_array-inl.h"
+#include "mirror/object-inl.h"
 #include "mirror/stack_trace_element.h"
 #include "runtime.h"
 #include "ScopedLocalRef.h"
diff --git a/src/exception_test.cc b/src/exception_test.cc
index f0bec1b2c3..1b4332f313 100644
--- a/src/exception_test.cc
+++ b/src/exception_test.cc
@@ -19,6 +19,7 @@
 #include "dex_file.h"
 #include "gtest/gtest.h"
 #include "mirror/object_array-inl.h"
+#include "mirror/object-inl.h"
 #include "mirror/stack_trace_element.h"
 #include "runtime.h"
 #include "scoped_thread_state_change.h"
diff --git a/src/heap.cc b/src/heap.cc
index a3a3a287f5..d7432a379c 100644
--- a/src/heap.cc
+++ b/src/heap.cc
@@ -192,7 +192,7 @@ Heap::Heap(size_t initial_size, size_t growth_limit, size_t min_free, size_t max
       total_wait_time_(0),
       measure_allocation_time_(false),
       total_allocation_time_(0),
-      verify_objects_(false) {
+      verify_object_mode_(kHeapVerificationNotPermitted) {
   if (VLOG_IS_ON(heap) || VLOG_IS_ON(startup)) {
     LOG(INFO) << "Heap() entering";
   }
@@ -587,15 +587,13 @@ bool Heap::IsLiveObjectLocked(const mirror::Object* obj) {
   return IsHeapAddress(obj) && GetLiveBitmap()->Test(obj);
 }
 
-#if VERIFY_OBJECT_ENABLED
-void Heap::VerifyObject(const Object* obj) {
-  if (obj == NULL || this == NULL || !verify_objects_ || Thread::Current() == NULL ||
+void Heap::VerifyObjectImpl(const mirror::Object* obj) {
+  if (Thread::Current() == NULL ||
       Runtime::Current()->GetThreadList()->GetLockOwner() == Thread::Current()->GetTid()) {
     return;
   }
   VerifyObjectBody(obj);
 }
-#endif
 
 void Heap::DumpSpaces() {
   // TODO: C++0x auto
@@ -632,7 +630,7 @@ void Heap::VerifyObjectBody(const mirror::Object* obj) {
   }
 
   // Ignore early dawn of the universe verifications
-  if (!VERIFY_OBJECT_FAST && GetObjectsAllocated() > 10) {
+  if (verify_object_mode_ != kVerifyAllFast && GetObjectsAllocated() > 10) {
     const byte* raw_addr = reinterpret_cast<const byte*>(obj) +
         mirror::Object::ClassOffset().Int32Value();
     const mirror::Class* c = *reinterpret_cast<mirror::Class* const *>(raw_addr);
@@ -1458,7 +1456,7 @@ void Heap::SwapStacks() {
   allocation_stack_.swap(live_stack_);
 
   // Sort the live stack so that we can quickly binary search it later.
-  if (VERIFY_OBJECT_ENABLED) {
+  if (verify_object_mode_ > kNoHeapVerification) {
     std::sort(live_stack_->Begin(), live_stack_->End());
   }
 }
diff --git a/src/heap.h b/src/heap.h
index 7af15cdd1b..a2c2d9867b 100644
--- a/src/heap.h
+++ b/src/heap.h
@@ -34,11 +34,6 @@
 #include "safe_map.h"
 #include "thread_pool.h"
 
-#define VERIFY_OBJECT_ENABLED 0
-
-// Fast verification means we do not verify the classes of objects.
-#define VERIFY_OBJECT_FAST 1
-
 namespace art {
 namespace mirror {
 class Class;
@@ -80,6 +75,15 @@ enum GcCause {
 };
 std::ostream& operator<<(std::ostream& os, const GcCause& policy);
 
+// How we want to sanity check the heap's correctness.
+enum HeapVerificationMode {
+  kHeapVerificationNotPermitted,  // Too early in runtime start-up for heap to be verified.
+  kNoHeapVerification,  // Production default.
+  kVerifyAllFast,  // Sanity check all heap accesses with quick(er) tests.
+  kVerifyAll  // Sanity check all heap accesses.
+};
+const HeapVerificationMode kDesiredHeapVerification = kNoHeapVerification;
+
 class Heap {
  public:
   static const size_t kDefaultInitialSize = 2 * MB;
@@ -106,14 +110,15 @@ class Heap {
   mirror::Object* AllocObject(Thread* self, mirror::Class* klass, size_t num_bytes)
       SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
 
-  // Check sanity of given reference. Requires the heap lock.
-#if VERIFY_OBJECT_ENABLED
-  void VerifyObject(const mirror::Object* o);
-#else
-  void VerifyObject(const mirror::Object*) {}
-#endif
+  // The given reference is believed to be to an object in the Java heap, check the soundness of it.
+  void VerifyObjectImpl(const mirror::Object* o);
+  void VerifyObject(const mirror::Object* o) {
+    if (o != NULL && this != NULL && verify_object_mode_ > kNoHeapVerification) {
+      VerifyObjectImpl(o);
+    }
+  }
 
-  // Check sanity of all live references. Requires the heap lock.
+  // Check sanity of all live references.
   void VerifyHeap() LOCKS_EXCLUDED(Locks::heap_bitmap_lock_);
   static void RootMatchesObjectVisitor(const mirror::Object* root, void* arg);
   bool VerifyHeapReferences()
@@ -219,19 +224,23 @@ class Heap {
     return finalizer_reference_zombie_offset_;
   }
 
+  // Enable verification of object references when the runtime is sufficiently initialized.
   void EnableObjectValidation() {
-#if VERIFY_OBJECT_ENABLED
-    VerifyHeap();
-#endif
-    verify_objects_ = true;
+    verify_object_mode_ = kDesiredHeapVerification;
+    if (verify_object_mode_ > kNoHeapVerification) {
+      VerifyHeap();
+    }
   }
 
+  // Disable object reference verification for image writing.
   void DisableObjectValidation() {
-    verify_objects_ = false;
+    verify_object_mode_ = kHeapVerificationNotPermitted;
   }
 
+  // Other checks may be performed if we know the heap should be in a sane state.
   bool IsObjectValidationEnabled() const {
-    return verify_objects_;
+    return kDesiredHeapVerification > kNoHeapVerification &&
+        verify_object_mode_ > kHeapVerificationNotPermitted;
   }
 
   void RecordFree(size_t freed_objects, size_t freed_bytes);
@@ -532,7 +541,9 @@ class Heap {
   const bool measure_allocation_time_;
   AtomicInteger total_allocation_time_;
 
-  bool verify_objects_;
+  // The current state of heap verification, may be enabled or disabled.
+  HeapVerificationMode verify_object_mode_;
+
   typedef std::vector<MarkSweep*> Collectors;
   Collectors mark_sweep_collectors_;
 
diff --git a/src/jni_internal_test.cc b/src/jni_internal_test.cc
index 4b820f9126..67020d874b 100644
--- a/src/jni_internal_test.cc
+++ b/src/jni_internal_test.cc
@@ -22,6 +22,7 @@
 #include "common_test.h"
 #include "mirror/abstract_method-inl.h"
 #include "mirror/object_array-inl.h"
+#include "mirror/object-inl.h"
 #include "ScopedLocalRef.h"
 #include "sirt_ref.h"
 
diff --git a/src/mirror/dex_cache_test.cc b/src/mirror/dex_cache_test.cc
index 98176602a1..3d753e1e15 100644
--- a/src/mirror/dex_cache_test.cc
+++ b/src/mirror/dex_cache_test.cc
@@ -19,6 +19,7 @@
 #include "dex_cache.h"
 #include "heap.h"
 #include "mirror/object_array-inl.h"
+#include "mirror/object-inl.h"
 #include "sirt_ref.h"
 
 #include <stdio.h>
diff --git a/src/mirror/object-inl.h b/src/mirror/object-inl.h
index b6c8008b1f..3913c817ee 100644
--- a/src/mirror/object-inl.h
+++ b/src/mirror/object-inl.h
@@ -263,6 +263,10 @@ inline void Object::WriteBarrierField(const Object* dst, MemberOffset field_offs
   Runtime::Current()->GetHeap()->WriteBarrierField(dst, field_offset, new_value);
 }
 
+inline void Object::VerifyObject(const Object* obj) {
+  Runtime::Current()->GetHeap()->VerifyObject(obj);
+}
+
 }  // namespace mirror
 }  // namespace art
 
diff --git a/src/mirror/object.cc b/src/mirror/object.cc
index 5c65b83593..4acb5679f9 100644
--- a/src/mirror/object.cc
+++ b/src/mirror/object.cc
@@ -19,13 +19,15 @@
 #include "array-inl.h"
 #include "class.h"
 #include "class-inl.h"
+#include "class_linker-inl.h"
 #include "field.h"
 #include "field-inl.h"
 #include "gc/card_table-inl.h"
 #include "heap.h"
+#include "iftable-inl.h"
 #include "monitor.h"
 #include "object-inl.h"
-#include "object_array.h"
+#include "object_array-inl.h"
 #include "object_utils.h"
 #include "runtime.h"
 #include "sirt_ref.h"
@@ -80,8 +82,7 @@ Object* Object::Clone(Thread* self) {
   return copy.get();
 }
 
-#if VERIFY_OBJECT_ENABLED
-void Object::CheckFieldAssignment(MemberOffset field_offset, const Object* new_value) {
+void Object::CheckFieldAssignmentImpl(MemberOffset field_offset, const Object* new_value) {
   const Class* c = GetClass();
   if (Runtime::Current()->GetClassLinker() == NULL ||
       !Runtime::Current()->GetHeap()->IsObjectValidationEnabled() ||
@@ -123,7 +124,6 @@ void Object::CheckFieldAssignment(MemberOffset field_offset, const Object* new_v
   LOG(FATAL) << "Failed to find field for assignment to " << reinterpret_cast<void*>(this)
       << " of type " << PrettyDescriptor(c) << " at offset " << field_offset;
 }
-#endif
 
 }  // namespace mirror
 }  // namespace art
diff --git a/src/mirror/object.h b/src/mirror/object.h
index c404b61393..0cce8d8eee 100644
--- a/src/mirror/object.h
+++ b/src/mirror/object.h
@@ -58,6 +58,8 @@ class Throwable;
 #define OFFSET_OF_OBJECT_MEMBER(type, field) \
     MemberOffset(OFFSETOF_MEMBER(type, field))
 
+const bool kCheckFieldAssignments = false;
+
 // C++ mirror of java.lang.Object
 class MANAGED Object {
  public:
@@ -231,15 +233,17 @@ class MANAGED Object {
   }
 
  private:
-#if VERIFY_OBJECT_ENABLED
   static void VerifyObject(const Object* obj);
-  void CheckFieldAssignment(MemberOffset field_offset, const Object* new_value)
+
+  // Verify the type correctness of stores to fields.
+  void CheckFieldAssignmentImpl(MemberOffset field_offset, const Object* new_value)
       SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
-#else
-  static void VerifyObject(const Object*) {}
-  void CheckFieldAssignment(MemberOffset, const Object*)
-      SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) {}
-#endif
+  void CheckFieldAssignment(MemberOffset field_offset, const Object* new_value)
+      SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) {
+    if (kCheckFieldAssignments) {
+      CheckFieldAssignmentImpl(field_offset, new_value);
+    }
+  }
 
   // Write barrier called post update to a reference bearing field.
   static void WriteBarrierField(const Object* dst, MemberOffset offset, const Object* new_value);
diff --git a/src/oat/runtime/arm/context_arm.cc b/src/oat/runtime/arm/context_arm.cc
index 4612986288..814e649ac2 100644
--- a/src/oat/runtime/arm/context_arm.cc
+++ b/src/oat/runtime/arm/context_arm.cc
@@ -17,6 +17,7 @@
 #include "context_arm.h"
 
 #include "mirror/abstract_method.h"
+#include "mirror/object-inl.h"
 #include "stack.h"
 #include "thread.h"
 
diff --git a/src/oat/runtime/callee_save_frame.h b/src/oat/runtime/callee_save_frame.h
index 08cf9d86be..dd2f3fa69e 100644
--- a/src/oat/runtime/callee_save_frame.h
+++ b/src/oat/runtime/callee_save_frame.h
@@ -18,7 +18,7 @@
 #define ART_SRC_OAT_RUNTIME_CALLEE_SAVE_FRAME_H_
 
 #include "base/mutex.h"
-#include "thread.h"
+#include "thread-inl.h"
 
 namespace art {
 namespace mirror {
diff --git a/src/oat/runtime/support_instrumentation.cc b/src/oat/runtime/support_instrumentation.cc
index f65717aa73..6598f196cd 100644
--- a/src/oat/runtime/support_instrumentation.cc
+++ b/src/oat/runtime/support_instrumentation.cc
@@ -17,7 +17,7 @@
 #include "base/logging.h"
 #include "instrumentation.h"
 #include "runtime.h"
-#include "thread.h"
+#include "thread-inl.h"
 #include "trace.h"
 
 namespace art {
diff --git a/src/oat_test.cc b/src/oat_test.cc
index c4bd60e858..a2ea71c4c1 100644
--- a/src/oat_test.cc
+++ b/src/oat_test.cc
@@ -17,6 +17,7 @@
 #include "mirror/abstract_method-inl.h"
 #include "mirror/class-inl.h"
 #include "mirror/object_array-inl.h"
+#include "mirror/object-inl.h"
 #include "oat_file.h"
 #include "oat_writer.h"
 #include "vector_output_stream.h"
diff --git a/src/thread-inl.h b/src/thread-inl.h
index 414b8d87a0..6c1ae59b60 100644
--- a/src/thread-inl.h
+++ b/src/thread-inl.h
@@ -124,6 +124,13 @@ inline ThreadState Thread::TransitionFromSuspendedToRunnable() {
   return static_cast<ThreadState>(old_state);
 }
 
+inline void Thread::VerifyStack() {
+  Heap* heap = Runtime::Current()->GetHeap();
+  if (heap->IsObjectValidationEnabled()) {
+    VerifyStackImpl();
+  }
+}
+
 }  // namespace art
 
 #endif  // ART_SRC_THREAD_INL_H_
diff --git a/src/thread.cc b/src/thread.cc
index 394d2636b5..94c437f385 100644
--- a/src/thread.cc
+++ b/src/thread.cc
@@ -2004,20 +2004,17 @@ void Thread::VisitRoots(RootVisitor* visitor, void* arg) {
   ReleaseLongJumpContext(context);
 }
 
-#if VERIFY_OBJECT_ENABLED
-static void VerifyObject(const Object* obj, void* arg) {
+static void VerifyObject(const mirror::Object* root, void* arg) {
   Heap* heap = reinterpret_cast<Heap*>(arg);
-  heap->VerifyObject(obj);
+  heap->VerifyObject(root);
 }
 
-void Thread::VerifyStack() {
+void Thread::VerifyStackImpl() {
   UniquePtr<Context> context(Context::Create());
   RootCallbackVisitor visitorToCallback(VerifyObject, Runtime::Current()->GetHeap());
-  ReferenceMapVisitor<RootCallbackVisitor> mapper(GetManagedStack(), GetInstrumentationStack(),
-                                                  context.get(), visitorToCallback);
+  ReferenceMapVisitor<RootCallbackVisitor> mapper(this, context.get(), visitorToCallback);
   mapper.WalkStack();
 }
-#endif
 
 // Set the stack end to that to be used during a stack overflow
 void Thread::SetStackEndForStackOverflow() {
diff --git a/src/thread.h b/src/thread.h
index 1992dc97e9..dd67a21edd 100644
--- a/src/thread.h
+++ b/src/thread.h
@@ -401,11 +401,7 @@ class PACKED(4) Thread {
   void VerifyRoots(VerifyRootVisitor* visitor, void* arg)
       SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
 
-#if VERIFY_OBJECT_ENABLED
   void VerifyStack() SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
-#else
-  void VerifyStack() SHARED_LOCKS_REQUIRED(Locks::mutator_lock_){}
-#endif
 
   //
   // Offsets of various members of native Thread class, used by compiled code.
@@ -610,6 +606,8 @@ class PACKED(4) Thread {
   }
   friend class SignalCatcher;  // For SetStateUnsafe.
 
+  void VerifyStackImpl() SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
+
   void DumpState(std::ostream& os) const SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
   void DumpStack(std::ostream& os) const
       LOCKS_EXCLUDED(Locks::thread_suspend_count_lock_)
diff --git a/test/StackWalk/stack_walk_jni.cc b/test/StackWalk/stack_walk_jni.cc
index a16d89615e..92cfa99b85 100644
--- a/test/StackWalk/stack_walk_jni.cc
+++ b/test/StackWalk/stack_walk_jni.cc
@@ -22,6 +22,7 @@
 #include "mirror/abstract_method.h"
 #include "mirror/abstract_method-inl.h"
 #include "mirror/object_array-inl.h"
+#include "mirror/object-inl.h"
 #include "object_utils.h"
 #include "jni.h"
 #include "scoped_thread_state_change.h"