Fix object verification.
Refactor VERIFY_OBJECT_ENABLED to become less brittle to change enum and global
constant.
Change-Id: Ie405106be81dce9a913730c7f46a5659582fa18b
diff --git a/src/base/mutex.cc b/src/base/mutex.cc
index 1975f3b..912e7fd 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 cbfc2ae..aef3a33 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 77dd51e..6c9a6df 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 f0bec1b..1b4332f 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 a3a3a28..d7432a3 100644
--- a/src/heap.cc
+++ b/src/heap.cc
@@ -192,7 +192,7 @@
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 @@
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 @@
}
// 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 @@
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 7af15cd..a2c2d98 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 @@
};
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 @@
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 @@
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 @@
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 4b820f9..67020d8 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 9817660..3d753e1 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 b6c8008..3913c81 100644
--- a/src/mirror/object-inl.h
+++ b/src/mirror/object-inl.h
@@ -263,6 +263,10 @@
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 5c65b83..4acb567 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 @@
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 @@
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 c404b61..0cce8d8 100644
--- a/src/mirror/object.h
+++ b/src/mirror/object.h
@@ -58,6 +58,8 @@
#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 @@
}
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 4612986..814e649 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 08cf9d8..dd2f3fa 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 f65717a..6598f19 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 c4bd60e..a2ea71c 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 414b8d8..6c1ae59 100644
--- a/src/thread-inl.h
+++ b/src/thread-inl.h
@@ -124,6 +124,13 @@
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 394d263..94c437f 100644
--- a/src/thread.cc
+++ b/src/thread.cc
@@ -2004,20 +2004,17 @@
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 1992dc9..dd67a21 100644
--- a/src/thread.h
+++ b/src/thread.h
@@ -401,11 +401,7 @@
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 @@
}
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 a16d896..92cfa99 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"
